Skip to content

Commit 39a448b

Browse files
authored
Merge pull request #3410 from MDoerner/ShadowedDeclarationsPerformanceTweaks
Performance tweaks for `ShadowedDeclarations` inspection. No other changes.
2 parents b69ddcb + ef4d724 commit 39a448b

File tree

1 file changed

+38
-83
lines changed

1 file changed

+38
-83
lines changed

Rubberduck.Inspections/Concrete/ShadowedDeclarationInspection.cs

Lines changed: 38 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,8 @@
22
using System.Collections.Generic;
33
using System.Globalization;
44
using System.Linq;
5-
using Antlr4.Runtime;
6-
using Antlr4.Runtime.Tree;
7-
using Rubberduck.Common;
85
using Rubberduck.Inspections.Abstract;
96
using Rubberduck.Inspections.Results;
10-
using Rubberduck.Parsing;
11-
using Rubberduck.Parsing.Grammar;
127
using Rubberduck.Parsing.Inspections.Abstract;
138
using Rubberduck.Parsing.Inspections.Resources;
149
using Rubberduck.Parsing.Symbols;
@@ -28,34 +23,6 @@ private enum DeclarationSite
2823
SameComponent = 3
2924
}
3025

31-
private class OptionPrivateModuleListener : VBAParserBaseListener
32-
{
33-
public List<VBAParser.ModuleContext> OptionPrivateModules { get; } = new List<VBAParser.ModuleContext>();
34-
35-
public override void EnterModule(VBAParser.ModuleContext context)
36-
{
37-
if (context.FindChildren<VBAParser.OptionPrivateModuleStmtContext>().Any())
38-
{
39-
OptionPrivateModules.Add(context);
40-
}
41-
}
42-
}
43-
44-
private class EnumerationRuleIndexListener : VBAParserBaseListener
45-
{
46-
public Dictionary<ParserRuleContext, int> DeclarationIndexes { get; } = new Dictionary<ParserRuleContext, int>();
47-
48-
public override void EnterEnumerationStmt(VBAParser.EnumerationStmtContext context)
49-
{
50-
DeclarationIndexes.Add(context, context.Start.TokenIndex);
51-
}
52-
53-
public override void EnterEnumerationStmt_Constant(VBAParser.EnumerationStmt_ConstantContext context)
54-
{
55-
DeclarationIndexes.Add(context, context.Start.TokenIndex);
56-
}
57-
}
58-
5926
public ShadowedDeclarationInspection(RubberduckParserState state) : base(state, CodeInspectionSeverity.DoNotShow)
6027
{
6128
}
@@ -66,50 +33,37 @@ public ShadowedDeclarationInspection(RubberduckParserState state) : base(state,
6633

6734
public override IEnumerable<IInspectionResult> GetInspectionResults()
6835
{
69-
var optionPrivateModuleListener = new OptionPrivateModuleListener();
70-
var enumerationRuleIndexListener = new EnumerationRuleIndexListener();
71-
72-
foreach (var module in State.DeclarationFinder.AllModules.Where(m => m.ComponentType == ComponentType.StandardModule))
73-
{
74-
ParseTreeWalker.Default.Walk(optionPrivateModuleListener, State.GetParseTree(module));
75-
}
76-
77-
foreach (var module in State.AllUserDeclarations.Where(d => d.DeclarationType == DeclarationType.ProceduralModule || d.DeclarationType == DeclarationType.ClassModule))
78-
{
79-
ParseTreeWalker.Default.Walk(enumerationRuleIndexListener, State.GetParseTree(module.QualifiedName.QualifiedModuleName));
80-
}
81-
8236
var builtInEventHandlers = State.DeclarationFinder.FindEventHandlers().ToHashSet();
8337

8438
var issues = new List<IInspectionResult>();
8539

86-
var allUserProjects = UserDeclarations.OfType(DeclarationType.Project).Cast<ProjectDeclaration>();
40+
var allUserProjects = State.DeclarationFinder.UserDeclarations(DeclarationType.Project).Cast<ProjectDeclaration>();
8741

8842
foreach (var userProject in allUserProjects)
8943
{
9044
var referencedProjectIds = userProject.ProjectReferences.Select(reference => reference.ReferencedProjectId).ToHashSet();
9145

92-
var userDeclarations = UserDeclarations.Where(d =>
93-
d.ProjectId == userProject.ProjectId &&
46+
var userDeclarations = UserDeclarations.Where(declaration =>
47+
declaration.ProjectId == userProject.ProjectId &&
9448
// User has no control over build-in event handlers or their parameters, so we skip them
95-
!DeclarationIsPartOfBuiltInEventHandler(d, builtInEventHandlers));
49+
!DeclarationIsPartOfBuiltInEventHandler(declaration, builtInEventHandlers));
9650

97-
foreach (var declaration in userDeclarations)
51+
foreach (var userDeclaration in userDeclarations)
9852
{
99-
var shadowedDeclaration = State.AllDeclarations.FirstOrDefault(d =>
100-
!Equals(d, declaration) &&
101-
string.Equals(d.IdentifierName, declaration.IdentifierName, StringComparison.OrdinalIgnoreCase) &&
102-
DeclarationCanBeShadowed(d, declaration, GetDeclarationSite(d, declaration, referencedProjectIds), optionPrivateModuleListener, enumerationRuleIndexListener));
53+
var shadowedDeclaration = State.DeclarationFinder
54+
.MatchName(userDeclaration.IdentifierName).FirstOrDefault(declaration =>
55+
!declaration.Equals(userDeclaration) &&
56+
DeclarationCanBeShadowed(declaration, userDeclaration, GetDeclarationSite(declaration, userDeclaration, referencedProjectIds)));
10357

10458
if (shadowedDeclaration != null)
10559
{
10660
issues.Add(new DeclarationInspectionResult(this,
10761
string.Format(InspectionsUI.ShadowedDeclarationInspectionResultFormat,
108-
RubberduckUI.ResourceManager.GetString("DeclarationType_" + declaration.DeclarationType, CultureInfo.CurrentUICulture),
109-
declaration.IdentifierName,
62+
RubberduckUI.ResourceManager.GetString("DeclarationType_" + userDeclaration.DeclarationType, CultureInfo.CurrentUICulture),
63+
userDeclaration.IdentifierName,
11064
RubberduckUI.ResourceManager.GetString("DeclarationType_" + shadowedDeclaration.DeclarationType, CultureInfo.CurrentUICulture),
11165
shadowedDeclaration.IdentifierName),
112-
declaration));
66+
userDeclaration));
11367
}
11468
}
11569
}
@@ -124,7 +78,7 @@ private static DeclarationSite GetDeclarationSite(Declaration originalDeclaratio
12478
return referencedProjectIds.Contains(originalDeclaration.ProjectId) ? DeclarationSite.ReferencedProject : DeclarationSite.NotApplicable;
12579
}
12680

