Skip to content

Commit 5116cfc

Browse files
committed
made RemoveParametersRefactoring tests pass. these tests highlight a number of little issues.
1 parent d3a6a0d commit 5116cfc

File tree

2 files changed

+10
-8
lines changed

2 files changed

+10
-8
lines changed

RetailCoder.VBE/Refactorings/RemoveParameters/RemoveParametersModel.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ private void LoadParameters()
4747
if (TargetDeclaration.DeclarationType == DeclarationType.PropertyLet ||
4848
TargetDeclaration.DeclarationType == DeclarationType.PropertySet)
4949
{
50-
Parameters.RemoveAt(Parameters.Count - 1);
50+
Parameters.Remove(Parameters.Last());
5151
}
5252
}
5353

RubberduckTests/Refactoring/RemoveParametersTests.cs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -221,19 +221,18 @@ public void RemoveParametersRefactoring_RemoveFromGetter()
221221
Assert.AreEqual(expectedCode, Module.Object.Lines());
222222
}
223223

224-
//bug: We shouldn't allow the only param in a setter to be removed, it will break the VBA code.
225224
[TestMethod]
226225
public void RemoveParametersRefactoring_RemoveFromSetter()
227226
{
228227
//Input
229228
const string inputCode =
230-
@"Private Property Set Foo(ByVal arg1 As Integer)
229+
@"Private Property Set Foo(ByVal arg1 As Integer)
231230
End Property";
232231
var selection = new Selection(1, 23, 1, 27); //startLine, startCol, endLine, endCol
233232

234233
//Expectation
235234
const string expectedCode =
236-
@"Private Property Set Foo()
235+
@"Private Property Set Foo(ByVal arg1 As Integer)
237236
End Property"; //note: The IDE strips out the extra whitespace
238237

239238
//Arrange
@@ -257,20 +256,23 @@ public void RemoveParametersRefactoring_RemoveFromSetter()
257256
Assert.AreEqual(expectedCode, Module.Object.Lines());
258257
}
259258

259+
// todo: write a test that confirms that Property Set and Property Let members have 1 less parameter than what the signature actually says.
260+
// ...or re-implement the code so that such a test isn't needed to document this surprising behavior.
261+
260262
//note: removing other params from setters is fine (In fact, we may want to create an inspection for this).
261263
[TestMethod]
262264
public void RemoveParametersRefactoring_RemoveSecondParamFromSetter()
263265
{
264266
//Input
265267
const string inputCode =
266-
@"Private Property Set Foo(ByVal arg1 As Integer, ByVal arg2 As String)
268+
@"Private Property Set Foo(ByVal arg1 As Integer, ByVal arg2 As String)
267269
End Property";
268270
var selection = new Selection(1, 23, 1, 27); //startLine, startCol, endLine, endCol
269271

270272
//Expectation
271273
const string expectedCode =
272-
@"Private Property Set Foo(ByVal arg1 As Integer )
273-
End Property"; //note: The IDE strips out the extra whitespace
274+
@"Private Property Set Foo( ByVal arg2 As String)
275+
End Property"; //note: The IDE strips out the extra whitespace // bug: the refactoring should be removing that extra whitespace.
274276

275277
//Arrange
276278
SetupProject(inputCode);
@@ -280,7 +282,7 @@ public void RemoveParametersRefactoring_RemoveSecondParamFromSetter()
280282

281283
//Specify Param(s) to remove
282284
var model = new RemoveParametersModel(parseResult, qualifiedSelection);
283-
model.Parameters[1].IsRemoved = true;
285+
model.Parameters[0].IsRemoved = true;
284286

285287
//SetupFactory
286288
var factory = SetupFactory(model);

0 commit comments

Comments
 (0)