Fix RemoveParameterRefactoring for properties in interfaces

Previously, we did not remove the parameters for the implementations of the setters and letters.

Author: MDoerner

Rubberduck.Refactorings/RemoveParameters/RemoveParametersRefactoring.cs

@@ -190,21 +190,17 @@ private void RemoveParameters(RemoveParametersModel model, IRewriteSession rewri
                 if (setter != null)
                 {
                     var setterModel = ModelForNewTarget(model, setter);
-                    AdjustReferences(setterModel, rewriteSession);
-                    AdjustSignatures(setterModel, rewriteSession);
+                    RemoveParameters(setterModel, rewriteSession);
                 }
 
                 var letter = GetLetterOrSetter(model.TargetDeclaration, DeclarationType.PropertyLet);
                 if (letter != null)
                 {
                     var letterModel = ModelForNewTarget(model, letter);
-                    AdjustReferences(letterModel, rewriteSession);
-                    AdjustSignatures(letterModel, rewriteSession);
+                    RemoveParameters(letterModel, rewriteSession);
                 }
             }
 
-            AdjustSignatures(model, rewriteSession);
-
             var eventImplementations = _declarationFinderProvider.DeclarationFinder
                 .FindEventHandlers(model.TargetDeclaration);
 
@@ -216,19 +212,17 @@ private void RemoveParameters(RemoveParametersModel model, IRewriteSession rewri
             }
 
             var interfaceImplementations = _declarationFinderProvider.DeclarationFinder
-                .FindAllInterfaceImplementingMembers()
-                .Where(item => item.ProjectId == model.TargetDeclaration.ProjectId
-                               && item.IdentifierName == $"{model.TargetDeclaration.ComponentName}_{model.TargetDeclaration.IdentifierName}");
+                .FindInterfaceImplementationMembers(model.TargetDeclaration);
 
-            foreach (var interfaceImplentation in interfaceImplementations)
+            foreach (var interfaceImplementation in interfaceImplementations)
             {
-                var interfaceImplementationModel = ModelForNewTarget(model, interfaceImplentation);
+                var interfaceImplementationModel = ModelForNewTarget(model, interfaceImplementation);
                 AdjustReferences(interfaceImplementationModel, rewriteSession);
                 AdjustSignatures(interfaceImplementationModel, rewriteSession);
             }
         }
 
-        private void AdjustReferences(RemoveParametersModel model, IRewriteSession rewriteSession)
+        private static void AdjustReferences(RemoveParametersModel model, IRewriteSession rewriteSession)
         {
             var parametersToRemove = model.RemoveParameters
                 .Select(parameter => parameter.Declaration)
@@ -241,7 +235,7 @@ private void AdjustReferences(RemoveParametersModel model, IRewriteSession rewri
             }
         }
 
-        private Dictionary<QualifiedModuleName, Dictionary<Selection, List<ArgumentReference>>> ArgumentReferencesByLocation(ICollection<ParameterDeclaration> parameters)
+        private static Dictionary<QualifiedModuleName, Dictionary<Selection, List<ArgumentReference>>> ArgumentReferencesByLocation(ICollection<ParameterDeclaration> parameters)
         {
             return parameters
                 .SelectMany(parameterDeclaration => parameterDeclaration.ArgumentReferences)
@@ -253,7 +247,7 @@ private Dictionary<QualifiedModuleName, Dictionary<Selection, List<ArgumentRefer
                         .ToDictionary(group => group.Key, group => group.ToList()));
         }
 
