Skip to content

Commit ae9e807

Browse files
retailcoderretailcoder
committed
all green?
1 parent b589d88 commit ae9e807

File tree

5 files changed

+149
-94
lines changed

5 files changed

+149
-94
lines changed

‎Rubberduck.CodeAnalysis/Inspections/Abstract/IdentifierReferenceInspectionFromDeclarationsBase.cs‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
using System.Collections.Generic;
22
using System.Linq;
33
using Rubberduck.CodeAnalysis.Inspections.Results;
4+
using Rubberduck.Parsing;
5+
using Rubberduck.Parsing.Grammar;
46
using Rubberduck.Parsing.Symbols;
57
using Rubberduck.Parsing.VBA;
68
using Rubberduck.Parsing.VBA.DeclarationCaching;
@@ -19,6 +21,67 @@ protected IdentifierReferenceInspectionFromDeclarationsBase(IDeclarationFinderPr
1921

2022
protected virtual ICollection<string> DisabledQuickFixes(IdentifierReference reference) => new List<string>();
2123

24+
/// <summary>
25+
/// Gets the possible <see cref="Declaration"/> that qualifies an identifier reference in a member access expression.
26+
/// </summary>
27+
protected IEnumerable<Declaration> GetQualifierCandidates(IdentifierReference reference, DeclarationFinder finder)
28+
{
29+
if (reference.Context.TryGetAncestor<VBAParser.CallStmtContext>(out var callStmt))
30+
{
31+
if (reference.Context.TryGetAncestor<VBAParser.LExpressionContext>(out var lExpression))
32+
{
33+
// reference is in lexpression of a call statement
34+
35+
if (lExpression is VBAParser.MemberAccessExprContext member)
36+
{
37+
if (member.lExpression() is VBAParser.SimpleNameExprContext name)
38+
{
39+
if (reference.IdentifierName.Equals(name.identifier().GetText(), System.StringComparison.InvariantCultureIgnoreCase))
40+
{
41+
// unqualified
42+
return Enumerable.Empty<Declaration>();
43+
}
44+
45+
return finder.MatchName(name.identifier().GetText())
46+
.Where(candidate => !candidate.Equals(reference.Declaration));
47+
}
48+
49+
// todo get the actual qualifying declaration?
50+
return finder.MatchName(member.lExpression().children.First().GetText());
51+
}
52+
}
53+
54+
55+
}
56+
57+
if (reference.Context.TryGetAncestor<VBAParser.MemberAccessExprContext>(out var memberAccess))
58+
{
59+
var parentModule = Declaration.GetModuleParent(reference.ParentScoping);
60+
var qualifyingExpression = memberAccess.lExpression();
61+
if (qualifyingExpression is VBAParser.SimpleNameExprContext simpleName)
62+
{
63+
if (simpleName.GetText().Equals(Tokens.Me, System.StringComparison.InvariantCultureIgnoreCase))
64+
{
65+
// qualifier is 'Me'
66+
return new[] { parentModule };
67+
}
68+
69+
// todo get the actual qualifying declaration?
70+
return finder.MatchName(simpleName.GetText());
71+
}
72+
}
73+
74+
if (reference.Context.TryGetAncestor<VBAParser.WithMemberAccessExprContext>(out var dot))
75+
{
76+
// qualifier is a With block
77+
var withBlock = dot.GetAncestor<VBAParser.WithStmtContext>();
78+
return finder.ContainedIdentifierReferences(new QualifiedSelection(reference.QualifiedModuleName, withBlock.GetSelection()))
79+
.Select(r => r.Declaration).Distinct();
80+
}
81+
82+
return Enumerable.Empty<Declaration>();
83+
}
84+
2285
protected override IEnumerable<IInspectionResult> DoGetInspectionResults(DeclarationFinder finder)
2386
{
2487
var objectionableReferences = ObjectionableReferences(finder);

‎Rubberduck.CodeAnalysis/Inspections/Abstract/ImplicitWorkbookReferenceInspectionBase.cs‎

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,23 +22,40 @@ internal ImplicitWorkbookReferenceInspectionBase(IDeclarationFinderProvider decl
2222
"_Global", "_Application", "Global", "Application", "_Workbook", "Workbook"
2323
};
2424

25+
private IReadOnlyList<ModuleDeclaration> _relevantClasses;
26+
private IReadOnlyList<PropertyGetDeclaration> _relevantProperties;
27+
28+
protected Declaration Excel { get; private set; }
29+
2530
protected override IEnumerable<Declaration> ObjectionableDeclarations(DeclarationFinder finder)
2631
{
27-
if (!finder.TryFindProjectDeclaration("Excel", out var excel))
32+
if (Excel == null)
2833
{
29-
return Enumerable.Empty<Declaration>();
34+
if (!finder.TryFindProjectDeclaration("Excel", out var excel))
35+
{
36+
return Enumerable.Empty<Declaration>();
37+
}
38+
Excel = excel;
3039
}
3140

32-
var relevantClasses = InterestingClasses
33-
.Select(className => finder.FindClassModule(className, excel, true))
34-
.OfType<ModuleDeclaration>();
41+
if (_relevantClasses == null)
42+
{
43+
_relevantClasses = InterestingClasses
44+
.Select(className => finder.FindClassModule(className, Excel, true))
45+
.OfType<ModuleDeclaration>()
46+
.ToList();
47+
}
3548

36-
var relevantProperties = relevantClasses
37-
.SelectMany(classDeclaration => classDeclaration.Members)
38-
.OfType<PropertyGetDeclaration>()
39-
.Where(member => InterestingMembers.Contains(member.IdentifierName));
49+
if (_relevantProperties == null)
50+
{
51+
_relevantProperties = _relevantClasses
52+
.SelectMany(classDeclaration => classDeclaration.Members)
53+
.OfType<PropertyGetDeclaration>()
54+
.Where(member => InterestingMembers.Contains(member.IdentifierName))
55+
.ToList();
56+
}
4057

41-
return relevantProperties;
58+
return _relevantProperties;
4259
}
4360
}
4461
}

