Skip to content

Commit ff22747

Browse files
committed
Modified AssignedByValParameterMakeLocalCopyQuickFix to prevent user from selecting names in use at module and global scopes
1 parent a6fb323 commit ff22747

File tree

6 files changed

+358
-71
lines changed

6 files changed

+358
-71
lines changed

RetailCoder.VBE/Inspections/AssignedByValParameterInspection.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,13 @@ namespace Rubberduck.Inspections
1313
public sealed class AssignedByValParameterInspection : InspectionBase
1414
{
1515
private IAssignedByValParameterQuickFixDialogFactory _dialogFactory;
16+
private RubberduckParserState _parserState;
1617
public AssignedByValParameterInspection(RubberduckParserState state, IAssignedByValParameterQuickFixDialogFactory dialogFactory)
1718
: base(state)
1819
{
1920
Severity = DefaultSeverity;
2021
_dialogFactory = dialogFactory;
22+
_parserState = state;
2123

2224
}
2325

@@ -35,7 +37,7 @@ public override IEnumerable<InspectionResultBase> GetInspectionResults()
3537
.ToList();
3638

3739
return parameters
38-
.Select(param => new AssignedByValParameterInspectionResult(this, param, _dialogFactory))
40+
.Select(param => new AssignedByValParameterInspectionResult(this, param, _parserState, _dialogFactory))
3941
.ToList();
4042
}
4143
}

RetailCoder.VBE/Inspections/QuickFixes/AssignedByValParameterMakeLocalCopyQuickFix.cs

Lines changed: 82 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,22 +11,25 @@
1111
using Antlr4.Runtime;
1212
using System.Collections.Generic;
1313
using Antlr4.Runtime.Tree;
14+
using Rubberduck.Parsing.VBA;
1415

