Code review

SonarSource · Oct 24, 2023 · cf4817a · cf4817a
1 parent 540384d
commit cf4817a
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 15 deletions.
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpFacade.cs
@@ -60,7 +60,9 @@ public IMethodParameterLookup MethodParameterLookup(SyntaxNode invocation, IMeth
         };
 
     public IMethodParameterLookup MethodParameterLookup(SyntaxNode invocation, SemanticModel semanticModel) =>
-        invocation != null ? new CSharpMethodParameterLookup(invocation.ArgumentList(), semanticModel) : null;
+        invocation?.ArgumentList() is { } argumentList
+            ? new CSharpMethodParameterLookup(argumentList, semanticModel)
+            : null;
 
     public string GetName(SyntaxNode expression) =>
         expression.GetName();

diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs
@@ -27,16 +27,16 @@ public abstract class ParametersCorrectOrderBase<TSyntaxKind> : SonarDiagnosticA
         protected abstract TSyntaxKind[] InvocationKinds { get; }
         protected override string MessageFormat => "Parameters to '{0}' have the same names but not the same order as the method arguments.";
 
-        protected ParametersCorrectOrderBase() : base(DiagnosticId)
-        {
-        }
+        protected ParametersCorrectOrderBase() : base(DiagnosticId) { }
 
         protected sealed override void Initialize(SonarAnalysisContext context) =>
             context.RegisterNodeAction(Language.GeneratedCodeRecognizer,
                 c =>
                 {
+                    const int MinNumberOfNameableArguments = 2;
                     if (!c.IsRedundantPrimaryConstructorBaseTypeContext()
-                        && Language.Syntax.ArgumentList(c.Node) is { Count: >= 2 } argumentList // there must be at least two arguments to be able to swap
+                        && Language.Syntax.ArgumentList(c.Node) is { Count: >= MinNumberOfNameableArguments } argumentList  // there must be at least two arguments to be able to swap, and further
+                        && argumentList.Select(ArgumentName).WhereNotNull().Take(MinNumberOfNameableArguments).Count() == MinNumberOfNameableArguments // at least two arguments with a "name"
                         && Language.MethodParameterLookup(c.Node, c.SemanticModel) is var methodParameterLookup)
                     {
                         foreach (var argument in argumentList)

diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicFacade.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicFacade.cs
@@ -51,10 +51,14 @@ public object FindConstantValue(SemanticModel model, SyntaxNode node) =>
         node.FindConstantValue(model);
 
     public IMethodParameterLookup MethodParameterLookup(SyntaxNode invocation, IMethodSymbol methodSymbol) =>
-        invocation != null ? new VisualBasicMethodParameterLookup(invocation.ArgumentList(), methodSymbol) : null;
+        invocation?.ArgumentList() is { } argumentList
+            ? new VisualBasicMethodParameterLookup(argumentList, methodSymbol)
+            : null;
 
     public IMethodParameterLookup MethodParameterLookup(SyntaxNode invocation, SemanticModel semanticModel) =>
-        invocation != null ? new VisualBasicMethodParameterLookup(invocation.ArgumentList(), semanticModel) : null;
+        invocation?.ArgumentList() is { } argumentList
+            ? new VisualBasicMethodParameterLookup(argumentList, semanticModel)
+            : null;
 
     public string GetName(SyntaxNode expression) =>
         expression.GetName();

diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Facade/SyntaxFacadeTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Facade/SyntaxFacadeTest.cs
@@ -155,6 +155,15 @@ public void ArgumentNameColon_VB_OmittedArgument()
         vb.ArgumentNameColon(argument).Should().BeNull();
     }
 
+    [TestMethod]
+    public void ArgumentNameColon_VB_RangeArgument()
+    {
+        var literal1 = VB.SyntaxFactory.LiteralExpression(VB.SyntaxKind.NumericLiteralExpression, VB.SyntaxFactory.Literal(1));
+        var literal2 = literal1.WithToken(VB.SyntaxFactory.Literal(2));
+        var argument = VB.SyntaxFactory.RangeArgument(literal1, literal2);
+        vb.ArgumentNameColon(argument).Should().BeNull();
+    }
+
     [TestMethod]
     public void ArgumentNameColon_VB_UnsupportedSyntaxKind()
     {

diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.CSharp12.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.CSharp12.cs
@@ -5,19 +5,25 @@ namespace Repro_8071
 {
     class BaseConstructor
     {
-        class Base(int a, int b) // Secondary [Base1, Base2, Base3, Base4, Base5, Base6]
+        class Base(int a, int b) // Secondary [Base1, Base2, Base3, Base4, Base5, Base6, Base7, Base8]
         {
-            Base(int a, int b, string c) : this(b, a) { }  // Noncompliant [Base1]
-            //                             ^^^^
-            Base(string c, int a, int b) : this(b, a) { }  // Noncompliant [Base2]
+            Base(int a, int b, string c) : this(b, a) { }                                            // Noncompliant [Base1]
+            //                             ^^^^                                                      
+            Base(string c, int a, int b) : this(b, a) { }                                            // Noncompliant [Base2]
         }
 
-        class ParamsFullyInverted(int a, int b) : Base(b, a);                                    // Noncompliant [Base3]
+        class ParamsFullyInverted(int a, int b) : Base(b, a);                                        // Noncompliant [Base3]
         //                                        ^^^^
-        class ParamsPartiallyInverted(int a, int b, int c) : BaseConstructor.Base(b, a);         // Noncompliant [Base4]
+        class ParamsPartiallyInverted(int a, int b, int c) : BaseConstructor.Base(b, a);             // Noncompliant [Base4]
         //                                                                   ^^^^
-        class ParamsFullyInvertedWithAdditionalParamAfter(int a, int b, string s) : Base(b, a);  // Noncompliant [Base5]
-        class ParamsFullyInvertedWithAdditionalParamBefore(string s, int a, int b) : Base(b, a); // Noncompliant [Base6]
+        class ParamsPartiallyInvertedWithAdditionalParamAfter(int a, int b, string s) : Base(b, a);  // Noncompliant [Base5]
+        class ParamsPartiallyInvertedWithAdditionalParamBefore(string s, int a, int b) : Base(b, a); // Noncompliant [Base6]
+
+        Base MyMethod(int b, int a)
+        {
+            _ = new Base(b, a);                                                                      // Noncompliant [Base7]
+            return new(b, a);                                                                        // Noncompliant [Base8]
+        }
     }
 
     class WithRecordStructs