Skip to content

Commit f40a5f3

Browse files
retailcoderretailcoder
authored
Merge pull request #2055 from Hosch250/Issue541
Parameter Can By ByVal and resolving IsAssignment
2 parents 982db17 + bce3a7c commit f40a5f3

File tree

6 files changed

+750
-46
lines changed

6 files changed

+750
-46
lines changed

‎RetailCoder.VBE/Common/DeclarationExtensions.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,22 @@ public static IEnumerable<Declaration> FindEventHandlers(this IEnumerable<Declar
262262
&& declaration.IdentifierName.StartsWith(control.IdentifierName + "_"));
263263
}
264264

265+
public static IEnumerable<Declaration> FindUserEventHandlers(this IEnumerable<Declaration> declarations)
266+
{
267+
var declarationList = declarations.ToList();
268+
269+
var userEvents =
270+
declarationList.Where(item => !item.IsBuiltIn && item.DeclarationType == DeclarationType.Event).ToList();
271+
272+
var handlers = new List<Declaration>();
273+
foreach (var @event in userEvents)
274+
{
275+
handlers.AddRange(declarations.FindHandlersForEvent(@event).Select(s => s.Item2));
276+
}
277+
278+
return handlers;
279+
}
280+
265281
public static IEnumerable<Declaration> FindBuiltInEventHandlers(this IEnumerable<Declaration> declarations)
266282
{
267283
var declarationList = declarations.ToList();
@@ -451,6 +467,12 @@ public static IEnumerable<Declaration> FindInterfaceImplementationMembers(this I
451467
.Where(m => m.IdentifierName.EndsWith(interfaceMember));
452468
}
453469

470+
public static IEnumerable<Declaration> FindInterfaceImplementationMembers(this IEnumerable<Declaration> declarations, Declaration interfaceDeclaration)
471+
{
472+
return FindInterfaceImplementationMembers(declarations)
473+
.Where(m => m.IdentifierName == interfaceDeclaration.ComponentName + "_" + interfaceDeclaration.IdentifierName);
474+
}
475+
454476
public static Declaration FindInterfaceMember(this IEnumerable<Declaration> declarations, Declaration implementation)
455477
{
456478
var members = FindInterfaceMembers(declarations);

‎RetailCoder.VBE/Inspections/ParameterCanBeByValInspection.cs

Lines changed: 67 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -21,39 +21,86 @@ public ParameterCanBeByValInspection(RubberduckParserState state)
2121
public override IEnumerable<InspectionResultBase> GetInspectionResults()
2222
{
2323
var declarations = UserDeclarations.ToList();
24+
var issues = new List<ParameterCanBeByValInspectionResult>();
2425

25-
IEnumerable<Declaration> interfaceMembers = null;
26-
interfaceMembers = declarations.FindInterfaceMembers()
27-
.Concat(declarations.FindInterfaceImplementationMembers())
28-
.ToList();
26+
var interfaceDeclarationMembers = declarations.FindInterfaceMembers().ToList();
27+
var interfaceScopes = declarations.FindInterfaceImplementationMembers().Concat(interfaceDeclarationMembers).Select(s => s.Scope);
2928

30-
var formEventHandlerScopes = State.FindFormEventHandlers()
31-
.Select(handler => handler.Scope);
29+
issues.AddRange(GetResults(declarations, interfaceDeclarationMembers));
3230

33-
var eventScopes = declarations.Where(item =>
34-
!item.IsBuiltIn && item.DeclarationType == DeclarationType.Event)
35-
.Select(e => e.Scope).Concat(State.AllDeclarations.FindBuiltInEventHandlers().Select(e => e.Scope));
31+
var eventMembers = declarations.Where(item => !item.IsBuiltIn && item.DeclarationType == DeclarationType.Event).ToList();
32+
var formEventHandlerScopes = State.FindFormEventHandlers().Select(handler => handler.Scope);
33+
var eventHandlerScopes = State.AllDeclarations.FindBuiltInEventHandlers().Concat(declarations.FindUserEventHandlers()).Select(e => e.Scope);
34+
var eventScopes = eventMembers.Select(s => s.Scope)
35+
.Concat(formEventHandlerScopes)
36+
.Concat(eventHandlerScopes);
37+
38+
issues.AddRange(GetResults(declarations, eventMembers));
3639

3740
var declareScopes = declarations.Where(item =>
3841
item.DeclarationType == DeclarationType.LibraryFunction
3942
|| item.DeclarationType == DeclarationType.LibraryProcedure)
4043
.Select(e => e.Scope);
41-
42-
var ignoredScopes = formEventHandlerScopes.Concat(eventScopes).Concat(declareScopes);
43-
44-
var issues = declarations.Where(declaration =>
44+
45+
issues.AddRange(declarations.Where(declaration =>
4546
!declaration.IsArray
46-
&& !ignoredScopes.Contains(declaration.ParentScope)
47+
&& !declareScopes.Contains(declaration.ParentScope)
48+
&& !eventScopes.Contains(declaration.ParentScope)
49+
&& !interfaceScopes.Contains(declaration.ParentScope)
4750
&& declaration.DeclarationType == DeclarationType.Parameter
48-
&& !interfaceMembers.Select(m => m.Scope).Contains(declaration.ParentScope)
4951
&& ((VBAParser.ArgContext)declaration.Context).BYVAL() == null
5052
&& !IsUsedAsByRefParam(declarations, declaration)
5153
&& !declaration.References.Any(reference => reference.IsAssignment))
52-
.Select(issue => new ParameterCanBeByValInspectionResult(this, issue, issue.Context, issue.QualifiedName));
54+
.Select(issue => new ParameterCanBeByValInspectionResult(this, State, issue, issue.Context, issue.QualifiedName)));
5355

5456
return issues;
5557
}
5658

59+
private IEnumerable<ParameterCanBeByValInspectionResult> GetResults(List<Declaration> declarations, List<Declaration> declarationMembers)
60+
{
61+
foreach (var declaration in declarationMembers)
62+
{
63+
var declarationParameters =
64+
declarations.Where(d => d.DeclarationType == DeclarationType.Parameter &&
65+
d.ParentDeclaration == declaration)
66+
.OrderBy(o => o.Selection.StartLine)
67+
.ThenBy(t => t.Selection.StartColumn)
68+
.ToList();
69+
70+
var parametersAreByRef = declarationParameters.Select(s => true).ToList();
71+
72+
var members = declarationMembers.Any(a => a.DeclarationType == DeclarationType.Event)
73+
? declarations.FindHandlersForEvent(declaration).Select(s => s.Item2).ToList()
74+
: declarations.FindInterfaceImplementationMembers(declaration).ToList();
75+
76+
foreach (var member in members)
77+
{
78+
var parameters =
79+
declarations.Where(d => d.DeclarationType == DeclarationType.Parameter &&
80+
d.ParentDeclaration == member)
81+
.OrderBy(o => o.Selection.StartLine)
82+
.ThenBy(t => t.Selection.StartColumn)
83+
.ToList();
84+
85+
for (var i = 0; i < parameters.Count; i++)
86+
{
87+
parametersAreByRef[i] = parametersAreByRef[i] && !IsUsedAsByRefParam(declarations, parameters[i]) &&
88+
((VBAParser.ArgContext)parameters[i].Context).BYVAL() == null &&
89+
!parameters[i].References.Any(reference => reference.IsAssignment);
90+
}
91+
}
92+
93+
for (var i = 0; i < declarationParameters.Count; i++)
94+
{
95+
if (parametersAreByRef[i])
96+
{
97+
yield return new ParameterCanBeByValInspectionResult(this, State, declarationParameters[i],
98+
declarationParameters[i].Context, declarationParameters[i].QualifiedName);
99+
}
100+
}
101+
}
102+
}
103+
57104
private static bool IsUsedAsByRefParam(IEnumerable<Declaration> declarations, Declaration parameter)
58105
{
59106
// find the procedure calls in the procedure of the parameter.
@@ -74,28 +121,16 @@ private static bool IsUsedAsByRefParam(IEnumerable<Declaration> declarations, De
74121
.ThenBy(arg => arg.Selection.StartColumn)
75122
.ToArray();
76123

77-
for (var i = 0; i < calledProcedureArgs.Count(); i++)
124+
foreach (var declaration in calledProcedureArgs)
78125
{
79-
if (((VBAParser.ArgContext)calledProcedureArgs[i].Context).BYVAL() != null)
126+
if (((VBAParser.ArgContext)declaration.Context).BYVAL() != null)
80127
{
81128
continue;
82129
}
83130

84-
foreach (var reference in item)
131+
if (declaration.References.Any(reference => reference.IsAssignment))
85132
{
86-
if (!(reference.Context is VBAParser.ArgContext))
87-
{
88-
continue;
89-
}
90-
var context = ((dynamic)reference.Context.Parent).argsCall() as VBAParser.ArgContext;
91-
if (context == null)
92-
{
93-
continue;
94-
}
95-
if (parameter.IdentifierName == context.GetText())
96-
{
97-
return true;
98-
}
133+
return true;
99134
}
100135
}
101136
}

‎RetailCoder.VBE/Inspections/ParameterCanBeByValInspectionResult.cs

Lines changed: 67 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
using System.Collections.Generic;
2+
using System.Linq;
23
using Antlr4.Runtime;
4+
using Rubberduck.Common;
35
using Rubberduck.Parsing.Grammar;
46
using Rubberduck.Parsing.Symbols;
7+
using Rubberduck.Parsing.VBA;
58
using Rubberduck.VBEditor;
69

710
namespace Rubberduck.Inspections
@@ -10,12 +13,12 @@ public class ParameterCanBeByValInspectionResult : InspectionResultBase
1013
{
1114
private readonly IEnumerable<CodeInspectionQuickFix> _quickFixes;
1215

13-
public ParameterCanBeByValInspectionResult(IInspection inspection, Declaration target, ParserRuleContext context, QualifiedMemberName qualifiedName)
16+
public ParameterCanBeByValInspectionResult(IInspection inspection, RubberduckParserState state, Declaration target, ParserRuleContext context, QualifiedMemberName qualifiedName)
1417
: base(inspection, qualifiedName.QualifiedModuleName, context, target)
1518
{
1619
_quickFixes = new CodeInspectionQuickFix[]
1720
{
18-
new PassParameterByValueQuickFix(Context, QualifiedSelection),
21+
new PassParameterByValueQuickFix(state, Target, Context, QualifiedSelection),
1922
new IgnoreOnceQuickFix(Context, QualifiedSelection, inspection.AnnotationName)
2023
};
2124
}
@@ -30,21 +33,76 @@ public override string Description
3033

3134
public class PassParameterByValueQuickFix : CodeInspectionQuickFix
3235
{
33-
public PassParameterByValueQuickFix(ParserRuleContext context, QualifiedSelection selection)
36+
private readonly RubberduckParserState _state;
37+
private readonly Declaration _target;
38+
39+
public PassParameterByValueQuickFix(RubberduckParserState state, Declaration target, ParserRuleContext context, QualifiedSelection selection)
3440
: base(context, selection, InspectionsUI.PassParameterByValueQuickFix)
3541
{
42+
_state = state;
43+
_target = target;
3644
}
3745

3846
public override void Fix()
3947
{
40-
var selection = Selection.Selection;
41-
var selectionLength = ((VBAParser.ArgContext) Context).BYREF() == null ? 0 : 6;
48+
if (_target.ParentDeclaration.DeclarationType == DeclarationType.Event ||
49+
_state.AllUserDeclarations.FindInterfaceMembers().Contains(_target.ParentDeclaration))
50+
{
51+
FixMethods();
52+
}
53+
else
54+
{
55+
FixMethod((VBAParser.ArgContext)Context, Selection);
56+
}
57+
}
58+
59+
private void FixMethods()
60+
{
61+
var declarationParameters =
62+
_state.AllUserDeclarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter &&
63+
declaration.ParentDeclaration == _target.ParentDeclaration)
64+
.OrderBy(o => o.Selection.StartLine)
65+
.ThenBy(t => t.Selection.StartColumn)
66+
.ToList();
67+
68+
var parameterIndex = declarationParameters.IndexOf(_target);
69+
if (parameterIndex == -1)
70+
{
71+
return; // should only happen if the parse results are stale; prevents a crash in that case
72+
}
73+
74+
var members = _target.ParentDeclaration.DeclarationType == DeclarationType.Event
75+
? _state.AllUserDeclarations.FindHandlersForEvent(_target.ParentDeclaration)
76+
.Select(s => s.Item2)
77+
.ToList()
78+
: _state.AllUserDeclarations.FindInterfaceImplementationMembers(_target.ParentDeclaration).ToList();
79+
80+
foreach (var member in members)
81+
{
82+
var parameters =
83+
_state.AllUserDeclarations.Where(declaration => declaration.DeclarationType == DeclarationType.Parameter &&
84+
declaration.ParentDeclaration == member)
85+
.OrderBy(o => o.Selection.StartLine)
86+
.ThenBy(t => t.Selection.StartColumn)
87+
.ToList();
88+
89+
FixMethod((VBAParser.ArgContext)parameters[parameterIndex].Context,
90+
parameters[parameterIndex].QualifiedSelection);
91+
}
92+
93+
FixMethod((VBAParser.ArgContext)declarationParameters[parameterIndex].Context,
94+
declarationParameters[parameterIndex].QualifiedSelection);
95+
}
96+
97+
private void FixMethod(VBAParser.ArgContext context, QualifiedSelection qualifiedSelection)
98+
{
99+
var selectionLength = context.BYREF() == null ? 0 : 6;
42100

43-
var module = Selection.QualifiedName.Component.CodeModule;
44-
var lines = module.Lines[selection.StartLine, 1];
101+
var module = qualifiedSelection.QualifiedName.Component.CodeModule;
102+
var lines = module.Lines[context.Start.Line, 1];
45103

46-
var result = lines.Remove(selection.StartColumn - 1, selectionLength).Insert(selection.StartColumn - 1, Tokens.ByVal + ' ');
47-
module.ReplaceLine(selection.StartLine, result);
104+
var result = lines.Remove(context.Start.Column, selectionLength).Insert(context.Start.Column, Tokens.ByVal + ' ');
105+
module.ReplaceLine(context.Start.Line, result);
48106
}
49107
}
50108
}

‎Rubberduck.Parsing/Symbols/IdentifierReferenceResolver.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ private void ResolveDefault(
163163
"Default Context: Failed to resolve {0}. Binding as much as we can.",
164164
expression.GetText()));
165165
}
166-
_boundExpressionVisitor.AddIdentifierReferences(boundExpression, _qualifiedModuleName, _currentScope, _currentParent, isAssignmentTarget, false);
166+
_boundExpressionVisitor.AddIdentifierReferences(boundExpression, _qualifiedModuleName, _currentScope, _currentParent, isAssignmentTarget, hasExplicitLetStatement);
167167
}
168168

