From dd03b74861729168f92e554e55508635655d6f24 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Fri, 20 Oct 2023 16:20:34 +0200 Subject: [PATCH 01/29] Rework S2234 --- .../Facade/CSharpFacade.cs | 4 +- .../Facade/CSharpSyntaxFacade.cs | 17 +- .../Rules/ParametersCorrectOrder.cs | 69 +------ .../SonarSyntaxNodeReportingContext.cs | 15 ++ .../Facade/SyntaxFacade.cs | 4 +- .../Helpers/MethodParameterLookupBase.cs | 1 + .../Rules/ParametersCorrectOrderBase.cs | 176 +++++------------- .../Facade/VisualBasicSyntaxFacade.cs | 17 +- .../Rules/ParametersCorrectOrder.cs | 65 +------ .../Rules/ParametersCorrectOrderTest.cs | 18 ++ .../ParametersCorrectOrder.CSharp12.cs | 38 ++-- .../TestCases/ParametersCorrectOrder.cs | 6 +- .../TestCases/ParametersCorrectOrder.vb | 6 +- 13 files changed, 154 insertions(+), 282 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpFacade.cs index df5a08632d0..7f360294900 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpFacade.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpFacade.cs @@ -68,9 +68,9 @@ private static ArgumentListSyntax GetArgumentList(SyntaxNode invocation) => ArgumentListSyntax x => x, ObjectCreationExpressionSyntax x => x.ArgumentList, InvocationExpressionSyntax x => x.ArgumentList, - _ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(invocation) => - ((ImplicitObjectCreationExpressionSyntaxWrapper)invocation).ArgumentList, ConstructorInitializerSyntax x => x.ArgumentList, + _ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(invocation) => ((ImplicitObjectCreationExpressionSyntaxWrapper)invocation).ArgumentList, + _ when PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(invocation) => ((PrimaryConstructorBaseTypeSyntaxWrapper)invocation).ArgumentList, _ => throw new ArgumentException($"{invocation.GetType()} does not contain an ArgumentList.", nameof(invocation)), }; diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs index 7c88f189f19..32e7a887394 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs @@ -26,14 +26,22 @@ public override bool AreEquivalent(SyntaxNode firstNode, SyntaxNode secondNode) SyntaxFactory.AreEquivalent(firstNode, secondNode); public override IEnumerable ArgumentExpressions(SyntaxNode node) => + ArgumentList(node)?.OfType().Select(x => x.Expression) ?? Enumerable.Empty(); + + public override IReadOnlyList ArgumentList(SyntaxNode node) => node switch { - ObjectCreationExpressionSyntax creation => creation.ArgumentList?.Arguments.Select(x => x.Expression) ?? Enumerable.Empty(), - null => Enumerable.Empty(), - var _ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(node) - => ((ImplicitObjectCreationExpressionSyntaxWrapper)node).ArgumentList?.Arguments.Select(x => x.Expression) ?? Enumerable.Empty(), + ObjectCreationExpressionSyntax creation => creation.ArgumentList.Arguments, + InvocationExpressionSyntax invocation => invocation.ArgumentList.Arguments, + ConstructorInitializerSyntax constructorInitializer => constructorInitializer.ArgumentList.Arguments, + null => ImmutableArray.Empty, + _ when PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(node) => ((PrimaryConstructorBaseTypeSyntaxWrapper)node).ArgumentList.Arguments, + _ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(node) + => ((ImplicitObjectCreationExpressionSyntaxWrapper)node).ArgumentList.Arguments, _ => throw InvalidOperation(node, nameof(ArgumentExpressions)) }; + public override SyntaxToken? ArgumentNameColon(SyntaxNode argument) => + (argument as ArgumentSyntax)?.NameColon?.Name?.Identifier; public override int? ArgumentIndex(SyntaxNode argument) => Cast(argument).GetArgumentIndex(); @@ -126,6 +134,7 @@ public override SyntaxNode NodeExpression(SyntaxNode node) => node switch { ArrowExpressionClauseSyntax x => x.Expression, + ArgumentSyntax argument => argument.Expression, AttributeArgumentSyntax x => x.Expression, InterpolationSyntax x => x.Expression, InvocationExpressionSyntax x => x.Expression, diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs index 0a992f109d8..0428209d232 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs @@ -21,66 +21,17 @@ namespace SonarAnalyzer.Rules.CSharp { [DiagnosticAnalyzer(LanguageNames.CSharp)] - public sealed class ParametersCorrectOrder : ParametersCorrectOrderBase + public sealed class ParametersCorrectOrder : ParametersCorrectOrderBase { - private static readonly DiagnosticDescriptor rule = - DescriptorFactory.Create(DiagnosticId, MessageFormat); - public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(rule); - - protected override void Initialize(SonarAnalysisContext context) - { - context.RegisterNodeAction( - c => - { - var methodCall = (InvocationExpressionSyntax)c.Node; - - var memberAccess = methodCall.Expression as MemberAccessExpressionSyntax; - Location getLocation() => - memberAccess == null - ? methodCall.Expression.GetLocation() - : memberAccess.Name.GetLocation(); - - AnalyzeArguments(c, methodCall.ArgumentList, getLocation); - }, SyntaxKind.InvocationExpression); - - context.RegisterNodeAction( - c => - { - var objectCreationCall = (ObjectCreationExpressionSyntax)c.Node; - - var qualifiedAccess = objectCreationCall.Type as QualifiedNameSyntax; - Location getLocation() => - qualifiedAccess == null - ? objectCreationCall.Type.GetLocation() - : qualifiedAccess.Right.GetLocation(); - - AnalyzeArguments(c, objectCreationCall.ArgumentList, getLocation); - }, SyntaxKind.ObjectCreationExpression); - } - - private void AnalyzeArguments(SonarSyntaxNodeReportingContext analysisContext, ArgumentListSyntax argumentList, - Func getLocation) - { - if (argumentList == null) + protected override SyntaxKind[] InvocationKinds => new[] { - return; - } - - var methodParameterLookup = new CSharpMethodParameterLookup(argumentList, analysisContext.SemanticModel); - - ReportIncorrectlyOrderedParameters(analysisContext, methodParameterLookup, argumentList.Arguments, getLocation); - } - - protected override TypeInfo GetArgumentTypeSymbolInfo(ArgumentSyntax argument, SemanticModel semanticModel) => - semanticModel.GetTypeInfo(argument.Expression); - - protected override Location GetMethodDeclarationIdentifierLocation(SyntaxNode syntaxNode) => - (syntaxNode as BaseMethodDeclarationSyntax)?.FindIdentifierLocation(); - - protected override SyntaxToken? GetArgumentIdentifier(ArgumentSyntax argument) => - (argument.Expression as IdentifierNameSyntax)?.Identifier; - - protected override SyntaxToken? GetNameColonArgumentIdentifier(ArgumentSyntax argument) => - argument.NameColon?.Name.Identifier; + SyntaxKind.InvocationExpression, + SyntaxKind.ObjectCreationExpression, + SyntaxKind.ThisConstructorInitializer, + SyntaxKindEx.PrimaryConstructorBaseType, + SyntaxKindEx.ImplicitObjectCreationExpression, + }; + + protected override ILanguageFacade Language => CSharpFacade.Instance; } } diff --git a/analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarSyntaxNodeReportingContext.cs b/analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarSyntaxNodeReportingContext.cs index 1166a774f72..8872c37dc98 100644 --- a/analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarSyntaxNodeReportingContext.cs +++ b/analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarSyntaxNodeReportingContext.cs @@ -46,6 +46,21 @@ internal SonarSyntaxNodeReportingContext(SonarAnalysisContext analysisContext, S public bool IsRedundantPositionalRecordContext() => Context.ContainingSymbol.Kind == SymbolKind.Method; + /// + /// Roslyn invokes the analyzer twice for PrimaryConstructorBaseType. The ContainingSymbol is first the type and second the constructor. This check filters can be used to filter + /// the first invocation. See also #roslyn/70488 + /// + /// + /// Returns for the invocation on the class declaration and for the ctor invocation. + /// + public bool IsRedundantPrimaryConstructorBaseTypeContext() => + Context.Compilation.Language == LanguageNames.CSharp + && Context is + { + Node.RawKind: (int)SyntaxKindEx.PrimaryConstructorBaseType, + ContainingSymbol.Kind: SymbolKind.Method, + }; + public bool IsAzureFunction() => AzureFunctionMethod() is not null; diff --git a/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs b/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs index 8d0c934e559..cbec044afbd 100644 --- a/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs @@ -24,8 +24,10 @@ public abstract class SyntaxFacade where TSyntaxKind : struct { public abstract bool AreEquivalent(SyntaxNode firstNode, SyntaxNode secondNode); + public abstract SyntaxToken? ArgumentNameColon(SyntaxNode argument); public abstract IEnumerable ArgumentExpressions(SyntaxNode node); public abstract int? ArgumentIndex(SyntaxNode argument); + public abstract IReadOnlyList ArgumentList(SyntaxNode node); public abstract SyntaxNode AssignmentLeft(SyntaxNode assignment); public abstract SyntaxNode AssignmentRight(SyntaxNode assignment); public abstract ImmutableArray AssignmentTargets(SyntaxNode assignment); @@ -35,8 +37,6 @@ public abstract class SyntaxFacade public abstract SyntaxNode CastExpression(SyntaxNode cast); public abstract ComparisonKind ComparisonKind(SyntaxNode node); public abstract IEnumerable EnumMembers(SyntaxNode @enum); - public abstract ImmutableArray FieldDeclarationIdentifiers(SyntaxNode node); - public abstract bool HasExactlyNArguments(SyntaxNode invocation, int count); public abstract SyntaxToken? InvocationIdentifier(SyntaxNode invocation); public abstract bool IsAnyKind(SyntaxNode node, ISet syntaxKinds); public abstract bool IsAnyKind(SyntaxNode node, params TSyntaxKind[] syntaxKinds); diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/MethodParameterLookupBase.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/MethodParameterLookupBase.cs index 03b9589b9ff..81ec92ab972 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/MethodParameterLookupBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/MethodParameterLookupBase.cs @@ -26,6 +26,7 @@ public interface IMethodParameterLookup bool TryGetSyntax(IParameterSymbol parameter, out ImmutableArray expressions); bool TryGetSyntax(string parameterName, out ImmutableArray expressions); bool TryGetNonParamsSyntax(IParameterSymbol parameter, out SyntaxNode expression); + IMethodSymbol MethodSymbol { get; } } // This should come from the Roslyn API (https://github.com/dotnet/roslyn/issues/9) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs index e01a83afc53..d09b7b07914 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs @@ -18,141 +18,63 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using Microsoft.CodeAnalysis; +using SonarAnalyzer.Extensions; + namespace SonarAnalyzer.Rules { - public abstract class ParametersCorrectOrderBase : SonarDiagnosticAnalyzer - where TArgumentSyntax : SyntaxNode + public abstract class ParametersCorrectOrderBase : SonarDiagnosticAnalyzer + where TSyntaxKind : struct { protected const string DiagnosticId = "S2234"; - protected const string MessageFormat = "Parameters to '{0}' have the same names but not the same order as the method arguments."; - - protected abstract TypeInfo GetArgumentTypeSymbolInfo(TArgumentSyntax argument, SemanticModel semanticModel); - protected abstract Location GetMethodDeclarationIdentifierLocation(SyntaxNode syntaxNode); - protected abstract SyntaxToken? GetArgumentIdentifier(TArgumentSyntax argument); - protected abstract SyntaxToken? GetNameColonArgumentIdentifier(TArgumentSyntax argument); + 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."; - internal void ReportIncorrectlyOrderedParameters(SonarSyntaxNodeReportingContext analysisContext, - MethodParameterLookupBase methodParameterLookup, SeparatedSyntaxList argumentList, - Func getLocationToReport) + protected ParametersCorrectOrderBase() : base(DiagnosticId) { - var argumentParameterMappings = methodParameterLookup.GetAllArgumentParameterMappings() - .ToDictionary(pair => pair.Node, pair => pair.Symbol); - - var methodSymbol = methodParameterLookup.MethodSymbol; - if (methodSymbol == null) - { - return; - } - - var parameterNames = argumentParameterMappings.Values - .Select(symbol => symbol.Name) - .Distinct() - .ToList(); - - var argumentIdentifiers = argumentList - .Select(argument => ConvertToArgumentIdentifier(argument)) - .ToList(); - var identifierNames = argumentIdentifiers - .Select(p => p.IdentifierName) - .ToList(); - - if (parameterNames.Intersect(identifierNames).Any() && - HasIncorrectlyOrderedParameters(argumentIdentifiers, argumentParameterMappings, parameterNames, identifierNames, - analysisContext.SemanticModel)) - { - // for VB the symbol does not contain the method syntax reference - var secondaryLocations = methodSymbol.DeclaringSyntaxReferences - .Select(s => GetMethodDeclarationIdentifierLocation(s.GetSyntax())) - .WhereNotNull(); - - analysisContext.ReportIssue(SupportedDiagnostics[0].CreateDiagnostic(analysisContext.Compilation, getLocationToReport(), secondaryLocations, properties: null, methodSymbol.Name)); - } } - private bool HasIncorrectlyOrderedParameters(List argumentIdentifiers, - Dictionary argumentParameterMappings, List parameterNames, - List identifierNames, SemanticModel semanticModel) - { - for (var i = 0; i < argumentIdentifiers.Count; i++) - { - var argumentIdentifier = argumentIdentifiers[i]; - var identifierName = argumentIdentifier.IdentifierName; - var parameter = argumentParameterMappings[argumentIdentifier.ArgumentSyntax]; - var parameterName = parameter.Name; - - if (string.IsNullOrEmpty(identifierName) || - !parameterNames.Contains(identifierName) || - !IdentifierWithSameNameAndTypeExists(parameter)) + protected override void Initialize(SonarAnalysisContext context) => + context.RegisterNodeAction(Language.GeneratedCodeRecognizer, + c => { - continue; - } - - if (argumentIdentifier is PositionalArgumentIdentifier positional && - (parameter.IsParams || identifierName == parameterName)) - { - continue; - } - - if (argumentIdentifier is NamedArgumentIdentifier named && - (!identifierNames.Contains(named.DeclaredName) || named.DeclaredName == named.IdentifierName)) - { - continue; - } - - return true; - } - - return false; - - bool IdentifierWithSameNameAndTypeExists(IParameterSymbol parameter) => - argumentIdentifiers.Any(ia => - ia.IdentifierName == parameter.Name && - GetArgumentTypeSymbolInfo(ia.ArgumentSyntax, semanticModel).ConvertedType.DerivesOrImplements(parameter.Type)); - } - - private ArgumentIdentifier ConvertToArgumentIdentifier(TArgumentSyntax argument) - { - var identifierName = GetArgumentIdentifier(argument)?.Text; - var nameColonIdentifier = GetNameColonArgumentIdentifier(argument); - - if (nameColonIdentifier == null) - { - return new PositionalArgumentIdentifier(identifierName, argument); - } - - return new NamedArgumentIdentifier(identifierName, argument, nameColonIdentifier.Value.Text); - } - - private class ArgumentIdentifier - { - protected ArgumentIdentifier(string identifierName, TArgumentSyntax argumentSyntax) - { - IdentifierName = identifierName; - ArgumentSyntax = argumentSyntax; - } - - public string IdentifierName { get; } - public TArgumentSyntax ArgumentSyntax { get; } - } - - private class PositionalArgumentIdentifier : ArgumentIdentifier - { - public PositionalArgumentIdentifier(string identifierName, TArgumentSyntax argumentSyntax) - : base(identifierName, argumentSyntax) - { - } - } - - private class NamedArgumentIdentifier : ArgumentIdentifier - { - public NamedArgumentIdentifier(string identifierName, TArgumentSyntax argumentSyntax, string declaredName) - : base(identifierName, argumentSyntax) - { - DeclaredName = declaredName; - } - - public string DeclaredName { get; } - } + if (c.IsRedundantPrimaryConstructorBaseTypeContext()) + { + return; + } + var arguments = Language.Syntax.ArgumentList(c.Node); + IMethodParameterLookup methodParameterLookup = null; + foreach (var argument in arguments) + { + methodParameterLookup ??= Language.MethodParameterLookup(c.Node, c.SemanticModel); + // Example void M(int x, int y) <- p_x and p_y are the parameter + // M(y, x) <- a_x and a_y are the arguments + if (methodParameterLookup.TryGetSymbol(argument, out var parameterSymbol) // argument = a_x and parameterSymbol = p_y + && parameterSymbol is { IsParams: false } + && ArgumentName(argument) is { } argumentName // "x" + && !MatchingNames(parameterSymbol, argumentName) // "x" != "y" + && Language.Syntax.NodeExpression(argument) is { } argumentExpression + && c.Context.SemanticModel.GetTypeInfo(argumentExpression).ConvertedType is { } argumentType + && methodParameterLookup.MethodSymbol.Parameters.FirstOrDefault(p => MatchingNames(p, argumentName) && argumentType.DerivesOrImplements(p.Type)) is + { IsParams: false } // p_x (there is another parameter that seems to be a better fit) + && arguments.FirstOrDefault(x => MatchingNames(parameterSymbol, ArgumentName(x))) is { }) // Look if there is an argument that matches the parameter p_y (yes: a_y) + { + var secondaryLocations = methodParameterLookup.MethodSymbol.DeclaringSyntaxReferences + .Select(s => Language.Syntax.NodeIdentifier(s.GetSyntax())?.GetLocation()) + .WhereNotNull(); + + c.ReportIssue(SupportedDiagnostics[0].CreateDiagnostic(c.Compilation, c.Node.GetLocation(), secondaryLocations, properties: null, methodParameterLookup.MethodSymbol.Name)); + return; + } + } + }, InvocationKinds); + + private static bool MatchingNames(IParameterSymbol parameter, string argumentName) => + parameter.Name == argumentName; + + private string ArgumentName(SyntaxNode argument) => + Language.Syntax.NodeIdentifier(Language.Syntax.NodeExpression(argument)) is SyntaxToken identifier + ? identifier.ValueText + : null; } } - diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs index 20fc27cd524..3e93d5a1fc5 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs @@ -18,6 +18,8 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using System.Collections.Generic; + namespace SonarAnalyzer.Helpers.Facade; internal sealed class VisualBasicSyntaxFacade : SyntaxFacade @@ -26,16 +28,23 @@ public override bool AreEquivalent(SyntaxNode firstNode, SyntaxNode secondNode) SyntaxFactory.AreEquivalent(firstNode, secondNode); public override IEnumerable ArgumentExpressions(SyntaxNode node) => - node switch + ArgumentList(node).OfType().Select(x => x.GetExpression()).WhereNotNull(); + + public override IReadOnlyList ArgumentList(SyntaxNode node) => + (node switch { - ObjectCreationExpressionSyntax creation => creation.ArgumentList?.Arguments.Select(x => x.GetExpression()) ?? Enumerable.Empty(), - null => Enumerable.Empty(), + ObjectCreationExpressionSyntax creation => creation.ArgumentList, + InvocationExpressionSyntax invocation => invocation.ArgumentList, + null => null, _ => throw InvalidOperation(node, nameof(ArgumentExpressions)) - }; + })?.Arguments ?? (IReadOnlyList)Array.Empty(); public override int? ArgumentIndex(SyntaxNode argument) => Cast(argument).GetArgumentIndex(); + public override SyntaxToken? ArgumentNameColon(SyntaxNode argument) => + (argument as SimpleArgumentSyntax)?.NameColonEquals?.Name.Identifier; + public override SyntaxNode AssignmentLeft(SyntaxNode assignment) => Cast(assignment).Left; diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ParametersCorrectOrder.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ParametersCorrectOrder.cs index 2169162b176..2c4e651fc1a 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ParametersCorrectOrder.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ParametersCorrectOrder.cs @@ -21,67 +21,14 @@ namespace SonarAnalyzer.Rules.VisualBasic { [DiagnosticAnalyzer(LanguageNames.VisualBasic)] - public sealed class ParametersCorrectOrder : ParametersCorrectOrderBase + public sealed class ParametersCorrectOrder : ParametersCorrectOrderBase { - private static readonly DiagnosticDescriptor rule = - DescriptorFactory.Create(DiagnosticId, MessageFormat); - public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(rule); - - protected override void Initialize(SonarAnalysisContext context) - { - context.RegisterNodeAction( - c => - { - var methodCall = (InvocationExpressionSyntax)c.Node; - - var memberAccess = methodCall.Expression as MemberAccessExpressionSyntax; - Location getLocation() => - memberAccess == null - ? methodCall.Expression.GetLocation() - : memberAccess.Name.GetLocation(); - - AnalyzeArguments(c, methodCall.ArgumentList, getLocation); - }, SyntaxKind.InvocationExpression); - - context.RegisterNodeAction( - c => - { - var objectCreationCall = (ObjectCreationExpressionSyntax)c.Node; - - var qualifiedAccess = objectCreationCall.Type as QualifiedNameSyntax; - Location getLocation() => - qualifiedAccess == null - ? objectCreationCall.Type.GetLocation() - : qualifiedAccess.Right.GetLocation(); - - AnalyzeArguments(c, objectCreationCall.ArgumentList, getLocation); - }, SyntaxKind.ObjectCreationExpression); - } - - private void AnalyzeArguments(SonarSyntaxNodeReportingContext analysisContext, ArgumentListSyntax argumentList, - Func getLocation) + protected override SyntaxKind[] InvocationKinds => new[] { - if (argumentList == null) - { - return; - } - - var methodParameterLookup = new VisualBasicMethodParameterLookup(argumentList, analysisContext.SemanticModel); - - ReportIncorrectlyOrderedParameters(analysisContext, methodParameterLookup, argumentList.Arguments, getLocation); - } - - protected override TypeInfo GetArgumentTypeSymbolInfo(ArgumentSyntax argument, SemanticModel semanticModel) => - semanticModel.GetTypeInfo(argument.GetExpression()); + SyntaxKind.ObjectCreationExpression, + SyntaxKind.InvocationExpression + }; - protected override Location GetMethodDeclarationIdentifierLocation(SyntaxNode syntaxNode) => - (syntaxNode as MethodBlockBaseSyntax)?.FindIdentifierLocation(); - - protected override SyntaxToken? GetArgumentIdentifier(ArgumentSyntax argument) => - (argument.GetExpression() as IdentifierNameSyntax)?.Identifier; - - protected override SyntaxToken? GetNameColonArgumentIdentifier(ArgumentSyntax argument) => - (argument as SimpleArgumentSyntax)?.NameColonEquals?.Name.Identifier; + protected override ILanguageFacade Language => VisualBasicFacade.Instance; } } - diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/ParametersCorrectOrderTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/ParametersCorrectOrderTest.cs index 7055eeed357..ce811e73164 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/ParametersCorrectOrderTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/ParametersCorrectOrderTest.cs @@ -71,5 +71,23 @@ Public Sub Bar() Dim y = New System. () End Sub End Class").WithErrorBehavior(CompilationErrorBehavior.Ignore).Verify(); + + [TestMethod] + public void ParametersCorrectOrder_InvalidCode_CS1() => + builderCS.AddSnippet(""" + class BaseConstructor + { + class Base(int a, int b) // Secondary [1, 2, 3, 4, 5, 6] + { + Base(int a, int b, string c) : this(b, a) { } // Noncompliant [1] + Base(string c, int a, int b) : this(b, a) { } // Noncompliant [2] + } + + class ParamsFullyInverted(int a, int b) : Base(b, a); // Noncompliant [3] + class ParamsPartiallyInverted(int a, int b, int c) : Base(b, a); // Noncompliant [4] + class ParamsFullyInvertedWithAdditionalParamAfter(int a, int b, string s) : Base(b, a); // Noncompliant [5] + class ParamsFullyInvertedWithAdditionalParamBefore(string s, int a, int b) : Base(b, a); // Noncompliant [6] + } + """).WithOptions(ParseOptionsHelper.FromCSharp12).WithConcurrentAnalysis(false).Verify(); } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.CSharp12.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.CSharp12.cs index 5b4c4f00f97..20b3cd14df7 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.CSharp12.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.CSharp12.cs @@ -5,39 +5,39 @@ namespace Repro_8071 { class BaseConstructor { - class Base(int a, int b) + class Base(int a, int b) // Secondary [Base1, Base2, Base3, Base4, Base5, Base6] { - Base(int a, int b, string c) : this(b, a) { } // FN: ctor params inverted with additional param after - Base(string c, int a, int b) : this(b, a) { } // FN: ctor params inverted with additional param before + 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); // FN - class ParamsPartiallyInverted(int a, int b, int c) : Base(b, a); // FN - class ParamsFullyInvertedWithAdditionalParamAfter(int a, int b, string s) : Base(b, a); // FN - class ParamsFullyInvertedWithAdditionalParamBefore(string s, int a, int b) : Base(b, a); // FN + class ParamsFullyInverted(int a, int b) : Base(b, a); // Noncompliant [Base3] + class ParamsPartiallyInverted(int a, int b, int c) : 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 WithRecordStructs { void Basics(int a, int b, int c) { - _ = new SomeRecord(b, a); // Noncompliant + _ = new SomeRecord(b, a); // Noncompliant [SomeRecord1] } void WithPromotion(short a, short b) { - _ = new SomeRecord(b, a); // Noncompliant + _ = new SomeRecord(b, a); // Noncompliant [SomeRecord2] } void WithCasting(long a, long b) { - _ = new SomeRecord((int)b, (int)a); // FN + _ = new SomeRecord((int)b, (int)a); // FN [SomeRecord3] } - record SomeRecord(int a, int b) + record SomeRecord(int a, int b) // Secondary [SomeRecord1, SomeRecord2, SomeRecord4, SomeRecord5] { - public SomeRecord(int a, int b, string c) : this(b, a) { } // FN - public SomeRecord(string c, int a, int b) : this(b, a) { } // FN + public SomeRecord(int a, int b, string c) : this(b, a) { } // Noncompliant [SomeRecord4] + public SomeRecord(string c, int a, int b) : this(b, a) { } // Noncompliant [SomeRecord5] } } @@ -45,23 +45,23 @@ class WithRecords { void Basics(int a, int b, int c) { - _ = new SomeRecordStruct(b, a); // Noncompliant + _ = new SomeRecordStruct(b, a); // Noncompliant [SomeRecordStruct1] } void WithPromotion(short a, short b) { - _ = new SomeRecordStruct(b, a); // Noncompliant + _ = new SomeRecordStruct(b, a); // Noncompliant [SomeRecordStruct2] } void WithCasting(long a, long b) { - _ = new SomeRecordStruct((int)b, (int)a); // FN + _ = new SomeRecordStruct((int)b, (int)a); // FN [SomeRecordStruct3] } - record struct SomeRecordStruct(int a, int b) + record struct SomeRecordStruct(int a, int b) // Secondary [SomeRecordStruct1, SomeRecordStruct2, SomeRecordStruct4, SomeRecordStruct5] { - public SomeRecordStruct(int a, int b, string c) : this(b, a) { } // FN - public SomeRecordStruct(string c, int a, int b) : this(b, a) { } // FN + public SomeRecordStruct(int a, int b, string c) : this(b, a) { } // Noncompliant [SomeRecordStruct4] + public SomeRecordStruct(string c, int a, int b) : this(b, a) { } // Noncompliant [SomeRecordStruct5] } } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.cs index 1dbf5bdaf6b..937d794880d 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.cs @@ -215,9 +215,9 @@ class InvokingConstructorViaThis { class SomeClass { - public SomeClass(int a, int b) { } - public SomeClass(int a, int b, string c) : this(b, a) { } // FN: ctor params fully inverted with additional par after - public SomeClass(string c, int a, int b) : this(b, a) { } // FN: ctor params fully inverted with additional par before + public SomeClass(int a, int b) { } // Secondary [SomeClass1, SomeClass2] + public SomeClass(int a, int b, string c) : this(b, a) { } // Noncompliant [SomeClass1] + public SomeClass(string c, int a, int b) : this(b, a) { } // Noncompliant [SomeClass2] } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.vb b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.vb index 52ca6b3965a..71e1e3cae91 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.vb +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.vb @@ -19,7 +19,7 @@ Foo.GlobalDivide(divisor, dividend) Divide(dividend, divisor) ' Noncompliant [1] {{Parameters to 'Divide' have the same names but not the same order as the method arguments.}} - Foo.GlobalDivide(dividend, divisor) ' Noncompliant + Foo.GlobalDivide(dividend, divisor) ' Noncompliant [2] Divide(dividend:=dividend, divisor:= divisor) @@ -53,11 +53,11 @@ Sub FooBar(ByVal value As Integer, ByVal ParamArray args() As String) End Sub - Public Function Divide(ByVal divisor As Integer, ByVal dividend As Integer) As Double ' Should be secondary-location for [1], but does not work + Public Function Divide(ByVal divisor As Integer, ByVal dividend As Integer) As Double ' Secondary [1] Return divisor / dividend End Function - Public Shared Function GlobalDivide(ByVal divisor As Integer, ByVal dividend As Integer) As Double + Public Shared Function GlobalDivide(ByVal divisor As Integer, ByVal dividend As Integer) As Double ' Secondary [2] Return divisor / dividend End Function From 3a0dc75d818b659e5a537a390ec83cd84448fba1 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Fri, 20 Oct 2023 16:40:42 +0200 Subject: [PATCH 02/29] Clean-up --- .../Facade/CSharpSyntaxFacade.cs | 3 +-- .../Rules/ParametersCorrectOrderBase.cs | 19 +++++++------------ .../Rules/ParametersCorrectOrderTest.cs | 18 ------------------ 3 files changed, 8 insertions(+), 32 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs index 32e7a887394..ed5f758a3a3 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs @@ -36,8 +36,7 @@ public override IReadOnlyList ArgumentList(SyntaxNode node) => ConstructorInitializerSyntax constructorInitializer => constructorInitializer.ArgumentList.Arguments, null => ImmutableArray.Empty, _ when PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(node) => ((PrimaryConstructorBaseTypeSyntaxWrapper)node).ArgumentList.Arguments, - _ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(node) - => ((ImplicitObjectCreationExpressionSyntaxWrapper)node).ArgumentList.Arguments, + _ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(node) => ((ImplicitObjectCreationExpressionSyntaxWrapper)node).ArgumentList.Arguments, _ => throw InvalidOperation(node, nameof(ArgumentExpressions)) }; public override SyntaxToken? ArgumentNameColon(SyntaxNode argument) => diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs index d09b7b07914..38baf80699d 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs @@ -18,9 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using Microsoft.CodeAnalysis; -using SonarAnalyzer.Extensions; - namespace SonarAnalyzer.Rules { public abstract class ParametersCorrectOrderBase : SonarDiagnosticAnalyzer @@ -42,22 +39,22 @@ protected override void Initialize(SonarAnalysisContext context) => { return; } - var arguments = Language.Syntax.ArgumentList(c.Node); IMethodParameterLookup methodParameterLookup = null; - foreach (var argument in arguments) + foreach (var argument in Language.Syntax.ArgumentList(c.Node)) { methodParameterLookup ??= Language.MethodParameterLookup(c.Node, c.SemanticModel); // Example void M(int x, int y) <- p_x and p_y are the parameter - // M(y, x) <- a_x and a_y are the arguments + // M(y, x); <- a_y and a_x are the arguments if (methodParameterLookup.TryGetSymbol(argument, out var parameterSymbol) // argument = a_x and parameterSymbol = p_y && parameterSymbol is { IsParams: false } && ArgumentName(argument) is { } argumentName // "x" && !MatchingNames(parameterSymbol, argumentName) // "x" != "y" && Language.Syntax.NodeExpression(argument) is { } argumentExpression && c.Context.SemanticModel.GetTypeInfo(argumentExpression).ConvertedType is { } argumentType - && methodParameterLookup.MethodSymbol.Parameters.FirstOrDefault(p => MatchingNames(p, argumentName) && argumentType.DerivesOrImplements(p.Type)) is - { IsParams: false } // p_x (there is another parameter that seems to be a better fit) - && arguments.FirstOrDefault(x => MatchingNames(parameterSymbol, ArgumentName(x))) is { }) // Look if there is an argument that matches the parameter p_y (yes: a_y) + // is there another parameter that seems to be a better fit (name and type match): p_x + && methodParameterLookup.MethodSymbol.Parameters.FirstOrDefault(p => MatchingNames(p, argumentName) && argumentType.DerivesOrImplements(p.Type)) is { IsParams: false } + // is there an argument that matches the parameter p_y by name: a_y + && Language.Syntax.ArgumentList(c.Node).FirstOrDefault(x => MatchingNames(parameterSymbol, ArgumentName(x))) is { }) { var secondaryLocations = methodParameterLookup.MethodSymbol.DeclaringSyntaxReferences .Select(s => Language.Syntax.NodeIdentifier(s.GetSyntax())?.GetLocation()) @@ -73,8 +70,6 @@ private static bool MatchingNames(IParameterSymbol parameter, string argumentNam parameter.Name == argumentName; private string ArgumentName(SyntaxNode argument) => - Language.Syntax.NodeIdentifier(Language.Syntax.NodeExpression(argument)) is SyntaxToken identifier - ? identifier.ValueText - : null; + Language.Syntax.NodeIdentifier(Language.Syntax.NodeExpression(argument))?.ValueText; } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/ParametersCorrectOrderTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/ParametersCorrectOrderTest.cs index ce811e73164..7055eeed357 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Rules/ParametersCorrectOrderTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Rules/ParametersCorrectOrderTest.cs @@ -71,23 +71,5 @@ Public Sub Bar() Dim y = New System. () End Sub End Class").WithErrorBehavior(CompilationErrorBehavior.Ignore).Verify(); - - [TestMethod] - public void ParametersCorrectOrder_InvalidCode_CS1() => - builderCS.AddSnippet(""" - class BaseConstructor - { - class Base(int a, int b) // Secondary [1, 2, 3, 4, 5, 6] - { - Base(int a, int b, string c) : this(b, a) { } // Noncompliant [1] - Base(string c, int a, int b) : this(b, a) { } // Noncompliant [2] - } - - class ParamsFullyInverted(int a, int b) : Base(b, a); // Noncompliant [3] - class ParamsPartiallyInverted(int a, int b, int c) : Base(b, a); // Noncompliant [4] - class ParamsFullyInvertedWithAdditionalParamAfter(int a, int b, string s) : Base(b, a); // Noncompliant [5] - class ParamsFullyInvertedWithAdditionalParamBefore(string s, int a, int b) : Base(b, a); // Noncompliant [6] - } - """).WithOptions(ParseOptionsHelper.FromCSharp12).WithConcurrentAnalysis(false).Verify(); } } From 7089603359760e92b853386c6d727a18c05af684 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Fri, 20 Oct 2023 17:46:19 +0200 Subject: [PATCH 03/29] Fix NRE --- .../Facade/CSharpSyntaxFacade.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs index ed5f758a3a3..26f075f453d 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs @@ -29,16 +29,16 @@ public override IEnumerable ArgumentExpressions(SyntaxNode node) => ArgumentList(node)?.OfType().Select(x => x.Expression) ?? Enumerable.Empty(); public override IReadOnlyList ArgumentList(SyntaxNode node) => - node switch + (node switch { - ObjectCreationExpressionSyntax creation => creation.ArgumentList.Arguments, - InvocationExpressionSyntax invocation => invocation.ArgumentList.Arguments, - ConstructorInitializerSyntax constructorInitializer => constructorInitializer.ArgumentList.Arguments, - null => ImmutableArray.Empty, - _ when PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(node) => ((PrimaryConstructorBaseTypeSyntaxWrapper)node).ArgumentList.Arguments, - _ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(node) => ((ImplicitObjectCreationExpressionSyntaxWrapper)node).ArgumentList.Arguments, + ObjectCreationExpressionSyntax creation => creation.ArgumentList, + InvocationExpressionSyntax invocation => invocation.ArgumentList, + ConstructorInitializerSyntax constructorInitializer => constructorInitializer.ArgumentList, + null => null, + _ when PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(node) => ((PrimaryConstructorBaseTypeSyntaxWrapper)node).ArgumentList, + _ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(node) => ((ImplicitObjectCreationExpressionSyntaxWrapper)node).ArgumentList, _ => throw InvalidOperation(node, nameof(ArgumentExpressions)) - }; + })?.Arguments ?? (IReadOnlyList)Array.Empty(); public override SyntaxToken? ArgumentNameColon(SyntaxNode argument) => (argument as ArgumentSyntax)?.NameColon?.Name?.Identifier; From b4c5a49334be01f55850dd6294621c28da7fba38 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 10:52:07 +0200 Subject: [PATCH 04/29] Move ArgumentList impl to extension mehtods in the language projects --- .../Extensions/SyntaxNodeExtensions.cs | 12 +++ .../Facade/CSharpSyntaxFacade.cs | 12 +-- .../Facade/VisualBasicSyntaxFacade.cs | 8 +- .../Helpers/VisualBasicSyntaxHelper.cs | 15 ++++ .../Extensions/SyntaxNodeExtensionsTest.cs | 86 +++++++++++++++++++ 5 files changed, 116 insertions(+), 17 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs b/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs index bad89563cc0..e8503b9c3da 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs @@ -342,6 +342,18 @@ public static bool IsParentKind(this SyntaxNode node, SyntaxKind kind, out T return false; } + public static IReadOnlyList ArgumentList(this SyntaxNode node) => + (node switch + { + ObjectCreationExpressionSyntax creation => creation.ArgumentList, + InvocationExpressionSyntax invocation => invocation.ArgumentList, + ConstructorInitializerSyntax constructorInitializer => constructorInitializer.ArgumentList, + null => null, + _ when PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(node) => ((PrimaryConstructorBaseTypeSyntaxWrapper)node).ArgumentList, + _ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(node) => ((ImplicitObjectCreationExpressionSyntaxWrapper)node).ArgumentList, + _ => throw new InvalidOperationException($"The {nameof(node)} of kind {node.Kind()} does not have an {nameof(ArgumentList)}."), + })?.Arguments ?? (IReadOnlyList)Array.Empty(); + public static ParameterListSyntax ParameterList(this SyntaxNode node) => node switch { diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs index 26f075f453d..e98db50f7c6 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs @@ -29,16 +29,8 @@ public override IEnumerable ArgumentExpressions(SyntaxNode node) => ArgumentList(node)?.OfType().Select(x => x.Expression) ?? Enumerable.Empty(); public override IReadOnlyList ArgumentList(SyntaxNode node) => - (node switch - { - ObjectCreationExpressionSyntax creation => creation.ArgumentList, - InvocationExpressionSyntax invocation => invocation.ArgumentList, - ConstructorInitializerSyntax constructorInitializer => constructorInitializer.ArgumentList, - null => null, - _ when PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(node) => ((PrimaryConstructorBaseTypeSyntaxWrapper)node).ArgumentList, - _ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(node) => ((ImplicitObjectCreationExpressionSyntaxWrapper)node).ArgumentList, - _ => throw InvalidOperation(node, nameof(ArgumentExpressions)) - })?.Arguments ?? (IReadOnlyList)Array.Empty(); + node.ArgumentList(); + public override SyntaxToken? ArgumentNameColon(SyntaxNode argument) => (argument as ArgumentSyntax)?.NameColon?.Name?.Identifier; diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs index 3e93d5a1fc5..c12aed450a2 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs @@ -31,13 +31,7 @@ public override IEnumerable ArgumentExpressions(SyntaxNode node) => ArgumentList(node).OfType().Select(x => x.GetExpression()).WhereNotNull(); public override IReadOnlyList ArgumentList(SyntaxNode node) => - (node switch - { - ObjectCreationExpressionSyntax creation => creation.ArgumentList, - InvocationExpressionSyntax invocation => invocation.ArgumentList, - null => null, - _ => throw InvalidOperation(node, nameof(ArgumentExpressions)) - })?.Arguments ?? (IReadOnlyList)Array.Empty(); + node.ArgumentList(); public override int? ArgumentIndex(SyntaxNode argument) => Cast(argument).GetArgumentIndex(); diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Helpers/VisualBasicSyntaxHelper.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Helpers/VisualBasicSyntaxHelper.cs index aa66680204d..767e6adea6c 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Helpers/VisualBasicSyntaxHelper.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Helpers/VisualBasicSyntaxHelper.cs @@ -209,6 +209,21 @@ public static ExpressionSyntax Get(this ArgumentListSyntax argumentList, int ind ? argumentList.Arguments[index].GetExpression().RemoveParentheses() : null; + public static IReadOnlyList ArgumentList(this SyntaxNode node) => + (node switch + { + ObjectCreationExpressionSyntax creation => creation.ArgumentList, + InvocationExpressionSyntax invocation => invocation.ArgumentList, + ModifiedIdentifierSyntax modified => modified.ArrayBounds, + AttributeSyntax attribute => attribute.ArgumentList, + MidExpressionSyntax mid => mid.ArgumentList, + RaiseEventStatementSyntax raise => raise.ArgumentList, + RedimClauseSyntax reDim => reDim.ArrayBounds, + ArrayCreationExpressionSyntax arrayCreation => arrayCreation.ArrayBounds, + null => null, + _ => throw new InvalidOperationException($"The {nameof(node)} of kind {node.Kind()} does not have an {nameof(ArgumentList)}."), + })?.Arguments ?? (IReadOnlyList)Array.Empty(); + /// /// Returns argument expressions for given parameter. /// diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs index d629edb96a9..489c5acac97 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs @@ -733,6 +733,92 @@ public void GetMappedFilePath(string code, string expectedFileName = DefaultFile syntaxTree.GetRoot().GetMappedFilePathFromRoot().Should().Be(expectedFileName); } + [DataTestMethod] + [DataRow("$$M(1)$$;")] + [DataRow("_ = $$new C(1)$$;")] + [DataRow("C c = $$new(1)$$;")] + public void ArgumentList_InvocationObjectCreation(string statement) + { + var code = $$""" + public class C(int p) { + public void M(int p) { + {{statement}} + } + } + """; + var node = NodeBetweenMarkers(code, LanguageNames.CSharp); + var argumentList = ExtensionsCS.ArgumentList(node); + var argument = argumentList.Should().ContainSingle().Which; + (argument is { Expression: SyntaxCS.LiteralExpressionSyntax { Token.ValueText: "1" } }).Should().BeTrue(); + } + + [DataTestMethod] + [DataRow("base")] + [DataRow("this")] + public void ArgumentList_ConstructorInitializer(string keyword) + { + var code = $$""" + public class Base(int p); + + public class C: Base + { + public C(): $${{keyword}}(1)$$ { } + public C(int p): base(p) { } + } + + """; + var node = NodeBetweenMarkers(code, LanguageNames.CSharp); + var argumentList = ExtensionsCS.ArgumentList(node); + var argument = argumentList.Should().ContainSingle().Which; + (argument is { Expression: SyntaxCS.LiteralExpressionSyntax { Token.ValueText: "1" } }).Should().BeTrue(); + } + + [TestMethod] + public void ArgumentList_PrimaryConstructorBaseType() + { + var code = """ + public class Base(int p); + public class Derived(int p): $$Base(1)$$; + """; + var node = NodeBetweenMarkers(code, LanguageNames.CSharp); + var argumentList = ExtensionsCS.ArgumentList(node); + var argument = argumentList.Should().ContainSingle().Which; + (argument is { Expression: SyntaxCS.LiteralExpressionSyntax { Token.ValueText: "1" } }).Should().BeTrue(); + } + + [DataTestMethod] + [DataRow("_ = $$new System.Collections.Generic.List { 0 }$$;")] + public void ArgumentList_NoList(string statement) + { + var code = $$""" + public class C { + public void M() { + {{statement}} + } + } + """; + var node = NodeBetweenMarkers(code, LanguageNames.CSharp); + var argumentList = ExtensionsCS.ArgumentList(node); + argumentList.Should().BeEmpty(); + } + + [DataTestMethod] + [DataRow("_ = $$new int[] { 1 }$$;")] + [DataRow("_ = $$new { A = 1 }$$;")] + public void ArgumentList_UnsupportedNodeKinds(string statement) + { + var code = $$""" + public class C { + public void M() { + {{statement}} + } + } + """; + var node = NodeBetweenMarkers(code, LanguageNames.CSharp); + var sut = () => ExtensionsCS.ArgumentList(node); + sut.Should().Throw(); + } + [DataTestMethod] [DataRow("""public C(int p) { }""")] [DataRow("""public void M(int p) { }""")] From 6dccb04af6529b89bac7a28db1fd5ba77ce1c4b9 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 12:09:20 +0200 Subject: [PATCH 05/29] member ordering --- .../src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs | 6 +++--- analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs index e98db50f7c6..a6552434341 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs @@ -31,12 +31,12 @@ public override IEnumerable ArgumentExpressions(SyntaxNode node) => public override IReadOnlyList ArgumentList(SyntaxNode node) => node.ArgumentList(); - public override SyntaxToken? ArgumentNameColon(SyntaxNode argument) => - (argument as ArgumentSyntax)?.NameColon?.Name?.Identifier; - public override int? ArgumentIndex(SyntaxNode argument) => Cast(argument).GetArgumentIndex(); + public override SyntaxToken? ArgumentNameColon(SyntaxNode argument) => + (argument as ArgumentSyntax)?.NameColon?.Name?.Identifier; + public override SyntaxNode AssignmentLeft(SyntaxNode assignment) => Cast(assignment).Left; diff --git a/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs b/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs index cbec044afbd..672d54f75e5 100644 --- a/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs @@ -24,10 +24,10 @@ public abstract class SyntaxFacade where TSyntaxKind : struct { public abstract bool AreEquivalent(SyntaxNode firstNode, SyntaxNode secondNode); - public abstract SyntaxToken? ArgumentNameColon(SyntaxNode argument); public abstract IEnumerable ArgumentExpressions(SyntaxNode node); public abstract int? ArgumentIndex(SyntaxNode argument); public abstract IReadOnlyList ArgumentList(SyntaxNode node); + public abstract SyntaxToken? ArgumentNameColon(SyntaxNode argument); public abstract SyntaxNode AssignmentLeft(SyntaxNode assignment); public abstract SyntaxNode AssignmentRight(SyntaxNode assignment); public abstract ImmutableArray AssignmentTargets(SyntaxNode assignment); @@ -37,6 +37,8 @@ public abstract class SyntaxFacade public abstract SyntaxNode CastExpression(SyntaxNode cast); public abstract ComparisonKind ComparisonKind(SyntaxNode node); public abstract IEnumerable EnumMembers(SyntaxNode @enum); + public abstract ImmutableArray FieldDeclarationIdentifiers(SyntaxNode node); + public abstract bool HasExactlyNArguments(SyntaxNode invocation, int count); public abstract SyntaxToken? InvocationIdentifier(SyntaxNode invocation); public abstract bool IsAnyKind(SyntaxNode node, ISet syntaxKinds); public abstract bool IsAnyKind(SyntaxNode node, params TSyntaxKind[] syntaxKinds); From 9daab19b1a0f50ec44d2f050abc47d61d7d1610e Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 13:46:17 +0200 Subject: [PATCH 06/29] Add VB tests for ArgumentList --- .../Extensions/SyntaxNodeExtensions.cs | 15 ++ .../Helpers/VisualBasicSyntaxHelper.cs | 15 -- .../Extensions/SyntaxNodeExtensionsTest.cs | 129 +++++++++++++++++- 3 files changed, 137 insertions(+), 22 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs index 28d86c2f12f..6503c7b8deb 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs @@ -100,6 +100,21 @@ static bool TakesExpressionTree(SymbolInfo info) _ => null, }; + public static IReadOnlyList ArgumentList(this SyntaxNode node) => + (node switch + { + ArrayCreationExpressionSyntax arrayCreation => arrayCreation.ArrayBounds, + AttributeSyntax attribute => attribute.ArgumentList, + InvocationExpressionSyntax invocation => invocation.ArgumentList, + MidExpressionSyntax mid => mid.ArgumentList, + ModifiedIdentifierSyntax modified => modified.ArrayBounds, + ObjectCreationExpressionSyntax creation => creation.ArgumentList, + RaiseEventStatementSyntax raise => raise.ArgumentList, + RedimClauseSyntax reDim => reDim.ArrayBounds, + null => null, + _ => throw new InvalidOperationException($"The {nameof(node)} of kind {node.Kind()} does not have an {nameof(ArgumentList)}."), + })?.Arguments ?? (IReadOnlyList)Array.Empty(); + /// /// Returns the left hand side of a conditional access expression. Returns c in case like a?.b?[0].c?.d.e?.f if d is passed. /// diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Helpers/VisualBasicSyntaxHelper.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Helpers/VisualBasicSyntaxHelper.cs index 767e6adea6c..aa66680204d 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Helpers/VisualBasicSyntaxHelper.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Helpers/VisualBasicSyntaxHelper.cs @@ -209,21 +209,6 @@ public static ExpressionSyntax Get(this ArgumentListSyntax argumentList, int ind ? argumentList.Arguments[index].GetExpression().RemoveParentheses() : null; - public static IReadOnlyList ArgumentList(this SyntaxNode node) => - (node switch - { - ObjectCreationExpressionSyntax creation => creation.ArgumentList, - InvocationExpressionSyntax invocation => invocation.ArgumentList, - ModifiedIdentifierSyntax modified => modified.ArrayBounds, - AttributeSyntax attribute => attribute.ArgumentList, - MidExpressionSyntax mid => mid.ArgumentList, - RaiseEventStatementSyntax raise => raise.ArgumentList, - RedimClauseSyntax reDim => reDim.ArrayBounds, - ArrayCreationExpressionSyntax arrayCreation => arrayCreation.ArrayBounds, - null => null, - _ => throw new InvalidOperationException($"The {nameof(node)} of kind {node.Kind()} does not have an {nameof(ArgumentList)}."), - })?.Arguments ?? (IReadOnlyList)Array.Empty(); - /// /// Returns argument expressions for given parameter. /// diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs index 489c5acac97..c32e017ae06 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs @@ -737,7 +737,7 @@ public void GetMappedFilePath(string code, string expectedFileName = DefaultFile [DataRow("$$M(1)$$;")] [DataRow("_ = $$new C(1)$$;")] [DataRow("C c = $$new(1)$$;")] - public void ArgumentList_InvocationObjectCreation(string statement) + public void ArgumentList_CS_InvocationObjectCreation(string statement) { var code = $$""" public class C(int p) { @@ -755,7 +755,7 @@ public void M(int p) { [DataTestMethod] [DataRow("base")] [DataRow("this")] - public void ArgumentList_ConstructorInitializer(string keyword) + public void ArgumentList_CS_ConstructorInitializer(string keyword) { var code = $$""" public class Base(int p); @@ -774,7 +774,7 @@ public C(int p): base(p) { } } [TestMethod] - public void ArgumentList_PrimaryConstructorBaseType() + public void ArgumentList_CS_PrimaryConstructorBaseType() { var code = """ public class Base(int p); @@ -788,7 +788,7 @@ public class Derived(int p): $$Base(1)$$; [DataTestMethod] [DataRow("_ = $$new System.Collections.Generic.List { 0 }$$;")] - public void ArgumentList_NoList(string statement) + public void ArgumentList_CS_NoList(string statement) { var code = $$""" public class C { @@ -805,7 +805,7 @@ public void M() { [DataTestMethod] [DataRow("_ = $$new int[] { 1 }$$;")] [DataRow("_ = $$new { A = 1 }$$;")] - public void ArgumentList_UnsupportedNodeKinds(string statement) + public void ArgumentList_CS_UnsupportedNodeKinds(string statement) { var code = $$""" public class C { @@ -819,6 +819,121 @@ public void M() { sut.Should().Throw(); } + [DataTestMethod] + [DataRow("$$M(1)$$")] + [DataRow("Call $$M(1)$$")] + [DataRow("Dim c = $$New C(1)$$")] + [DataRow("$$RaiseEvent SomeEvent(1)$$")] + public void ArgumentList_VB_Invocations(string statement) + { + var code = $$""" + Imports System + + Public Class C + Public Event SomeEvent As Action(Of Integer) + + Public Sub New(p As Integer) + End Sub + + Public Sub M(p As Integer) + Dim s As String = "Test" + {{statement}} + End Sub + End Class + """; + var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic, getInnermostNodeForTie: true); + var argumentList = ExtensionsVB.ArgumentList(node); + var argument = argumentList.Should().ContainSingle().Which; + (argument.GetExpression() is SyntaxVB.LiteralExpressionSyntax { Token.ValueText: "1" }).Should().BeTrue(); + } + + [TestMethod] + public void ArgumentList_VB_Mid() + { + var code = $$""" + Public Class C + Public Sub M() + Dim s As String = "Test" + $$Mid(s, 1)$$ = "Test" + End Sub + End Class + """; + var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic, getInnermostNodeForTie: true); + var argumentList = ExtensionsVB.ArgumentList(node); + argumentList.Should().SatisfyRespectively( + a => (a.GetExpression() is SyntaxVB.IdentifierNameSyntax { Identifier.ValueText: "s" }).Should().BeTrue(), + a => (a.GetExpression() is SyntaxVB.LiteralExpressionSyntax { Token.ValueText: "1" }).Should().BeTrue()); + } + + [TestMethod] + public void ArgumentList_VB_Attribute() + { + var code = """ + <$$System.Obsolete("1")$$> + Public Class C + End Class + """; + var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic, getInnermostNodeForTie: true); + var argumentList = ExtensionsVB.ArgumentList(node); + var argument = argumentList.Should().ContainSingle().Which; + (argument.GetExpression() is SyntaxVB.LiteralExpressionSyntax { Token.ValueText: "1" }).Should().BeTrue(); + } + + [DataTestMethod] + [DataRow("Dim $$i(1)$$ As Integer")] + [DataRow("Dim sales()() As Double = $$New Double(1)() { }$$")] + [DataRow("ReDim $$arr(1)$$")] + public void ArgumentList_VB_ArrayBounds(string statement) + { + var code = $$""" + Public Class C + Public Sub M() + Dim arr(0) As Integer + {{statement}} + End Sub + End Class + """; + var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic, getInnermostNodeForTie: true); + var argumentList = ExtensionsVB.ArgumentList(node); + var argument = argumentList.Should().ContainSingle().Which; + (argument.GetExpression() is SyntaxVB.LiteralExpressionSyntax { Token.ValueText: "1" }).Should().BeTrue(); + } + + [TestMethod] + public void ArgumentList_VB_Call() + { + var code = $$""" + Public Class C + Public Sub M() + Call $$M$$ + End Sub + End Class + """; + var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic, getInnermostNodeForTie: false); + var argumentList = ExtensionsVB.ArgumentList(node); + argumentList.Should().BeEmpty(); + } + + [TestMethod] + public void ArgumentList_VB_Null() => + ExtensionsVB.ArgumentList(null).Should().BeEmpty(); + + [DataTestMethod] + [DataRow("$$Dim a = 1$$")] + public void ArgumentList_VB_UnsupportedNodeKinds(string statement) + { + var code = $$""" + Public Class C + Public Sub M() + {{statement}} + End Sub + End Class + """; + var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic, getInnermostNodeForTie: true); + var sut = () => ExtensionsVB.ArgumentList(node); + sut.Should().Throw(); + } + [DataTestMethod] [DataRow("""public C(int p) { }""")] [DataRow("""public void M(int p) { }""")] @@ -1016,14 +1131,14 @@ public void GetIdentifier_CompilationUnit(string member, string expected) } } - private static SyntaxNode NodeBetweenMarkers(string code, string language) + private static SyntaxNode NodeBetweenMarkers(string code, string language, bool getInnermostNodeForTie = false) { var position = code.IndexOf("$$"); var lastPosition = code.LastIndexOf("$$"); var length = lastPosition == position ? 0 : lastPosition - position - "$$".Length; code = code.Replace("$$", string.Empty); var (tree, _) = IsCSharp() ? TestHelper.CompileCS(code) : TestHelper.CompileVB(code); - var node = tree.GetRoot().FindNode(new TextSpan(position, length)); + var node = tree.GetRoot().FindNode(new TextSpan(position, length), getInnermostNodeForTie: getInnermostNodeForTie); return node; bool IsCSharp() => language == LanguageNames.CSharp; From 36d3ca8b0aa2f2f6fb7318d2b9a895a86173b7a3 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 13:54:59 +0200 Subject: [PATCH 07/29] Add links to Roslyn --- .../src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs | 1 + .../SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs | 1 + 2 files changed, 2 insertions(+) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs b/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs index e8503b9c3da..ab725141a50 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs @@ -342,6 +342,7 @@ public static bool IsParentKind(this SyntaxNode node, SyntaxKind kind, out T return false; } + // based on Type="ArgumentListSyntax" in https://github.com/dotnet/roslyn/blob/main/src/Compilers/CSharp/Portable/Syntax/Syntax.xml public static IReadOnlyList ArgumentList(this SyntaxNode node) => (node switch { diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs index 6503c7b8deb..716c7e370c7 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs @@ -100,6 +100,7 @@ static bool TakesExpressionTree(SymbolInfo info) _ => null, }; + // based on kind="ArgumentList" in https://github.com/dotnet/roslyn/blob/main/src/Compilers/VisualBasic/Portable/Syntax/Syntax.xml public static IReadOnlyList ArgumentList(this SyntaxNode node) => (node switch { From bd4f63412a47022c230a66ffd8da932ca05261f9 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 14:41:12 +0200 Subject: [PATCH 08/29] Refactor --- .../Rules/ParametersCorrectOrderBase.cs | 48 +++++++++---------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs index 38baf80699d..74832f926dc 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs @@ -35,33 +35,31 @@ protected override void Initialize(SonarAnalysisContext context) => context.RegisterNodeAction(Language.GeneratedCodeRecognizer, c => { - if (c.IsRedundantPrimaryConstructorBaseTypeContext()) + 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.MethodParameterLookup(c.Node, c.SemanticModel) is var methodParameterLookup) { - return; - } - IMethodParameterLookup methodParameterLookup = null; - foreach (var argument in Language.Syntax.ArgumentList(c.Node)) - { - methodParameterLookup ??= Language.MethodParameterLookup(c.Node, c.SemanticModel); - // Example void M(int x, int y) <- p_x and p_y are the parameter - // M(y, x); <- a_y and a_x are the arguments - if (methodParameterLookup.TryGetSymbol(argument, out var parameterSymbol) // argument = a_x and parameterSymbol = p_y - && parameterSymbol is { IsParams: false } - && ArgumentName(argument) is { } argumentName // "x" - && !MatchingNames(parameterSymbol, argumentName) // "x" != "y" - && Language.Syntax.NodeExpression(argument) is { } argumentExpression - && c.Context.SemanticModel.GetTypeInfo(argumentExpression).ConvertedType is { } argumentType - // is there another parameter that seems to be a better fit (name and type match): p_x - && methodParameterLookup.MethodSymbol.Parameters.FirstOrDefault(p => MatchingNames(p, argumentName) && argumentType.DerivesOrImplements(p.Type)) is { IsParams: false } - // is there an argument that matches the parameter p_y by name: a_y - && Language.Syntax.ArgumentList(c.Node).FirstOrDefault(x => MatchingNames(parameterSymbol, ArgumentName(x))) is { }) + foreach (var argument in argumentList) { - var secondaryLocations = methodParameterLookup.MethodSymbol.DeclaringSyntaxReferences - .Select(s => Language.Syntax.NodeIdentifier(s.GetSyntax())?.GetLocation()) - .WhereNotNull(); - - c.ReportIssue(SupportedDiagnostics[0].CreateDiagnostic(c.Compilation, c.Node.GetLocation(), secondaryLocations, properties: null, methodParameterLookup.MethodSymbol.Name)); - return; + // Example void M(int x, int y) <- p_x and p_y are the parameter + // M(y, x); <- a_y and a_x are the arguments + if (methodParameterLookup.TryGetSymbol(argument, out var parameterSymbol) // argument = a_x and parameterSymbol = p_y + && parameterSymbol is { IsParams: false } + && ArgumentName(argument) is { } argumentName // "x" + && !MatchingNames(parameterSymbol, argumentName) // "x" != "y" + && Language.Syntax.NodeExpression(argument) is { } argumentExpression + && c.Context.SemanticModel.GetTypeInfo(argumentExpression).ConvertedType is { } argumentType + // is there another parameter that seems to be a better fit (name and type match): p_x + && methodParameterLookup.MethodSymbol.Parameters.FirstOrDefault(p => MatchingNames(p, argumentName) && argumentType.DerivesOrImplements(p.Type)) is { IsParams: false } + // is there an argument that matches the parameter p_y by name: a_y + && Language.Syntax.ArgumentList(c.Node).FirstOrDefault(x => MatchingNames(parameterSymbol, ArgumentName(x))) is { }) + { + var secondaryLocations = methodParameterLookup.MethodSymbol.DeclaringSyntaxReferences + .Select(s => Language.Syntax.NodeIdentifier(s.GetSyntax())?.GetLocation()) + .WhereNotNull(); + c.ReportIssue(Diagnostic.Create(SupportedDiagnostics[0], c.Node.GetLocation(), secondaryLocations, properties: null, methodParameterLookup.MethodSymbol.Name)); + return; + } } } }, InvocationKinds); From 5b3260c614c578358455893e83abf9909a7c2e29 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 14:56:20 +0200 Subject: [PATCH 09/29] Support primary location handling --- .../Rules/ParametersCorrectOrder.cs | 12 +++++++++++ .../Rules/ParametersCorrectOrderBase.cs | 7 +++++-- .../TestCases/ParametersCorrectOrder.cs | 21 ++++++++++++------- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs index 0428209d232..5f43c87843d 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs @@ -28,10 +28,22 @@ public sealed class ParametersCorrectOrder : ParametersCorrectOrderBase Language => CSharpFacade.Instance; + + protected override Location PrimaryLocation(SyntaxNode node) => + node switch + { + InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax { Name: { } name } } => name.GetLocation(), + InvocationExpressionSyntax { Expression: { } expression } => expression.GetLocation(), + ObjectCreationExpressionSyntax { Type: QualifiedNameSyntax { Right: { } right } } => right.GetLocation(), + ObjectCreationExpressionSyntax { Type: { } type } => type.GetLocation(), + ConstructorInitializerSyntax { ThisOrBaseKeyword: { } keyword } => keyword.GetLocation(), + _ => base.PrimaryLocation(node), + }; } } diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs index 74832f926dc..a8d9a3986fd 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs @@ -31,7 +31,7 @@ protected ParametersCorrectOrderBase() : base(DiagnosticId) { } - protected override void Initialize(SonarAnalysisContext context) => + protected sealed override void Initialize(SonarAnalysisContext context) => context.RegisterNodeAction(Language.GeneratedCodeRecognizer, c => { @@ -57,13 +57,16 @@ protected override void Initialize(SonarAnalysisContext context) => var secondaryLocations = methodParameterLookup.MethodSymbol.DeclaringSyntaxReferences .Select(s => Language.Syntax.NodeIdentifier(s.GetSyntax())?.GetLocation()) .WhereNotNull(); - c.ReportIssue(Diagnostic.Create(SupportedDiagnostics[0], c.Node.GetLocation(), secondaryLocations, properties: null, methodParameterLookup.MethodSymbol.Name)); + c.ReportIssue(Diagnostic.Create(SupportedDiagnostics[0], PrimaryLocation(c.Node), secondaryLocations, properties: null, methodParameterLookup.MethodSymbol.Name)); return; } } } }, InvocationKinds); + protected virtual Location PrimaryLocation(SyntaxNode node) + => node.GetLocation(); + private static bool MatchingNames(IParameterSymbol parameter, string argumentName) => parameter.Name == argumentName; diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.cs index 937d794880d..8e0180dfb50 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.cs @@ -30,6 +30,7 @@ public static void call() var b = "2"; Comparer.Default.Compare(b, a); // Noncompliant + // ^^^^^^^ } } @@ -75,6 +76,7 @@ public void doTheThing() var some = 6; divide(dividend, 1 + 1, divisor, other2: 6); // Noncompliant [1] operation succeeds, but result is unexpected +// ^^^^^^ divide(divisor, other2, dividend); divide(divisor, other2, dividend, other2: someOther); // Noncompliant [2] {{Parameters to 'divide' have the same names but not the same order as the method arguments.}} @@ -200,8 +202,10 @@ class InvokingConstructorViaNew { void Test(int a, int b, int c) { - _ = new SomeClass(b, a); // Noncompliant, fully inverted - _ = new SomeClass(b, a, c); // Noncompliant, partially inverted + _ = new SomeClass(b, a); // Noncompliant, fully inverted + // ^^^^^^^^^ + _ = new InvokingConstructorViaNew.SomeClass(b, a, c); // Noncompliant, partially inverted + // ^^^^^^^^^ } class SomeClass @@ -225,28 +229,29 @@ class InvokingConstructorViaBase { class Base { - public Base(int a, int b) { } - public Base(int a, int b, int c) { } + public Base(int a, int b) { } // Secondary [Base1, Base3, Base4] + public Base(int a, int b, int c) { } // Secondary [Base2] } class ParamsFullyInverted : Base { - public ParamsFullyInverted(int a, int b) : base(b, a) { } // FN + public ParamsFullyInverted(int a, int b) : base(b, a) { } // Noncompliant [Base1] + // ^^^^ } class ParamsPartiallyInverted : Base { - public ParamsPartiallyInverted(int a, int b, int c) : base(b, a, c) { } // FN + public ParamsPartiallyInverted(int a, int b, int c) : base(b, a, c) { } // Noncompliant [Base2] } class ParamsFullyInvertedWithAdditionalParamAfter : Base { - public ParamsFullyInvertedWithAdditionalParamAfter(int a, int b, string s) : base(b, a) { } // FN + public ParamsFullyInvertedWithAdditionalParamAfter(int a, int b, string s) : base(b, a) { } // Noncompliant [Base3] } class ParamsFullyInvertedWithAdditionalParamBefore : Base { - public ParamsFullyInvertedWithAdditionalParamBefore(string s, int a, int b) : base(b, a) { } // FN + public ParamsFullyInvertedWithAdditionalParamBefore(string s, int a, int b) : base(b, a) { } // Noncompliant [Base4] } } } From c687e033478cf84e7aa4c616904ad0d64667e3d4 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 15:00:45 +0200 Subject: [PATCH 10/29] Location for PrimaryConstructorBaseTypeSyntax --- .../src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs | 2 ++ .../TestCases/ParametersCorrectOrder.CSharp12.cs | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs index 5f43c87843d..a1134ee0692 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs @@ -43,6 +43,8 @@ protected override Location PrimaryLocation(SyntaxNode node) => ObjectCreationExpressionSyntax { Type: QualifiedNameSyntax { Right: { } right } } => right.GetLocation(), ObjectCreationExpressionSyntax { Type: { } type } => type.GetLocation(), ConstructorInitializerSyntax { ThisOrBaseKeyword: { } keyword } => keyword.GetLocation(), + _ when PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(node) && ((PrimaryConstructorBaseTypeSyntaxWrapper)node).Type is { } type => + type is QualifiedNameSyntax { Right: { } right } ? right.GetLocation() : type.GetLocation(), _ => base.PrimaryLocation(node), }; } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.CSharp12.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.CSharp12.cs index 20b3cd14df7..0f543a58312 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.CSharp12.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.CSharp12.cs @@ -8,11 +8,14 @@ class BaseConstructor class Base(int a, int b) // Secondary [Base1, Base2, Base3, Base4, Base5, Base6] { 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 ParamsPartiallyInverted(int a, int b, int c) : 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] } From ce9613d494cd81e37e84e01bafc00291188ba4e1 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 15:04:40 +0200 Subject: [PATCH 11/29] Add support for VB primary location --- .../Rules/ParametersCorrectOrder.cs | 9 +++++++++ .../TestCases/ParametersCorrectOrder.vb | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ParametersCorrectOrder.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ParametersCorrectOrder.cs index 2c4e651fc1a..bc5094151fa 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ParametersCorrectOrder.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ParametersCorrectOrder.cs @@ -30,5 +30,14 @@ public sealed class ParametersCorrectOrder : ParametersCorrectOrderBase Language => VisualBasicFacade.Instance; + protected override Location PrimaryLocation(SyntaxNode node) => + node switch + { + InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax { Name: { } name } } => name.GetLocation(), + InvocationExpressionSyntax { Expression: { } expression } => expression.GetLocation(), + ObjectCreationExpressionSyntax { Type: QualifiedNameSyntax { Right: { } right } } => right.GetLocation(), + ObjectCreationExpressionSyntax { Type: { } type } => type.GetLocation(), + _ => base.PrimaryLocation(node), + }; } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.vb b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.vb index 71e1e3cae91..5c740bbd6c3 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.vb +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.vb @@ -19,7 +19,9 @@ Foo.GlobalDivide(divisor, dividend) Divide(dividend, divisor) ' Noncompliant [1] {{Parameters to 'Divide' have the same names but not the same order as the method arguments.}} +' ^^^^^^ Foo.GlobalDivide(dividend, divisor) ' Noncompliant [2] + ' ^^^^^^^^^^^^ Divide(dividend:=dividend, divisor:= divisor) @@ -41,8 +43,10 @@ x = New Foo x = New Foo(right, left) ' Noncompliant + ' ^^^ x = New FooFoo(right, left) ' Noncompliant x = New Foo.FooFoo(right, left) ' Noncompliant + ' ^^^^^^ x = New Foo(ValProp, Foo.ValProp) End Sub From 0861d96ad88148aab58d24079bda975d36d7056c Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 15:06:47 +0200 Subject: [PATCH 12/29] naming --- analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs index a6552434341..fb0a2d57008 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs @@ -125,7 +125,7 @@ public override SyntaxNode NodeExpression(SyntaxNode node) => node switch { ArrowExpressionClauseSyntax x => x.Expression, - ArgumentSyntax argument => argument.Expression, + ArgumentSyntax x => x.Expression, AttributeArgumentSyntax x => x.Expression, InterpolationSyntax x => x.Expression, InvocationExpressionSyntax x => x.Expression, From 6600fd11c0a57a29a0752f37549ea44a8039adbf Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 15:24:29 +0200 Subject: [PATCH 13/29] Use Language comparrsion --- .../Rules/ParametersCorrectOrderBase.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs index a8d9a3986fd..9c406a370b5 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs @@ -64,11 +64,11 @@ protected sealed override void Initialize(SonarAnalysisContext context) => } }, InvocationKinds); - protected virtual Location PrimaryLocation(SyntaxNode node) - => node.GetLocation(); + protected virtual Location PrimaryLocation(SyntaxNode node) => + node.GetLocation(); - private static bool MatchingNames(IParameterSymbol parameter, string argumentName) => - parameter.Name == argumentName; + private bool MatchingNames(IParameterSymbol parameter, string argumentName) => + Language.NameComparer.Equals(parameter.Name, argumentName); private string ArgumentName(SyntaxNode argument) => Language.Syntax.NodeIdentifier(Language.Syntax.NodeExpression(argument))?.ValueText; From 722840867c556b349767dcea21b53ce3f0ce72d4 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 15:27:40 +0200 Subject: [PATCH 14/29] Commets --- .../Rules/ParametersCorrectOrder.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ParametersCorrectOrder.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ParametersCorrectOrder.cs index bc5094151fa..0c3c72f5999 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ParametersCorrectOrder.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Rules/ParametersCorrectOrder.cs @@ -30,13 +30,14 @@ public sealed class ParametersCorrectOrder : ParametersCorrectOrderBase Language => VisualBasicFacade.Instance; + protected override Location PrimaryLocation(SyntaxNode node) => node switch { - InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax { Name: { } name } } => name.GetLocation(), - InvocationExpressionSyntax { Expression: { } expression } => expression.GetLocation(), - ObjectCreationExpressionSyntax { Type: QualifiedNameSyntax { Right: { } right } } => right.GetLocation(), - ObjectCreationExpressionSyntax { Type: { } type } => type.GetLocation(), + InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax { Name: { } name } } => name.GetLocation(), // A.B.C() -> C + InvocationExpressionSyntax { Expression: { } expression } => expression.GetLocation(), // A() -> A + ObjectCreationExpressionSyntax { Type: QualifiedNameSyntax { Right: { } right } } => right.GetLocation(), // New A.B.C() -> C + ObjectCreationExpressionSyntax { Type: { } type } => type.GetLocation(), // New A() -> A _ => base.PrimaryLocation(node), }; } From b1e53e5dfc6e1f894bbbca38338722582ca9ede8 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 15:30:22 +0200 Subject: [PATCH 15/29] Comments --- .../Rules/ParametersCorrectOrder.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs index a1134ee0692..8e9cef06f60 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs @@ -38,13 +38,15 @@ public sealed class ParametersCorrectOrder : ParametersCorrectOrderBase node switch { - InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax { Name: { } name } } => name.GetLocation(), - InvocationExpressionSyntax { Expression: { } expression } => expression.GetLocation(), - ObjectCreationExpressionSyntax { Type: QualifiedNameSyntax { Right: { } right } } => right.GetLocation(), - ObjectCreationExpressionSyntax { Type: { } type } => type.GetLocation(), - ConstructorInitializerSyntax { ThisOrBaseKeyword: { } keyword } => keyword.GetLocation(), + InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax { Name: { } name } } => name.GetLocation(), // A.B.C() -> C + InvocationExpressionSyntax { Expression: { } expression } => expression.GetLocation(), // A() -> A + ObjectCreationExpressionSyntax { Type: QualifiedNameSyntax { Right: { } right } } => right.GetLocation(), // new A.B.C() -> C + ObjectCreationExpressionSyntax { Type: { } type } => type.GetLocation(), // new A() -> A + ConstructorInitializerSyntax { ThisOrBaseKeyword: { } keyword } => keyword.GetLocation(), // this() -> this _ when PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(node) && ((PrimaryConstructorBaseTypeSyntaxWrapper)node).Type is { } type => - type is QualifiedNameSyntax { Right: { } right } ? right.GetLocation() : type.GetLocation(), + type is QualifiedNameSyntax { Right: { } right } + ? right.GetLocation() // class A: B.C() -> C + : type.GetLocation(), // class A: B() -> B _ => base.PrimaryLocation(node), }; } From b11b572a0a34371f7a42f5db91f4662360d708d6 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 15:54:16 +0200 Subject: [PATCH 16/29] Use `Rule` and add null test --- .../SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs | 2 +- .../Extensions/SyntaxNodeExtensionsTest.cs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs index 9c406a370b5..cecb295f8ab 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs @@ -57,7 +57,7 @@ protected sealed override void Initialize(SonarAnalysisContext context) => var secondaryLocations = methodParameterLookup.MethodSymbol.DeclaringSyntaxReferences .Select(s => Language.Syntax.NodeIdentifier(s.GetSyntax())?.GetLocation()) .WhereNotNull(); - c.ReportIssue(Diagnostic.Create(SupportedDiagnostics[0], PrimaryLocation(c.Node), secondaryLocations, properties: null, methodParameterLookup.MethodSymbol.Name)); + c.ReportIssue(Diagnostic.Create(Rule, PrimaryLocation(c.Node), secondaryLocations, properties: null, methodParameterLookup.MethodSymbol.Name)); return; } } diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs index c32e017ae06..1616a830ce1 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs @@ -802,6 +802,10 @@ public void M() { argumentList.Should().BeEmpty(); } + [TestMethod] + public void ArgumentList_CS_Null() => + ExtensionsCS.ArgumentList(null).Should().BeEmpty(); + [DataTestMethod] [DataRow("_ = $$new int[] { 1 }$$;")] [DataRow("_ = $$new { A = 1 }$$;")] From 076d3f6a6798dc455c62153f542b73cee67d76c6 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 16:19:02 +0200 Subject: [PATCH 17/29] Add tests for ArgumentNameColon --- .../Facade/SyntaxFacadeTest.cs | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Facade/SyntaxFacadeTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Facade/SyntaxFacadeTest.cs index 520c9c5d2fb..73b580b3419 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Facade/SyntaxFacadeTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Facade/SyntaxFacadeTest.cs @@ -131,4 +131,57 @@ public void RemoveConditionalAccess_Null_CS() => [DataRow("A.B?.C?.D", ".D")] public void RemoveConditionalAccess_SimpleInvocation_CS(string invocation, string expected) => cs.RemoveConditionalAccess(CS.SyntaxFactory.ParseExpression(invocation)).ToString().Should().Be(expected); + + [TestMethod] + public void ArgumentNameColon_VB_SimpleNameWithNameColonEquals() + { + var expression = VB.SyntaxFactory.LiteralExpression(VB.SyntaxKind.TrueLiteralExpression, VB.SyntaxFactory.Token(VB.SyntaxKind.TrueKeyword)); + var argument = VB.SyntaxFactory.SimpleArgument(VB.SyntaxFactory.NameColonEquals(VB.SyntaxFactory.IdentifierName("a")), expression); + vb.ArgumentNameColon(argument).Should().BeOfType().Subject.ValueText.Should().Be("a"); + } + + [TestMethod] + public void ArgumentNameColon_VB_SimpleNameWithoutNameColonEquals() + { + var expression = VB.SyntaxFactory.LiteralExpression(VB.SyntaxKind.TrueLiteralExpression, VB.SyntaxFactory.Token(VB.SyntaxKind.TrueKeyword)); + var argument = VB.SyntaxFactory.SimpleArgument(expression); + vb.ArgumentNameColon(argument).Should().BeNull(); + } + + [TestMethod] + public void ArgumentNameColon_VB_OmittedArgument() + { + var argument = VB.SyntaxFactory.OmittedArgument(); + vb.ArgumentNameColon(argument).Should().BeNull(); + } + + [TestMethod] + public void ArgumentNameColon_VB_UnsupportedSyntaxKind() + { + var expression = VB.SyntaxFactory.LiteralExpression(VB.SyntaxKind.TrueLiteralExpression, VB.SyntaxFactory.Token(VB.SyntaxKind.TrueKeyword)); + vb.ArgumentNameColon(expression).Should().BeNull(); + } + + [TestMethod] + public void ArgumentNameColon_CS_WithNameColon() + { + var expression = CS.SyntaxFactory.LiteralExpression(CS.SyntaxKind.TrueLiteralExpression, CS.SyntaxFactory.Token(CS.SyntaxKind.TrueKeyword)); + var argument = CS.SyntaxFactory.Argument(CS.SyntaxFactory.NameColon(CS.SyntaxFactory.IdentifierName("a")), CS.SyntaxFactory.Token(CS.SyntaxKind.None), expression); + cs.ArgumentNameColon(argument).Should().BeOfType().Subject.ValueText.Should().Be("a"); + } + + [TestMethod] + public void ArgumentNameColon_CS_WithoutNameColon() + { + var expression = CS.SyntaxFactory.LiteralExpression(CS.SyntaxKind.TrueLiteralExpression, CS.SyntaxFactory.Token(CS.SyntaxKind.TrueKeyword)); + var argument = CS.SyntaxFactory.Argument(expression); + cs.ArgumentNameColon(argument).Should().BeNull(); + } + + [TestMethod] + public void ArgumentNameColon_CS_UnsupportedSyntaxKind() + { + var expression = CS.SyntaxFactory.LiteralExpression(CS.SyntaxKind.TrueLiteralExpression, CS.SyntaxFactory.Token(CS.SyntaxKind.TrueKeyword)); + cs.ArgumentNameColon(expression).Should().BeNull(); + } } From 2b60c99ca964f7f9bcfd7934c7fdaef6fc971ef6 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 16:21:00 +0200 Subject: [PATCH 18/29] Simplify test --- .../tests/SonarAnalyzer.UnitTest/Facade/SyntaxFacadeTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Facade/SyntaxFacadeTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Facade/SyntaxFacadeTest.cs index 73b580b3419..3d5edf1dfc0 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Facade/SyntaxFacadeTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Facade/SyntaxFacadeTest.cs @@ -165,7 +165,7 @@ public void ArgumentNameColon_VB_UnsupportedSyntaxKind() [TestMethod] public void ArgumentNameColon_CS_WithNameColon() { - var expression = CS.SyntaxFactory.LiteralExpression(CS.SyntaxKind.TrueLiteralExpression, CS.SyntaxFactory.Token(CS.SyntaxKind.TrueKeyword)); + var expression = CS.SyntaxFactory.LiteralExpression(CS.SyntaxKind.TrueLiteralExpression); var argument = CS.SyntaxFactory.Argument(CS.SyntaxFactory.NameColon(CS.SyntaxFactory.IdentifierName("a")), CS.SyntaxFactory.Token(CS.SyntaxKind.None), expression); cs.ArgumentNameColon(argument).Should().BeOfType().Subject.ValueText.Should().Be("a"); } @@ -173,7 +173,7 @@ public void ArgumentNameColon_CS_WithNameColon() [TestMethod] public void ArgumentNameColon_CS_WithoutNameColon() { - var expression = CS.SyntaxFactory.LiteralExpression(CS.SyntaxKind.TrueLiteralExpression, CS.SyntaxFactory.Token(CS.SyntaxKind.TrueKeyword)); + var expression = CS.SyntaxFactory.LiteralExpression(CS.SyntaxKind.TrueLiteralExpression); var argument = CS.SyntaxFactory.Argument(expression); cs.ArgumentNameColon(argument).Should().BeNull(); } From 282169f8f59a9d03cca38d005cbae4bfde4491f6 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 16:43:21 +0200 Subject: [PATCH 19/29] Coverage --- analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs index fb0a2d57008..4bcf0a9d036 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs @@ -35,7 +35,7 @@ public override IReadOnlyList ArgumentList(SyntaxNode node) => Cast(argument).GetArgumentIndex(); public override SyntaxToken? ArgumentNameColon(SyntaxNode argument) => - (argument as ArgumentSyntax)?.NameColon?.Name?.Identifier; + (argument as ArgumentSyntax)?.NameColon?.Name.Identifier; public override SyntaxNode AssignmentLeft(SyntaxNode assignment) => Cast(assignment).Left; From 47acd12b09dac3f0617eae01f64746114d44fa8e Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 16:57:01 +0200 Subject: [PATCH 20/29] Fix IsRedundantPrimaryConstructorBaseTypeContext --- .../AnalysisContext/SonarSyntaxNodeReportingContext.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarSyntaxNodeReportingContext.cs b/analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarSyntaxNodeReportingContext.cs index 8872c37dc98..53ea274867c 100644 --- a/analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarSyntaxNodeReportingContext.cs +++ b/analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarSyntaxNodeReportingContext.cs @@ -48,17 +48,17 @@ public bool IsRedundantPositionalRecordContext() => /// /// Roslyn invokes the analyzer twice for PrimaryConstructorBaseType. The ContainingSymbol is first the type and second the constructor. This check filters can be used to filter - /// the first invocation. See also #roslyn/70488 + /// the first invocation. See also #Roslyn/70488. /// /// - /// Returns for the invocation on the class declaration and for the ctor invocation. + /// Returns for the invocation with PrimaryConstructorBaseType and ContainingSymbol being the type for everything else. /// public bool IsRedundantPrimaryConstructorBaseTypeContext() => - Context.Compilation.Language == LanguageNames.CSharp - && Context is + Context is { Node.RawKind: (int)SyntaxKindEx.PrimaryConstructorBaseType, - ContainingSymbol.Kind: SymbolKind.Method, + Compilation.Language: LanguageNames.CSharp, + ContainingSymbol.Kind: SymbolKind.NamedType, }; public bool IsAzureFunction() => From b0c93d0172acb0ea33a517f251127f4fa96efba3 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 16:58:45 +0200 Subject: [PATCH 21/29] Fix comment --- .../AnalysisContext/SonarSyntaxNodeReportingContext.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarSyntaxNodeReportingContext.cs b/analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarSyntaxNodeReportingContext.cs index 53ea274867c..ce368ae5f8f 100644 --- a/analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarSyntaxNodeReportingContext.cs +++ b/analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarSyntaxNodeReportingContext.cs @@ -51,7 +51,8 @@ public bool IsRedundantPositionalRecordContext() => /// the first invocation. See also #Roslyn/70488. /// /// - /// Returns for the invocation with PrimaryConstructorBaseType and ContainingSymbol being the type for everything else. + /// Returns for the invocation with PrimaryConstructorBaseType and ContainingSymbol being and + /// for everything else. /// public bool IsRedundantPrimaryConstructorBaseTypeContext() => Context is From 808976317b80f8cfd5b7c6acc11231e642a192e0 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 17:12:17 +0200 Subject: [PATCH 22/29] Primary clocation: ImplicitObjectCreationExpressionSyntaxWrapper --- .../src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs | 4 +++- .../TestCases/ParametersCorrectOrder.CSharp12.cs | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs index 8e9cef06f60..8db62904da5 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs @@ -39,10 +39,12 @@ protected override Location PrimaryLocation(SyntaxNode node) => node switch { InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax { Name: { } name } } => name.GetLocation(), // A.B.C() -> C - InvocationExpressionSyntax { Expression: { } expression } => expression.GetLocation(), // A() -> A + InvocationExpressionSyntax { Expression: { } expression } => expression.GetLocation(), // A() -> A ObjectCreationExpressionSyntax { Type: QualifiedNameSyntax { Right: { } right } } => right.GetLocation(), // new A.B.C() -> C ObjectCreationExpressionSyntax { Type: { } type } => type.GetLocation(), // new A() -> A ConstructorInitializerSyntax { ThisOrBaseKeyword: { } keyword } => keyword.GetLocation(), // this() -> this + _ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(node) => + ((ImplicitObjectCreationExpressionSyntaxWrapper)node).NewKeyword.GetLocation(), // new() -> new _ when PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(node) && ((PrimaryConstructorBaseTypeSyntaxWrapper)node).Type is { } type => type is QualifiedNameSyntax { Right: { } right } ? right.GetLocation() // class A: B.C() -> C diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.CSharp12.cs b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.CSharp12.cs index 0f543a58312..c4a95250abe 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.CSharp12.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/TestCases/ParametersCorrectOrder.CSharp12.cs @@ -29,7 +29,8 @@ void Basics(int a, int b, int c) void WithPromotion(short a, short b) { - _ = new SomeRecord(b, a); // Noncompliant [SomeRecord2] + SomeRecord _ = new(b, a); // Noncompliant [SomeRecord2] + // ^^^ } void WithCasting(long a, long b) From 64bd12fdf9952c227847ee3e4cc22fb8176574d1 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 22:06:54 +0200 Subject: [PATCH 23/29] Return ArgumentListSyntax from ArgumentList() and use it in MethodParameterLookup --- .../Extensions/SyntaxNodeExtensions.cs | 6 ++--- .../Facade/CSharpFacade.cs | 16 ++----------- .../Facade/CSharpSyntaxFacade.cs | 2 +- .../Extensions/SyntaxNodeExtensions.cs | 6 ++--- .../Facade/VisualBasicFacade.cs | 14 ++--------- .../Facade/VisualBasicSyntaxFacade.cs | 4 +--- .../Extensions/SyntaxNodeExtensionsTest.cs | 24 +++++++++---------- 7 files changed, 23 insertions(+), 49 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs b/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs index ab725141a50..9f882e24cfa 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs @@ -343,8 +343,8 @@ public static bool IsParentKind(this SyntaxNode node, SyntaxKind kind, out T } // based on Type="ArgumentListSyntax" in https://github.com/dotnet/roslyn/blob/main/src/Compilers/CSharp/Portable/Syntax/Syntax.xml - public static IReadOnlyList ArgumentList(this SyntaxNode node) => - (node switch + public static ArgumentListSyntax ArgumentList(this SyntaxNode node) => + node switch { ObjectCreationExpressionSyntax creation => creation.ArgumentList, InvocationExpressionSyntax invocation => invocation.ArgumentList, @@ -353,7 +353,7 @@ public static IReadOnlyList ArgumentList(this SyntaxNode node) = _ when PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(node) => ((PrimaryConstructorBaseTypeSyntaxWrapper)node).ArgumentList, _ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(node) => ((ImplicitObjectCreationExpressionSyntaxWrapper)node).ArgumentList, _ => throw new InvalidOperationException($"The {nameof(node)} of kind {node.Kind()} does not have an {nameof(ArgumentList)}."), - })?.Arguments ?? (IReadOnlyList)Array.Empty(); + }; public static ParameterListSyntax ParameterList(this SyntaxNode node) => node switch diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpFacade.cs index 7f360294900..49bdadf3986 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpFacade.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpFacade.cs @@ -56,23 +56,11 @@ public IMethodParameterLookup MethodParameterLookup(SyntaxNode invocation, IMeth { null => null, AttributeSyntax x => new CSharpAttributeParameterLookup(x, methodSymbol), - _ => new CSharpMethodParameterLookup(GetArgumentList(invocation), methodSymbol), + _ => new CSharpMethodParameterLookup(invocation.ArgumentList(), methodSymbol), }; public IMethodParameterLookup MethodParameterLookup(SyntaxNode invocation, SemanticModel semanticModel) => - invocation != null ? new CSharpMethodParameterLookup(GetArgumentList(invocation), semanticModel) : null; - - private static ArgumentListSyntax GetArgumentList(SyntaxNode invocation) => - invocation switch - { - ArgumentListSyntax x => x, - ObjectCreationExpressionSyntax x => x.ArgumentList, - InvocationExpressionSyntax x => x.ArgumentList, - ConstructorInitializerSyntax x => x.ArgumentList, - _ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(invocation) => ((ImplicitObjectCreationExpressionSyntaxWrapper)invocation).ArgumentList, - _ when PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(invocation) => ((PrimaryConstructorBaseTypeSyntaxWrapper)invocation).ArgumentList, - _ => throw new ArgumentException($"{invocation.GetType()} does not contain an ArgumentList.", nameof(invocation)), - }; + invocation != null ? new CSharpMethodParameterLookup(invocation.ArgumentList(), semanticModel) : null; public string GetName(SyntaxNode expression) => expression.GetName(); diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs index 4bcf0a9d036..0f6e3cf1c0a 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs @@ -29,7 +29,7 @@ public override IEnumerable ArgumentExpressions(SyntaxNode node) => ArgumentList(node)?.OfType().Select(x => x.Expression) ?? Enumerable.Empty(); public override IReadOnlyList ArgumentList(SyntaxNode node) => - node.ArgumentList(); + node.ArgumentList()?.Arguments; public override int? ArgumentIndex(SyntaxNode argument) => Cast(argument).GetArgumentIndex(); diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs index 716c7e370c7..7c722d70167 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Extensions/SyntaxNodeExtensions.cs @@ -101,8 +101,8 @@ static bool TakesExpressionTree(SymbolInfo info) }; // based on kind="ArgumentList" in https://github.com/dotnet/roslyn/blob/main/src/Compilers/VisualBasic/Portable/Syntax/Syntax.xml - public static IReadOnlyList ArgumentList(this SyntaxNode node) => - (node switch + public static ArgumentListSyntax ArgumentList(this SyntaxNode node) => + node switch { ArrayCreationExpressionSyntax arrayCreation => arrayCreation.ArrayBounds, AttributeSyntax attribute => attribute.ArgumentList, @@ -114,7 +114,7 @@ public static IReadOnlyList ArgumentList(this SyntaxNode node) = RedimClauseSyntax reDim => reDim.ArrayBounds, null => null, _ => throw new InvalidOperationException($"The {nameof(node)} of kind {node.Kind()} does not have an {nameof(ArgumentList)}."), - })?.Arguments ?? (IReadOnlyList)Array.Empty(); + }; /// /// Returns the left hand side of a conditional access expression. Returns c in case like a?.b?[0].c?.d.e?.f if d is passed. diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicFacade.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicFacade.cs index b3c09a8465b..1e6b064f96a 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicFacade.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicFacade.cs @@ -51,20 +51,10 @@ public object FindConstantValue(SemanticModel model, SyntaxNode node) => node.FindConstantValue(model); public IMethodParameterLookup MethodParameterLookup(SyntaxNode invocation, IMethodSymbol methodSymbol) => - invocation != null ? new VisualBasicMethodParameterLookup(GetArgumentList(invocation), methodSymbol) : null; + invocation != null ? new VisualBasicMethodParameterLookup(invocation.ArgumentList(), methodSymbol) : null; public IMethodParameterLookup MethodParameterLookup(SyntaxNode invocation, SemanticModel semanticModel) => - invocation != null ? new VisualBasicMethodParameterLookup(GetArgumentList(invocation), semanticModel) : null; - - private static ArgumentListSyntax GetArgumentList(SyntaxNode invocation) => - invocation switch - { - ArgumentListSyntax x => x, - AttributeSyntax x => x.ArgumentList, - ObjectCreationExpressionSyntax x => x.ArgumentList, - InvocationExpressionSyntax x => x.ArgumentList, - _ => throw new ArgumentException($"{invocation.GetType()} does not contain an ArgumentList.", nameof(invocation)), - }; + invocation != null ? new VisualBasicMethodParameterLookup(invocation.ArgumentList(), semanticModel) : null; public string GetName(SyntaxNode expression) => expression.GetName(); diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs index c12aed450a2..efe2f3bd05c 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs @@ -18,8 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System.Collections.Generic; - namespace SonarAnalyzer.Helpers.Facade; internal sealed class VisualBasicSyntaxFacade : SyntaxFacade @@ -31,7 +29,7 @@ public override IEnumerable ArgumentExpressions(SyntaxNode node) => ArgumentList(node).OfType().Select(x => x.GetExpression()).WhereNotNull(); public override IReadOnlyList ArgumentList(SyntaxNode node) => - node.ArgumentList(); + node.ArgumentList()?.Arguments; public override int? ArgumentIndex(SyntaxNode argument) => Cast(argument).GetArgumentIndex(); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs index 1616a830ce1..3207d3afe97 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs @@ -747,7 +747,7 @@ public void M(int p) { } """; var node = NodeBetweenMarkers(code, LanguageNames.CSharp); - var argumentList = ExtensionsCS.ArgumentList(node); + var argumentList = ExtensionsCS.ArgumentList(node).Arguments; var argument = argumentList.Should().ContainSingle().Which; (argument is { Expression: SyntaxCS.LiteralExpressionSyntax { Token.ValueText: "1" } }).Should().BeTrue(); } @@ -768,7 +768,7 @@ public C(int p): base(p) { } """; var node = NodeBetweenMarkers(code, LanguageNames.CSharp); - var argumentList = ExtensionsCS.ArgumentList(node); + var argumentList = ExtensionsCS.ArgumentList(node).Arguments; var argument = argumentList.Should().ContainSingle().Which; (argument is { Expression: SyntaxCS.LiteralExpressionSyntax { Token.ValueText: "1" } }).Should().BeTrue(); } @@ -781,7 +781,7 @@ public class Base(int p); public class Derived(int p): $$Base(1)$$; """; var node = NodeBetweenMarkers(code, LanguageNames.CSharp); - var argumentList = ExtensionsCS.ArgumentList(node); + var argumentList = ExtensionsCS.ArgumentList(node).Arguments; var argument = argumentList.Should().ContainSingle().Which; (argument is { Expression: SyntaxCS.LiteralExpressionSyntax { Token.ValueText: "1" } }).Should().BeTrue(); } @@ -798,13 +798,12 @@ public void M() { } """; var node = NodeBetweenMarkers(code, LanguageNames.CSharp); - var argumentList = ExtensionsCS.ArgumentList(node); - argumentList.Should().BeEmpty(); + ExtensionsCS.ArgumentList(node).Should().BeNull(); } [TestMethod] public void ArgumentList_CS_Null() => - ExtensionsCS.ArgumentList(null).Should().BeEmpty(); + ExtensionsCS.ArgumentList(null).Should().BeNull(); [DataTestMethod] [DataRow("_ = $$new int[] { 1 }$$;")] @@ -847,7 +846,7 @@ End Class """; var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic, getInnermostNodeForTie: true); var argumentList = ExtensionsVB.ArgumentList(node); - var argument = argumentList.Should().ContainSingle().Which; + var argument = argumentList.Arguments.Should().ContainSingle().Which; (argument.GetExpression() is SyntaxVB.LiteralExpressionSyntax { Token.ValueText: "1" }).Should().BeTrue(); } @@ -864,7 +863,7 @@ End Class """; var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic, getInnermostNodeForTie: true); var argumentList = ExtensionsVB.ArgumentList(node); - argumentList.Should().SatisfyRespectively( + argumentList.Arguments.Should().SatisfyRespectively( a => (a.GetExpression() is SyntaxVB.IdentifierNameSyntax { Identifier.ValueText: "s" }).Should().BeTrue(), a => (a.GetExpression() is SyntaxVB.LiteralExpressionSyntax { Token.ValueText: "1" }).Should().BeTrue()); } @@ -879,7 +878,7 @@ End Class """; var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic, getInnermostNodeForTie: true); var argumentList = ExtensionsVB.ArgumentList(node); - var argument = argumentList.Should().ContainSingle().Which; + var argument = argumentList.Arguments.Should().ContainSingle().Which; (argument.GetExpression() is SyntaxVB.LiteralExpressionSyntax { Token.ValueText: "1" }).Should().BeTrue(); } @@ -899,7 +898,7 @@ End Class """; var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic, getInnermostNodeForTie: true); var argumentList = ExtensionsVB.ArgumentList(node); - var argument = argumentList.Should().ContainSingle().Which; + var argument = argumentList.Arguments.Should().ContainSingle().Which; (argument.GetExpression() is SyntaxVB.LiteralExpressionSyntax { Token.ValueText: "1" }).Should().BeTrue(); } @@ -914,13 +913,12 @@ End Sub End Class """; var node = NodeBetweenMarkers(code, LanguageNames.VisualBasic, getInnermostNodeForTie: false); - var argumentList = ExtensionsVB.ArgumentList(node); - argumentList.Should().BeEmpty(); + ExtensionsVB.ArgumentList(node).Should().BeNull(); } [TestMethod] public void ArgumentList_VB_Null() => - ExtensionsVB.ArgumentList(null).Should().BeEmpty(); + ExtensionsVB.ArgumentList(null).Should().BeNull(); [DataTestMethod] [DataRow("$$Dim a = 1$$")] From 5568b57465c8d99a192d6f968598868ef5b3f85f Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 22:20:54 +0200 Subject: [PATCH 24/29] Fix AssertionArgsShouldBePassedInCorrectOrder tests --- .../Rules/AssertionArgsShouldBePassedInCorrectOrder.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionArgsShouldBePassedInCorrectOrder.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionArgsShouldBePassedInCorrectOrder.cs index 73b5562d287..2669e95d124 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionArgsShouldBePassedInCorrectOrder.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionArgsShouldBePassedInCorrectOrder.cs @@ -37,7 +37,7 @@ protected override void Initialize(SonarAnalysisContext context) => && c.SemanticModel.GetSymbolInfo(invocation).AllSymbols() .SelectMany(symbol => symbol is IMethodSymbol { IsStatic: true, ContainingSymbol: INamedTypeSymbol container } methodSymbol - ? knownAssertParameters.Select(knownParameters => FindWrongArguments(c.SemanticModel, container, methodSymbol, argumentList, knownParameters)) + ? knownAssertParameters.Select(knownParameters => FindWrongArguments(c.SemanticModel, container, methodSymbol, invocation, knownParameters)) : Enumerable.Empty()) .FirstOrDefault(x => x is not null) is (Expected: var expected, Actual: var actual)) { @@ -79,10 +79,10 @@ private static KnownAssertParameters[] GetParameters(string name) => private static WrongArguments? FindWrongArguments(SemanticModel semanticModel, INamedTypeSymbol container, IMethodSymbol symbol, - ArgumentListSyntax argumentList, + InvocationExpressionSyntax invocation, KnownAssertParameters knownParameters) => container.Is(knownParameters.AssertClass) - && CSharpFacade.Instance.MethodParameterLookup(argumentList, symbol) is var parameterLookup + && CSharpFacade.Instance.MethodParameterLookup(invocation, symbol) is var parameterLookup && parameterLookup.TryGetSyntax(knownParameters.ExpectedParameterName, out var expectedArguments) && expectedArguments.FirstOrDefault() is { } expected && semanticModel.GetConstantValue(expected).HasValue is false From d29167ac8e9b49a8b001a5ffbb3fd1154587a8b3 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 23 Oct 2023 22:29:47 +0200 Subject: [PATCH 25/29] Fix failing UTs --- .../Facade/VisualBasicSyntaxFacade.cs | 2 +- .../SonarAnalyzer.UnitTest/Facade/CSharpFacadeTests.cs | 8 ++++---- .../Facade/VisualBasicFacadeTests.cs | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs index efe2f3bd05c..7c638f4b3f1 100644 --- a/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs +++ b/analyzers/src/SonarAnalyzer.VisualBasic/Facade/VisualBasicSyntaxFacade.cs @@ -26,7 +26,7 @@ public override bool AreEquivalent(SyntaxNode firstNode, SyntaxNode secondNode) SyntaxFactory.AreEquivalent(firstNode, secondNode); public override IEnumerable ArgumentExpressions(SyntaxNode node) => - ArgumentList(node).OfType().Select(x => x.GetExpression()).WhereNotNull(); + ArgumentList(node)?.OfType().Select(x => x.GetExpression()).WhereNotNull() ?? Enumerable.Empty(); public override IReadOnlyList ArgumentList(SyntaxNode node) => node.ArgumentList()?.Arguments; diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Facade/CSharpFacadeTests.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Facade/CSharpFacadeTests.cs index 67202ed4dac..b8f192ab74d 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Facade/CSharpFacadeTests.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Facade/CSharpFacadeTests.cs @@ -116,10 +116,10 @@ public int M(int arg) => """; var (tree, model) = TestHelper.CompileCS(code); var root = tree.GetRoot(); - var invocation = root.DescendantNodes().OfType().First(); + var argumentList = root.DescendantNodes().OfType().First(); var method = model.GetDeclaredSymbol(root.DescendantNodes().OfType().First()); - var actual = sut.MethodParameterLookup(invocation, method); - actual.Should().NotBeNull().And.BeOfType(); + var actual = () => sut.MethodParameterLookup(argumentList, method); + actual.Should().Throw(); } [TestMethod] @@ -138,7 +138,7 @@ public int M(int arg) => var methodDeclaration = root.DescendantNodes().OfType().First(); var method = model.GetDeclaredSymbol(methodDeclaration); var actual = () => sut.MethodParameterLookup(methodDeclaration, method); // MethodDeclarationSyntax passed instead of invocation - actual.Should().Throw().Which.Message.Should().StartWith("Microsoft.CodeAnalysis.CSharp.Syntax.MethodDeclarationSyntax does not contain an ArgumentList."); + actual.Should().Throw().Which.Message.Should().StartWith("The node of kind MethodDeclaration does not have an ArgumentList."); } [TestMethod] diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Facade/VisualBasicFacadeTests.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Facade/VisualBasicFacadeTests.cs index 89a11532d84..2f988aee165 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Facade/VisualBasicFacadeTests.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Facade/VisualBasicFacadeTests.cs @@ -97,8 +97,8 @@ End Class var root = tree.GetRoot(); var argumentList = root.DescendantNodes().OfType().First(); var method = model.GetDeclaredSymbol(root.DescendantNodes().OfType().First()); - var actual = sut.MethodParameterLookup(argumentList, method); - actual.Should().NotBeNull().And.BeOfType(); + var actual = () => sut.MethodParameterLookup(argumentList, method); + actual.Should().Throw(); } [TestMethod] @@ -117,7 +117,7 @@ End Class var methodDeclaration = root.DescendantNodes().OfType().First(); var method = model.GetDeclaredSymbol(methodDeclaration); var actual = () => sut.MethodParameterLookup(methodDeclaration, method); // MethodDeclarationSyntax passed instead of invocation - actual.Should().Throw().Which.Message.Should().StartWith("Microsoft.CodeAnalysis.VisualBasic.Syntax.MethodStatementSyntax does not contain an ArgumentList."); + actual.Should().Throw().Which.Message.Should().StartWith("The node of kind FunctionStatement does not have an ArgumentList."); } [TestMethod] From be996338b402a49e3384669ad2452ffe9edf8792 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 24 Oct 2023 09:03:06 +0200 Subject: [PATCH 26/29] Simplify --- .../Rules/AssertionArgsShouldBePassedInCorrectOrder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionArgsShouldBePassedInCorrectOrder.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionArgsShouldBePassedInCorrectOrder.cs index 2669e95d124..cf460897e73 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionArgsShouldBePassedInCorrectOrder.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionArgsShouldBePassedInCorrectOrder.cs @@ -32,7 +32,7 @@ public sealed class AssertionArgsShouldBePassedInCorrectOrder : SonarDiagnosticA protected override void Initialize(SonarAnalysisContext context) => context.RegisterNodeAction(c => { - if (c.Node is InvocationExpressionSyntax { ArgumentList: { Arguments.Count: >= 2 } argumentList } invocation + if (c.Node is InvocationExpressionSyntax { ArgumentList.Arguments.Count: >= 2 } invocation && GetParameters(invocation.GetName()) is { } knownAssertParameters && c.SemanticModel.GetSymbolInfo(invocation).AllSymbols() .SelectMany(symbol => From 9870e05a519397cc6b8cbedad5b3e5522e432f32 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 24 Oct 2023 17:13:38 +0200 Subject: [PATCH 27/29] Code review --- .../Facade/CSharpFacade.cs | 4 +++- .../Rules/ParametersCorrectOrderBase.cs | 8 +++---- .../Facade/VisualBasicFacade.cs | 8 +++++-- .../Facade/SyntaxFacadeTest.cs | 9 ++++++++ .../ParametersCorrectOrder.CSharp12.cs | 22 ++++++++++++------- 5 files changed, 36 insertions(+), 15 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpFacade.cs b/analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpFacade.cs index 49bdadf3986..f2781b52d64 100644 --- 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 index cecb295f8ab..831d3227ab1 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs @@ -27,16 +27,16 @@ public abstract class ParametersCorrectOrderBase : 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 index 1e6b064f96a..4042cb02804 100644 --- 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 index 3d5edf1dfc0..0ebac1fe305 100644 --- 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 index c4a95250abe..7b2bb68eec9 100644 --- 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 From 0ebf9030a1e6c1a552d895a8e118cb0f43e10046 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 25 Oct 2023 12:38:19 +0200 Subject: [PATCH 28/29] Use "NodeIdentifier" for PrimaryLocation --- .../Extensions/SyntaxNodeExtensions.cs | 4 +++ .../Rules/ParametersCorrectOrder.cs | 17 ----------- .../Rules/ParametersCorrectOrderBase.cs | 2 +- .../Extensions/SyntaxNodeExtensionsTest.cs | 28 +++++++++++++++++++ 4 files changed, 33 insertions(+), 18 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs b/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs index 9f882e24cfa..60ab4182eea 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Extensions/SyntaxNodeExtensions.cs @@ -146,6 +146,7 @@ public static SyntaxNode WalkUpParentheses(this SyntaxNode node) AttributeSyntax { Name: { } name } => GetIdentifier(name), BaseTypeDeclarationSyntax { Identifier: var identifier } => identifier, ConstructorDeclarationSyntax { Identifier: var identifier } => identifier, + ConstructorInitializerSyntax { ThisOrBaseKeyword: var keyword } => keyword, ConversionOperatorDeclarationSyntax { Type: { } type } => GetIdentifier(type), DelegateDeclarationSyntax { Identifier: var identifier } => identifier, DestructorDeclarationSyntax { Identifier: var identifier } => identifier, @@ -177,8 +178,11 @@ public static SyntaxNode WalkUpParentheses(this SyntaxNode node) PostfixUnaryExpressionSyntax { Operand: { } operand } => GetIdentifier(operand), UsingDirectiveSyntax { Alias.Name: { } name } => GetIdentifier(name), VariableDeclaratorSyntax { Identifier: var identifier } => identifier, + { } implicitNew when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(implicitNew) => ((ImplicitObjectCreationExpressionSyntaxWrapper)implicitNew).NewKeyword, { } fileScoped when FileScopedNamespaceDeclarationSyntaxWrapper.IsInstance(fileScoped) && ((FileScopedNamespaceDeclarationSyntaxWrapper)fileScoped).Name is { } name => GetIdentifier(name), + { } primary when PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(primary) + && ((PrimaryConstructorBaseTypeSyntaxWrapper)primary).Type is { } type => GetIdentifier(type), { } refType when RefTypeSyntaxWrapper.IsInstance(refType) => GetIdentifier(((RefTypeSyntaxWrapper)refType).Type), _ => null }; diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs index 8db62904da5..009fe01e722 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs @@ -34,22 +34,5 @@ public sealed class ParametersCorrectOrder : ParametersCorrectOrderBase Language => CSharpFacade.Instance; - - protected override Location PrimaryLocation(SyntaxNode node) => - node switch - { - InvocationExpressionSyntax { Expression: MemberAccessExpressionSyntax { Name: { } name } } => name.GetLocation(), // A.B.C() -> C - InvocationExpressionSyntax { Expression: { } expression } => expression.GetLocation(), // A() -> A - ObjectCreationExpressionSyntax { Type: QualifiedNameSyntax { Right: { } right } } => right.GetLocation(), // new A.B.C() -> C - ObjectCreationExpressionSyntax { Type: { } type } => type.GetLocation(), // new A() -> A - ConstructorInitializerSyntax { ThisOrBaseKeyword: { } keyword } => keyword.GetLocation(), // this() -> this - _ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(node) => - ((ImplicitObjectCreationExpressionSyntaxWrapper)node).NewKeyword.GetLocation(), // new() -> new - _ when PrimaryConstructorBaseTypeSyntaxWrapper.IsInstance(node) && ((PrimaryConstructorBaseTypeSyntaxWrapper)node).Type is { } type => - type is QualifiedNameSyntax { Right: { } right } - ? right.GetLocation() // class A: B.C() -> C - : type.GetLocation(), // class A: B() -> B - _ => base.PrimaryLocation(node), - }; } } diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs index 831d3227ab1..3720bc35e35 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs @@ -65,7 +65,7 @@ protected sealed override void Initialize(SonarAnalysisContext context) => }, InvocationKinds); protected virtual Location PrimaryLocation(SyntaxNode node) => - node.GetLocation(); + Language.Syntax.NodeIdentifier(node)?.GetLocation() ?? node.GetLocation(); private bool MatchingNames(IParameterSymbol parameter, string argumentName) => Language.NameComparer.Equals(parameter.Name, argumentName); diff --git a/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs b/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs index 3207d3afe97..111c9754add 100644 --- a/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs +++ b/analyzers/tests/SonarAnalyzer.UnitTest/Extensions/SyntaxNodeExtensionsTest.cs @@ -1056,6 +1056,8 @@ public class C [DataRow("""record struct $$T$$ { }""", "T")] // BaseTypeDeclarationSyntax [DataRow("""enum $$T$$ { }""", "T")] // BaseTypeDeclarationSyntax [DataRow("""$$Test() { }$$""", "Test")] // ConstructorDeclarationSyntax + [DataRow("""Test() : $$this(1)$$ { }""", "this")] // ConstructorInitializerSyntax + [DataRow("""Test() : $$base()$$ { }""", "base")] // ConstructorInitializerSyntax [DataRow("""$$public static implicit operator int(Test t) => 1;$$""", "int")] // ConversionOperatorDeclarationSyntax [DataRow("""$$delegate void D();$$""", "D")] // DelegateDeclarationSyntax [DataRow("""$$~Test() { }$$""", "Test")] // DestructorDeclarationSyntax @@ -1086,6 +1088,7 @@ public class C [DataRow("""void M() where $$T : class$$ { }""", "T")] // TypeParameterConstraintClauseSyntax [DataRow("""void M<$$T$$>() { }""", "T")] // TypeParameterSyntax [DataRow("""int $$i$$;""", "i")] // VariableDeclaratorSyntax + [DataRow("""object o = $$new()$$;""", "new")] // ImplicitObjectCreationExpressionSyntax [DataRow("""void M(int p) { $$ref int$$ i = ref p; }""", "int")] // RefTypeSyntax public void GetIdentifier_Members(string member, string expected) { @@ -1096,6 +1099,7 @@ public void GetIdentifier_Members(string member, string expected) unsafe class Test { static Func Fun() => default; + public Test(int i) { } {{member}} } """, LanguageNames.CSharp); @@ -1133,6 +1137,30 @@ public void GetIdentifier_CompilationUnit(string member, string expected) } } + [DataTestMethod] + [DataRow(""" : $$Base(i)$$""", "Base")] // NamespaceDeclarationSyntax + [DataRow(""" : $$Test.Base(i)$$""", "Base")] // NamespaceDeclarationSyntax + public void GetIdentifier_PrimaryConstructor(string baseType, string expected) + { + var node = NodeBetweenMarkers($$""" + namespace Test; + public class Base(int i) + { + } + public class Derived(int i) {{baseType}} { } + """, LanguageNames.CSharp); + var actual = ExtensionsCS.GetIdentifier(node); + if (expected is null) + { + actual.Should().BeNull(); + } + else + { + actual.Should().NotBeNull(); + actual.Value.ValueText.Should().Be(expected); + } + } + private static SyntaxNode NodeBetweenMarkers(string code, string language, bool getInnermostNodeForTie = false) { var position = code.IndexOf("$$"); From a9fe51c95344041b2db9794c8f738afead87a048 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 25 Oct 2023 12:41:19 +0200 Subject: [PATCH 29/29] Split condition --- .../SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs index 3720bc35e35..540374a3f38 100644 --- a/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs +++ b/analyzers/src/SonarAnalyzer.Common/Rules/ParametersCorrectOrderBase.cs @@ -50,7 +50,8 @@ protected sealed override void Initialize(SonarAnalysisContext context) => && Language.Syntax.NodeExpression(argument) is { } argumentExpression && c.Context.SemanticModel.GetTypeInfo(argumentExpression).ConvertedType is { } argumentType // is there another parameter that seems to be a better fit (name and type match): p_x - && methodParameterLookup.MethodSymbol.Parameters.FirstOrDefault(p => MatchingNames(p, argumentName) && argumentType.DerivesOrImplements(p.Type)) is { IsParams: false } + && methodParameterLookup.MethodSymbol.Parameters.FirstOrDefault(p => MatchingNames(p, argumentName)) is { IsParams: false } otherParameter + && argumentType.DerivesOrImplements(otherParameter.Type) // is there an argument that matches the parameter p_y by name: a_y && Language.Syntax.ArgumentList(c.Node).FirstOrDefault(x => MatchingNames(parameterSymbol, ArgumentName(x))) is { }) {