Skip to content

Commit 56130cb

Browse files
committed
#1942 - AssignedByValParameter new QuickFIx
Added QuickFix to replace ByVal parameter with local copy. Provides a dialog to retrieve a user provided name. Refactored variable name validations to avoid duplicate code and standardize validations for 'UseMeaningfulName' inspections and user's variable name input for this quickfix. Refactored validations to a POCO 'VariableNameValidator'.
1 parent 95024c2 commit 56130cb

File tree

6 files changed

+164
-176
lines changed

6 files changed

+164
-176
lines changed

RetailCoder.VBE/Inspections/QuickFixes/AssignedByValParameterQuickFix.cs

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,46 +16,51 @@ public class AssignedByValParameterQuickFix : QuickFixBase
1616

1717
private Declaration _target;
1818
private string _localCopyVariableName;
19-
private bool _forceUseOfGeneratedName;
19+
private bool _forceUseOfSuggestedName;
2020
private string[] _originalCodeLines;
2121

2222
public AssignedByValParameterQuickFix(Declaration target, QualifiedSelection selection)
2323
: base(target.Context, selection, InspectionsUI.AssignedByValParameterQuickFix)
2424
{
2525
_target = target;
2626
_localCopyVariableName = "local" + CapitalizeFirstLetter(_target.IdentifierName);
27-
_forceUseOfGeneratedName = false;
27+
_forceUseOfSuggestedName = false;
2828

2929
_originalCodeLines = GetMethodLines();
3030
}
3131
public override bool CanFixInModule { get { return false; } }
3232
public override bool CanFixInProject { get { return false; } }
3333