169169
private void ResolveType(ParserRuleContext expression)
@@ -444,7 +444,7 @@ private void ResolveRecordRange(VBAParser.RecordRangeContext recordRange)
444444
public void Resolve(VBAParser.LineInputStmtContext context)
445445
{
446446
ResolveDefault(context.markedFileNumber().expression());
447-
ResolveDefault(context.variableName().expression());
447+
ResolveDefault(context.variableName().expression(), isAssignmentTarget: true);
448448
}
449449

450450
public void Resolve(VBAParser.WidthStmtContext context)
@@ -496,7 +496,7 @@ public void Resolve(VBAParser.InputStmtContext context)
496496
ResolveDefault(context.markedFileNumber().expression());
497497
foreach (var inputVariable in context.inputList().inputVariable())
498498
{
499-
ResolveDefault(inputVariable.expression());
499+
ResolveDefault(inputVariable.expression(), isAssignmentTarget: true);
500500
}
501501
}
502502

@@ -522,7 +522,7 @@ public void Resolve(VBAParser.GetStmtContext context)
522522
}
523523
if (context.variable() != null)
524524
{
525-
ResolveDefault(context.variable().expression());
525+
ResolveDefault(context.variable().expression(), isAssignmentTarget: true);
526526
}
527527
}
528528

0 commit comments

Comments
 (0)