127-
if (originalDeclaration.QualifiedName.QualifiedModuleName.Name != userDeclaration.QualifiedName.QualifiedModuleName.Name)
81+
if (originalDeclaration.QualifiedName.QualifiedModuleName.ComponentName != userDeclaration.QualifiedName.QualifiedModuleName.ComponentName)
12882
{
12983
return DeclarationSite.OtherComponent;
13084
}
@@ -144,8 +98,7 @@ private static bool DeclarationIsPartOfBuiltInEventHandler(Declaration declarati
14498
return parameterDeclaration != null && builtInEventHandlers.Contains(parameterDeclaration.ParentDeclaration);
14599
}
146100

147-
private static bool DeclarationCanBeShadowed(Declaration originalDeclaration, Declaration userDeclaration, DeclarationSite originalDeclarationSite,
148-
OptionPrivateModuleListener optionPrivateModuleListener, EnumerationRuleIndexListener enumerationRuleIndexListener)
101+
private static bool DeclarationCanBeShadowed(Declaration originalDeclaration, Declaration userDeclaration, DeclarationSite originalDeclarationSite)
149102
{
150103
if (originalDeclarationSite == DeclarationSite.NotApplicable)
151104
{
@@ -154,20 +107,20 @@ private static bool DeclarationCanBeShadowed(Declaration originalDeclaration, De
154107

155108
if (originalDeclarationSite == DeclarationSite.ReferencedProject)
156109
{
157-
return DeclarationInReferencedProjectCanBeShadowed(originalDeclaration, userDeclaration, optionPrivateModuleListener);
110+
return DeclarationInReferencedProjectCanBeShadowed(originalDeclaration, userDeclaration);
158111
}
159112

160113
if (originalDeclarationSite == DeclarationSite.OtherComponent)
161114
{
162-
return DeclarationInAnotherComponentCanBeShadowed(originalDeclaration, userDeclaration, optionPrivateModuleListener);
115+
return DeclarationInAnotherComponentCanBeShadowed(originalDeclaration, userDeclaration);
163116
}
164117

165-
return DeclarationInTheSameComponentCanBeShadowed(originalDeclaration, userDeclaration, enumerationRuleIndexListener);
118+
return DeclarationInTheSameComponentCanBeShadowed(originalDeclaration, userDeclaration);
166119
}
167120

168-
private static bool DeclarationInReferencedProjectCanBeShadowed(Declaration originalDeclaration, Declaration userDeclaration, OptionPrivateModuleListener listener)
121+
private static bool DeclarationInReferencedProjectCanBeShadowed(Declaration originalDeclaration, Declaration userDeclaration)
169122
{
170-
if (DeclarationIsInsideOptionPrivateModule(originalDeclaration, listener))
123+
if (DeclarationIsInsideOptionPrivateModule(originalDeclaration))
171124
{
172125
return false;
173126
}
@@ -231,9 +184,9 @@ private static bool DeclarationInReferencedProjectCanBeShadowed(Declaration orig
231184
return DeclarationAccessibilityCanBeShadowed(originalDeclaration);
232185
}
233186

234-
private static bool DeclarationInAnotherComponentCanBeShadowed(Declaration originalDeclaration, Declaration userDeclaration, OptionPrivateModuleListener listener)
187+
private static bool DeclarationInAnotherComponentCanBeShadowed(Declaration originalDeclaration, Declaration userDeclaration)
235188
{
236-
if (DeclarationIsInsideOptionPrivateModule(originalDeclaration, listener))
189+
if (DeclarationIsInsideOptionPrivateModule(originalDeclaration))
237190
{
238191
return false;
239192
}
@@ -292,7 +245,7 @@ private static bool DeclarationInAnotherComponentCanBeShadowed(Declaration origi
292245
return DeclarationAccessibilityCanBeShadowed(originalDeclaration);
293246
}
294247

295-
private static bool DeclarationInTheSameComponentCanBeShadowed(Declaration originalDeclaration, Declaration userDeclaration, EnumerationRuleIndexListener listener)
248+
private static bool DeclarationInTheSameComponentCanBeShadowed(Declaration originalDeclaration, Declaration userDeclaration)
296249
{
297250
// Shadowing the component containing the declaration is not a problem, because it is possible to directly access declarations inside that component
298251
if (originalDeclaration.DeclarationType == DeclarationType.ProceduralModule || originalDeclaration.DeclarationType == DeclarationType.ClassModule ||
@@ -323,18 +276,20 @@ private static bool DeclarationInTheSameComponentCanBeShadowed(Declaration origi
323276
{
324277
return DeclarationIsLocal(userDeclaration);
325278
}
279+
280+
// Shadowing between two enumerations or enumeration members is not possible inside one component.
281+
if (((originalDeclaration.DeclarationType == DeclarationType.Enumeration
282+
&& userDeclaration.DeclarationType == DeclarationType.EnumerationMember)
283+
|| (originalDeclaration.DeclarationType == DeclarationType.EnumerationMember
284+
&& userDeclaration.DeclarationType == DeclarationType.Enumeration)))
285+
{
286+
var originalDeclarationIndex = originalDeclaration.Context.start.StartIndex;
287+
var userDeclarationIndex = userDeclaration.Context.start.StartIndex;
326288

327-
if (listener.DeclarationIndexes.ContainsKey(originalDeclaration.Context) && listener.DeclarationIndexes.ContainsKey(userDeclaration.Context))
328-
{
329-
var originalDeclarationIndex = listener.DeclarationIndexes[originalDeclaration.Context];
330-
var userDeclarationIndex = listener.DeclarationIndexes[userDeclaration.Context];
331-
332-
// The same declaration type means that both declarations are enumerations or enumeration members, and such shadowing is not possible inside one component
333-
return originalDeclaration.DeclarationType != userDeclaration.DeclarationType &&
334-
// First declaration wins
335-
originalDeclarationIndex > userDeclarationIndex &&
336-
// Enumeration member can have the same name as enclosing enumeration
337-
!Equals(originalDeclaration.ParentDeclaration, userDeclaration);
289+
// First declaration wins
290+
return originalDeclarationIndex > userDeclarationIndex
291+
// Enumeration member can have the same name as enclosing enumeration
292+
&& !userDeclaration.Equals(originalDeclaration.ParentDeclaration);
338293
}
339294

340295
// Events don't have a body, so their parameters can't be accessed
@@ -355,20 +310,20 @@ private static bool DeclarationAccessibilityCanBeShadowed(Declaration originalDe
355310
(originalDeclaration.DeclarationType == DeclarationType.EnumerationMember && originalDeclaration.ParentDeclaration.Accessibility == Accessibility.Public);
356311
}
357312

358-
private static bool DeclarationIsInsideOptionPrivateModule(Declaration declaration, OptionPrivateModuleListener listener)
313+
private static bool DeclarationIsInsideOptionPrivateModule(Declaration declaration)
359314
{
360315
if (declaration.QualifiedName.QualifiedModuleName.ComponentType != ComponentType.StandardModule)
361316
{
362317
return false;
363318
}
364319

365-
var moduleDeclaration = declaration as ProceduralModuleDeclaration;
320+
var moduleDeclaration = Declaration.GetModuleParent(declaration) as ProceduralModuleDeclaration;
366321
if (moduleDeclaration != null)
367322
{
368323
return moduleDeclaration.IsPrivateModule;
369324
}
370325

371-
return listener.OptionPrivateModules.Any(moduleContext => ParserRuleContextHelper.HasParent(declaration.Context, moduleContext));
326+
return false;
372327
}
373328

374329
private static bool DeclarationIsProjectOrComponent(Declaration declaration)

0 commit comments

Comments
 (0)