Analyze comparison with boolean constants (#41349)

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

@@ -2750,22 +2750,86 @@ protected override void VisitBinaryOperatorChildren(ArrayBuilder<BoundBinaryOper
             var binary = stack.Pop();
 
             var (leftOperand, leftConversion) = RemoveConversion(binary.Left, includeExplicitConversions: false);
-            VisitRvalue(leftOperand);
+            Visit(leftOperand);
 
             while (true)
             {
-                AfterLeftChildHasBeenVisited(leftOperand, leftConversion, binary);
+                if (!learnFromBooleanConstantTest())
+                {
+                    Unsplit(); // VisitRvalue does this
+                    UseRvalueOnly(leftOperand); // drop lvalue part
+
+                    AfterLeftChildHasBeenVisited(leftOperand, leftConversion, binary);
+                }
 
                 if (stack.Count == 0)
                 {
                     break;
                 }
 
-                Unsplit(); // VisitRvalue does this
                 leftOperand = binary;
                 leftConversion = Conversion.Identity;
                 binary = stack.Pop();
             }
+
+            // learn from true/false constant
+            bool learnFromBooleanConstantTest()
+            {
+                if (!IsConditionalState)
+                {
+                    return false;
+                }
+
+                if (!leftConversion.IsIdentity)
+                {
+                    return false;
+                }
+
+                BinaryOperatorKind op = binary.OperatorKind.Operator();
+                if (op != BinaryOperatorKind.Equal && op != BinaryOperatorKind.NotEqual)
+                {
+                    return false;
+                }
+
+                bool isSense;
+                if (binary.Right.ConstantValue?.IsBoolean == true)
+                {
+                    UseRvalueOnly(leftOperand); // record result for the left
+
+                    var stateWhenTrue = this.StateWhenTrue.Clone();
+                    var stateWhenFalse = this.StateWhenFalse.Clone();
+
+                    Unsplit();
+                    Visit(binary.Right);
+                    UseRvalueOnly(binary.Right); // record result for the right
+
+                    SetConditionalState(stateWhenTrue, stateWhenFalse);
+                    isSense = (op == BinaryOperatorKind.Equal) == binary.Right.ConstantValue.BooleanValue;
+                }
+                else if (binary.Left.ConstantValue?.IsBoolean == true)
+                {
+                    Unsplit();
+                    UseRvalueOnly(leftOperand); // record result for the left
+
+                    Visit(binary.Right);
+                    UseRvalueOnly(binary.Right); // record result for the right
+                    isSense = (op == BinaryOperatorKind.Equal) == binary.Left.ConstantValue.BooleanValue;
+                }
+                else
+                {
+                    return false;
+                }
+
+                if (!isSense && IsConditionalState)
+                {
+                    SetConditionalState(StateWhenFalse, StateWhenTrue);
+                }
+
+                // record result for the binary
+                Debug.Assert(binary.Type.SpecialType == SpecialType.System_Boolean);
+                SetResult(binary, TypeWithState.ForType(binary.Type), TypeWithAnnotations.Create(binary.Type));
+                return true;
+            }
         }
 
         private void AfterLeftChildHasBeenVisited(BoundExpression leftOperand, Conversion leftConversion, BoundBinaryOperator binary)
@@ -2863,11 +2927,10 @@ private void AfterLeftChildHasBeenVisited(BoundExpression leftOperand, Conversio
 
             BinaryOperatorKind op = binary.OperatorKind.Operator();
 
-            // learn from null constant
             if (op == BinaryOperatorKind.Equal || op == BinaryOperatorKind.NotEqual)
             {
+                // learn from null constant
                 BoundExpression operandComparedToNull = null;
-
                 if (binary.Right.ConstantValue?.IsNull == true)
                 {
                     operandComparedToNull = binary.Left;

src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs

@@ -21990,7 +21990,7 @@ public void Main()
                 );
         }
 
-        [Fact]
+        [Fact, WorkItem(29855, "https://github.com/dotnet/roslyn/issues/29855")]
         public void MaybeNullWhenTrue_EqualsTrue()
         {
             var c = CreateCompilation(new[] { @"
@@ -22014,14 +22014,352 @@ public void Main(string s)
 }
 ", MaybeNullWhenAttributeDefinition }, options: WithNonNullTypesTrue());
 
-            // https://github.com/dotnet/roslyn/issues/29855: there should only be one diagnostic
             c.VerifyDiagnostics(
                 // (9,13): warning CS8602: Dereference of a possibly null reference.
                 //             s.ToString(); // warn
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(9, 13)
+                );
+        }
+
+        [Fact, WorkItem(29855, "https://github.com/dotnet/roslyn/issues/29855")]
+        public void IsNullEqualsTrue()
+        {
+            var c = CreateNullableCompilation(@"
+class C
+{
+    void M(string s)
+    {
+        if ((s is null) == true)
+            s.ToString(); // 1
+        else
+            s.ToString();
+    }
+
+    void M2(string s)
+    {
+        if (true == (s is null))
+            s.ToString(); // 2
+        else
+            s.ToString();
+    }
+}
+");
+
+            c.VerifyDiagnostics(
+                // (7,13): warning CS8602: Dereference of a possibly null reference.
+                //             s.ToString(); // 1
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(7, 13),
+                // (15,13): warning CS8602: Dereference of a possibly null reference.
+                //             s.ToString(); // 2
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(15, 13)
+                );
+        }
+
+        [Fact, WorkItem(29855, "https://github.com/dotnet/roslyn/issues/29855")]
+        public void IsNullEqualsTrueEqualsFalse()
+        {
+            var c = CreateNullableCompilation(@"
+class C
+{
+    void M(string s)
+    {
+        if (((s is null) == true) == false)
+            s.ToString();
+        else
+            s.ToString(); // 1
+    }
+
+    void M2(string s)
+    {
+        if (false == (true == (s is null)))
+            s.ToString();
+        else
+            s.ToString(); // 2
+    }
+}
+");
+            c.VerifyDiagnostics(
+                // (9,13): warning CS8602: Dereference of a possibly null reference.
+                //             s.ToString(); // 1
                 Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(9, 13),
+                // (17,13): warning CS8602: Dereference of a possibly null reference.
+                //             s.ToString(); // 2
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(17, 13)
+                );
+        }
+
+        [Fact, WorkItem(29855, "https://github.com/dotnet/roslyn/issues/29855")]
+        public void IsNullEqualsFalse()
+        {
+            var c = CreateNullableCompilation(@"
+class C
+{
+    void M(string s)
+    {
+        if ((s is null) == false)
+            s.ToString();
+        else
+            s.ToString(); // 1
+    }
+
+    void M2(string s)
+    {
+        if (false == (s is null))
+            s.ToString();
+        else
+            s.ToString(); // 2
+    }
+}
+");
+
+            c.VerifyDiagnostics(
+                // (9,13): warning CS8602: Dereference of a possibly null reference.
+                //             s.ToString(); // 1
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(9, 13),
+                // (17,13): warning CS8602: Dereference of a possibly null reference.
+                //             s.ToString(); // 2
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(17, 13)
+                );
+        }
+
+        [Fact, WorkItem(29855, "https://github.com/dotnet/roslyn/issues/29855")]
+        public void IsNullNotEqualsTrue()
+        {
+            var c = CreateNullableCompilation(@"
+class C
+{
+    void M(string s)
+    {
+        if ((s is null) != true)
+            s.ToString();
+        else
+            s.ToString(); // 1
+    }
+
+    void M2(string s)
+    {
+        if (true != (s is null))
+            s.ToString();
+        else
+            s.ToString(); // 2
+    }
+}
+");
+
+            c.VerifyDiagnostics(
+                // (9,13): warning CS8602: Dereference of a possibly null reference.
+                //             s.ToString(); // 1
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(9, 13),
+                // (17,13): warning CS8602: Dereference of a possibly null reference.
+                //             s.ToString(); // 2
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(17, 13)
+                );
+        }
+
+        [Fact, WorkItem(29855, "https://github.com/dotnet/roslyn/issues/29855")]
+        public void IsNullNotEqualsFalse()
+        {
+            var c = CreateNullableCompilation(@"
+class C
+{
+    void M(string s)
+    {
+        if ((s is null) != false)
+            s.ToString(); // 1
+        else
+            s.ToString();
+    }
+
+    void M2(string s)
+    {
+        if (false != (s is null))
+            s.ToString(); // 2
+        else
+            s.ToString();
+    }
+}
+");
+
+            c.VerifyDiagnostics(
+                // (7,13): warning CS8602: Dereference of a possibly null reference.
+                //             s.ToString(); // 1
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(7, 13),
+                // (15,13): warning CS8602: Dereference of a possibly null reference.
+                //             s.ToString(); // 2
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(15, 13)
+                );
+        }
+
+        [Fact, WorkItem(29855, "https://github.com/dotnet/roslyn/issues/29855")]
+        public void OnlySwapConditionalStatesWhenInConditionalState()
+        {
+            var c = CreateNullableCompilation(@"
+class C
+{
+    void M(bool b)
+    {
+        if (b == (true != b))
+        {
+        }
+    }
+}
+");
+
+            c.VerifyDiagnostics();
+        }
+
+        [Fact, WorkItem(41107, "https://github.com/dotnet/roslyn/issues/41107")]
+        public void NullConditionalEqualsTrue()
+        {
+            var c = CreateNullableCompilation(new[] { @"
+public class C
+{
+    public C? field;
+    public bool value;
+
+    static public void M(C c)
+    {
+        if (c.field?.value == true)
+        {
+            c.field.ToString();
+        }
+        else
+        {
+            c.field.ToString(); // 1
+        }
+    }
+
+    static public void M2(C c)
+    {
+        if (false == c.field?.value)
+        {
+            c.field.ToString();
+        }
+        else
+        {
+            c.field.ToString(); // 2
+        }
+    }
+}
+", NotNullWhenAttributeDefinition });
+
+            c.VerifyDiagnostics(
+                // (15,13): warning CS8602: Dereference of a possibly null reference.
+                //             c.field.ToString(); // 1
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c.field").WithLocation(15, 13),
+                // (27,13): warning CS8602: Dereference of a possibly null reference.
+                //             c.field.ToString(); // 2
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c.field").WithLocation(27, 13)
+                );
+        }
+
+        [Fact, WorkItem(41107, "https://github.com/dotnet/roslyn/issues/41107")]
+        public void EqualsComplicatedTrue()
+        {
+            var c = CreateNullableCompilation(new[] { @"
+public class C
+{
+    public C? field;
+    public bool value;
+
+    static C? MaybeNull() => throw null!;
+
+    static public void M(C c)
+    {
+        if ((c.field is null) == (true || MaybeNull()?.value == true))
+        {
+            c.field.ToString(); // 1
+        }
+        else
+        {
+            c.field.ToString(); // 2
+        }
+    }
+
+    static public void M2(C c)
+    {
+        if ((false && MaybeNull()?.value == true) == (c.field is null))
+        {
+            c.field.ToString(); // 3
+        }
+        else
+        {
+            c.field.ToString(); // 4
+        }
+    }
+}
+", NotNullWhenAttributeDefinition });
+
+            c.VerifyDiagnostics(
                 // (13,13): warning CS8602: Dereference of a possibly null reference.
-                //             s.ToString(); // ok
-                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(13, 13)
+                //             c.field.ToString(); // 1
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c.field").WithLocation(13, 13),
+                // (17,13): warning CS8602: Dereference of a possibly null reference.
+                //             c.field.ToString(); // 2
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c.field").WithLocation(17, 13),
+                // (25,13): warning CS8602: Dereference of a possibly null reference.
+                //             c.field.ToString(); // 3
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c.field").WithLocation(25, 13),
+                // (29,13): warning CS8602: Dereference of a possibly null reference.
+                //             c.field.ToString(); // 4
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c.field").WithLocation(29, 13)
+                );
+        }
+
+        [Fact, WorkItem(41107, "https://github.com/dotnet/roslyn/issues/41107")]
+        public void NullConditionalWithExtensionMethodEqualsTrue()
+        {
+            var c = CreateNullableCompilation(new[] { @"
+using System.Diagnostics.CodeAnalysis;
+
+public class C
+{
+    public C? field;
+
+    static public void M(C c)
+    {
+        if (c.field?.field.IsKind() == true)
+        {
+            c.field.field.ToString(); // 1
+        }
+    }
+
+    static public void M2(C c)
+    {
+        if (true == c.field?.field.IsKind())
+        {
+            c.field.field.ToString(); // 2
+        }
+    }
+
+
+    static public void M3(C c)
+    {
+        if (Extension.IsKind(c.field?.field) == true)
+        {
+            c.field.field.ToString();
+        }
+    }
+}
+
+public static class Extension
+{
+    public static bool IsKind([NotNullWhen(true)] this C? c)
+    {
+        throw null!;
+    }
+}
+", NotNullWhenAttributeDefinition });
+
+            // Note: when we're done analyzing `c.field?.field.IsKind()` we have an unconditional state.
+            // If we wanted to analyze this properly, we would probably need a WhenTrue/WhenFalse/WhenNull split.
+            c.VerifyDiagnostics(
+                // (12,13): warning CS8602: Dereference of a possibly null reference.
+                //             c.field.field.ToString(); // 1
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c.field.field").WithLocation(12, 13),
+                // (20,13): warning CS8602: Dereference of a possibly null reference.
+                //             c.field.field.ToString(); // 2
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c.field.field").WithLocation(20, 13)
                 );
         }
 
@@ -22480,14 +22818,10 @@ public void Main(string? s)
 }
 ", NotNullWhenAttributeDefinition }, options: WithNonNullTypesTrue());
 
-            // https://github.com/dotnet/roslyn/issues/29855: there should only be one diagnostic
             c.VerifyDiagnostics(
                 // (9,13): warning CS8602: Dereference of a possibly null reference.
                 //             s.ToString(); // warn
-                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(9, 13),
-                // (13,13): warning CS8602: Dereference of a possibly null reference.
-                //             s.ToString(); // ok
-                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(13, 13)
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(9, 13)
                 );
         }
 
@@ -22610,11 +22944,7 @@ public void Main(string? s)
 }
 ", NotNullWhenAttributeDefinition }, options: WithNonNullTypesTrue());
 
-            // https://github.com/dotnet/roslyn/issues/29855: there should only be one diagnostic
             c.VerifyDiagnostics(
-                // (9,13): warning CS8602: Dereference of a possibly null reference.
-                //             s.ToString(); // ok
-                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(9, 13),
                 // (13,13): warning CS8602: Dereference of a possibly null reference.
                 //             s.ToString(); // warn
                 Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(13, 13)
@@ -22646,14 +22976,10 @@ public void Main(string? s)
 }
 ", NotNullWhenAttributeDefinition }, options: WithNonNullTypesTrue());
 
-            // https://github.com/dotnet/roslyn/issues/29855: there should only be one diagnostic
             c.VerifyDiagnostics(
                 // (9,13): warning CS8602: Dereference of a possibly null reference.
                 //             s.ToString(); // warn
-                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(9, 13),
-                // (13,13): warning CS8602: Dereference of a possibly null reference.
-                //             s.ToString(); // ok
-                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(13, 13)
+                Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(9, 13)
                 );
         }
 
@@ -27880,6 +28206,7 @@ void M(string? s1)
         }
 
         [Fact]
+
         public void NotNull_ConditionalMethodInReleaseMode()
         {
             CSharpCompilation c = CreateCompilation(new[] { @"
@@ -35219,56 +35546,6 @@ void M(C c)
             comp.VerifyDiagnostics();
         }
 
-        [Fact, WorkItem(35949, "https://github.com/dotnet/roslyn/issues/35949")]
-        public void NotNull_Complexity()
-        {
-            var source = @"
-using System;
-using System.Diagnostics.CodeAnalysis;
-class C
-{
-    C f = null!;
-
-    void M(C c)
-    {
-        c.f = c;
-        c.NotNull(
-            x => x.f.NotNull(
-                y => y.f.NotNull(
-                    z => z.f.NotNull(
-                        q => q.f.NotNull(
-                            w => w.f.NotNull(
-                                e => e.f.NotNull(
-                                    r => r.f.NotNull(
-                                        _ =>
-                                        {
-                                            """".NotNull(s => s);
-                                            """".NotNull(s => s);
-                                            """".NotNull(s => s);
-                                            """".NotNull(s => s);
-                                            """".NotNull(s => s);
-                                            """".NotNull(s => s);
-                                            """".NotNull(s => s);
-                                            """".NotNull(s => s);
-                                            """".NotNull(s => s);
-                                            """".NotNull(s => s);
-                                            """".NotNull(s => s);
-                                            """".NotNull(s => s);
-
-                                            return """";
-                                        }))))))));
-    }
-}
-
-static class Ext
-{
-    public static V NotNull<T, V>([NotNull] this T t, Func<T, V> f) => throw null!;
-}
-";
-            var comp = CreateNullableCompilation(new[] { NotNullAttributeDefinition, source });
-            comp.VerifyDiagnostics();
-        }
-
         [Fact]
         public void NotNull_OutParameter_GenericType()
         {

src/Compilers/CSharp/Test/Semantic/Semantics/SlowTests.cs

+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
+using Roslyn.Test.Utilities;
+using Xunit;
+
+namespace Microsoft.CodeAnalysis.CSharp.UnitTests.Semantics
+{
+    public class SlowTests : CSharpTestBase
+    {
+        [Fact, WorkItem(35949, "https://github.com/dotnet/roslyn/issues/35949")]
+        public void NotNull_Complexity()
+        {
+            var source = @"
+#nullable enable
+
+using System;
+using System.Diagnostics.CodeAnalysis;
+class C
+{
+    C f = null!;
+
+    void M(C c)
+    {
+        c.f = c;
+        c.NotNull(
+            x => x.f.NotNull(
+                y => y.f.NotNull(
+                    z => z.f.NotNull(
+                        q => q.f.NotNull(
+                            w => w.f.NotNull(
+                                e => e.f.NotNull(
+                                    r => r.f.NotNull(
+                                        _ =>
+                                        {
+                                            """".NotNull(s => s);
+                                            """".NotNull(s => s);
+                                            """".NotNull(s => s);
+                                            """".NotNull(s => s);
+                                            """".NotNull(s => s);
+                                            """".NotNull(s => s);
+                                            """".NotNull(s => s);
+                                            """".NotNull(s => s);
+                                            """".NotNull(s => s);
+                                            """".NotNull(s => s);
+                                            """".NotNull(s => s);
+                                            """".NotNull(s => s);
+
+                                            return """";
+                                        }))))))));
+    }
+}
+
+static class Ext
+{
+    public static V NotNull<T, V>([NotNull] this T t, Func<T, V> f) => throw null!;
+}
+";
+            var comp = CreateCompilation(new[] { NotNullAttributeDefinition, source });
+            comp.VerifyDiagnostics();
+        }
+    }
+}