Skip to content

Commit 8e4a56b

Browse files
BZngrBZngr
committed
Final updates per PR comments
1 parent 6871731 commit 8e4a56b

7 files changed

+79
-108
lines changed

‎RetailCoder.VBE/Inspections/QuickFixes/AssignedByValParameterMakeLocalCopyQuickFix.cs

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ public AssignedByValParameterMakeLocalCopyQuickFix(Declaration target, Qualified
2626
{
2727
_target = target;
2828
_dialogFactory = dialogFactory;
29-
_localCopyVariableName = "x" + _target.IdentifierName.CapitalizeFirstLetter();
3029
_variableNamesAccessibleToProcedureContext = GetVariableNamesAccessibleToProcedureContext(_target.Context.Parent.Parent);
30+
SetValidLocalCopyVariableNameSuggestion();
3131
}
3232

3333
public override bool CanFixInModule { get { return false; } }
@@ -37,7 +37,7 @@ public override void Fix()
3737
{
3838
RequestLocalCopyVariableName();
3939

40-
if (!ProposedLocalVariableNameIsValid() || IsCancelled)
40+
if (!VariableNameIsValid(_localCopyVariableName) || IsCancelled)
4141
{
4242
return;
4343
}
@@ -62,12 +62,31 @@ private void RequestLocalCopyVariableName()
6262
}
6363
}
6464

65-
private bool ProposedLocalVariableNameIsValid()
65+
private void SetValidLocalCopyVariableNameSuggestion()
66+
{
67+
_localCopyVariableName = "x" + _target.IdentifierName.CapitalizeFirstLetter();
68+
if (VariableNameIsValid(_localCopyVariableName)) { return; }
69+
70+
//If the initial suggestion is not valid, keep pre-pending x's until it is
71+
for ( int attempt = 2; attempt < 10; attempt++)
72+
{
73+
_localCopyVariableName = "x" + _localCopyVariableName;
74+
if (VariableNameIsValid(_localCopyVariableName))
75+
{
76+
return;
77+
}
78+
}
79+
//if "xxFoo" to "xxxxxxxxxxFoo" isn't unique, give up and go with the original suggestion.
80+
//The QuickFix will leave the code as-is unless it receives a name that is free of conflicts
81+
_localCopyVariableName = "x" + _target.IdentifierName.CapitalizeFirstLetter();
82+
}
83+
84+
private bool VariableNameIsValid(string variableName)
6685
{
6786
var validator = new VariableNameValidator(_localCopyVariableName);
6887
return validator.IsValidName()
6988
&& !_variableNamesAccessibleToProcedureContext
70-
.Any(name => name.ToUpper().Equals(_localCopyVariableName.ToUpper()));
89+
.Any(name => name.Equals(_localCopyVariableName, System.StringComparison.InvariantCultureIgnoreCase));
7190
}
7291

