Re-infer unary and binary operators in NullableWalker (#38320)

docs/compilers/CSharp/Compiler Breaking Changes - post VS2019.md

@@ -51,3 +51,19 @@ Each entry should include a short description of the break, followed by either a
 6. https://github.com/dotnet/roslyn/issues/37527 The constant folding behavior of the compiler differed depending on your host architecture when converting a floating-point constant to an integral type where that conversion would be a compile-time error if not in an `unchecked` context.  We now yield a zero result for such conversions on all host architectures.
 
 7. https://github.com/dotnet/roslyn/issues/38226 When there exists a common type among those arms of a switch expression that have a type, but there are some arms that have an expression without a type (e.g. `null`) that cannot convert to that common type, the compiler improperly inferred that common type as the natural type of the switch expression. That would cause an error.  In Visual Studio 2019 Update 4, we fixed the compiler to no longer consider such a switch expression to have a common type.  This may permit some programs to compile without error that would produce an error in the previous version.
+
+8. User-defined unary and binary operators are re-inferred from the nullability of the arguments. This may result in additional warnings:
+    ```C#
+    struct S<T>
+    {
+        public static S<T> operator~(S<T> s) { ... }
+        public T F;
+    }
+    static S<T> Create<T>(T t) { ... }
+    static void F()
+    {
+        object o = null;
+        var s = ~Create(o);
+        s.F.ToString(); // warning: s.F may be null
+    }
+    ```
\ No newline at end of file

src/Compilers/CSharp/Portable/BoundTree/NullabilityRewriter.cs

 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 #nullable enable
-using System.Collections.Immutable;
 using System.Diagnostics;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.PooledObjects;
@@ -51,10 +50,10 @@ private BoundNode VisitBinaryOperatorBase(BoundBinaryOperatorBase binaryOperator
                 var right = (BoundExpression)Visit(currentBinary.Right);
                 var type = foundInfo ? infoAndType.Type : currentBinary.Type;
 
-                // https://github.com/dotnet/roslyn/issues/35031: We'll need to update the symbols for the internal methods/operators used in the binary operators
                 currentBinary = currentBinary switch
                 {
-                    BoundBinaryOperator binary => binary.Update(binary.OperatorKind, binary.ConstantValueOpt, binary.MethodOpt, binary.ResultKind, binary.OriginalUserDefinedOperatorsOpt, leftChild, right, type),
+                    BoundBinaryOperator binary => binary.Update(binary.OperatorKind, binary.ConstantValueOpt, GetUpdatedSymbol(binary, binary.MethodOpt), binary.ResultKind, binary.OriginalUserDefinedOperatorsOpt, leftChild, right, type),
+                    // https://github.com/dotnet/roslyn/issues/35031: We'll need to update logical.LogicalOperator
                     BoundUserDefinedConditionalLogicalOperator logical => logical.Update(logical.OperatorKind, logical.LogicalOperator, logical.TrueOperator, logical.FalseOperator, logical.ResultKind, logical.OriginalUserDefinedOperatorsOpt, leftChild, right, type),
                     _ => throw ExceptionUtilities.UnexpectedValue(currentBinary.Kind),
                 };

src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs

@@ -2084,29 +2084,27 @@ private void VisitBinaryOperatorChildren(BoundBinaryOperator node)
             //
             // Of course we must ensure that we visit the left hand side before the right hand side.
             var stack = ArrayBuilder<BoundBinaryOperator>.GetInstance();
-            stack.Push(node);
 
-            BoundBinaryOperator binary;
-            BoundExpression child = node.Left;
-
-            while (true)
+            BoundBinaryOperator binary = node;
+            do
             {
-                binary = child as BoundBinaryOperator;
-                if (binary == null || binary.OperatorKind.IsLogical())
-                {
-                    break;
-                }
-
                 stack.Push(binary);
-                child = binary.Left;
+                binary = binary.Left as BoundBinaryOperator;
             }
+            while (binary != null && !binary.OperatorKind.IsLogical());
 
-            VisitRvalue(child);
+            VisitBinaryOperatorChildren(stack);
+            stack.Free();
+        }
+
+        protected virtual void VisitBinaryOperatorChildren(ArrayBuilder<BoundBinaryOperator> stack)
+        {
+            var binary = stack.Pop();
+            VisitRvalue(binary.Left);
 
             while (true)
             {
-                binary = stack.Pop();
-                AfterLeftChildHasBeenVisited(binary);
+                VisitRvalue(binary.Right);
 
                 if (stack.Count == 0)
                 {
@@ -2114,15 +2112,8 @@ private void VisitBinaryOperatorChildren(BoundBinaryOperator node)
                 }
 
                 Unsplit(); // VisitRvalue does this
+                binary = stack.Pop();
             }
-
-            Debug.Assert((object)binary == node);
-            stack.Free();
-        }
-
-        protected virtual void AfterLeftChildHasBeenVisited(BoundBinaryOperator binary)
-        {
-            VisitRvalue(binary.Right);
         }
 
         public override BoundNode VisitUnaryOperator(BoundUnaryOperator node)

src/Compilers/CSharp/Test/Symbol/Symbols/Source/NullablePublicAPITests.cs

@@ -1399,7 +1399,6 @@ void M(C? x, C x2)
         [Fact]
         public void SpeculativeGetTypeInfo_Basic()
         {
-
             var source = @"
 class C
 {
@@ -2636,7 +2635,6 @@ void verifyAnnotation(ExpressionSyntax expr)
         [Fact]
         public void GetSymbolInfo_ReinferredCollectionInitializerAdd_MultiElementAdds()
         {
-
             var source = @"
 using System.Collections;
 class C : IEnumerable
@@ -2680,7 +2678,6 @@ void verifyAnnotation(ExpressionSyntax expr, PublicNullableAnnotation annotation
         [Fact]
         public void GetSymbolInfo_ReinferredCollectionInitializerAdd_MultiElementAdds_LinkedTypes()
         {
-
             var source = @"
 using System.Collections;
 class C : IEnumerable
@@ -2842,7 +2839,6 @@ void verifyIndexer(IPropertySymbol propertySymbol)
         [Fact]
         public void GetSymbolInfo_RangeReinferred()
         {
-
             var source = @"
 using System;
 
@@ -2881,5 +2877,93 @@ void verifyAnnotation(ElementAccessExpressionSyntax indexer, PublicNullableAnnot
                 Assert.Equal(annotation, spanType.TypeArgumentNullableAnnotations[0]);
             }
         }
+
+        [Fact]
+        public void GetSymbolInfo_UnaryOperator()
+        {
+            var source =
+@"#nullable enable
+struct S<T>
+{
+    public static S<T> operator~(S<T> s) => s;
+}
+class Program
+{
+    static S<T> Create1<T>(T t) => new S<T>();
+    static S<T>? Create2<T>(T t) => null;
+    static void F<T>() where T : class, new()
+    {
+        T x = null;
+        var sx = Create1(x);
+        _ = ~sx;
+        T? y = new T();
+        var sy = Create2(y);
+        _ = ~sy;
+    }
+}";
+            var comp = CreateCompilation(source);
+            comp.VerifyDiagnostics(
+                // (12,15): warning CS8600: Converting null literal or possible null value to non-nullable type.
+                //         T x = null;
+                Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "null").WithLocation(12, 15));
+
+            var syntaxTree = comp.SyntaxTrees[0];
+            var root = syntaxTree.GetRoot();
+            var model = comp.GetSemanticModel(syntaxTree);
+            var operators = root.DescendantNodes().OfType<PrefixUnaryExpressionSyntax>().ToList();
+            verifyAnnotations(operators[0], PublicNullableAnnotation.Annotated, "S<T?> S<T?>.operator ~(S<T?> s)");
+            verifyAnnotations(operators[1], PublicNullableAnnotation.NotAnnotated, "S<T!> S<T!>.operator ~(S<T!> s)");
+
+            void verifyAnnotations(PrefixUnaryExpressionSyntax syntax, PublicNullableAnnotation annotation, string expected)
+            {
+                var method = (IMethodSymbol)model.GetSymbolInfo(syntax).Symbol;
+                Assert.Equal(expected, method.ToTestDisplayString(includeNonNullable: true));
+                Assert.Equal(annotation, method.ContainingType.TypeArgumentNullableAnnotations[0]);
+            }
+        }
+
+        [Fact]
+        public void GetSymbolInfo_BinaryOperator()
+        {
+            var source =
+@"#nullable enable
+struct S<T>
+{
+    public static S<T> operator+(S<T> x, S<T> y) => x;
+}
+class Program
+{
+    static S<T> Create1<T>(T t) => new S<T>();
+    static S<T>? Create2<T>(T t) => null;
+    static void F<T>() where T : class, new()
+    {
+        T x = null;
+        var sx = Create1(x);
+        _ = sx + sx;
+        T? y = new T();
+        var sy = Create2(y);
+        _ = sy + sy;
+    }
+}";
+            var comp = CreateCompilation(source);
+            comp.VerifyDiagnostics(
+                // (12,15): warning CS8600: Converting null literal or possible null value to non-nullable type.
+                //         T x = null;
+                Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "null").WithLocation(12, 15));
+
+            var syntaxTree = comp.SyntaxTrees[0];
+            var root = syntaxTree.GetRoot();
+            var model = comp.GetSemanticModel(syntaxTree);
+            var operators = root.DescendantNodes().OfType<BinaryExpressionSyntax>().ToList();
+            verifyAnnotations(operators[0], PublicNullableAnnotation.Annotated, "S<T?> S<T?>.operator +(S<T?> x, S<T?> y)");
+            verifyAnnotations(operators[1], PublicNullableAnnotation.NotAnnotated, "S<T!> S<T!>.operator +(S<T!> x, S<T!> y)");
+
+            void verifyAnnotations(BinaryExpressionSyntax syntax, PublicNullableAnnotation annotation, string expected)
+            {
+                var method = (IMethodSymbol)model.GetSymbolInfo(syntax).Symbol;
+                Assert.Equal(expected, method.ToTestDisplayString(includeNonNullable: true));
+                Assert.Equal(annotation, method.ContainingType.TypeArgumentNullableAnnotations[0]);
+            }
+        }
     }
 }