Some performance improvements for the ShadowedDeclarationsInspection

Author: MDoerner

Rubberduck.Inspections/Concrete/ShadowedDeclarationInspection.cs

@@ -28,31 +28,24 @@ private enum DeclarationSite
             SameComponent = 3
         }
 
-        private class OptionPrivateModuleListener : VBAParserBaseListener
-        {
-            public List<VBAParser.ModuleContext> OptionPrivateModules { get; } = new List<VBAParser.ModuleContext>();
-
-            public override void EnterModule(VBAParser.ModuleContext context)
-            {
-                if (context.FindChildren<VBAParser.OptionPrivateModuleStmtContext>().Any())
-                {
-                    OptionPrivateModules.Add(context);
-                }
-            }
-        }
-
         private class EnumerationRuleIndexListener : VBAParserBaseListener
         {
             public Dictionary<ParserRuleContext, int> DeclarationIndexes { get; } = new Dictionary<ParserRuleContext, int>();
 
             public override void EnterEnumerationStmt(VBAParser.EnumerationStmtContext context)
             {
-                DeclarationIndexes.Add(context, context.Start.TokenIndex);
+                if (!DeclarationIndexes.ContainsKey(context))
+                {
+                    DeclarationIndexes.Add(context, context.Start.TokenIndex);
+                }
             }
 
             public override void EnterEnumerationStmt_Constant(VBAParser.EnumerationStmt_ConstantContext context)
             {
-                DeclarationIndexes.Add(context, context.Start.TokenIndex);
+                if (!DeclarationIndexes.ContainsKey(context))
+                {
+                    DeclarationIndexes.Add(context, context.Start.TokenIndex);
+                }
             }
         }
 
@@ -66,50 +59,37 @@ public ShadowedDeclarationInspection(RubberduckParserState state) : base(state,
 
         public override IEnumerable<IInspectionResult> GetInspectionResults()
         {
-            var optionPrivateModuleListener = new OptionPrivateModuleListener();
-            var enumerationRuleIndexListener = new EnumerationRuleIndexListener();
-
-            foreach (var module in State.DeclarationFinder.AllModules.Where(m => m.ComponentType == ComponentType.StandardModule))
-            {
-                ParseTreeWalker.Default.Walk(optionPrivateModuleListener, State.GetParseTree(module));
-            }
-
-            foreach (var module in State.AllUserDeclarations.Where(d => d.DeclarationType == DeclarationType.ProceduralModule || d.DeclarationType == DeclarationType.ClassModule))
-            {
-                ParseTreeWalker.Default.Walk(enumerationRuleIndexListener, State.GetParseTree(module.QualifiedName.QualifiedModuleName));
-            }
-
             var builtInEventHandlers = State.DeclarationFinder.FindEventHandlers().ToHashSet();
 
             var issues = new List<IInspectionResult>();
 
-            var allUserProjects = UserDeclarations.OfType(DeclarationType.Project).Cast<ProjectDeclaration>();
+            var allUserProjects = State.DeclarationFinder.UserDeclarations(DeclarationType.Project).Cast<ProjectDeclaration>();
 
             foreach (var userProject in allUserProjects)
             {
                 var referencedProjectIds = userProject.ProjectReferences.Select(reference => reference.ReferencedProjectId).ToHashSet();
 
-                var userDeclarations = UserDeclarations.Where(d =>
-                    d.ProjectId == userProject.ProjectId &&
+                var userDeclarations = UserDeclarations.Where(declaration =>
+                    declaration.ProjectId == userProject.ProjectId &&
                     // User has no control over build-in event handlers or their parameters, so we skip them
-                    !DeclarationIsPartOfBuiltInEventHandler(d, builtInEventHandlers));
+                    !DeclarationIsPartOfBuiltInEventHandler(declaration, builtInEventHandlers));
 
-                foreach (var declaration in userDeclarations)
+                foreach (var userDeclaration in userDeclarations)
                 {
-                    var shadowedDeclaration = State.AllDeclarations.FirstOrDefault(d =>
-                        !Equals(d, declaration) &&
-                        string.Equals(d.IdentifierName, declaration.IdentifierName, StringComparison.OrdinalIgnoreCase) &&
-                        DeclarationCanBeShadowed(d, declaration, GetDeclarationSite(d, declaration, referencedProjectIds), optionPrivateModuleListener, enumerationRuleIndexListener));
+                    var shadowedDeclaration = State.DeclarationFinder
+                        .MatchName(userDeclaration.IdentifierName).FirstOrDefault(declaration => 
+                            !declaration.Equals(userDeclaration) &&
+                            DeclarationCanBeShadowed(declaration, userDeclaration, GetDeclarationSite(declaration, userDeclaration, referencedProjectIds)));
 
                     if (shadowedDeclaration != null)
                     {
                         issues.Add(new DeclarationInspectionResult(this,
                             string.Format(InspectionsUI.ShadowedDeclarationInspectionResultFormat,
-                                RubberduckUI.ResourceManager.GetString("DeclarationType_" + declaration.DeclarationType, CultureInfo.CurrentUICulture),
-                                declaration.IdentifierName,
+                                RubberduckUI.ResourceManager.GetString("DeclarationType_" + userDeclaration.DeclarationType, CultureInfo.CurrentUICulture),
+                                userDeclaration.IdentifierName,
                                 RubberduckUI.ResourceManager.GetString("DeclarationType_" + shadowedDeclaration.DeclarationType, CultureInfo.CurrentUICulture),
                                 shadowedDeclaration.IdentifierName),
-                            declaration));
+                            userDeclaration));
                     }
                 }
             }