1516
namespace Rubberduck.Inspections.QuickFixes
1617
{
1718
public class AssignedByValParameterMakeLocalCopyQuickFix : QuickFixBase
1819
{
1920
private readonly Declaration _target;
2021
private readonly IAssignedByValParameterQuickFixDialogFactory _dialogFactory;
22+
private readonly RubberduckParserState _parserState;
2123
private string _localCopyVariableName;
2224
private string[] _variableNamesAccessibleToProcedureContext;
2325

24-
public AssignedByValParameterMakeLocalCopyQuickFix(Declaration target, QualifiedSelection selection, IAssignedByValParameterQuickFixDialogFactory dialogFactory)
26+
public AssignedByValParameterMakeLocalCopyQuickFix(Declaration target, QualifiedSelection selection, RubberduckParserState parserState, IAssignedByValParameterQuickFixDialogFactory dialogFactory)
2527
: base(target.Context, selection, InspectionsUI.AssignedByValParameterMakeLocalCopyQuickFix)
2628
{
2729
_target = target;
2830
_dialogFactory = dialogFactory;
29-
_variableNamesAccessibleToProcedureContext = GetVariableNamesAccessibleToProcedureContext(_target.Context.Parent.Parent);
31+
_parserState = parserState;
32+
_variableNamesAccessibleToProcedureContext = GetUserDefinedNamesAccessibleToProcedureContext(_target.Context.Parent.Parent);
3033
SetValidLocalCopyVariableNameSuggestion();
3134
}
3235

@@ -100,7 +103,7 @@ private void ReplaceAssignedByValParameterReferences()
100103

101104
private void InsertLocalVariableDeclarationAndAssignment()
102105
{
103-
var blocks = QuickFixHelper.GetBlockStmtContextsForContext(_target.Context.Parent.Parent);
106+
var blocks = QuickFixHelper.GetBlockStmtContexts(_target.Context.Parent.Parent);
104107
string[] lines = { BuildLocalCopyDeclaration(), BuildLocalCopyAssignment() };
105108
var module = Selection.QualifiedName.Component.CodeModule;
106109
module.InsertLines(blocks.FirstOrDefault().Start.Line, lines);
@@ -118,58 +121,93 @@ private string BuildLocalCopyAssignment()
118121
+ _localCopyVariableName + " = " + _target.IdentifierName;
119122
}
120123

121-
private string[] GetVariableNamesAccessibleToProcedureContext(RuleContext ruleContext)
124+
private string[] GetUserDefinedNamesAccessibleToProcedureContext(RuleContext ruleContext)
122125
{
123126
var allIdentifiers = new HashSet<string>();
124127

125-
var blocks = QuickFixHelper.GetBlockStmtContextsForContext(ruleContext);
128+
//Locally declared variable names
129+
var blocks = QuickFixHelper.GetBlockStmtContexts(ruleContext);
130+
131+
var blockStmtIdentifierContexts = GetIdentifierContexts(blocks);
132+
var blockStmtIdentifiers = GetVariableNamesFromRuleContexts(blockStmtIdentifierContexts.ToArray());
126133

127-
var blockStmtIdentifiers = GetIdentifierNames(blocks);
128134
allIdentifiers.UnionWith(blockStmtIdentifiers);
129135

130-
var args = QuickFixHelper.GetArgContextsForContext(ruleContext);
136+
//The parameters of the procedure that are unreferenced in the procedure body
137+
var args = QuickFixHelper.GetArgContexts(ruleContext);
138+
139+
var potentiallyUnreferencedIdentifierContexts = GetIdentifierContexts(args);
140+
var potentiallyUnreferencedParameters = GetVariableNamesFromRuleContexts(potentiallyUnreferencedIdentifierContexts.ToArray());
131141

132-
var potentiallyUnreferencedParameters = GetIdentifierNames(args);
133142
allIdentifiers.UnionWith(potentiallyUnreferencedParameters);
134143

135-
//TODO: add module and global scope variableNames to the list.
144+
//All declarations within the same module, but outside of all procedures (e.g., member variables, procedure names)
145+
var sameModuleDeclarations = _parserState.AllUserDeclarations
146+
.Where(item => item.ComponentName == _target.ComponentName
147+
&& !IsProceduralContext(item.ParentDeclaration.Context))
148+
.ToList();
149+
150+
allIdentifiers.UnionWith(sameModuleDeclarations.Select(d => d.IdentifierName));
151+
152+
//Public declarations anywhere within the project other than Public members and
153+
//procedures of Class modules
154+
var allPublicDeclarations = _parserState.AllUserDeclarations
155+
.Where(item => (item.Accessibility == Accessibility.Public
156+
|| ((item.Accessibility == Accessibility.Implicit)
157+
&& item.ParentScopeDeclaration is ProceduralModuleDeclaration))
158+
&& !(item.ParentScopeDeclaration is ClassModuleDeclaration))
159+
.ToList();
160+
161+
allIdentifiers.UnionWith(allPublicDeclarations.Select(d => d.IdentifierName));
136162

137163
return allIdentifiers.ToArray();
138164
}
139165

140-
private HashSet<string> GetIdentifierNames(IReadOnlyList<RuleContext> ruleContexts)
166+
private HashSet<string> GetVariableNamesFromRuleContexts(RuleContext[] ruleContexts)
167+
{
168+
var tokenValues = typeof(Tokens).GetFields().Select(item => item.GetValue(null)).Cast<string>().Select(item => item);
169+
var results = new HashSet<string>();
170+
171+
foreach( var ruleContext in ruleContexts)
172+
{
173+
var name = Identifier.GetName((VBAParser.IdentifierContext)ruleContext);
174+
if (!tokenValues.Contains(name))
175+
{
176+
results.Add(name);
177+
}
178+
}
179+
return results;
180+
}
181+
182+
private HashSet<RuleContext> GetIdentifierContexts(IReadOnlyList<RuleContext> ruleContexts)
141183
{
142-
var identifiers = new HashSet<string>();
184+
var identifiers = new HashSet<RuleContext>();
143185
foreach (RuleContext ruleContext in ruleContexts)
144186
{
145-
var identifiersForThisContext = GetIdentifierNames(ruleContext);
187+
var identifiersForThisContext = GetIdentifierContexts(ruleContext);
146188
identifiers.UnionWith(identifiersForThisContext);
147189
}
148190
return identifiers;
149191
}
150192

151-
private HashSet<string> GetIdentifierNames(RuleContext ruleContext)
193+
private HashSet<RuleContext> GetIdentifierContexts(RuleContext ruleContext)
152194
{
153195
//Recursively work through the tree to get all IdentifierContexts
154-
var results = new HashSet<string>();
155-
var tokenValues = typeof(Tokens).GetFields().Select(item => item.GetValue(null)).Cast<string>().Select(item => item);
196+
var results = new HashSet<RuleContext>();
156197
var children = GetChildren(ruleContext);
157198

158199
foreach (IParseTree child in children)
159200
{
160201
if (child is VBAParser.IdentifierContext)
161202
{
162203
var childName = Identifier.GetName((VBAParser.IdentifierContext)child);
163-
if (!tokenValues.Contains(childName))
164-
{
165-
results.Add(childName);
166-
}
204+
results.Add((RuleContext)child);
167205
}
168206
else
169207
{
170208
if (!(child is TerminalNodeImpl))
171209
{
172-
results.UnionWith(GetIdentifierNames((RuleContext)child));
210+
results.UnionWith(GetIdentifierContexts((RuleContext)child));
173211
}
174212
}
175213
}
@@ -185,5 +223,29 @@ private static List<IParseTree> GetChildren(RuleContext ruleCtx)
185223
}
186224
return result;
187225
}
226+
private bool IsProceduralContext(RuleContext context)
227+
{
228+
if (context is VBAParser.SubStmtContext)
229+
{
230+
return true;
231+
}
232+
else if (context is VBAParser.FunctionStmtContext)
233+
{
234+
return true;
235+
}
236+
else if (context is VBAParser.PropertyLetStmtContext)
237+
{
238+
return true;
239+
}
240+
else if (context is VBAParser.PropertyGetStmtContext)
241+
{
242+
return true;
243+
}
244+
else if (context is VBAParser.PropertySetStmtContext)
245+
{
246+
return true;
247+
}
248+
return false;
249+
}
188250
}
189251
}