-        private void AdjustReferences(
+        private static void AdjustReferences(
             QualifiedModuleName module, 
             Dictionary<Selection, List<ArgumentReference>> argumentReferences,
             IRewriteSession rewriteSession)
@@ -294,7 +288,7 @@ private static void ReplaceDictionaryAccess(ArgumentReference dictionaryAccessAr
             rewriter.Replace(new Interval(startTokenIndex, stopTokenIndex), replacementString);
         }
 
-        private void AdjustReferences(IReadOnlyCollection<ArgumentReference> argumentReferences, IModuleRewriter rewriter)
+        private static void AdjustReferences(IReadOnlyCollection<ArgumentReference> argumentReferences, IModuleRewriter rewriter)
         {
             if (!argumentReferences.Any())
             {
@@ -314,7 +308,7 @@ private void AdjustReferences(IReadOnlyCollection<ArgumentReference> argumentRef
             }
         }
 
-        private IEnumerable<(int startIndex, int stopIndex)> IndexRanges(IEnumerable<int> indices)
+        private static IEnumerable<(int startIndex, int stopIndex)> IndexRanges(IEnumerable<int> indices)
         {
             var sortedIndices = indices.OrderBy(num => num).ToList();
             var ranges = new List<(int startIndex, int stopIndex)>();
@@ -346,7 +340,7 @@ private void AdjustReferences(IReadOnlyCollection<ArgumentReference> argumentRef
             return ranges;
         }
 
-        private IEnumerable<(int startIndex, int stopIndex)> WithTrailingMissingArguments(
+        private static IEnumerable<(int startIndex, int stopIndex)> WithTrailingMissingArguments(
             IEnumerable<(int startIndex, int stopIndex)> argumentRanges, 
             VBAParser.ArgumentListContext argumentList)
         {
@@ -384,7 +378,7 @@ private void AdjustReferences(IReadOnlyCollection<ArgumentReference> argumentRef
             return newRanges;
         }
 
-        private void RemoveArgumentRange(
+        private static void RemoveArgumentRange(
             int startArgumentIndex, 
             int stopArgumentIndex,
             VBAParser.ArgumentListContext argumentList, 
@@ -394,7 +388,7 @@ private void RemoveArgumentRange(
             rewriter.RemoveRange(startTokenIndex, stopTokenIndex);
         }
 
-        private (int startTokenIndex, int stopTokenIndex) TokenIndexRange(
+        private static (int startTokenIndex, int stopTokenIndex) TokenIndexRange(
             int startIndex, 
             int stopIndex,
             IReadOnlyList<ParserRuleContext> contexts)
@@ -416,26 +410,7 @@ private void RemoveArgumentRange(
             return (startTokenIndex, stopTokenIndex);
         }
 
-        private RemoveParametersModel ModelForNewTarget(RemoveParametersModel oldModel, Declaration newTarget)
-        {
-            var newModel = new RemoveParametersModel(newTarget);
-            var toRemoveIndices = oldModel.RemoveParameters.Select(param => oldModel.Parameters.IndexOf(param));
-            var newToRemoveParams = toRemoveIndices
-                .Select(index => newModel.Parameters[index])
-                .ToList();
-            newModel.RemoveParameters = newToRemoveParams;
-            return newModel;
-        }
-
-        private Declaration GetLetterOrSetter(Declaration declaration, DeclarationType declarationType)
-        {
-            return _declarationFinderProvider.DeclarationFinder
-                .UserDeclarations(declarationType)
-                .FirstOrDefault(item => item.QualifiedModuleName.Equals(declaration.QualifiedModuleName)
-                && item.IdentifierName == declaration.IdentifierName);
-        }
-
-        private void AdjustSignatures(RemoveParametersModel model, IRewriteSession rewriteSession)
+        private static void AdjustSignatures(RemoveParametersModel model, IRewriteSession rewriteSession)
         {
             var rewriter = rewriteSession.CheckOutModuleRewriter(model.TargetDeclaration.QualifiedModuleName);
 
@@ -451,7 +426,7 @@ private void AdjustSignatures(RemoveParametersModel model, IRewriteSession rewri
             }
         }
 
-        private void RemoveParameterRange(
+        private static void RemoveParameterRange(
             int startArgumentIndex,
             int stopArgumentIndex,
             VBAParser.ArgListContext argList,
@@ -461,6 +436,25 @@ private void RemoveParameterRange(
             rewriter.RemoveRange(startTokenIndex, stopTokenIndex);
         }
 
+        private Declaration GetLetterOrSetter(Declaration declaration, DeclarationType declarationType)
+        {
+            return _declarationFinderProvider.DeclarationFinder
+                .UserDeclarations(declarationType)
+                .FirstOrDefault(item => item.QualifiedModuleName.Equals(declaration.QualifiedModuleName)
+                                        && item.IdentifierName == declaration.IdentifierName);
+        }
+
+        private static RemoveParametersModel ModelForNewTarget(RemoveParametersModel oldModel, Declaration newTarget)
+        {
+            var newModel = new RemoveParametersModel(newTarget);
+            var toRemoveIndices = oldModel.RemoveParameters.Select(param => oldModel.Parameters.IndexOf(param));
+            var newToRemoveParams = toRemoveIndices
+                .Select(index => newModel.Parameters[index])
+                .ToList();
+            newModel.RemoveParameters = newToRemoveParams;
+            return newModel;
+        }
+
         public static readonly DeclarationType[] ValidDeclarationTypes =
         {
             DeclarationType.Event,

RubberduckTests/Refactoring/RemoveParametersTests.cs

@@ -1342,6 +1342,72 @@ Private Sub IClass1_DoSomething(ByVal i As Integer)
             Assert.AreEqual(expectedCode3, actualCode["Class2"]);
         }
 
+        [Test]
+        [Category("Refactorings")]
+        [Category("Remove Parameters")]
+        public void RemoveParametersRefactoring_InterfaceGetterParam_ImplementationLetAndSetParamRemoved()
+        {
+            //Input
+            const string inputCode1 =
+                @"Public Property Get Foo(ByVal a As Integer, ByVal b As String) As Variant
+End Property
+
+Public Property Let Foo(ByVal a As Integer, ByVal b As String, RHS As Variant)
+End Property
+
+Public Property Set Foo(ByVal a As Integer, ByVal b As String, RHS As Variant)
+End Property";
+            const string inputCode2 =
+                @"Implements IClass1
+
+Private Property Get IClass1_Foo(ByVal a As Integer, ByVal b As String) As Variant
+End Property
+
+Private Property Let IClass1_Foo(ByVal a As Integer, ByVal b As String, RHS As Variant)
+End Property
+
+Private Property Set IClass1_Foo(ByVal a As Integer, ByVal b As String, RHS As Variant)
+End Property";
+
+            var selection = new Selection(1, 23, 1, 27);
+
+            //Expectation
+            const string expectedCode1 =
+                @"Public Property Get Foo(ByVal a As Integer) As Variant
+End Property
+
+Public Property Let Foo(ByVal a As Integer, RHS As Variant)
+End Property
+
+Public Property Set Foo(ByVal a As Integer, RHS As Variant)
+End Property";
+            const string expectedCode2 =
+                @"Implements IClass1
+
+Private Property Get IClass1_Foo(ByVal a As Integer) As Variant
+End Property
+
+Private Property Let IClass1_Foo(ByVal a As Integer, RHS As Variant)
+End Property
+
+Private Property Set IClass1_Foo(ByVal a As Integer, RHS As Variant)
+End Property";   
+
+            var paramIndices = new[] { 1 }.ToList();
+            var presenterAction = StandardPresenterAction(paramIndices);
+            var actualCode = RefactoredCode(
+                "IClass1",
+                selection,
+                presenterAction,
+                null,
+                false,
+                ("IClass1", inputCode1, ComponentType.ClassModule),
+                ("Class1", inputCode2, ComponentType.ClassModule));
+
+            Assert.AreEqual(expectedCode1, actualCode["IClass1"]);
+            Assert.AreEqual(expectedCode2, actualCode["Class1"]);
+        }
+
         [Test]
         [Category("Refactorings")]
         [Category("Remove Parameters")]