Skip to content

Commit 8941329

Browse files
committed
Fix reorder params caller bugs
1 parent e5c3cda commit 8941329

File tree

2 files changed

+74
-28
lines changed

2 files changed

+74
-28
lines changed

RetailCoder.VBE/Refactorings/ReorderParameters/ReorderParametersRefactoring.cs

Lines changed: 23 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using Rubberduck.Parsing;
55
using Rubberduck.Parsing.Grammar;
66
using Rubberduck.Parsing.Symbols;
7-
using Rubberduck.Parsing.VBA;
87
using Rubberduck.UI;
98
using Rubberduck.VBEditor;
109
using Rubberduck.VBEditor.Extensions;
@@ -106,7 +105,6 @@ private void AdjustReferences(IEnumerable<IdentifierReference> references)
106105
{
107106
foreach (var reference in references.Where(item => item.Context != _model.TargetDeclaration.Context))
108107
{
109-
dynamic proc = reference.Context;
110108
var module = reference.QualifiedModuleName.Component.CodeModule;
111109
VBAParser.ArgumentListContext argumentList = null;
112110
var callStmt = ParserRuleContextHelper.GetParent<VBAParser.CallStmtContext>(reference.Context);
@@ -121,55 +119,52 @@ private void AdjustReferences(IEnumerable<IdentifierReference> references)
121119

122120
private void RewriteCall(VBAParser.ArgumentListContext paramList, CodeModule module)
123121
{
124-
List<string> paramNames = new List<string>();
122+
var argValues = new List<string>();
125123
if (paramList.positionalOrNamedArgumentList().positionalArgumentOrMissing() != null)
126124
{
127-
paramNames.AddRange(paramList.positionalOrNamedArgumentList().positionalArgumentOrMissing().Select(p =>
125+
argValues.AddRange(paramList.positionalOrNamedArgumentList().positionalArgumentOrMissing().Select(p =>
128126
{
129127
if (p is VBAParser.SpecifiedPositionalArgumentContext)
130128
{
131129
return ((VBAParser.SpecifiedPositionalArgumentContext)p).positionalArgument().GetText();
132130
}
133-
else
134-
{
135-
return string.Empty;
136-
}
131+
132+
return string.Empty;
137133
}).ToList());
138134
}
139135
if (paramList.positionalOrNamedArgumentList().namedArgumentList() != null)
140136
{
141-
paramNames.AddRange(paramList.positionalOrNamedArgumentList().namedArgumentList().namedArgument().Select(p => p.GetText()).ToList());
137+
argValues.AddRange(paramList.positionalOrNamedArgumentList().namedArgumentList().namedArgument().Select(p => p.GetText()).ToList());
142138
}
143139
if (paramList.positionalOrNamedArgumentList().requiredPositionalArgument() != null)
144140
{
145-
paramNames.Add(paramList.positionalOrNamedArgumentList().requiredPositionalArgument().GetText());
141+
argValues.Add(paramList.positionalOrNamedArgumentList().requiredPositionalArgument().GetText());
146142
}
147143

148144
var lineCount = paramList.Stop.Line - paramList.Start.Line + 1; // adjust for total line count
149145

150-
var newContent = module.Lines[paramList.Start.Line, lineCount].Replace(" _" + Environment.NewLine, string.Empty).RemoveExtraSpacesLeavingIndentation();
151-
152-
var parameterIndex = 0;
153-
var currentStringIndex = 0;
146+
var newContent = module.Lines[paramList.Start.Line, lineCount];
147+
newContent = newContent.Remove(paramList.Start.Column, paramList.GetText().Length);
154148

155-
for (var i = 0; i < paramNames.Count && parameterIndex < _model.Parameters.Count; i++)
149+
var reorderedArgValues = new List<string>();
150+
foreach (var param in _model.Parameters)
156151
{
157-
var parameterStringIndex = newContent.IndexOf(paramNames.ElementAt(i), currentStringIndex, StringComparison.Ordinal);
158-
159-
if (parameterStringIndex <= -1) { continue; }
160-
161-
var oldParameterString = paramNames.ElementAt(i);
162-
var newParameterString = paramNames.ElementAt(_model.Parameters.ElementAt(parameterIndex).Index);
163-
var beginningSub = newContent.Substring(0, parameterStringIndex);
164-
var replaceSub = newContent.Substring(parameterStringIndex).Replace(oldParameterString, newParameterString);
165-
166-
newContent = beginningSub + replaceSub;
152+
var argAtIndex = argValues.ElementAtOrDefault(param.Index);
153+
if (argAtIndex != null)
154+
{
155+
reorderedArgValues.Add(argAtIndex);
156+
}
157+
}
167158

168-
parameterIndex++;
169-
currentStringIndex = beginningSub.Length + newParameterString.Length;
159+
// property let/set and paramarrays
160+
for (var index = reorderedArgValues.Count; index < argValues.Count; index++)
161+
{
162+
reorderedArgValues.Add(argValues[index]);
170163
}
171164

172-
module.ReplaceLine(paramList.Start.Line, newContent);
165+
newContent = newContent.Insert(paramList.Start.Column, string.Join(", ", reorderedArgValues));
166+
167+
module.ReplaceLine(paramList.Start.Line, newContent.Replace(" _" + Environment.NewLine, string.Empty));
173168
module.DeleteLines(paramList.Start.Line + 1, lineCount - 1);
174169
}
175170

RubberduckTests/Refactoring/ReorderParametersTests.cs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,57 @@ public void ReorderParams_SwapPositions_SignatureContainsParamName()
108108
Assert.AreEqual(expectedCode, module.Lines());
109109
}
110110

111+
[TestMethod]
112+
public void ReorderParams_SwapPositions_ReferenceValueContainsOtherReferenceValue()
113+
{
114+
//Input
115+
const string inputCode =
116+
@"Private Sub Foo(a, ba)
117+
End Sub
118+
119+
Sub Goo()
120+
Foo 1, 121
121+
End Sub";
122+
var selection = new Selection(1, 16, 1, 16);
123+
124+
//Expectation
125+
const string expectedCode =
126+
@"Private Sub Foo(ba, a)
127+
End Sub
128+
129+
Sub Goo()
130+
Foo 121, 1
131+
End Sub";
132+
133+
//Arrange
134+
var builder = new MockVbeBuilder();
135+
VBComponent component;
136+
var vbe = builder.BuildFromSingleStandardModule(inputCode, out component, selection);
137+
var project = vbe.Object.VBProjects.Item(0);
138+
var module = project.VBComponents.Item(0).CodeModule;
139+
var mockHost = new Mock<IHostApplication>();
140+
mockHost.SetupAllProperties();
141+
var parser = MockParser.Create(vbe.Object, new RubberduckParserState(new Mock<ISinks>().Object));
142+
143+
parser.Parse(new CancellationTokenSource());
144+
if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
145+
146+
var qualifiedSelection = new QualifiedSelection(new QualifiedModuleName(component), selection);
147+
148+
//set up model
149+
var model = new ReorderParametersModel(parser.State, qualifiedSelection, null);
150+
model.Parameters.Reverse();
151+
152+
var factory = SetupFactory(model);
153+
154+
//act
155+
var refactoring = new ReorderParametersRefactoring(vbe.Object, factory.Object, null);
156+
refactoring.Refactor(qualifiedSelection);
157+
158+
//assert
159+
Assert.AreEqual(expectedCode, module.Lines());
160+
}
161+
111162
[TestMethod]
112163
public void ReorderParams_RefactorDeclaration()
113164
{

0 commit comments

Comments
 (0)