7392
private void ReplaceAssignedByValParameterReferences()
@@ -92,6 +111,7 @@ private string BuildLocalCopyDeclaration()
92111
return Tokens.Dim + " " + _localCopyVariableName + " " + Tokens.As
93112
+ " " + _target.AsTypeName;
94113
}
114+
95115
private string BuildLocalCopyAssignment()
96116
{
97117
return (SymbolList.ValueTypes.Contains(_target.AsTypeName) ? string.Empty : Tokens.Set + " ")
@@ -112,7 +132,7 @@ private string[] GetVariableNamesAccessibleToProcedureContext(RuleContext ruleCo
112132
var potentiallyUnreferencedParameters = GetIdentifierNames(args);
113133
allIdentifiers.UnionWith(potentiallyUnreferencedParameters);
114134

115-
//TODO: add module and global scope variableNames.
135+
//TODO: add module and global scope variableNames to the list.
116136

117137
return allIdentifiers.ToArray();
118138
}

‎RetailCoder.VBE/Rubberduck.csproj

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -473,10 +473,8 @@
473473
<Compile Include="UI\EnvironmentProvider.cs" />
474474
<Compile Include="UI\ModernFolderBrowser.cs" />
475475
<Compile Include="UI\Refactorings\AssignedByValParameterQuickFixDialogFactory.cs" />
476-
<Compile Include="UI\Refactorings\AssignedByValParameterQuickFixMockDialogFactory.cs" />
477476
<Compile Include="UI\Refactorings\IAssignedByValParameterQuickFixDialog.cs" />
478477
<Compile Include="UI\Refactorings\IAssignedByValParameterQuickFixDialogFactory.cs" />
479-
<Compile Include="UI\Refactorings\AssignedByValParameterQuickFixMockDialog.cs" />
480478
<Compile Include="UI\SelectionChangeService.cs" />
481479
<Compile Include="VersionCheck\IVersionCheck.cs" />
482480
<Compile Include="UI\Command\MenuItems\CommandBars\AppCommandBarBase.cs" />

‎RetailCoder.VBE/UI/REfactorings/AssignedByValParameterQuickFixMockDialog.cs

Lines changed: 0 additions & 36 deletions
This file was deleted.

‎RetailCoder.VBE/UI/Refactorings/AssignedByValParameterQuickFixMockDialogFactory.cs

Lines changed: 0 additions & 20 deletions
This file was deleted.

‎RubberduckTests/Inspections/AssignedByValParameterInspectionTests.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ End Sub
118118
.AddComponent("Module1", ComponentType.StandardModule, caller)
119119
.MockVbeBuilder()
120120
.Build();
121-
var results = GetInspectionResults(vbe);
121+
var results = GetInspectionResults(vbe.Object);
122122
Assert.AreEqual(0, results.Count());
123123
}
124124

@@ -168,29 +168,29 @@ private void AssertVbaFragmentYieldsExpectedInspectionResultCount(string inputCo
168168
private string ApplyIgnoreOnceQuickFixToCodeFragment(string inputCode)
169169
{
170170
var vbe = BuildMockVBEStandardModuleForVBAFragment(inputCode);
171-
var inspectionResults = GetInspectionResults(vbe);
171+
var inspectionResults = GetInspectionResults(vbe.Object);
172172

173173
inspectionResults.First().QuickFixes.Single(s => s is IgnoreOnceQuickFix).Fix();
174174

175-
return GetModuleContent(vbe);
175+
return GetModuleContent(vbe.Object);
176176
}
177177

178-
private string GetModuleContent(Mock<IVBE> vbe)
178+
private string GetModuleContent(IVBE vbe)
179179
{
180-
var project = vbe.Object.VBProjects[0];
180+
var project = vbe.VBProjects[0];
181181
var module = project.VBComponents[0].CodeModule;
182182
return module.Content();
183183
}
184184

185185
private IEnumerable<Rubberduck.Inspections.Abstract.InspectionResultBase> GetInspectionResults(string inputCode)
186186
{
187187
var vbe = BuildMockVBEStandardModuleForVBAFragment(inputCode);
188-
return GetInspectionResults(vbe);
188+
return GetInspectionResults(vbe.Object);
189189
}
190190

191-
private IEnumerable<Rubberduck.Inspections.Abstract.InspectionResultBase> GetInspectionResults(Mock<IVBE> vbe)
191+
private IEnumerable<Rubberduck.Inspections.Abstract.InspectionResultBase> GetInspectionResults(IVBE vbe)
192192
{
193-
var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object));
193+
var parser = MockParser.Create(vbe, new RubberduckParserState(vbe));
194194
parser.Parse(new CancellationTokenSource());
195195
if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
196196

‎RubberduckTests/Inspections/AssignedByValParameterMakeLocalCopyQuickFixTests.cs

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
using RubberduckTests.Mocks;
1111
using System.Threading;
1212
using Rubberduck.UI.Refactorings;
13-
13+
using System.Windows.Forms;
1414

