Fix S1125 FN: recognize "is" and "is not" keyword with constant patte…

…rn (#7687)
SonarSource · Aug 4, 2023 · 8f6674f · 8f6674f
1 parent 85c2441
commit 8f6674f
Show file tree

Hide file tree

Showing 20 changed files with 342 additions and 147 deletions.
diff --git a/...s/expected/Ember-MM/Ember Media Manager-{9B57D3AB-AF12-4012-B945-284C2448DC81}-S1125.json b/...s/expected/Ember-MM/Ember Media Manager-{9B57D3AB-AF12-4012-B945-284C2448DC81}-S1125.json
@@ -59,7 +59,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Ember-MM/Ember%20Media%20Manager/dlgManualEdit.vb#L259",
 "region":  {
 "startLine":  259,
-"startColumn":  24,
+"startColumn":  22,
 "endLine":  259,
 "endColumn":  29
 }
@@ -72,7 +72,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Ember-MM/Ember%20Media%20Manager/dlgManualEdit.vb#L376",
 "region":  {
 "startLine":  376,
-"startColumn":  22,
+"startColumn":  20,
 "endLine":  376,
 "endColumn":  27
 }

diff --git a/analyzers/its/expected/Ember-MM/EmberAPI-{208AA35E-C6AE-4D2D-A9DD-B6EFD19A4279}-S1125.json b/analyzers/its/expected/Ember-MM/EmberAPI-{208AA35E-C6AE-4D2D-A9DD-B6EFD19A4279}-S1125.json
@@ -46,7 +46,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Ember-MM/EmberAPI/clsAPIDatabase.vb#L112",
 "region":  {
 "startLine":  112,
-"startColumn":  64,
+"startColumn":  62,
 "endLine":  112,
 "endColumn":  69
 }

diff --git a/analyzers/its/expected/Nancy/Nancy--net452-S1125.json b/analyzers/its/expected/Nancy/Nancy--net452-S1125.json
@@ -20,7 +20,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Nancy/src/Nancy/TinyIoc/TinyIoC.cs#L3289",
 "region":  {
 "startLine":  3289,
-"startColumn":  76,
+"startColumn":  73,
 "endLine":  3289,
 "endColumn":  81
 }
@@ -111,7 +111,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Nancy/src/Nancy/ViewEngines/SuperSimpleViewEngine/SuperSimpleViewEngine.cs#L284",
 "region":  {
 "startLine":  284,
-"startColumn":  44,
+"startColumn":  41,
 "endLine":  284,
 "endColumn":  49
 }
@@ -124,7 +124,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Nancy/src/Nancy/ViewEngines/SuperSimpleViewEngine/SuperSimpleViewEngine.cs#L306",
 "region":  {
 "startLine":  306,
-"startColumn":  45,
+"startColumn":  42,
 "endLine":  306,
 "endColumn":  50
 }
@@ -137,7 +137,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Nancy/src/Nancy/ViewEngines/SuperSimpleViewEngine/SuperSimpleViewEngine.cs#L487",
 "region":  {
 "startLine":  487,
-"startColumn":  53,
+"startColumn":  50,
 "endLine":  487,
 "endColumn":  58
 }

diff --git a/analyzers/its/expected/Nancy/Nancy--netstandard2.0-S1125.json b/analyzers/its/expected/Nancy/Nancy--netstandard2.0-S1125.json
@@ -20,7 +20,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Nancy/src/Nancy/TinyIoc/TinyIoC.cs#L3289",
 "region":  {
 "startLine":  3289,
-"startColumn":  76,
+"startColumn":  73,
 "endLine":  3289,
 "endColumn":  81
 }
@@ -111,7 +111,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Nancy/src/Nancy/ViewEngines/SuperSimpleViewEngine/SuperSimpleViewEngine.cs#L284",
 "region":  {
 "startLine":  284,
-"startColumn":  44,
+"startColumn":  41,
 "endLine":  284,
 "endColumn":  49
 }
@@ -124,7 +124,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Nancy/src/Nancy/ViewEngines/SuperSimpleViewEngine/SuperSimpleViewEngine.cs#L306",
 "region":  {
 "startLine":  306,
-"startColumn":  45,
+"startColumn":  42,
 "endLine":  306,
 "endColumn":  50
 }
@@ -137,7 +137,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/Nancy/src/Nancy/ViewEngines/SuperSimpleViewEngine/SuperSimpleViewEngine.cs#L487",
 "region":  {
 "startLine":  487,
-"startColumn":  53,
+"startColumn":  50,
 "endLine":  487,
 "endColumn":  58
 }

diff --git a/analyzers/its/expected/akka.net/Akka--netstandard2.0-S1125.json b/analyzers/its/expected/akka.net/Akka--netstandard2.0-S1125.json
@@ -7,7 +7,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/akka.net/src/core/Akka/Configuration/Config.cs#L445",
 "region":  {
 "startLine":  445,
-"startColumn":  36,
+"startColumn":  33,
 "endLine":  445,
 "endColumn":  41
 }
@@ -20,7 +20,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/akka.net/src/core/Akka/Dispatch/Mailbox.cs#L385",
 "region":  {
 "startLine":  385,
-"startColumn":  80,
+"startColumn":  77,
 "endLine":  385,
 "endColumn":  85
 }

diff --git a/analyzers/its/expected/akka.net/Akka.Cluster--netstandard2.0-S1125.json b/analyzers/its/expected/akka.net/Akka.Cluster--netstandard2.0-S1125.json
@@ -7,7 +7,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/akka.net/src/core/Akka.Cluster/ClusterHeartbeat.cs#L663",
 "region":  {
 "startLine":  663,
-"startColumn":  48,
+"startColumn":  45,
 "endLine":  663,
 "endColumn":  53
 }

diff --git a/analyzers/its/expected/akka.net/Akka.MultiNodeTestRunner.Shared--netstandard2.0-S1125.json b/analyzers/its/expected/akka.net/Akka.MultiNodeTestRunner.Shared--netstandard2.0-S1125.json
@@ -7,7 +7,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/akka.net/src/core/Akka.MultiNodeTestRunner.Shared/Sinks/ConsoleMessageSinkActor.cs#L54",
 "region":  {
 "startLine":  54,
-"startColumn":  57,
+"startColumn":  54,
 "endLine":  54,
 "endColumn":  62
 }
@@ -20,7 +20,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/akka.net/src/core/Akka.MultiNodeTestRunner.Shared/Sinks/ConsoleMessageSinkActor.cs#L59",
 "region":  {
 "startLine":  59,
-"startColumn":  71,
+"startColumn":  68,
 "endLine":  59,
 "endColumn":  76
 }

diff --git a/analyzers/its/expected/akka.net/Akka.Remote.TestKit--netstandard2.0-S1125.json b/analyzers/its/expected/akka.net/Akka.Remote.TestKit--netstandard2.0-S1125.json
@@ -7,7 +7,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/akka.net/src/core/Akka.Remote.TestKit/Player.cs#L510",
 "region":  {
 "startLine":  510,
-"startColumn":  113,
+"startColumn":  110,
 "endLine":  510,
 "endColumn":  118
 }

diff --git a/analyzers/its/expected/akka.net/Akka.TestKit--netstandard2.0-S1125.json b/analyzers/its/expected/akka.net/Akka.TestKit--netstandard2.0-S1125.json
@@ -7,7 +7,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/akka.net/src/core/Akka.TestKit/EventFilter/Internal/EventFilterApplier.cs#L410",
 "region":  {
 "startLine":  410,
-"startColumn":  42,
+"startColumn":  39,
 "endLine":  410,
 "endColumn":  47
 }
@@ -20,7 +20,7 @@
 "uri":  "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/sources/akka.net/src/core/Akka.TestKit/EventFilter/Internal/EventFilterApplier.cs#L433",
 "region":  {
 "startLine":  433,
-"startColumn":  42,
+"startColumn":  39,
 "endLine":  433,
 "endColumn":  47
 }

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/BooleanLiteralUnnecessary.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/BooleanLiteralUnnecessary.cs
@@ -21,32 +21,48 @@
 namespace SonarAnalyzer.Rules.CSharp
 {
     [DiagnosticAnalyzer(LanguageNames.CSharp)]
-    public sealed class BooleanLiteralUnnecessary : BooleanLiteralUnnecessaryBase<BinaryExpressionSyntax, SyntaxKind>
+    public sealed class BooleanLiteralUnnecessary : BooleanLiteralUnnecessaryBase<SyntaxKind>
     {
         protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;
 
-        protected override bool IsBooleanLiteral(SyntaxNode node) => IsTrueLiteralKind(node) || IsFalseLiteralKind(node);
-
-        protected override SyntaxNode GetLeftNode(BinaryExpressionSyntax binaryExpression) => binaryExpression.Left;
-
-        protected override SyntaxNode GetRightNode(BinaryExpressionSyntax binaryExpression) => binaryExpression.Right;
-
-        protected override SyntaxToken GetOperatorToken(BinaryExpressionSyntax binaryExpression) => binaryExpression.OperatorToken;
+        protected override SyntaxToken? GetOperatorToken(SyntaxNode node) =>
+            node switch
+            {
+                BinaryExpressionSyntax binary => binary.OperatorToken,
+                _ when IsPatternExpressionSyntaxWrapper.IsInstance(node) => ((IsPatternExpressionSyntaxWrapper)node).IsKeyword,
+                _ => null,
+            };
 
-        protected override bool IsTrueLiteralKind(SyntaxNode syntaxNode) => syntaxNode.IsKind(SyntaxKind.TrueLiteralExpression);
+        protected override bool IsTrue(SyntaxNode syntaxNode) => syntaxNode.IsTrue();
 
-        protected override bool IsFalseLiteralKind(SyntaxNode syntaxNode) => syntaxNode.IsKind(SyntaxKind.FalseLiteralExpression);
+        protected override bool IsFalse(SyntaxNode syntaxNode) => syntaxNode.IsFalse();
 
-        protected override bool IsInsideTernaryWithThrowExpression(BinaryExpressionSyntax syntaxNode) =>
+        protected override bool IsInsideTernaryWithThrowExpression(SyntaxNode syntaxNode) =>
             syntaxNode.Parent is ConditionalExpressionSyntax conditionalExpression
             && (IsThrowExpression(conditionalExpression.WhenTrue) || IsThrowExpression(conditionalExpression.WhenFalse));
 
+        protected override SyntaxNode GetLeftNode(SyntaxNode node) =>
+            node switch
+            {
+                BinaryExpressionSyntax binaryExpression => binaryExpression.Left,
+                _ when IsPatternExpressionSyntaxWrapper.IsInstance(node) => ((IsPatternExpressionSyntaxWrapper)node).Expression,
+                _ => null
+            };
+
+        protected override SyntaxNode GetRightNode(SyntaxNode node) =>
+            node switch
+            {
+                BinaryExpressionSyntax binaryExpression => binaryExpression.Right,
+                _ when IsPatternExpressionSyntaxWrapper.IsInstance(node) => ((IsPatternExpressionSyntaxWrapper)node).Pattern,
+                _ => null
+            };
+
         protected override void Initialize(SonarAnalysisContext context)
         {
             context.RegisterNodeAction(CheckLogicalNot, SyntaxKind.LogicalNotExpression);
             context.RegisterNodeAction(CheckAndExpression, SyntaxKind.LogicalAndExpression);
             context.RegisterNodeAction(CheckOrExpression, SyntaxKind.LogicalOrExpression);
-            context.RegisterNodeAction(CheckEquals, SyntaxKind.EqualsExpression);
+            context.RegisterNodeAction(CheckEquals, SyntaxKind.EqualsExpression, SyntaxKindEx.IsPatternExpression);
             context.RegisterNodeAction(CheckNotEquals, SyntaxKind.NotEqualsExpression);
             context.RegisterNodeAction(CheckConditional, SyntaxKind.ConditionalExpression);
             context.RegisterNodeAction(CheckForLoopCondition, SyntaxKind.ForStatement);
@@ -67,13 +83,10 @@ private void CheckLogicalNot(SonarSyntaxNodeReportingContext context)
         {
             var logicalNot = (PrefixUnaryExpressionSyntax)context.Node;
             var logicalNotOperand = logicalNot.Operand.RemoveParentheses();
-            if (IsBooleanLiteral(logicalNotOperand))
+            if (IsTrue(logicalNotOperand) || IsFalse(logicalNotOperand))
             {
                 context.ReportIssue(Diagnostic.Create(Rule, logicalNot.Operand.GetLocation()));
             }
-
-            static bool IsBooleanLiteral(SyntaxNode node) =>
-                node.IsKind(SyntaxKind.TrueLiteralExpression) || node.IsKind(SyntaxKind.FalseLiteralExpression);
         }
 
         private void CheckConditional(SonarSyntaxNodeReportingContext context)
@@ -94,7 +107,7 @@ private void CheckConditional(SonarSyntaxNodeReportingContext context)
             {
                 return;
             }
-            CheckTernaryExpressionBranches(context, conditional.SyntaxTree, whenTrue, whenFalse);
+            CheckTernaryExpressionBranches(context, whenTrue, whenFalse);
         }
 
         private static bool IsThrowExpression(ExpressionSyntax expressionSyntax) =>

diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/BooleanLiteralUnnecessaryCodeFix.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/BooleanLiteralUnnecessaryCodeFix.cs
@@ -20,6 +20,7 @@
 
 using Microsoft.CodeAnalysis.CodeFixes;
 using Microsoft.CodeAnalysis.Formatting;
+using Microsoft.CodeAnalysis.Simplification;
 
 namespace SonarAnalyzer.Rules.CSharp
 {
@@ -54,6 +55,11 @@ protected override Task RegisterCodeFixesAsync(SyntaxNode root, SonarCodeFixCont
                 return Task.CompletedTask;
             }
 
+            if (IsPatternExpressionSyntaxWrapper.IsInstance(syntaxNode))
+            {
+                RegisterPatternExpressionReplacement(context, root, (IsPatternExpressionSyntaxWrapper)syntaxNode);
+            }
+
             if (syntaxNode is not LiteralExpressionSyntax literal)
             {
                 return Task.CompletedTask;
@@ -86,6 +92,31 @@ protected override Task RegisterCodeFixesAsync(SyntaxNode root, SonarCodeFixCont
             return Task.CompletedTask;
         }
 
+        private static void RegisterPatternExpressionReplacement(SonarCodeFixContext context, SyntaxNode root, IsPatternExpressionSyntaxWrapper patternExpression)
+        {
+            var replacement = patternExpression.Pattern.SyntaxNode.IsTrue()
+                ? patternExpression.Expression
+                : GetNegatedExpression(patternExpression.Expression);
+
+            if (replacement.IsTrue())
+            {
+                replacement = CSharpSyntaxHelper.TrueLiteralExpression;
+            }
+            else if (replacement.IsFalse())
+            {
+                replacement = CSharpSyntaxHelper.FalseLiteralExpression;
+            }
+
+            context.RegisterCodeFix(
+                Title,
+                c =>
+                {
+                    var newRoot = root.ReplaceNode(patternExpression.SyntaxNode, replacement.WithAdditionalAnnotations(Formatter.Annotation));
+                    return Task.FromResult(context.Document.WithSyntaxRoot(newRoot));
+                },
+                context.Diagnostics);
+        }
+
         private static void RegisterForStatementConditionRemoval(SonarCodeFixContext context, SyntaxNode root, ForStatementSyntax forStatement) =>
             context.RegisterCodeFix(
                 Title,
@@ -109,7 +140,7 @@ private static void RegisterBinaryExpressionRemoval(SonarCodeFixContext context,
                 Title,
                 c =>
                 {
-                    var newExpression = GetNegatedExpression(otherNode);
+                    var newExpression = GetNegatedExpression(otherNode).WithAdditionalAnnotations(Simplifier.Annotation);
                     var newRoot = root.ReplaceNode(binaryParent, newExpression
                         .WithAdditionalAnnotations(Formatter.Annotation));
 
@@ -141,7 +172,7 @@ private static void RegisterBinaryExpressionReplacement(SonarCodeFixContext cont
                 Title,
                 c =>
                 {
-                    var keepThisNode = FindNodeToKeep(binary);
+                    var keepThisNode = FindNodeToKeep(binary).WithAdditionalAnnotations(Simplifier.Annotation);
                     var newRoot = root.ReplaceNode(syntaxNode, keepThisNode
                         .WithAdditionalAnnotations(Formatter.Annotation));
                     return Task.FromResult(context.Document.WithSyntaxRoot(newRoot));