34+
//This function exists solely to support unit testing - by preventing the popup dialog
35+
public void TESTONLY_FixUsingAutoGeneratedName()
36+
{
37+
_forceUseOfSuggestedName = true;
38+
Fix();
39+
}
40+
3441
public override void Fix()
3542
{
36-
if (!_forceUseOfGeneratedName)
37-
{
38-
GetLocalCopyVariableNameFromUser();
43+
GetLocalCopyVariableNameFromUser();
3944

40-
if (IsCancelled) { return; }
45+
if (!IsCancelled)
46+
{
47+
modifyBlockToUseLocalCopyVariable();
4148
}
42-
43-
modifyBlockToUseLocalCopyVariable();
44-
45-
}
46-
//This function created solely to support unit testing - prevents popup dialog
47-
public void ForceFixUsingGeneratedName()
48-
{
49-
_forceUseOfGeneratedName = true;
50-
Fix();
5149
}
5250

5351
private void GetLocalCopyVariableNameFromUser()
5452
{
53+
if (_forceUseOfSuggestedName)
54+
{
55+
_localCopyVariableName = AutoSuggestedName();
56+
IsCancelled = false;
57+
return;
58+
}
59+
5560
using (var view = new AssignedByValParameterQuickFixDialog(_originalCodeLines))
5661
{
5762
view.Target = _target;
58-
view.NewName = _localCopyVariableName;
63+
view.NewName = AutoSuggestedName();
5964
view.ShowDialog();
6065

6166
IsCancelled = view.DialogResult == DialogResult.Cancel;
@@ -140,7 +145,7 @@ private string[] GetMethodLines()
140145
string endStatement = GetEndOfScopeStatementForDeclaration(allLines[zbIndex]);
141146

142147
bool IsInScope = true;
143-
System.Collections.Generic.List<string> codeBlockLines = new System.Collections.Generic.List<string>();
148+
var codeBlockLines = new List<string>();
144149
for ( ; IsInScope && zbIndex < allLines.Count(); zbIndex++)
145150
{
146151
codeBlockLines.Add(allLines[zbIndex]);
@@ -167,6 +172,10 @@ private string GetEndOfScopeStatementForDeclaration(string declaration)
167172
Tokens.String,
168173
Tokens.Variant
169174
};
175+
private string AutoSuggestedName()
176+
{
177+
return "local" + CapitalizeFirstLetter(_target.IdentifierName);
178+
}
170179
private string CapitalizeFirstLetter(string name)
171180
{
172181
return name.Substring(0, 1).ToUpper() + name.Substring(1);

RetailCoder.VBE/Inspections/UseMeaningfulNameInspection.cs

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -60,23 +60,8 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()
6060

6161
private static bool IsBadIdentifier(string identifier)
6262
{
63-
return identifier.Length < 3 ||
64-
char.IsDigit(identifier.Last()) ||
65-
!HasVowels(identifier) ||
66-
NameIsASingleRepeatedLetter(identifier);
67-
}
68-
69-
private static bool HasVowels(string identifier)
70-
{
71-
const string vowels = "aeiouyàâäéèêëïîöôùûü";
72-
return identifier.Any(character => vowels.Any(vowel =>
73-
string.Compare(vowel.ToString(), character.ToString(), StringComparison.OrdinalIgnoreCase) == 0));
74-
}
75-
private static bool NameIsASingleRepeatedLetter(string identifierName)
76-
{
77-
string firstLetter = identifierName.First().ToString();
78-
return identifierName.All(a => string.Compare(a.ToString(), firstLetter,
79-
StringComparison.OrdinalIgnoreCase) == 0);
63+
var validator = new VariableNameValidator(identifier);
64+
return !validator.IsMeaningfulName();
8065
}
8166
}
8267
}
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
using Rubberduck.Parsing.Grammar;
2+
using System;
3+
using System.Collections.Generic;
4+
using System.Linq;
5+
using System.Text;
6+
using System.Threading.Tasks;
7+
8+
namespace Rubberduck.Inspections
9+
{
10+
public class VariableNameValidator
11+
{
12+
public VariableNameValidator() { }
13+
public VariableNameValidator(string identifier) { _identifier = identifier; }
14+
15+
16+
private const string _ALL_VOWELS = "aeiouyàâäéèêëïîöôùûü";
17+
private const int _MIN_VARIABLE_NAME_LENGTH = 3;
18+
19+
/**** Meaningful Name Characteristics ************/
20+
public bool HasVowels { get { return hasVowels(); } }
21+
public bool HasConsonants { get { return hasConsonants(); } }
22+
public bool IsSingleRepeatedLetter { get { return nameIsASingleRepeatedLetter(); } }
23+
public bool IsTooShort { get { return _identifier.Length < _MIN_VARIABLE_NAME_LENGTH; } }
24+
public bool EndsWithNumber { get { return endsWithNumber(); } }
25+
26+
/**** Invalid Name Characteristics ************/
27+
public bool StartsWithNumber { get { return FirstLetterIsDigit(); } }
28+
public bool IsReservedName { get { return isReservedName(); } }
29+
public bool ContainsSpecialCharacters { get { return UsesSpecialCharacters(); } }
30+
31+
private string _identifier;
32+
public string Identifier
33+
{
34+
get { return _identifier; }
35+
set { _identifier = value; }
36+
}
37+
public bool IsValidName()
38+
{
39+
return !StartsWithNumber
40+
&& !IsReservedName
41+
&& !ContainsSpecialCharacters;
42+
}
43+
public bool IsMeaningfulName()
44+
{
45+
return HasVowels
46+
&& HasConsonants
47+
&& !IsSingleRepeatedLetter
48+
&& !IsTooShort
49+
&& !EndsWithNumber;
50+
}
51+
private bool IsEmpty()
52+
{
53+
return _identifier.Equals(string.Empty);
54+
}
55+
private bool FirstLetterIsDigit()
56+
{
57+
return !char.IsLetter(_identifier.FirstOrDefault());
58+
}
59+
private bool isReservedName()
60+
{
61+
var tokenValues = typeof(Tokens).GetFields().Select(item => item.GetValue(null)).Cast<string>().Select(item => item);
62+
return tokenValues.Contains(_identifier, StringComparer.InvariantCultureIgnoreCase);
63+
}
64+
private bool UsesSpecialCharacters()
65+
{
66+
return _identifier.Any(c => !char.IsLetterOrDigit(c) && c != '_');
67+
}
68+
69+
private bool hasVowels()
70+
{
71+
return _identifier.Any(character => _ALL_VOWELS.Any(vowel =>
72+
string.Compare(vowel.ToString(), character.ToString(), StringComparison.OrdinalIgnoreCase) == 0));
73+
}
74+
private bool hasConsonants()
75+
{
76+
return !_identifier.All(character => _ALL_VOWELS.Any(vowel =>
77+
string.Compare(vowel.ToString(), character.ToString(), StringComparison.OrdinalIgnoreCase) == 0));
78+
}
79+
private bool nameIsASingleRepeatedLetter()
80+
{
81+
string firstLetter = _identifier.First().ToString();
82+
return _identifier.All(a => string.Compare(a.ToString(), firstLetter,
83+
StringComparison.OrdinalIgnoreCase) == 0);
84+
}
85+
private bool endsWithNumber()
86+
{
87+
return char.IsDigit(_identifier.Last());
88+
}
89+
}
90+
}

RetailCoder.VBE/Rubberduck.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,7 @@
435435
<Compile Include="Inspections\QuickFixes\SpecifyExplicitPublicModifierQuickFix.cs" />
436436
<Compile Include="Inspections\QuickFixes\WriteOnlyPropertyQuickFix.cs" />
437437
<Compile Include="Inspections\Results\WriteOnlyPropertyInspectionResult.cs" />
438+
<Compile Include="Inspections\VariableNameValidator.cs" />
438439
<Compile Include="Navigation\CodeExplorer\ICodeExplorerDeclarationViewModel.cs" />
439440
<Compile Include="Navigation\Folders\FolderHelper.cs" />
440441
<Compile Include="Refactorings\ExtractMethod\ExtractedMethod.cs" />

0 commit comments

Comments
 (0)