Fix codeFix

SonarSource · Aug 3, 2023 · 0a50eb2 · 0a50eb2
1 parent f06ad51
commit 0a50eb2
Show file tree

Hide file tree

Showing 6 changed files with 68 additions and 14 deletions.
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs b/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs
@@ -449,6 +449,30 @@ private static string GetUnknownType(SyntaxKind kind) =>
 
 #endif
 
+        public static bool IsTrue(this SyntaxNode node) =>
+            node switch
+            {
+                { RawKind: (int)SyntaxKind.TrueLiteralExpression } => true, // true
+                { RawKind: (int)SyntaxKind.LogicalNotExpression } => IsFalse(((PrefixUnaryExpressionSyntax)node).Operand), // !false
+                { RawKind: (int)SyntaxKindEx.ConstantPattern } => IsTrue(((ConstantPatternSyntaxWrapper)node).Expression), // is true
+                { RawKind: (int)SyntaxKindEx.NotPattern } => IsFalse(((UnaryPatternSyntaxWrapper)node).Pattern), // is not false
+                { RawKind: (int)SyntaxKind.ParenthesizedExpression } => IsTrue(((ParenthesizedExpressionSyntax)node).Expression), // (true)
+                { RawKind: (int)SyntaxKindEx.ParenthesizedPattern } => IsTrue(((ParenthesizedPatternSyntaxWrapper)node).Pattern), // is (true)
+                _ => false,
+            };
+
+        public static bool IsFalse(this SyntaxNode node) =>
+            node switch
+            {
+            { RawKind: (int)SyntaxKind.FalseLiteralExpression } => true, // false
+            { RawKind: (int)SyntaxKind.LogicalNotExpression } => IsTrue(((PrefixUnaryExpressionSyntax)node).Operand), // !true
+            { RawKind: (int)SyntaxKindEx.ConstantPattern } => IsFalse(((ConstantPatternSyntaxWrapper)node).Expression), // is false
+            { RawKind: (int)SyntaxKindEx.NotPattern } => IsTrue(((UnaryPatternSyntaxWrapper)node).Pattern), // is not true
+            { RawKind: (int)SyntaxKind.ParenthesizedExpression } => IsFalse(((ParenthesizedExpressionSyntax)node).Expression), // (false)
+            { RawKind: (int)SyntaxKindEx.ParenthesizedPattern } => IsFalse(((ParenthesizedPatternSyntaxWrapper)node).Pattern), // is (false)
+            _ => false,
+        };
+
         private readonly record struct PathPosition(int Index, int TupleLength);
 
         private sealed class ControlFlowGraphCache : ControlFlowGraphCacheBase

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/BooleanLiteralUnnecessaryCodeFix.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/BooleanLiteralUnnecessaryCodeFix.cs
@@ -250,8 +250,7 @@ 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,
@@ -263,8 +262,7 @@ private static Document RewriteConditional(Document document, SyntaxNode root, S
                 return document.WithSyntaxRoot(newRoot);
             }
 
-            if (whenTrue.Equals(syntaxNode)
-                && CSharpEquivalenceChecker.AreEquivalent(syntaxNode, CSharpSyntaxHelper.FalseLiteralExpression))
+            if (whenTrue.Equals(syntaxNode) && syntaxNode.IsFalse())
             {
                 var newRoot = ReplaceExpressionWithBinary(
                     conditional,
@@ -278,8 +276,7 @@ private static Document RewriteConditional(Document document, SyntaxNode root, S
 
             var whenFalse = conditional.WhenFalse.RemoveParentheses();
 
-            if (whenFalse.Equals(syntaxNode)
-                && CSharpEquivalenceChecker.AreEquivalent(syntaxNode, CSharpSyntaxHelper.TrueLiteralExpression))
+            if (whenFalse.Equals(syntaxNode) && syntaxNode.IsTrue())
             {
                 var newRoot = ReplaceExpressionWithBinary(
                     conditional,
@@ -291,8 +288,7 @@ private static Document RewriteConditional(Document document, SyntaxNode root, S
                 return document.WithSyntaxRoot(newRoot);
             }
 
-            if (whenFalse.Equals(syntaxNode)
-                && CSharpEquivalenceChecker.AreEquivalent(syntaxNode, CSharpSyntaxHelper.FalseLiteralExpression))
+            if (whenFalse.Equals(syntaxNode) && syntaxNode.IsFalse())
             {
                 var newRoot = ReplaceExpressionWithBinary(
                     conditional,
@@ -310,12 +306,11 @@ private static Document RewriteConditional(Document document, SyntaxNode root, S
         private static ExpressionSyntax GetNegatedExpression(ExpressionSyntax expression)
         {
             var exp = expression;
-            if (expression is BinaryExpressionSyntax
-               || expression is ConditionalExpressionSyntax)
+            if (expression is BinaryExpressionSyntax or ConditionalExpressionSyntax
+                || IsPatternExpressionSyntaxWrapper.IsInstance(expression))
             {
                 exp = SyntaxFactory.ParenthesizedExpression(expression);
             }
-
             return SyntaxFactory.PrefixUnaryExpression(SyntaxKind.LogicalNotExpression, exp);
         }
     }

diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/BooleanLiteralUnnecessary.CSharp9.Fixed.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/BooleanLiteralUnnecessary.CSharp9.Fixed.cs
@@ -29,5 +29,17 @@ public bool SomeMethod2()
         {
             return true;
         }
+
+        // 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
@@ -29,5 +29,17 @@ public bool SomeMethod2()
         {
             return true;
         }
+
+        // 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
@@ -84,6 +84,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()

diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/BooleanLiteralUnnecessary.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/BooleanLiteralUnnecessary.cs
@@ -76,9 +76,6 @@ public BooleanLiteralUnnecessary(bool a, bool b, bool? c, Item item)
             booleanVariable = condition ? throw new Exception() : true; // Compliant, we don't raise for throw expressions
             booleanVariable = condition ? throw new Exception() : false; // Compliant, we don't raise for throw expressions
 
-            // Reproducer for https://github.com/SonarSource/sonar-dotnet/issues/2618
-            booleanVariable = item is Item myItem ? myItem.Required : false; // Noncompliant
-
             booleanVariable = condition ? exp : exp2;
 
             b = x || booleanVariable ? false : true; // Noncompliant
@@ -99,6 +96,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()