Fix S1117 FN: support primary constructors (#8251)

analyzers/src/SonarAnalyzer.CSharp/Extensions/ISymbolExtensions.cs

@@ -25,6 +25,14 @@ namespace SonarAnalyzer.Extensions
 {
     internal static class ISymbolExtensions
     {
+        private static readonly SyntaxKind[] DeclarationsTypesWithPrimaryConstructor =
+        {
+            SyntaxKind.ClassDeclaration,
+            SyntaxKind.StructDeclaration,
+            SyntaxKindEx.RecordClassDeclaration,
+            SyntaxKindEx.RecordStructDeclaration
+        };
+
         public static bool HasConstraint(this ISymbol symbol, SymbolicConstraint constraint, ProgramState programState)
         {
             var symbolicValue = programState.GetSymbolValue(symbol);
@@ -92,5 +100,10 @@ public static ImmutableArray<SyntaxToken> DeclaringReferenceIdentifiers(this ISy
                .Select(x => x.GetSyntax().GetIdentifier())
                .WhereNotNull()
                .ToImmutableArray();
+
+        public static bool IsPrimaryConstructor(this ISymbol symbol) =>
+            symbol.IsConstructor()
+            && symbol.DeclaringSyntaxReferences.FirstOrDefault()?.GetSyntax() is { } syntax
+            && DeclarationsTypesWithPrimaryConstructor.Contains(syntax.Kind());
     }
 }

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

@@ -30,73 +30,79 @@ public sealed class VariableShadowsField : SonarDiagnosticAnalyzer
 
         public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
 
-        protected override void Initialize(SonarAnalysisContext context)
-        {
-            context.RegisterNodeAction(c =>
-                {
-                    var declaration = (ForEachStatementSyntax)c.Node;
-
-                    var variableSymbol = c.SemanticModel.GetDeclaredSymbol(declaration);
-                    if (variableSymbol == null)
-                    {
-                        return;
-                    }
-
-                    var members = GetMembers(variableSymbol.ContainingType);
-
-                    ReportOnVariableMatchingField(c, members, declaration.Identifier);
-                },
+        protected override void Initialize(SonarAnalysisContext context) =>
+            context.RegisterNodeAction(c => Process(c, GetDeclarationOrDesignation(c.Node)),
+                SyntaxKind.LocalDeclarationStatement,
+                SyntaxKind.ForStatement,
+                SyntaxKind.UsingStatement,
+                SyntaxKind.FixedStatement,
+                SyntaxKindEx.DeclarationExpression,
+                SyntaxKindEx.RecursivePattern,
+                SyntaxKindEx.VarPattern,
+                SyntaxKindEx.DeclarationPattern,
+                SyntaxKindEx.ListPattern,
                 SyntaxKind.ForEachStatement);
 
-            context.RegisterNodeAction(c => ProcessVariableDeclaration(c, ((LocalDeclarationStatementSyntax)c.Node).Declaration), SyntaxKind.LocalDeclarationStatement);
-            context.RegisterNodeAction(c => ProcessVariableDeclaration(c, ((ForStatementSyntax)c.Node).Declaration), SyntaxKind.ForStatement);
-            context.RegisterNodeAction(c => ProcessVariableDeclaration(c, ((UsingStatementSyntax)c.Node).Declaration), SyntaxKind.UsingStatement);
-            context.RegisterNodeAction(c => ProcessVariableDeclaration(c, ((FixedStatementSyntax)c.Node).Declaration), SyntaxKind.FixedStatement);
-
-            context.RegisterNodeAction(c => ProcessVariableDesignation(c, ((DeclarationExpressionSyntaxWrapper)c.Node).Designation), SyntaxKindEx.DeclarationExpression);
-            context.RegisterNodeAction(c => ProcessVariableDesignation(c, ((RecursivePatternSyntaxWrapper)c.Node).Designation), SyntaxKindEx.RecursivePattern);
-            context.RegisterNodeAction(c => ProcessVariableDesignation(c, ((VarPatternSyntaxWrapper)c.Node).Designation), SyntaxKindEx.VarPattern);
-            context.RegisterNodeAction(c => ProcessVariableDesignation(c, ((DeclarationPatternSyntaxWrapper)c.Node).Designation), SyntaxKindEx.DeclarationPattern);
-            context.RegisterNodeAction(c => ProcessVariableDesignation(c, ((ListPatternSyntaxWrapper)c.Node).Designation), SyntaxKindEx.ListPattern);
-        }
+        private static SyntaxNode GetDeclarationOrDesignation(SyntaxNode node) =>
+            node switch
+            {
+                LocalDeclarationStatementSyntax localDeclaration => localDeclaration.Declaration,
+                ForStatementSyntax forStatement => forStatement.Declaration,
+                UsingStatementSyntax usingStatement => usingStatement.Declaration,
+                FixedStatementSyntax fixedStatement => fixedStatement.Declaration,
+                ForEachStatementSyntax forEachStatement => forEachStatement,
+                _ when DeclarationExpressionSyntaxWrapper.IsInstance(node) => ((DeclarationExpressionSyntaxWrapper)node).Designation,
+                _ when RecursivePatternSyntaxWrapper.IsInstance(node) => ((RecursivePatternSyntaxWrapper)node).Designation,
+                _ when VarPatternSyntaxWrapper.IsInstance(node) => ((VarPatternSyntaxWrapper)node).Designation,
+                _ when DeclarationPatternSyntaxWrapper.IsInstance(node) => ((DeclarationPatternSyntaxWrapper)node).Designation,
+                _ when ListPatternSyntaxWrapper.IsInstance(node) => ((ListPatternSyntaxWrapper)node).Designation,
+                _ => null
+            };
 
-        private static void ProcessVariableDesignation(SonarSyntaxNodeReportingContext context, VariableDesignationSyntaxWrapper variableDesignation)
+        private static void Process(SonarSyntaxNodeReportingContext context, SyntaxNode node)
         {
-            if (variableDesignation.AllVariables() is { Length: > 0 } variables
-                && context.ContainingSymbol.ContainingType is { } containingType
-                && GetMembers(containingType) is var members)
+            if (ExtractIdentifiers(node) is { Count: > 0 } identifiers
+                && GetContextSymbols(context) is var members)
             {
-                foreach (var variable in variables)
+                foreach (var identifier in identifiers)
                 {
-                    ReportOnVariableMatchingField(context, members, variable.Identifier);
+                    ReportOnVariableMatchingField(context, members, identifier);
                 }
             }
         }
 
-        private static void ProcessVariableDeclaration(SonarSyntaxNodeReportingContext context, VariableDeclarationSyntax variableDeclaration)
-        {
-            if (variableDeclaration is { Variables: { Count: > 0 } variables }
-                && context.ContainingSymbol.ContainingType is { } containingType
-                && GetMembers(containingType) is var members)
+        private static List<SyntaxToken> ExtractIdentifiers(SyntaxNode node) =>
+            node switch
             {
-                foreach (var variable in variables)
-                {
-                    ReportOnVariableMatchingField(context, members, variable.Identifier);
-                }
-            }
+                VariableDeclarationSyntax variableDeclaration => variableDeclaration.Variables.Select(x => x.Identifier).ToList(),
+                ForEachStatementSyntax foreachStatement => new() { foreachStatement.Identifier },
+                _ when VariableDesignationSyntaxWrapper.IsInstance(node) => ((VariableDesignationSyntaxWrapper)node).AllVariables().Select(x => x.Identifier).ToList(),
+                _ => new()
+            };
+
+        private static List<ISymbol> GetContextSymbols(SonarSyntaxNodeReportingContext context)
+        {
+            var members = context.ContainingSymbol.ContainingType.GetMembers();
+            var primaryConstructorParameters = members.FirstOrDefault(x => x.IsPrimaryConstructor())?.GetParameters();
+            var fieldsAndProperties = members.Where(x => x is IPropertySymbol or IFieldSymbol).ToList();
+            return primaryConstructorParameters is null ? fieldsAndProperties : fieldsAndProperties.Concat(primaryConstructorParameters).ToList();
         }
 
         private static void ReportOnVariableMatchingField(SonarSyntaxNodeReportingContext context, IEnumerable<ISymbol> members, SyntaxToken identifier)
         {
-            if (members.FirstOrDefault(m => m.Name == identifier.ValueText) is { } matchingMember)
+            if (members.FirstOrDefault(x => x.Name == identifier.ValueText) is { } matchingMember)
             {
-                context.ReportIssue(Diagnostic.Create(Rule, identifier.GetLocation(), identifier.Text, matchingMember is IFieldSymbol ? "field" : "property"));
+                context.ReportIssue(Diagnostic.Create(Rule, identifier.GetLocation(), identifier.Text, GetSymbolName(matchingMember)));
             }
         }
 
-        private static List<ISymbol> GetMembers(INamespaceOrTypeSymbol classSymbol) =>
-            classSymbol.GetMembers()
-                       .Where(member => member is IFieldSymbol or IPropertySymbol)
-                       .ToList();
+        private static string GetSymbolName(ISymbol symbol) =>
+            symbol switch
+            {
+                IFieldSymbol => "field",
+                IPropertySymbol => "property",
+                IParameterSymbol => "primary constructor parameter",
+                _ => string.Empty
+            };
     }
 }

analyzers/tests/SonarAnalyzer.UnitTest/Rules/VariableShadowsFieldTest.cs

@@ -60,6 +60,12 @@ public void VariableShadowsField_CSharp12() =>
                 .WithOptions(ParseOptionsHelper.FromCSharp12)
                 .Verify();
 
+        [TestMethod]
+        public void VariableShadowsField_PartialClass() =>
+            builder.AddPaths("VariableShadowsField_PartialClass1.cs", "VariableShadowsField_PartialClass2.cs")
+                .WithOptions(ParseOptionsHelper.FromCSharp12)
+                .Verify();
+
 #endif
 
     }

analyzers/tests/SonarAnalyzer.UnitTest/TestCases/VariableShadowsField.CSharp12.cs

@@ -1,11 +1,54 @@
-class MyClass(int first, int second, int third, int fourth)
+using System;
+
+class MyParentClass(int parentClassParam)
 {
-    private int first = first; // Compliant
-    private int second; // FN
+    class MyClass(int a, int b, int c, int d, int e, int f, int g, int h, int i, int j)
+    {
+        public int a = a; // Compliant
+        public int b; // FN
+
+        public int c { get; set; } = c; // Compliant
+        public int d { get; set; } // FN
+
+        public int g = a; // FN {{Rename 'g' which hides the primary constructor parameter with the same name.}}
+
+        class MyChildClass(int childClassParam) { }
+
+        MyClass(int ctorParam, int f) : this(ctorParam, f, 1, 1, 1, 1, 1, 1, 1, 1) { } // Compliant
+
+        void MyMethod()
+        {
+            int parentClassParam = 42; // Compliant
+            int childClassParam = 42; // Compliant
+            int ctorParam = 42; // Compliant
 
-    void MyMethod()
+            int f = 42; // Noncompliant {{Rename 'f' which hides the primary constructor parameter with the same name.}}
+            _ = a is object g; // Noncompliant
+            for (int h = 0; h < 10; h++) { } // Noncompliant {{Rename 'h' which hides the primary constructor parameter with the same name.}}
+            for (int a = 0; a < 10; a++) { } // Noncompliant {{Rename 'a' which hides the field with the same name.}}
+            for (int c = 0; c < 10; c++) { } // Noncompliant {{Rename 'c' which hides the property with the same name.}}
+            var lambda = (int h = 1) => h; // Compliant
+            foreach (var (i, j) in new (int, int)[0]) { } // Noncompliant
+                                                          // Noncompliant@-1
+        }
+    }
+}
+
+// Reproducer for https://github.com/SonarSource/sonar-dotnet/issues/8260
+class Repro_8260(int primaryCtorParam)
+{
+    private int instanceField = 1;
+    private int instanceProperty { get; set; } = 1;
+    private static int staticField = 1;
+    private static int staticProperty = 1;
+
+    public static void StaticMethod()
     {
-        int third = 1; // FN
-        _ = first is object fourth; // FN
+        var instanceField = 2; // Noncompliant FP (instance field is not accessible from static method)
+        var instanceProperty = 2; // Noncompliant FP (instance property is not accessible from static method)
+        var primaryCtorParam = 2; // Noncompliant FP (primary ctor parameter is not accessible from static method)
+
+        var staticField = 2; // Noncompliant
+        var staticProperty = 2; // Noncompliant
     }
 }

analyzers/tests/SonarAnalyzer.UnitTest/TestCases/VariableShadowsField_PartialClass1.cs

@@ -0,0 +1,7 @@
+public partial class VariableShadowsField
+{
+    public int myField;
+    public int myProperty { get; set; }
+}
+
+public partial class VariableShadowsFieldPrimaryConstructor(int a) { }

analyzers/tests/SonarAnalyzer.UnitTest/TestCases/VariableShadowsField_PartialClass2.cs

@@ -0,0 +1,19 @@
+public partial class VariableShadowsField
+{
+    public void Method(object someParameter)
+    {
+        int myField = 0; // Noncompliant
+        //  ^^^^^^^
+        int myProperty = 0; // Noncompliant
+        //  ^^^^^^^^^^
+    }
+}
+
+public partial class VariableShadowsFieldPrimaryConstructor
+{
+    void Method()
+    {
+        var a = 0; // Noncompliant
+        //  ^
+    }
+}