‎Rubberduck.CodeAnalysis/Inspections/Concrete/ImplicitActiveWorkbookReferenceInspection.cs‎

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,19 +43,48 @@ namespace Rubberduck.CodeAnalysis.Inspections.Concrete
4343
internal sealed class ImplicitActiveWorkbookReferenceInspection : ImplicitWorkbookReferenceInspectionBase
4444
{
4545
public ImplicitActiveWorkbookReferenceInspection(IDeclarationFinderProvider declarationFinderProvider)
46-
: base(declarationFinderProvider)
47-
{}
46+
: base(declarationFinderProvider) { }
4847

49-
private static readonly List<string> _alwaysActiveWorkbookReferenceParents = new List<string>
50-
{
51-
"_Application", "Application"
52-
};
48+
private IReadOnlyList<Declaration> _applicationCandidates;
5349

5450
protected override bool IsResultReference(IdentifierReference reference, DeclarationFinder finder)
5551
{
56-
return !(Declaration.GetModuleParent(reference.ParentNonScoping) is DocumentModuleDeclaration document)
57-
|| !document.SupertypeNames.Contains("Workbook")
58-
|| _alwaysActiveWorkbookReferenceParents.Contains(reference.Declaration.ParentDeclaration.IdentifierName);
52+
var qualifiers = base.GetQualifierCandidates(reference, finder);
53+
var isQualified = qualifiers.Any();
54+
var document = Declaration.GetModuleParent(reference.ParentNonScoping) as DocumentModuleDeclaration;
55+
56+
var isHostWorkbook = (document?.SupertypeNames.Contains("Workbook") ?? false)
57+
&& (document?.ProjectId?.Equals(reference.QualifiedModuleName.ProjectId) ?? false);
58+
59+
if (!isQualified)
60+
{
61+
// unqualified calls aren't referring to ActiveWorkbook only inside a Workbook module:
62+
return !isHostWorkbook;
63+
}
64+
else
65+
{
66+
if (_applicationCandidates == null)
67+
{
68+
var applicationClass = finder.FindClassModule("Application", base.Excel, includeBuiltIn: true);
69+
// note: underscored declarations would be for unqualified calls
70+
var workbookClass = finder.FindClassModule("Workbook", base.Excel, includeBuiltIn: true);
71+
var worksheetClass = finder.FindClassModule("Worksheet", base.Excel, includeBuiltIn: true);
72+
var hostBook = finder.UserDeclarations(DeclarationType.Document)
73+
.Cast<DocumentModuleDeclaration>()
74+
.SingleOrDefault(doc => doc.ProjectId.Equals(reference.QualifiedModuleName.ProjectId)
75+
&& doc.SupertypeNames.Contains("Workbook"));
76+
77+
_applicationCandidates = finder.MatchName("Application")
78+
.Where(m => m.Equals(applicationClass)
79+
|| (m.ParentDeclaration.Equals(workbookClass) && m.DeclarationType.HasFlag(DeclarationType.PropertyGet))
80+
|| (m.ParentDeclaration.Equals(worksheetClass) && m.DeclarationType.HasFlag(DeclarationType.PropertyGet))
81+
|| (m.ParentDeclaration.Equals(hostBook) && m.DeclarationType.HasFlag(DeclarationType.PropertyGet)))
82+
.ToList();
83+
}
84+
85+
// qualified calls are referring to ActiveWorkbook if qualifier is the Application object:
86+
return _applicationCandidates.Any(candidate => qualifiers.Any(q => q.Equals(candidate)));
87+
}
5988
}
6089

6190
protected override string ResultDescription(IdentifierReference reference)

‎Rubberduck.CodeAnalysis/Inspections/Concrete/ImplicitContainingWorkbookReferenceInspection.cs‎

Lines changed: 2 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -61,38 +61,10 @@ protected override IEnumerable<Declaration> ObjectionableDeclarations(Declaratio
6161

6262
protected override bool IsResultReference(IdentifierReference reference, DeclarationFinder finder)
6363
{
64-
var qualifier = GetQualifier(reference, finder);
64+
var qualifiers = base.GetQualifierCandidates(reference, finder);
6565
return Declaration.GetModuleParent(reference.ParentScoping) is DocumentModuleDeclaration document
6666
&& document.SupertypeNames.Contains("Workbook")
67-
&& (qualifier == null);
68-
}
69-
70-
private Declaration GetQualifier(IdentifierReference reference, DeclarationFinder finder)
71-
{
72-
if (reference.Context.TryGetAncestor<VBAParser.MemberAccessExprContext>(out var memberAccess))
73-
{
74-
var parentModule = Declaration.GetModuleParent(reference.ParentScoping);
75-
var qualifyingExpression = memberAccess.lExpression();
76-
var qualifier = qualifyingExpression.children[qualifyingExpression.ChildCount - 1];
77-
var qualifyingIdentifier = qualifier.GetText();
78-
if (qualifyingIdentifier.Equals(Tokens.Me, System.StringComparison.InvariantCultureIgnoreCase))
79-
{
80-
// qualifier is 'Me'
81-
return parentModule;
82-
}
83-
84-
var matches = finder.MatchName(qualifyingIdentifier)
85-
.Where(m => m.ParentScopeDeclaration.Equals(reference.ParentScoping));
86-
return matches.FirstOrDefault(); // doesn't matter which, as long as not null.
87-
}
88-
89-
if (reference.Context.TryGetAncestor<VBAParser.WithMemberAccessExprContext>(out var withStatement))
90-
{
91-
// qualifier is a With block
92-
return reference.ParentScoping; // just return any non-null, don't bother resolving the actual With qualifier
93-
}
94-
95-
return null;
67+
&& (qualifiers.Any());
9668
}
9769

9870
protected override string ResultDescription(IdentifierReference reference)

‎RubberduckTests/Inspections/ImplicitActiveWorkbookReferenceInspectionTests.cs‎

Lines changed: 19 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Collections.Generic;
12
using System.Linq;
23
using System.Threading;
34
using NUnit.Framework;
@@ -13,6 +14,12 @@ namespace RubberduckTests.Inspections
1314
[TestFixture]
1415
public class ImplicitActiveWorkbookReferenceInspectionTests : InspectionTestsBase
1516
{
17+
private static readonly IDictionary<string, IEnumerable<string>> DefaultDocumentModuleSupertypeNames = new Dictionary<string, IEnumerable<string>>
18+
{
19+
["ThisWorkbook"] = new[] { "Workbook", "_Workbook" },
20+
["Sheet1"] = new[] { "Worksheet", "_Worksheet" }
21+
};
22+
1623
[Test]
1724
[Category("Inspections")]
1825
public void ImplicitActiveWorkbookReference_ReportsWorksheets()
@@ -172,21 +179,10 @@ Sub foo()
172179
Dim sheet As Worksheet
173180
Set sheet = Worksheets(""Sheet1"")
174181
End Sub";
175-
var module = ("SomeWorkbook", inputCode, ComponentType.Document);
176-
var vbe = MockVbeBuilder.BuildFromModules(module, ReferenceLibrary.Excel).Object;
177-
178-
using (var state = MockParser.CreateAndParse(vbe))
179-
{
180-
var documentModule = state.DeclarationFinder.UserDeclarations(DeclarationType.Document)
181-
.OfType<DocumentModuleDeclaration>()
182-
.Single();
183-
documentModule.AddSupertypeName("Workbook");
184-
185-
var inspection = InspectionUnderTest(state);
186-
var inspectionResults = inspection.GetInspectionResults(CancellationToken.None);
182+
const int expected = 0;
183+
var actual = ArrangeAndGetInspectionCount(inputCode, "ThisWorkbook", ComponentType.Document);
187184

188-
Assert.AreEqual(0, inspectionResults.Count());
189-
}
185+
Assert.AreEqual(expected, actual);
190186
}
191187

192188
[Test]
@@ -199,21 +195,10 @@ Sub foo()
199195
Dim sheet As Worksheet
200196
Set sheet = Application.Worksheets(""Sheet1"")
201197
End Sub";
202-
var module = ("SomeWorkbook", inputCode, ComponentType.Document);
203-
var vbe = MockVbeBuilder.BuildFromModules(module, ReferenceLibrary.Excel).Object;
204-
205-
using (var state = MockParser.CreateAndParse(vbe))
206-
{
207-
var documentModule = state.DeclarationFinder.UserDeclarations(DeclarationType.Document)
208-
.OfType<DocumentModuleDeclaration>()
209-
.Single();
210-
documentModule.AddSupertypeName("Workbook");
211-
212-
var inspection = InspectionUnderTest(state);
213-
var inspectionResults = inspection.GetInspectionResults(CancellationToken.None);
198+
const int expected = 1;
199+
var actual = ArrangeAndGetInspectionCount(inputCode, "ThisWorkbook", ComponentType.Document);
214200

215-
Assert.AreEqual(1, inspectionResults.Count());
216-
}
201+
Assert.AreEqual(expected, actual);
217202
}
218203

219204
[Test]
@@ -226,21 +211,10 @@ Sub foo()
226211
Dim sheet As Worksheet
227212
Set sheet = Worksheets(""Sheet1"")
228213
End Sub";
229-
var module = ("Sheet1", inputCode, ComponentType.Document);
230-
var vbe = MockVbeBuilder.BuildFromModules(module, ReferenceLibrary.Excel).Object;
231-
232-
using (var state = MockParser.CreateAndParse(vbe))
233-
{
234-
var documentModule = state.DeclarationFinder.UserDeclarations(DeclarationType.Document)
235-
.OfType<DocumentModuleDeclaration>()
236-
.Single();
237-
documentModule.AddSupertypeName("Worksheet");
238-
239-
var inspection = InspectionUnderTest(state);
240-
var inspectionResults = inspection.GetInspectionResults(CancellationToken.None);
214+
const int expected = 1;
215+
var actual = ArrangeAndGetInspectionCount(inputCode, "Sheet1", ComponentType.Document);
241216

242-
Assert.AreEqual(1, inspectionResults.Count());
243-
}
217+
Assert.AreEqual(expected, actual);
244218
}
245219

246220
[Test]
@@ -262,10 +236,10 @@ Dim sheet As Worksheet
262236
Assert.AreEqual(expected, actual);
263237
}
264238

265-
private int ArrangeAndGetInspectionCount(string code)
239+
private int ArrangeAndGetInspectionCount(string code, string moduleName = "Module1", ComponentType moduleType = ComponentType.StandardModule)
266240
{
267-
var modules = new(string, string, ComponentType)[] { ("Module1", code, ComponentType.StandardModule) };
268-
return InspectionResultsForModules(modules, ReferenceLibrary.Excel).Count();
241+
var modules = new(string, string, ComponentType)[] { (moduleName, code, moduleType) };
242+
return InspectionResultsForModules(modules, ReferenceLibrary.Excel, documentModuleSupertypeNames: DefaultDocumentModuleSupertypeNames).Count();
269243
}
270244

271245
[Test]

0 commit comments

Comments
 (0)