RetailCoder.VBE/Inspections/QuickFixes/PassParameterByReferenceQuickFix.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public PassParameterByReferenceQuickFix(Declaration target, QualifiedSelection s
2626
public override void Fix()
2727
{
2828
var module = Selection.QualifiedName.Component.CodeModule;
29-
var argContext = QuickFixHelper.GetArgContextsForContext(Context.Parent.Parent)
29+
var argContext = QuickFixHelper.GetArgContexts(Context.Parent.Parent)
3030
.SingleOrDefault(parameter => Identifier.GetName(parameter.unrestrictedIdentifier())
3131
.Equals(_target.IdentifierName));
3232

RetailCoder.VBE/Inspections/QuickFixes/QuickFixHelper.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ namespace Rubberduck.Inspections.QuickFixes
88
public static class QuickFixHelper
99
{
1010

11-
public static IReadOnlyList<VBAParser.BlockStmtContext> GetBlockStmtContextsForContext(RuleContext context)
11+
public static IReadOnlyList<VBAParser.BlockStmtContext> GetBlockStmtContexts(RuleContext context)
1212
{
1313
if (context is VBAParser.SubStmtContext)
1414
{
@@ -33,7 +33,7 @@ public static class QuickFixHelper
3333
return Enumerable.Empty<VBAParser.BlockStmtContext>().ToArray();
3434
}
3535

36-
public static IReadOnlyList<VBAParser.ArgContext> GetArgContextsForContext(RuleContext context)
36+
public static IReadOnlyList<VBAParser.ArgContext> GetArgContexts(RuleContext context)
3737
{
3838
if (context is VBAParser.SubStmtContext)
3939
{

RetailCoder.VBE/Inspections/Results/AssignedByValParameterInspectionResult.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,21 @@
55
using Rubberduck.Inspections.Resources;
66
using Rubberduck.Parsing.Symbols;
77
using Rubberduck.UI.Refactorings;
8+
using Rubberduck.Parsing.VBA;
89

910
namespace Rubberduck.Inspections.Results
1011
{
1112
public class AssignedByValParameterInspectionResult : InspectionResultBase
1213
{
1314
private IEnumerable<QuickFixBase> _quickFixes;
15+
private RubberduckParserState _parserState;
1416
private IAssignedByValParameterQuickFixDialogFactory _dialogFactory;
1517

16-
public AssignedByValParameterInspectionResult(IInspection inspection, Declaration target, IAssignedByValParameterQuickFixDialogFactory dialogFactory)
18+
public AssignedByValParameterInspectionResult(IInspection inspection, Declaration target, RubberduckParserState parserState, IAssignedByValParameterQuickFixDialogFactory dialogFactory)
1719
: base(inspection, target)
1820
{
1921
_dialogFactory = dialogFactory;
22+
_parserState = parserState;
2023
}
2124

2225
public override string Description
@@ -33,7 +36,7 @@ public override IEnumerable<QuickFixBase> QuickFixes
3336
{
3437
return _quickFixes ?? (_quickFixes = new QuickFixBase[]
3538
{
36-
new AssignedByValParameterMakeLocalCopyQuickFix(Target, QualifiedSelection, _dialogFactory),
39+
new AssignedByValParameterMakeLocalCopyQuickFix(Target, QualifiedSelection, _parserState, _dialogFactory),
3740
new PassParameterByReferenceQuickFix(Target, QualifiedSelection),
3841
new IgnoreOnceQuickFix(Context, QualifiedSelection, Inspection.AnnotationName)
3942
});

0 commit comments

Comments
 (0)