From 5116cfc18903fba578e27cd84c3a99e97cec92dd Mon Sep 17 00:00:00 2001 From: retailcoder Date: Wed, 8 Jul 2015 02:13:58 -0400 Subject: [PATCH] made RemoveParametersRefactoring tests pass. these tests highlight a number of little issues. --- .../RemoveParameters/RemoveParametersModel.cs | 2 +- .../Refactoring/RemoveParametersTests.cs | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/RetailCoder.VBE/Refactorings/RemoveParameters/RemoveParametersModel.cs b/RetailCoder.VBE/Refactorings/RemoveParameters/RemoveParametersModel.cs index 57b4d454b1..20eb156a60 100644 --- a/RetailCoder.VBE/Refactorings/RemoveParameters/RemoveParametersModel.cs +++ b/RetailCoder.VBE/Refactorings/RemoveParameters/RemoveParametersModel.cs @@ -47,7 +47,7 @@ private void LoadParameters() if (TargetDeclaration.DeclarationType == DeclarationType.PropertyLet || TargetDeclaration.DeclarationType == DeclarationType.PropertySet) { - Parameters.RemoveAt(Parameters.Count - 1); + Parameters.Remove(Parameters.Last()); } } diff --git a/RubberduckTests/Refactoring/RemoveParametersTests.cs b/RubberduckTests/Refactoring/RemoveParametersTests.cs index 0262196485..e2295ec55b 100644 --- a/RubberduckTests/Refactoring/RemoveParametersTests.cs +++ b/RubberduckTests/Refactoring/RemoveParametersTests.cs @@ -221,19 +221,18 @@ public void RemoveParametersRefactoring_RemoveFromGetter() Assert.AreEqual(expectedCode, Module.Object.Lines()); } - //bug: We shouldn't allow the only param in a setter to be removed, it will break the VBA code. [TestMethod] public void RemoveParametersRefactoring_RemoveFromSetter() { //Input const string inputCode = -@"Private Property Set Foo(ByVal arg1 As Integer) +@"Private Property Set Foo(ByVal arg1 As Integer) End Property"; var selection = new Selection(1, 23, 1, 27); //startLine, startCol, endLine, endCol //Expectation const string expectedCode = -@"Private Property Set Foo() +@"Private Property Set Foo(ByVal arg1 As Integer) End Property"; //note: The IDE strips out the extra whitespace //Arrange @@ -257,20 +256,23 @@ public void RemoveParametersRefactoring_RemoveFromSetter() Assert.AreEqual(expectedCode, Module.Object.Lines()); } + // todo: write a test that confirms that Property Set and Property Let members have 1 less parameter than what the signature actually says. + // ...or re-implement the code so that such a test isn't needed to document this surprising behavior. + //note: removing other params from setters is fine (In fact, we may want to create an inspection for this). [TestMethod] public void RemoveParametersRefactoring_RemoveSecondParamFromSetter() { //Input const string inputCode = -@"Private Property Set Foo(ByVal arg1 As Integer, ByVal arg2 As String) +@"Private Property Set Foo(ByVal arg1 As Integer, ByVal arg2 As String) End Property"; var selection = new Selection(1, 23, 1, 27); //startLine, startCol, endLine, endCol //Expectation const string expectedCode = -@"Private Property Set Foo(ByVal arg1 As Integer ) -End Property"; //note: The IDE strips out the extra whitespace +@"Private Property Set Foo( ByVal arg2 As String) +End Property"; //note: The IDE strips out the extra whitespace // bug: the refactoring should be removing that extra whitespace. //Arrange SetupProject(inputCode); @@ -280,7 +282,7 @@ public void RemoveParametersRefactoring_RemoveSecondParamFromSetter() //Specify Param(s) to remove var model = new RemoveParametersModel(parseResult, qualifiedSelection); - model.Parameters[1].IsRemoved = true; + model.Parameters[0].IsRemoved = true; //SetupFactory var factory = SetupFactory(model);