Fix S1125 CodeFix: C# pattern matching in conditional operator (#7719)

SonarSource · Aug 4, 2023 · b9bab9b · b9bab9b
1 parent 8f6674f
commit b9bab9b
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 22 deletions.
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/BooleanLiteralUnnecessaryCodeFix.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/BooleanLiteralUnnecessaryCodeFix.cs
@@ -281,73 +281,63 @@ private static SyntaxNode ReplaceExpressionWithBinary(SyntaxNode nodeToReplace,
         private static Document RewriteConditional(Document document, SyntaxNode root, SyntaxNode syntaxNode, ConditionalExpressionSyntax conditional)
         {
             var whenTrue = conditional.WhenTrue.RemoveParentheses();
-            if (whenTrue.Equals(syntaxNode)
-                && CSharpEquivalenceChecker.AreEquivalent(syntaxNode, CSharpSyntaxHelper.TrueLiteralExpression))
+            if (whenTrue.Equals(syntaxNode) && syntaxNode.IsTrue())
             {
                 var newRoot = ReplaceExpressionWithBinary(
                     conditional,
                     root,
                     SyntaxKind.LogicalOrExpression,
                     conditional.Condition,
-                    conditional.WhenFalse);
+                    AddParenthesis(conditional.WhenFalse));
 
                 return document.WithSyntaxRoot(newRoot);
             }
 
-            if (whenTrue.Equals(syntaxNode)
-                && CSharpEquivalenceChecker.AreEquivalent(syntaxNode, CSharpSyntaxHelper.FalseLiteralExpression))
+            if (whenTrue.Equals(syntaxNode) && syntaxNode.IsFalse())
             {
                 var newRoot = ReplaceExpressionWithBinary(
                     conditional,
                     root,
                     SyntaxKind.LogicalAndExpression,
                     GetNegatedExpression(conditional.Condition),
-                    conditional.WhenFalse);
+                    AddParenthesis(conditional.WhenFalse));
 
                 return document.WithSyntaxRoot(newRoot);
             }
 
             var whenFalse = conditional.WhenFalse.RemoveParentheses();
 
-            if (whenFalse.Equals(syntaxNode)
-                && CSharpEquivalenceChecker.AreEquivalent(syntaxNode, CSharpSyntaxHelper.TrueLiteralExpression))
+            if (whenFalse.Equals(syntaxNode) && syntaxNode.IsTrue())
             {
                 var newRoot = ReplaceExpressionWithBinary(
                     conditional,
                     root,
                     SyntaxKind.LogicalOrExpression,
                     GetNegatedExpression(conditional.Condition),
-                    conditional.WhenTrue);
+                    AddParenthesis(conditional.WhenTrue));
 
                 return document.WithSyntaxRoot(newRoot);
             }
 
-            if (whenFalse.Equals(syntaxNode)
-                && CSharpEquivalenceChecker.AreEquivalent(syntaxNode, CSharpSyntaxHelper.FalseLiteralExpression))
+            if (whenFalse.Equals(syntaxNode) && syntaxNode.IsFalse())
             {
                 var newRoot = ReplaceExpressionWithBinary(
                     conditional,
                     root,
                     SyntaxKind.LogicalAndExpression,
                     conditional.Condition,
-                    conditional.WhenTrue);
+                    AddParenthesis(conditional.WhenTrue));
 
                 return document.WithSyntaxRoot(newRoot);
             }
 
             return document;
         }
 
-        private static ExpressionSyntax GetNegatedExpression(ExpressionSyntax expression)
-        {
-            var exp = expression;
-            if (expression is BinaryExpressionSyntax
-               || expression is ConditionalExpressionSyntax)
-            {
-                exp = SyntaxFactory.ParenthesizedExpression(expression);
-            }
+        private static ExpressionSyntax GetNegatedExpression(ExpressionSyntax expression) =>
+            SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, AddParenthesis(expression));
 
