Skip to content

Commit e8f8d1a

Browse files
bclothierbclothier
authored
Merge pull request #5165 from MDoerner/ProcedureCoercionCallInspection
Inspection warning about procedure coercion calls
2 parents 33d0ebb + cdc67ab commit e8f8d1a

21 files changed

+927
-40
lines changed

‎Rubberduck.CodeAnalysis/Inspections/ObjectWhereProcedureIsRequiredInspection.cs

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
using System.Collections.Generic;
2+
using System.Linq;
3+
using Rubberduck.Inspections.Abstract;
4+
using Rubberduck.Resources.Inspections;
5+
using Rubberduck.Parsing.Symbols;
6+
using Rubberduck.Parsing.VBA;
7+
using Rubberduck.Inspections.Inspections.Extensions;
8+
using Rubberduck.Inspections.Results;
9+
using Rubberduck.Parsing.Inspections;
10+
using Rubberduck.Parsing.Inspections.Abstract;
11+
using Rubberduck.Parsing.VBA.DeclarationCaching;
12+
using Rubberduck.VBEditor;
13+
14+
namespace Rubberduck.Inspections.Concrete
15+
{
16+
/// <summary>
17+
/// Identifies places in which an object is used but a procedure is required and a default member exists on the object.
18+
/// </summary>
19+
/// <why>
20+
/// Providing an object where a procedure is required leads to an implicit call to the object's default member.
21+
/// This behavior is not obvious, and most likely unintended.
22+
/// </why>
23+
/// <example hasResult="true">
24+
/// <module name="Class1" type="Class Module">
25+
/// <![CDATA[
26+
/// Public Function Foo() As Long
27+
/// Attibute VB_UserMemId = 0
28+
/// Foo = 42
29+
/// End Function
30+
/// ]]>
31+
/// </module>
32+
/// <module name="Module" type="Standard Module">
33+
/// <![CDATA[
34+
/// Public Sub DoSomething(ByVal arg As Class1)
35+
/// arg
36+
/// End Sub
37+
/// ]]>
38+
/// </module>
39+
/// </example>
40+
/// <example hasResult="false">
41+
/// <module name="Class1" type="Class Module">
42+
/// <![CDATA[
43+
/// Public Function Foo() As Long
44+
/// Attibute VB_UserMemId = 0
45+
/// Foo = 42
46+
/// End Function
47+
/// ]]>
48+
/// </module>
49+
/// <module name="Module" type="Standard Module">
50+
/// <![CDATA[
51+
/// Public Sub DoSomething(ByVal arg As Class1)
52+
/// arg.Foo
53+
/// End Sub
54+
/// ]]>
55+
/// </module>
56+
/// </example>
57+
public sealed class ObjectWhereProcedureIsRequiredInspection : InspectionBase
58+
{
59+
private readonly IDeclarationFinderProvider _declarationFinderProvider;
60+
61+
public ObjectWhereProcedureIsRequiredInspection(RubberduckParserState state)
62+
: base(state)
63+
{
64+
_declarationFinderProvider = state;
65+
Severity = CodeInspectionSeverity.Warning;
66+
}
67+
68+
protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
69+
{
70+
var results = new List<IInspectionResult>();
71+
foreach (var moduleDeclaration in State.DeclarationFinder.UserDeclarations(DeclarationType.Module))
72+
{
73+
if (moduleDeclaration == null || moduleDeclaration.IsIgnoringInspectionResultFor(AnnotationName))
74+
{
75+
continue;
76+
}
77+
78+
var module = moduleDeclaration.QualifiedModuleName;
79+
results.AddRange(DoGetInspectionResults(module));
80+
}
81+
82+
return results;
83+
}
84+
85+
private IEnumerable<IInspectionResult> DoGetInspectionResults(QualifiedModuleName module)
86+
{
87+
var finder = _declarationFinderProvider.DeclarationFinder;
88+
return BoundInspectionResults(module, finder)
89+
.Concat(UnboundInspectionResults(module, finder));
90+
}
91+
92+
private IEnumerable<IInspectionResult> BoundInspectionResults(QualifiedModuleName module, DeclarationFinder finder)
93+
{
94+
var objectionableReferences = finder
95+
.IdentifierReferences(module)
96+
.Where(IsResultReference);
97+
98+
return objectionableReferences
99+
.Select(reference => BoundInspectionResult(reference, _declarationFinderProvider))
100+
.ToList();
101+
}
102+
103+
private bool IsResultReference(IdentifierReference reference)
104+
{
105+
return reference.IsProcedureCoercion
106+
&& !reference.IsIgnoringInspectionResultFor(AnnotationName);
107+
}
108+
109+
private IInspectionResult BoundInspectionResult(IdentifierReference reference, IDeclarationFinderProvider declarationFinderProvider)
110+
{
111+
return new IdentifierReferenceInspectionResult(
112+
this,
113+
BoundResultDescription(reference),
114+
declarationFinderProvider,
115+
reference);
116+
}
117+
118+
private string BoundResultDescription(IdentifierReference reference)
119+
{
120+
var expression = reference.IdentifierName;
121+
var defaultMember = reference.Declaration.QualifiedName.ToString();
122+
return string.Format(InspectionResults.ObjectWhereProcedureIsRequiredInspection, expression, defaultMember);
123+
}
124+
125+
private IEnumerable<IInspectionResult> UnboundInspectionResults(QualifiedModuleName module, DeclarationFinder finder)
126+
{
127+
var objectionableReferences = finder
128+
.UnboundDefaultMemberAccesses(module)
129+
.Where(IsResultReference);
130+
131+
return objectionableReferences
132+
.Select(reference => UnboundInspectionResult(reference, _declarationFinderProvider))
133+
.ToList();
134+
}
135+
136+
private IInspectionResult UnboundInspectionResult(IdentifierReference reference, IDeclarationFinderProvider declarationFinderProvider)
137+
{
138+
var result = new IdentifierReferenceInspectionResult(
139+
this,
140+
UnboundResultDescription(reference),
141+
declarationFinderProvider,
142+
reference);
143+
result.Properties.DisableFixes = "ExpandDefaultMemberQuickFix";
144+
return result;
145+
}
146+
147+
private string UnboundResultDescription(IdentifierReference reference)
148+
{
149+
var expression = reference.IdentifierName;
150+
return string.Format(InspectionResults.ObjectWhereProcedureIsRequiredInspection_Unbound, expression);
151+
}
152+
}
153+
}