1515
namespace RubberduckTests.Inspections
1616
{
@@ -55,8 +55,10 @@ End Sub"
5555
string expectedCode =
5656
@"
5757
Public Sub Foo(ByVal arg1 As String)
58+
Dim xxArg1 As String
59+
xxArg1 = arg1
5860
xArg1 = 6
59-
Let arg1 = ""test""
61+
Let xxArg1 = ""test""
6062
End Sub"
6163
;
6264

@@ -75,8 +77,10 @@ End Sub"
7577
expectedCode =
7678
@"
7779
Public Sub Foo(ByVal arg1 As String)
80+
Dim xxArg1 As String
81+
xxArg1 = arg1
7882
Dim fooVar, xArg1 As Long
79-
Let arg1 = ""test""
83+
Let xxArg1 = ""test""
8084
End Sub"
8185
;
8286

@@ -96,9 +100,11 @@ End Sub"
96100
expectedCode =
97101
@"
98102
Public Sub Foo(ByVal arg1 As String)
103+
Dim xxArg1 As String
104+
xxArg1 = arg1
99105
Dim fooVar, _
100106
xArg1 As Long
101-
Let arg1 = ""test""
107+
Let xxArg1 = ""test""
102108
End Sub"
103109
;
104110
quickFixResult = ApplyLocalVariableQuickFixToVBAFragment(inputCode);
@@ -137,7 +143,7 @@ End Sub
137143
Assert.AreEqual(expectedCode, quickFixResult);
138144

139145
//Punt if the user-defined or auto-generated name is already present as an parameter name
140-
userEnteredName = "theSecondArg";
146+
userEnteredName = "moduleScopeName";
141147

142148
inputCode =
143149
@"
@@ -306,7 +312,6 @@ End Sub
306312

307313
quickFixResult = ApplyLocalVariableQuickFixToVBAFragment(inputCode);
308314
Assert.AreEqual(expectedCode, quickFixResult);
309-
310315
}
311316

312317
[TestMethod]
@@ -368,6 +373,7 @@ End Sub
368373
var quickFixResult = ApplyLocalVariableQuickFixToVBAFragment(inputCode);
369374
Assert.AreEqual(expectedCode, quickFixResult);
370375
}
376+
371377
[TestMethod]
372378
[TestCategory("Inspections")]
373379
public void AssignedByValParameter_ProperPlacementOfDeclaration()
@@ -433,7 +439,6 @@ End Function
433439
Assert.AreEqual(expectedCode, quickFixResult);
434440
}
435441

436-
437442
[TestMethod]
438443
[TestCategory("Inspections")]
439444
public void InspectionType()
@@ -455,11 +460,13 @@ public void InspectionName()
455460
private string ApplyLocalVariableQuickFixToVBAFragment(string inputCode, string userEnteredName = "")
456461
{
457462
var vbe = BuildMockVBEStandardModuleForVBAFragment(inputCode);
458-
var inspectionResults = GetInspectionResults(vbe, userEnteredName);
459463

464+
var mockDialogFactory = BuildMockDialogFactory(userEnteredName);
465+
466+
var inspectionResults = GetInspectionResults(vbe.Object, mockDialogFactory.Object);
460467
inspectionResults.First().QuickFixes.Single(s => s is AssignedByValParameterMakeLocalCopyQuickFix).Fix();
461468

462-
return GetModuleContent(vbe);
469+
return GetModuleContent(vbe.Object);
463470
}
464471

465472
private Mock<IVBE> BuildMockVBEStandardModuleForVBAFragment(string inputCode)
@@ -468,19 +475,38 @@ private Mock<IVBE> BuildMockVBEStandardModuleForVBAFragment(string inputCode)
468475
IVBComponent component;
469476
return builder.BuildFromSingleStandardModule(inputCode, out component);
470477
}
471-
private IEnumerable<Rubberduck.Inspections.Abstract.InspectionResultBase> GetInspectionResults(Mock<IVBE> vbe, string userEnteredName)
478+
479+
private IEnumerable<Rubberduck.Inspections.Abstract.InspectionResultBase> GetInspectionResults(IVBE vbe, IAssignedByValParameterQuickFixDialogFactory mockDialogFactory)
472480
{
473-
var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object));
481+
var parser = MockParser.Create(vbe, new RubberduckParserState(vbe));
474482
parser.Parse(new CancellationTokenSource());
475483
if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
476484