-            return SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, exp);
-        }
+        private static ExpressionSyntax AddParenthesis(ExpressionSyntax expression) =>
+            SyntaxFactory.ParenthesizedExpression(expression).WithAdditionalAnnotations(Simplifier.Annotation);
     }
 }
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/BooleanLiteralUnnecessary.CSharp9.Fixed.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/BooleanLiteralUnnecessary.CSharp9.Fixed.cs
@@ -61,5 +61,17 @@ void IsNotPattern(bool a, bool? b)
 
             var x = a is not true ? throw new Exception() : false;
         }
+
+        // https://github.com/SonarSource/sonar-dotnet/issues/2618
+        public void Repro_2618(Item item)
+        {
+            var booleanVariable = !(item is not Item myItem) && myItem.Required; // Fixed
+            booleanVariable = item is not Item myItem2 || myItem2.Required; // Fixed
+        }
+    }
+
+    public class Item
+    {
+        public bool Required { get; set; }
     }
 }
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/BooleanLiteralUnnecessary.CSharp9.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/BooleanLiteralUnnecessary.CSharp9.cs
@@ -63,5 +63,17 @@ void IsNotPattern(bool a, bool? b)
 
             var x = a is not true ? throw new Exception() : false;
         }
+
+        // https://github.com/SonarSource/sonar-dotnet/issues/2618
+        public void Repro_2618(Item item)
+        {
+            var booleanVariable = item is not Item myItem ? false : myItem.Required; // Noncompliant
+            booleanVariable = item is not Item myItem2 ? true : myItem2.Required; // Noncompliant
+        }
+    }
+
+    public class Item
+    {
+        public bool Required { get; set; }
     }
 }
diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/BooleanLiteralUnnecessary.Fixed.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/BooleanLiteralUnnecessary.Fixed.cs
@@ -99,6 +99,13 @@ public BooleanLiteralUnnecessary(bool a, bool b, bool? c, Item item)
             };
         }
 
+        // https://github.com/SonarSource/sonar-dotnet/issues/2618
+        public void Repro_2618(Item item)
+        {
+            var booleanVariable = item is Item myItem && myItem.Required; // Fixed
+            booleanVariable = !(item is Item myItem2) || myItem2.Required; // Fixed
+        }
+
         public static void SomeFunc(bool x) { }
 
         private bool Foo()
@@ -174,6 +181,12 @@ public void ThrowExpressionIsIgnored(bool condition, int number)
                 x = condition is true ? throw new Exception() : false;
             }
         }
+
+        // https://github.com/SonarSource/sonar-dotnet/issues/7462
+        public void Repro_7462(object obj)
+        {
+            var a = !(obj == null) && (obj.ToString() == "x" || obj.ToString() == "y"); // Fixed
+        }
     }
 
     public class Item

diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/BooleanLiteralUnnecessary.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/BooleanLiteralUnnecessary.cs
@@ -116,6 +116,13 @@ public BooleanLiteralUnnecessary(bool a, bool b, bool? c, Item item)
             };
         }
 
+        // https://github.com/SonarSource/sonar-dotnet/issues/2618
+        public void Repro_2618(Item item)
+        {
+            var booleanVariable = item is Item myItem ? myItem.Required : false; // Noncompliant
+            booleanVariable = item is Item myItem2 ? myItem2.Required : true; // Noncompliant
+        }
+
         public static void SomeFunc(bool x) { }
 
         private bool Foo()
@@ -191,6 +198,12 @@ public void ThrowExpressionIsIgnored(bool condition, int number)
                 x = condition is true ? throw new Exception() : false;
             }
         }
+
+        // https://github.com/SonarSource/sonar-dotnet/issues/7462
+        public void Repro_7462(object obj)
+        {
+            var a = obj == null ? false : obj.ToString() == "x" || obj.ToString() == "y"; // Noncompliant
+        }
     }
 
     public class Item