‎Rubberduck.CodeAnalysis/QuickFixes/ExpandDefaultMemberQuickFix.cs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
using System.Linq;
2+
using Antlr4.Runtime;
3+
using Rubberduck.Inspections.Abstract;
4+
using Rubberduck.Inspections.Concrete;
5+
using Rubberduck.Parsing.Inspections.Abstract;
6+
using Rubberduck.Parsing.Rewriter;
7+
using Rubberduck.Parsing.VBA;
8+
using Rubberduck.Parsing.VBA.DeclarationCaching;
9+
using Rubberduck.VBEditor;
10+
11+
namespace Rubberduck.CodeAnalysis.QuickFixes
12+
{
13+
public class ExpandDefaultMemberQuickFix : QuickFixBase
14+
{
15+
private readonly IDeclarationFinderProvider _declarationFinderProvider;
16+
17+
public ExpandDefaultMemberQuickFix(IDeclarationFinderProvider declarationFinderProvider)
18+
: base(typeof(ObjectWhereProcedureIsRequiredInspection))
19+
{
20+
_declarationFinderProvider = declarationFinderProvider;
21+
}
22+
23+
public override void Fix(IInspectionResult result, IRewriteSession rewriteSession)
24+
{
25+
var rewriter = rewriteSession.CheckOutModuleRewriter(result.QualifiedSelection.QualifiedName);
26+
var finder = _declarationFinderProvider.DeclarationFinder;
27+
28+
var lExpressionContext = result.Context;
29+
var selection = result.QualifiedSelection;
30+
InsertDefaultMember(lExpressionContext, selection, finder, rewriter);
31+
}
32+
33+
private void InsertDefaultMember(ParserRuleContext lExpressionContext, QualifiedSelection selection, DeclarationFinder finder, IModuleRewriter rewriter)
34+
{
35+
var defaultMemberAccessCode = DefaultMemberAccessCode(selection, finder);
36+
rewriter.InsertAfter(lExpressionContext.Stop.TokenIndex, defaultMemberAccessCode);
37+
}
38+
39+
private string DefaultMemberAccessCode(QualifiedSelection selection, DeclarationFinder finder)
40+
{
41+
var defaultMemberAccesses = finder.IdentifierReferences(selection).Where(reference => reference.DefaultMemberRecursionDepth > 0);
42+
var defaultMemberNames = defaultMemberAccesses.Select(reference => reference.Declaration.IdentifierName);
43+
return $".{string.Join("().", defaultMemberNames)}";
44+
}
45+
46+
public override string Description(IInspectionResult result)
47+
{
48+
return Resources.Inspections.QuickFixes.ExpandDefaultMemberQuickFix;
49+
}
50+
51+
public override bool CanFixInProcedure => true;
52+
public override bool CanFixInModule => true;
53+
public override bool CanFixInProject => true;
54+
}
55+
}

