Fix S2234 FN: primary constructors for records, classes and structs (#…

…8238)

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
             };
@@ -342,6 +346,19 @@ public static bool IsParentKind<T>(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 ArgumentListSyntax 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)}."),
+            };
+
         public static ParameterListSyntax ParameterList(this SyntaxNode node) =>
             node switch
             {

analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpFacade.cs

@@ -56,23 +56,13 @@ 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,
-            _ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(invocation) =>
-                ((ImplicitObjectCreationExpressionSyntaxWrapper)invocation).ArgumentList,
-            ConstructorInitializerSyntax x => x.ArgumentList,
-            _ => throw new ArgumentException($"{invocation.GetType()} does not contain an ArgumentList.", nameof(invocation)),
-        };
+        invocation?.ArgumentList() is { } argumentList
+            ? new CSharpMethodParameterLookup(argumentList, semanticModel)
+            : null;
 
     public string GetName(SyntaxNode expression) =>
         expression.GetName();

analyzers/src/SonarAnalyzer.CSharp/Facade/CSharpSyntaxFacade.cs

@@ -26,18 +26,17 @@ public override bool AreEquivalent(SyntaxNode firstNode, SyntaxNode secondNode)
         SyntaxFactory.AreEquivalent(firstNode, secondNode);
 
     public override IEnumerable<SyntaxNode> ArgumentExpressions(SyntaxNode node) =>
-        node switch
-        {
-            ObjectCreationExpressionSyntax creation => creation.ArgumentList?.Arguments.Select(x => x.Expression) ?? Enumerable.Empty<SyntaxNode>(),
-            null => Enumerable.Empty<SyntaxNode>(),
-            var _ when ImplicitObjectCreationExpressionSyntaxWrapper.IsInstance(node)
-                => ((ImplicitObjectCreationExpressionSyntaxWrapper)node).ArgumentList?.Arguments.Select(x => x.Expression) ?? Enumerable.Empty<SyntaxNode>(),
-            _ => throw InvalidOperation(node, nameof(ArgumentExpressions))
-        };
+        ArgumentList(node)?.OfType<ArgumentSyntax>().Select(x => x.Expression) ?? Enumerable.Empty<SyntaxNode>();
+
+    public override IReadOnlyList<SyntaxNode> ArgumentList(SyntaxNode node) =>
+        node.ArgumentList()?.Arguments;
 
     public override int? ArgumentIndex(SyntaxNode argument) =>
         Cast<ArgumentSyntax>(argument).GetArgumentIndex();
 
+    public override SyntaxToken? ArgumentNameColon(SyntaxNode argument) =>
+        (argument as ArgumentSyntax)?.NameColon?.Name.Identifier;
+
     public override SyntaxNode AssignmentLeft(SyntaxNode assignment) =>
         Cast<AssignmentExpressionSyntax>(assignment).Left;
 
@@ -126,6 +125,7 @@ public override SyntaxNode NodeExpression(SyntaxNode node) =>
         node switch
         {
             ArrowExpressionClauseSyntax x => x.Expression,
+            ArgumentSyntax x => x.Expression,
             AttributeArgumentSyntax x => x.Expression,
             InterpolationSyntax x => x.Expression,
             InvocationExpressionSyntax x => x.Expression,

analyzers/src/SonarAnalyzer.CSharp/Rules/AssertionArgsShouldBePassedInCorrectOrder.cs

@@ -32,12 +32,12 @@ 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 =>
                         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<WrongArguments?>())
                     .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

analyzers/src/SonarAnalyzer.CSharp/Rules/ParametersCorrectOrder.cs

@@ -21,66 +21,18 @@
 namespace SonarAnalyzer.Rules.CSharp
 {
     [DiagnosticAnalyzer(LanguageNames.CSharp)]
-    public sealed class ParametersCorrectOrder : ParametersCorrectOrderBase<ArgumentSyntax>
+    public sealed class ParametersCorrectOrder : ParametersCorrectOrderBase<SyntaxKind>
     {
-        private static readonly DiagnosticDescriptor rule =
-            DescriptorFactory.Create(DiagnosticId, MessageFormat);
-        public override ImmutableArray<DiagnosticDescriptor> 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<Location> 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,
+                SyntaxKind.BaseConstructorInitializer,
+                SyntaxKindEx.PrimaryConstructorBaseType,
+                SyntaxKindEx.ImplicitObjectCreationExpression,
+            };
+
+        protected override ILanguageFacade<SyntaxKind> Language => CSharpFacade.Instance;
     }
 }

analyzers/src/SonarAnalyzer.Common/AnalysisContext/SonarSyntaxNodeReportingContext.cs

@@ -46,6 +46,22 @@ internal SonarSyntaxNodeReportingContext(SonarAnalysisContext analysisContext, S
     public bool IsRedundantPositionalRecordContext() =>
         Context.ContainingSymbol.Kind == SymbolKind.Method;
 
+    /// <summary>
+    /// 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 <seealso href="https://github.com/dotnet/roslyn/issues/70488">#Roslyn/70488</seealso>.
+    /// </summary>
+    /// <returns>
+    /// Returns <see langword="true"/> for the invocation with PrimaryConstructorBaseType and ContainingSymbol being <see cref="SymbolKind.NamedType"/> and
+    /// <see langword="false"/> for everything else.
+    /// </returns>
+    public bool IsRedundantPrimaryConstructorBaseTypeContext() =>
+        Context is
+        {
+            Node.RawKind: (int)SyntaxKindEx.PrimaryConstructorBaseType,
+            Compilation.Language: LanguageNames.CSharp,
+            ContainingSymbol.Kind: SymbolKind.NamedType,
+        };
+
     public bool IsAzureFunction() =>
         AzureFunctionMethod() is not null;
 

analyzers/src/SonarAnalyzer.Common/Facade/SyntaxFacade.cs

@@ -26,6 +26,8 @@ public abstract class SyntaxFacade<TSyntaxKind>
     public abstract bool AreEquivalent(SyntaxNode firstNode, SyntaxNode secondNode);
     public abstract IEnumerable<SyntaxNode> ArgumentExpressions(SyntaxNode node);
     public abstract int? ArgumentIndex(SyntaxNode argument);
+    public abstract IReadOnlyList<SyntaxNode> ArgumentList(SyntaxNode node);
+    public abstract SyntaxToken? ArgumentNameColon(SyntaxNode argument);
     public abstract SyntaxNode AssignmentLeft(SyntaxNode assignment);
     public abstract SyntaxNode AssignmentRight(SyntaxNode assignment);
     public abstract ImmutableArray<SyntaxNode> AssignmentTargets(SyntaxNode assignment);

analyzers/src/SonarAnalyzer.Common/Helpers/MethodParameterLookupBase.cs

@@ -26,6 +26,7 @@ public interface IMethodParameterLookup
     bool TryGetSyntax(IParameterSymbol parameter, out ImmutableArray<SyntaxNode> expressions);
     bool TryGetSyntax(string parameterName, out ImmutableArray<SyntaxNode> 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)