@@ -124,7 +104,7 @@ private static DeclarationSite GetDeclarationSite(Declaration originalDeclaratio
                 return referencedProjectIds.Contains(originalDeclaration.ProjectId) ? DeclarationSite.ReferencedProject : DeclarationSite.NotApplicable;
             }
 
-            if (originalDeclaration.QualifiedName.QualifiedModuleName.Name != userDeclaration.QualifiedName.QualifiedModuleName.Name)
+            if (originalDeclaration.QualifiedName.QualifiedModuleName.ComponentName != userDeclaration.QualifiedName.QualifiedModuleName.ComponentName)
             {
                 return DeclarationSite.OtherComponent;
             }
@@ -144,8 +124,7 @@ private static bool DeclarationIsPartOfBuiltInEventHandler(Declaration declarati
             return parameterDeclaration != null && builtInEventHandlers.Contains(parameterDeclaration.ParentDeclaration);
         }
 
-        private static bool DeclarationCanBeShadowed(Declaration originalDeclaration, Declaration userDeclaration, DeclarationSite originalDeclarationSite,
-            OptionPrivateModuleListener optionPrivateModuleListener, EnumerationRuleIndexListener enumerationRuleIndexListener)
+        private bool DeclarationCanBeShadowed(Declaration originalDeclaration, Declaration userDeclaration, DeclarationSite originalDeclarationSite)
         {
             if (originalDeclarationSite == DeclarationSite.NotApplicable)
             {
@@ -154,20 +133,20 @@ private static bool DeclarationCanBeShadowed(Declaration originalDeclaration, De
 
             if (originalDeclarationSite == DeclarationSite.ReferencedProject)
             {
-                return DeclarationInReferencedProjectCanBeShadowed(originalDeclaration, userDeclaration, optionPrivateModuleListener);
+                return DeclarationInReferencedProjectCanBeShadowed(originalDeclaration, userDeclaration);
             }
 
             if (originalDeclarationSite == DeclarationSite.OtherComponent)
             {
-                return DeclarationInAnotherComponentCanBeShadowed(originalDeclaration, userDeclaration, optionPrivateModuleListener);
+                return DeclarationInAnotherComponentCanBeShadowed(originalDeclaration, userDeclaration);
             }
 
-            return DeclarationInTheSameComponentCanBeShadowed(originalDeclaration, userDeclaration, enumerationRuleIndexListener);
+            return DeclarationInTheSameComponentCanBeShadowed(originalDeclaration, userDeclaration);
         }
 
-        private static bool DeclarationInReferencedProjectCanBeShadowed(Declaration originalDeclaration, Declaration userDeclaration, OptionPrivateModuleListener listener)
+        private bool DeclarationInReferencedProjectCanBeShadowed(Declaration originalDeclaration, Declaration userDeclaration)
         {
-            if (DeclarationIsInsideOptionPrivateModule(originalDeclaration, listener))
+            if (DeclarationIsInsideOptionPrivateModule(originalDeclaration))
             {
                 return false;
             }
@@ -231,9 +210,9 @@ private static bool DeclarationInReferencedProjectCanBeShadowed(Declaration orig
             return DeclarationAccessibilityCanBeShadowed(originalDeclaration);
         }
 
-        private static bool DeclarationInAnotherComponentCanBeShadowed(Declaration originalDeclaration, Declaration userDeclaration, OptionPrivateModuleListener listener)
+        private static bool DeclarationInAnotherComponentCanBeShadowed(Declaration originalDeclaration, Declaration userDeclaration)
         {
-            if (DeclarationIsInsideOptionPrivateModule(originalDeclaration, listener))
+            if (DeclarationIsInsideOptionPrivateModule(originalDeclaration))
             {
                 return false;
             }
@@ -292,7 +271,7 @@ private static bool DeclarationInAnotherComponentCanBeShadowed(Declaration origi
             return DeclarationAccessibilityCanBeShadowed(originalDeclaration);
         }
 
-        private static bool DeclarationInTheSameComponentCanBeShadowed(Declaration originalDeclaration, Declaration userDeclaration, EnumerationRuleIndexListener listener)
+        private bool DeclarationInTheSameComponentCanBeShadowed(Declaration originalDeclaration, Declaration userDeclaration)
         {
             // Shadowing the component containing the declaration is not a problem, because it is possible to directly access declarations inside that component
             if (originalDeclaration.DeclarationType == DeclarationType.ProceduralModule || originalDeclaration.DeclarationType == DeclarationType.ClassModule ||
@@ -323,18 +302,31 @@ private static bool DeclarationInTheSameComponentCanBeShadowed(Declaration origi
             {
                 return DeclarationIsLocal(userDeclaration);
             }
-
-            if (listener.DeclarationIndexes.ContainsKey(originalDeclaration.Context) && listener.DeclarationIndexes.ContainsKey(userDeclaration.Context))
+            
+            // Shadowing between two enumerations or enumeration members is not possible inside one component.
+            if (((originalDeclaration.DeclarationType == DeclarationType.Enumeration 
+                    && userDeclaration.DeclarationType == DeclarationType.EnumerationMember)
+                || (originalDeclaration.DeclarationType == DeclarationType.EnumerationMember
+                    && userDeclaration.DeclarationType == DeclarationType.Enumeration)))
             {
-                var originalDeclarationIndex = listener.DeclarationIndexes[originalDeclaration.Context];
-                var userDeclarationIndex = listener.DeclarationIndexes[userDeclaration.Context];
+                var listener = new EnumerationRuleIndexListener();
+                if (!listener.DeclarationIndexes.ContainsKey(originalDeclaration.Context))
+                {
+                    ParseTreeWalker.Default.Walk(listener,
+                        State.GetParseTree(originalDeclaration.QualifiedName.QualifiedModuleName));
+                }
 
-                // The same declaration type means that both declarations are enumerations or enumeration members, and such shadowing is not possible inside one component
-                return originalDeclaration.DeclarationType != userDeclaration.DeclarationType &&
-                       // First declaration wins
-                       originalDeclarationIndex > userDeclarationIndex &&
-                       // Enumeration member can have the same name as enclosing enumeration
-                       !Equals(originalDeclaration.ParentDeclaration, userDeclaration);
+                if (listener.DeclarationIndexes.ContainsKey(originalDeclaration.Context) &&
+                    listener.DeclarationIndexes.ContainsKey(userDeclaration.Context))
+                {
+                    var originalDeclarationIndex = listener.DeclarationIndexes[originalDeclaration.Context];
+                    var userDeclarationIndex = listener.DeclarationIndexes[userDeclaration.Context];
+
+                    // First declaration wins
+                    return originalDeclarationIndex > userDeclarationIndex 
+                           // Enumeration member can have the same name as enclosing enumeration
+                           && !userDeclaration.Equals(originalDeclaration.ParentDeclaration);
+                }
             }
 
             // Events don't have a body, so their parameters can't be accessed
@@ -355,20 +347,20 @@ private static bool DeclarationAccessibilityCanBeShadowed(Declaration originalDe
                    (originalDeclaration.DeclarationType == DeclarationType.EnumerationMember && originalDeclaration.ParentDeclaration.Accessibility == Accessibility.Public);
         }
 
-        private static bool DeclarationIsInsideOptionPrivateModule(Declaration declaration, OptionPrivateModuleListener listener)
+        private static bool DeclarationIsInsideOptionPrivateModule(Declaration declaration)
         {
             if (declaration.QualifiedName.QualifiedModuleName.ComponentType != ComponentType.StandardModule)
             {
                 return false;
             }
 
-            var moduleDeclaration = declaration as ProceduralModuleDeclaration;
+            var moduleDeclaration = Declaration.GetModuleParent(declaration) as ProceduralModuleDeclaration;
             if (moduleDeclaration != null)
             {
                 return moduleDeclaration.IsPrivateModule;
             }
 
-            return listener.OptionPrivateModules.Any(moduleContext => ParserRuleContextHelper.HasParent(declaration.Context, moduleContext));
+            return false;
         }
 
         private static bool DeclarationIsProjectOrComponent(Declaration declaration)