‎Rubberduck.Parsing/Symbols/Declaration.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,8 @@ public void AddReference(
366366
bool isIndexedDefaultMemberAccess = false,
367367
bool isNonIndexedDefaultMemberAccess = false,
368368
int defaultMemberRecursionDepth = 0,
369-
bool isArrayAccess = false
369+
bool isArrayAccess = false,
370+
bool isProcedureCoercion = false
370371
)
371372
{
372373
var oldReference = _references.FirstOrDefault(r =>
@@ -397,7 +398,8 @@ public void AddReference(
397398
isIndexedDefaultMemberAccess,
398399
isNonIndexedDefaultMemberAccess,
399400
defaultMemberRecursionDepth,
400-
isArrayAccess);
401+
isArrayAccess,
402+
isProcedureCoercion);
401403
_references.AddOrUpdate(newReference, 1, (key, value) => 1);
402404
}
403405

‎Rubberduck.Parsing/Symbols/IdentifierReference.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ public IdentifierReference(
2727
bool isIndexedDefaultMemberAccess = false,
2828
bool isNonIndexedDefaultMemberAccess = false,
2929
int defaultMemberRecursionDepth = 0,
30-
bool isArrayAccess = false)
30+
bool isArrayAccess = false,
31+
bool isProcedureCoercion = false)
3132
{
3233
ParentScoping = parentScopingDeclaration;
3334
ParentNonScoping = parentNonScopingDeclaration;
@@ -43,6 +44,7 @@ public IdentifierReference(
4344
IsNonIndexedDefaultMemberAccess = isNonIndexedDefaultMemberAccess;
4445
DefaultMemberRecursionDepth = defaultMemberRecursionDepth;
4546
IsArrayAccess = isArrayAccess;
47+
IsProcedureCoercion = isProcedureCoercion;
4648
Annotations = annotations ?? new List<IParseTreeAnnotation>();
4749
}
4850

@@ -71,6 +73,7 @@ public IdentifierReference(
7173
public bool IsIndexedDefaultMemberAccess { get; }
7274
public bool IsNonIndexedDefaultMemberAccess { get; }
7375
public bool IsDefaultMemberAccess => IsIndexedDefaultMemberAccess || IsNonIndexedDefaultMemberAccess;
76+
public bool IsProcedureCoercion { get; }
7477
public int DefaultMemberRecursionDepth { get; }
7578

7679
public bool IsArrayAccess { get; }

‎Rubberduck.Parsing/VBA/ReferenceManagement/BoundExpressionVisitor.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,8 @@ private void AddDefaultMemberReference(
455455
selection,
456456
FindIdentifierAnnotations(module, selection.StartLine),
457457
isNonIndexedDefaultMemberAccess: true,
458-
defaultMemberRecursionDepth: expression.DefaultMemberRecursionDepth);
458+
defaultMemberRecursionDepth: expression.DefaultMemberRecursionDepth,
459+
isProcedureCoercion: true);
459460
}
460461

