Merge pull request #64432 from jnm2/fix_invert_lifted

Fix 'invert if' mishandling of lifted nullable operators
dotnet · Oct 2, 2022 · 95278f4 · 95278f4
2 parents 6260291 + 5cdd0b0
commit 95278f4
Show file tree

Hide file tree

Showing 2 changed files with 180 additions and 0 deletions.
diff --git a/src/EditorFeatures/CSharpTest/InvertIf/InvertIfTests.cs b/src/EditorFeatures/CSharpTest/InvertIf/InvertIfTests.cs
@@ -1291,5 +1291,174 @@ int M()
     }
 }", parseOptions: CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp9));
         }
+
+        [Fact, WorkItem(63311, "https://github.com/dotnet/roslyn/issues/63311")]
+        public async Task TestLiftedNullable_GreaterThan()
+        {
+            await TestInRegularAndScriptAsync(
+                """
+                class C
+                {
+                    void M(int? p)
+                    {
+                        [||]if (p > 10)
+                        {
+                            System.Console.WriteLine("p is not null and p.Value > 10");
+                        }
+                    }
+                }
+                """,
+                """
+                class C
+                {
+                    void M(int? p)
+                    {
+                        if (!(p > 10))
+                        {
+                            return;
+                        }
+                        System.Console.WriteLine("p is not null and p.Value > 10");
+                    }
+                }
+                """);
+        }
+
+        [Fact, WorkItem(63311, "https://github.com/dotnet/roslyn/issues/63311")]
+        public async Task TestLiftedNullable_GreaterThanOrEqual()
+        {
+            await TestInRegularAndScriptAsync(
+                """
+                class C
+                {
+                    void M(int? p)
+                    {
+                        [||]if (p >= 10)
+                        {
+                            System.Console.WriteLine("p is not null and p.Value >= 10");
+                        }
+                    }
+                }
+                """,
+                """
+                class C
+                {
+                    void M(int? p)
+                    {
+                        if (!(p >= 10))
+                        {
+                            return;
+                        }
+                        System.Console.WriteLine("p is not null and p.Value >= 10");
+                    }
+                }
+                """);
+        }
+
+        [Fact, WorkItem(63311, "https://github.com/dotnet/roslyn/issues/63311")]
+        public async Task TestLiftedNullable_LessThan()
+        {
+            await TestInRegularAndScriptAsync(
+                """
+                class C
+                {
+                    void M(int? p)
+                    {
+                        [||]if (p < 10)
+                        {
+                            System.Console.WriteLine("p is not null and p.Value < 10");
+                        }
+                    }
+                }
+                """,
+                """
+                class C
+                {
+                    void M(int? p)
+                    {
+                        if (!(p < 10))
+                        {
+                            return;
+                        }
+                        System.Console.WriteLine("p is not null and p.Value < 10");
+                    }
+                }
+                """);
+        }
+
+        [Fact, WorkItem(63311, "https://github.com/dotnet/roslyn/issues/63311")]
+        public async Task TestLiftedNullable_LessThanOrEqual()
+        {
+            await TestInRegularAndScriptAsync(
+                """
+                class C
+                {
+                    void M(int? p)
+                    {
+                        [||]if (p <= 10)
+                        {
+                            System.Console.WriteLine("p is not null and p.Value <= 10");
+                        }
+                    }
+                }
+                """,
+                """
+                class C
+                {
+                    void M(int? p)
+                    {
+                        if (!(p <= 10))
+                        {
+                            return;
+                        }
+                        System.Console.WriteLine("p is not null and p.Value <= 10");
+                    }
+                }
+                """);
+        }
+
+        [Fact, WorkItem(63311, "https://github.com/dotnet/roslyn/issues/63311")]
+        public async Task TestNullableReference_GreaterThan()
+        {
+            await TestInRegularAndScriptAsync(
+                """
+                #nullable enable
+                using System;
+                class C
+                {
+                    void M(C? p)
+                    {
+                        [||]if (p > new C())
+                        {
+                            Console.WriteLine("Null-handling semantics may actually change depending on the operator implementation");
+                        }
+                    }
+
+                    public static bool operator <(C? left, C? right) => throw new NotImplementedException();
+                    public static bool operator >(C? left, C? right) => throw new NotImplementedException();
+                    public static bool operator <=(C? left, C? right) => throw new NotImplementedException();
+                    public static bool operator >=(C? left, C? right) => throw new NotImplementedException();
+                }
+                """,
+                """
+                #nullable enable
+                using System;
+                class C
+                {
+                    void M(C? p)
+                    {
+                        if (p <= new C())
+                        {
+                            return;
+                        }
+                        Console.WriteLine("Null-handling semantics may actually change depending on the operator implementation");
+                    }
+
+                    public static bool operator <(C? left, C? right) => throw new NotImplementedException();
+                    public static bool operator >(C? left, C? right) => throw new NotImplementedException();
+                    public static bool operator <=(C? left, C? right) => throw new NotImplementedException();
+                    public static bool operator >=(C? left, C? right) => throw new NotImplementedException();
+                }
+                """);
+        }
     }
 }
diff --git a/...haredUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs b/...haredUtilitiesAndExtensions/Workspace/Core/Extensions/SyntaxGeneratorExtensions_Negate.cs
@@ -157,6 +157,17 @@ private static SyntaxNode GetNegationOfBinaryExpression(
             if (!s_negatedBinaryMap.TryGetValue(binaryOperation.OperatorKind, out var negatedKind))
                 return generator.LogicalNotExpression(expressionNode);
 
+            // Lifted relational operators return false if either operand is null.
+            // Inverting the operator fails to invert the behavior when an operand is null.
+            if (binaryOperation.IsLifted
+                && binaryOperation.OperatorKind is BinaryOperatorKind.LessThan or
+                                                   BinaryOperatorKind.LessThanOrEqual or
+                                                   BinaryOperatorKind.GreaterThan or
+                                                   BinaryOperatorKind.GreaterThanOrEqual)
+            {
+                return generator.LogicalNotExpression(expressionNode);
+            }
+
             if (binaryOperation.OperatorKind is BinaryOperatorKind.Or or
                                                 BinaryOperatorKind.And or
                                                 BinaryOperatorKind.ConditionalAnd or