Fix S1172: code fix does not delete unused parameter in function call

SonarSource · Jul 18, 2024 · d611740 · d611740
1 parent 0fb0f76
commit d611740
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 17 deletions.
diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/MethodParameterUnusedCodeFix.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/MethodParameterUnusedCodeFix.cs
@@ -19,39 +19,82 @@
  */
 
 using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.FindSymbols;
+using Microsoft.CodeAnalysis.Text;
 
-namespace SonarAnalyzer.Rules.CSharp
+namespace SonarAnalyzer.Rules.CSharp;
+
+[ExportCodeFixProvider(LanguageNames.CSharp)]
+public sealed class MethodParameterUnusedCodeFix : SonarCodeFix
 {
-    [ExportCodeFixProvider(LanguageNames.CSharp)]
-    public sealed class MethodParameterUnusedCodeFix : SonarCodeFix
+    private const string Title = "Remove unused parameter";
+
+    public override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(MethodParameterUnused.DiagnosticId);
+
+    protected override async Task RegisterCodeFixesAsync(SyntaxNode root, SonarCodeFixContext context)
     {
-        private const string Title = "Remove unused parameter";
+        var diagnostic = context.Diagnostics.First();
+        var diagnosticSpan = diagnostic.Location.SourceSpan;
+        var parameter = root.FindNode(diagnosticSpan, getInnermostNodeForTie: true) as ParameterSyntax;
+        Dictionary<Document, IList<SyntaxNode>> nodesToRemove = [];
 
-        public override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(MethodParameterUnused.DiagnosticId);
+        if (!bool.Parse(diagnostic.Properties[MethodParameterUnused.IsRemovableKey]))
+        {
+            return;
+        }
 
-        protected override Task RegisterCodeFixesAsync(SyntaxNode root, SonarCodeFixContext context)
+        if (parameter?.Parent.Parent is BaseMethodDeclarationSyntax methodDeclaration)
         {
-            var diagnostic = context.Diagnostics.First();
-            var diagnosticSpan = diagnostic.Location.SourceSpan;
-            var parameter = root.FindNode(diagnosticSpan, getInnermostNodeForTie: true) as ParameterSyntax;
+            var model = await context.Document.GetSemanticModelAsync(context.CancellationToken);
+            var parameterIndex = methodDeclaration.ParameterList.Parameters.IndexOf(parameter);
 
-            if (!bool.Parse(diagnostic.Properties[MethodParameterUnused.IsRemovableKey]))
+            if (parameterIndex >= 0 && model.GetDeclaredSymbol(methodDeclaration, context.CancellationToken) is { } methodSymbol)
             {
-                return Task.CompletedTask;
+                var callers = await SymbolFinder.FindCallersAsync(
+                    methodSymbol,
+                    context.Document.Project.Solution,
+                    context.CancellationToken);
+
+                nodesToRemove = callers
+                    .SelectMany(x => x.Locations)
+                    .Aggregate(nodesToRemove, (args, location) =>
+                    {
+                        args.GetOrAdd(context.Document.Project.Solution.GetDocument(location.SourceTree), _ => new List<SyntaxNode>())
+                            .Add(GetArgumentFromInvocation(root, location.SourceSpan, parameterIndex));
+                        return args;
+                    });
             }
+        }
+
+        nodesToRemove.GetOrAdd(context.Document, _ => new List<SyntaxNode>()).Add(parameter);
 
+        foreach (var methodUsage in nodesToRemove)
+        {
             context.RegisterCodeFix(
                 Title,
-                c =>
+                _ =>
                 {
-                    var newRoot = root.RemoveNode(
-                        parameter,
+                    var newRoot = root.RemoveNodes(
+                        methodUsage.Value,
                         SyntaxRemoveOptions.KeepLeadingTrivia | SyntaxRemoveOptions.AddElasticMarker);
-                    return Task.FromResult(context.Document.WithSyntaxRoot(newRoot));
+
+                    return Task.FromResult(methodUsage.Key.WithSyntaxRoot(newRoot));
                 },
                 context.Diagnostics);
+        }
 
-            return Task.CompletedTask;
+        SyntaxNode GetArgumentFromInvocation(SyntaxNode rootNode, TextSpan span, int parameterIndex)
+        {
+            var node = rootNode.FindNode(span, getInnermostNodeForTie: true);
+            try
+            {
+                var argumentList = (node is IdentifierNameSyntax identifierName ? identifierName.Parent : node).ArgumentList();
+                return argumentList.Arguments[parameterIndex];
+            }
+            catch (Exception)
+            {
+                return null;
+            }
         }
     }
 }
diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodParameterUnused.RoslynCfg.Fixed.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodParameterUnused.RoslynCfg.Fixed.cs
@@ -16,7 +16,7 @@ internal interface MyPrivateInterface
 
     public class BasicTests : MyPrivateInterface
     {
-        private BasicTests(int a) : this(a, 42) // Compliant
+        private BasicTests(int a) : this(a) // Compliant
         { }
 
         private BasicTests(
@@ -645,4 +645,18 @@ private static class AssemblyResolver
             }
         }
     }
+
+    // https://github.com/SonarSource/sonar-dotnet/issues/8187
+    public class Repro_8187_CodeFix
+    {
+        void Method()
+        {
+            DoSomething(21);
+        }
+
+        void DoSomething(int a) // Fixed
+        {
+            Console.WriteLine(a);
+        }
+    }
 }
diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodParameterUnused.RoslynCfg.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/MethodParameterUnused.RoslynCfg.cs
@@ -647,4 +647,18 @@ private static class AssemblyResolver
             }
         }
     }
+
+    // https://github.com/SonarSource/sonar-dotnet/issues/8187
+    public class Repro_8187_CodeFix
+    {
+        void Method()
+        {
+            DoSomething(21, 42);
+        }
+
+        void DoSomething(int a, int b) // Noncompliant
+        {
+            Console.WriteLine(a);
+        }
+    }
 }