461462
private void AddUnboundDefaultMemberReference(
@@ -481,7 +482,8 @@ private void AddUnboundDefaultMemberReference(
481482
FindIdentifierAnnotations(module, selection.StartLine),
482483
false,
483484
isNonIndexedDefaultMemberAccess: true,
484-
defaultMemberRecursionDepth: expression.DefaultMemberRecursionDepth);
485+
defaultMemberRecursionDepth: expression.DefaultMemberRecursionDepth,
486+
isProcedureCoercion: true);
485487
_declarationFinder.AddUnboundDefaultMemberAccess(reference);
486488
}
487489

‎Rubberduck.Resources/Inspections/InspectionInfo.Designer.cs

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Rubberduck.Resources/Inspections/InspectionInfo.de.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,4 +409,7 @@ Falls der Parameter 'null' sein kann, bitte dieses Auftreten ignorieren. 'null'
409409
<data name="UseOfUnboundBangNotationInspection" xml:space="preserve">
410410
<value>Die Ausrufezeichennotation erweckt den Eindruck, dass es sich um einen Zugriff handelt, der Typchecks unterliegt. Allerdings handelt es sich lediglich um einen Zugriff auf den Standardmember des Objekts, auf das sie angewendet wird, bei dem das Argument als Zeichenkette übergeben wird. Dies ist besonders verwirrend, wenn der Standardmember nicht zum Zeitpunkt des Kompilierens ermittelt werden kann.</value>
411411
</data>
412+
<data name="ObjectWhereProcedureIsRequiredInspection" xml:space="preserve">
413+
<value>Wenn ein Objekt mit einem Standardmember an einer Stelle verwendet wird, die nach einer Prozedur verlangt, wird implizit der Standardmember aufgerufen. Die ist wahrscheinlich nicht beabsichtigt, minder aber zu Mindest die Lesbarkeit des Programms.</value>
414+
</data>
412415
</root>

‎Rubberduck.Resources/Inspections/InspectionInfo.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,4 +412,7 @@ If the parameter can be null, ignore this inspection result; passing a null valu
412412
<data name="UseOfUnboundBangNotationInspection" xml:space="preserve">
413413
<value>Bang notation, formally known as dictionary access expression, looks like it is strongly typed. However, it is actually a stringly-typed access to the parameterized default member of the object it is used on. This is especially misleading the default member cannot be determined at compile time.</value>
414414
</data>
415+
<data name="ObjectWhereProcedureIsRequiredInspection" xml:space="preserve">
416+
<value>Using an object with a default member in a place that requires a procedure leads to an implicit invocation of the default member. This is most likely unintentional and negatively affects readability.</value>
417+
</data>
415418
</root>

‎Rubberduck.Resources/Inspections/InspectionNames.Designer.cs

Lines changed: 10 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Rubberduck.Resources/Inspections/InspectionNames.de.resx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@
382382
<value>Objekt statt Wert verwendet</value>
383383
</data>
384384
<data name="ProcedureRequiredInspection" xml:space="preserve">
385-
<value>Objekt statt Prozedur verwendet</value>
385+
<value>Objekt ohne Standardmember statt Prozedur verwendet</value>
386386
</data>
387387
<data name="DefaultMemberRequiredInspection" xml:space="preserve">
388388
<value>Standardmemberzugriff ohne Standardmember</value>
@@ -396,4 +396,7 @@
396396
<data name="UseOfUnboundBangNotationInspection" xml:space="preserve">
397397
<value>Verwendung nicht gebundener Ausrufezeichennotation</value>
398398
</data>
399+
<data name="ObjectWhereProcedureIsRequiredInspection" xml:space="preserve">
400+
<value>Objekt an Stell einer Prozedur verwendet</value>
401+
</data>
399402
</root>

0 commit comments

Comments
 (0)