477-
var inspection = new AssignedByValParameterInspection(parser.State,new AssignedByValParameterQuickFixMockDialogFactory(userEnteredName));
485+
var inspection = new AssignedByValParameterInspection(parser.State, mockDialogFactory);
478486
return inspection.GetInspectionResults();
479487
}
480488

481-
private string GetModuleContent(Mock<IVBE> vbe)
489+
private Mock<IAssignedByValParameterQuickFixDialogFactory> BuildMockDialogFactory(string userEnteredName)
490+
{
491+
var mockDialog = new Mock<IAssignedByValParameterQuickFixDialog>();
492+
493+
mockDialog.SetupAllProperties();
494+
495+
if (userEnteredName.Length > 0)
496+
{
497+
mockDialog.SetupGet(m => m.NewName).Returns(() => userEnteredName);
498+
}
499+
mockDialog.SetupGet(m => m.DialogResult).Returns(() => DialogResult.OK);
500+
501+
var mockDialogFactory = new Mock<IAssignedByValParameterQuickFixDialogFactory>();
502+
mockDialogFactory.Setup(f => f.Create(It.IsAny<string>(), It.IsAny<string>())).Returns(mockDialog.Object);
503+
504+
return mockDialogFactory;
505+
}
506+
507+
private string GetModuleContent(IVBE vbe)
482508
{
483-
var project = vbe.Object.VBProjects[0];
509+
var project = vbe.VBProjects[0];
484510
var module = project.VBComponents[0].CodeModule;
485511
return module.Content();
486512
}

‎RubberduckTests/Inspections/PassParameterByReferenceQuickFixTests.cs

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -162,44 +162,27 @@ End Sub
162162
quickFixResult = ApplyPassParameterByReferenceQuickFixToVBAFragment(inputCode);
163163
Assert.AreEqual(expectedCode, quickFixResult);
164164
}
165-
[TestMethod]
166-
[TestCategory("Inspections")]
167-
public void InspectionType()
168-
{
169-
var inspection = new AssignedByValParameterInspection(null,null);
170-
Assert.AreEqual(CodeInspectionType.CodeQualityIssues, inspection.InspectionType);
171-
}
172-
173-
[TestMethod]
174-
[TestCategory("Inspections")]
175-
public void InspectionName()
176-
{
177-
const string inspectionName = "AssignedByValParameterInspection";
178-
var inspection = new AssignedByValParameterInspection(null,null);
179-
180-
Assert.AreEqual(inspectionName, inspection.Name);
181-
}
182165

183166
private string ApplyPassParameterByReferenceQuickFixToVBAFragment(string inputCode)
184167
{
185168
var vbe = BuildMockVBEStandardModuleForVBAFragment(inputCode);
186-
var inspectionResults = GetInspectionResults(vbe);
169+
var inspectionResults = GetAssignedByValParameterInspectionResults(vbe.Object);
187170

188171
inspectionResults.First().QuickFixes.Single(s => s is PassParameterByReferenceQuickFix).Fix();
189172

190-
return GetModuleContent(vbe);
173+
return GetModuleContent(vbe.Object);
191174
}
192175

193-
private string GetModuleContent(Mock<IVBE> vbe)
176+
private string GetModuleContent(IVBE vbe)
194177
{
195-
var project = vbe.Object.VBProjects[0];
178+
var project = vbe.VBProjects[0];
196179
var module = project.VBComponents[0].CodeModule;
197180
return module.Content();
198181
}
199182

200-
private IEnumerable<Rubberduck.Inspections.Abstract.InspectionResultBase> GetInspectionResults(Mock<IVBE> vbe)
183+
private IEnumerable<Rubberduck.Inspections.Abstract.InspectionResultBase> GetAssignedByValParameterInspectionResults(IVBE vbe)
201184
{
202-
var parser = MockParser.Create(vbe.Object, new RubberduckParserState(vbe.Object));
185+
var parser = MockParser.Create(vbe, new RubberduckParserState(vbe));
203186
parser.Parse(new CancellationTokenSource());
204187
if (parser.State.Status >= ParserState.Error) { Assert.Inconclusive("Parser Error"); }
205188

0 commit comments

Comments
 (0)