Pass declaration finder along in UnreacableCaseInspection

MDoerner · MDoerner · commit e5efb4885118 · 2020-03-02T00:48:39.000+01:00
diff --git a/Rubberduck.CodeAnalysis/Inspections/Concrete/UnreachableCaseInspection/UnreachableCaseInspection.cs b/Rubberduck.CodeAnalysis/Inspections/Concrete/UnreachableCaseInspection/UnreachableCaseInspection.cs
@@ -144,38 +144,38 @@ public UnreachableCaseInspection(IDeclarationFinderProvider declarationFinderPro
         protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
         {
             var finder = DeclarationFinderProvider.DeclarationFinder;
-            var enumStmts = _listener.EnumerationStmtContexts();
-            var parseTreeValueVisitor = CreateParseTreeValueVisitor(enumStmts, GetIdentifierReferenceForContext);
 
             return finder.UserDeclarations(DeclarationType.Module)
                 .Where(module => module != null)
-                .SelectMany(module => DoGetInspectionResults(module.QualifiedModuleName, finder, parseTreeValueVisitor))
+                .SelectMany(module => DoGetInspectionResults(module.QualifiedModuleName, finder))
                 .ToList();
         }
 
         protected override IEnumerable<IInspectionResult> DoGetInspectionResults(QualifiedModuleName module)
         {
             var finder = DeclarationFinderProvider.DeclarationFinder;
-            var enumStmts = _listener.EnumerationStmtContexts();
-            var parseTreeValueVisitor = CreateParseTreeValueVisitor(enumStmts, GetIdentifierReferenceForContext);
-            return DoGetInspectionResults(module, finder, parseTreeValueVisitor);
+            return DoGetInspectionResults(module, finder);
         }
 
-        private IEnumerable<IInspectionResult> DoGetInspectionResults(QualifiedModuleName module, DeclarationFinder finder, IParseTreeValueVisitor parseTreeValueVisitor)
+        private IEnumerable<IInspectionResult> DoGetInspectionResults(QualifiedModuleName module, DeclarationFinder finder)
         {
             var qualifiedSelectCaseStmts = Listener.Contexts(module)
                 // ignore filtering here to make the search space smaller
                 .Where(result => !result.IsIgnoringInspectionResultFor(finder, AnnotationName));
 
+            var enumStmts = _listener.EnumerationStmtContexts();
+            var parseTreeValueVisitor = CreateParseTreeValueVisitor(enumStmts, context => GetIdentifierReferenceForContextFunction(finder)(module, context));
+
             return qualifiedSelectCaseStmts
                 .SelectMany(context => ResultsForContext(context, finder, parseTreeValueVisitor))
                 .ToList();
         }
 
         private IEnumerable<IInspectionResult> ResultsForContext(QualifiedContext<ParserRuleContext> qualifiedSelectCaseStmt, DeclarationFinder finder, IParseTreeValueVisitor parseTreeValueVisitor)
         {
-            var contextValues = qualifiedSelectCaseStmt.Context.Accept(parseTreeValueVisitor);
-            var selectCaseInspector = _unreachableCaseInspectorFactory.Create((VBAParser.SelectCaseStmtContext)qualifiedSelectCaseStmt.Context, contextValues, GetVariableTypeName);
+            var module = qualifiedSelectCaseStmt.ModuleName;
+            var contextValues = parseTreeValueVisitor.VisitChildren(qualifiedSelectCaseStmt.Context);
+            var selectCaseInspector = _unreachableCaseInspectorFactory.Create((VBAParser.SelectCaseStmtContext)qualifiedSelectCaseStmt.Context, contextValues, GetVariableTypeNameFunction(module, finder));
 
             var results = selectCaseInspector.InspectForUnreachableCases();
 
@@ -216,28 +216,33 @@ private IInspectionResult CreateInspectionResult(QualifiedContext<ParserRuleCont
                         new QualifiedContext<ParserRuleContext>(selectStmt.ModuleName, unreachableBlock));
         }
 
-        public IParseTreeValueVisitor CreateParseTreeValueVisitor(IReadOnlyList<VBAParser.EnumerationStmtContext> allEnums, Func<ParserRuleContext, (bool success, IdentifierReference idRef)> func)
-            => _parseTreeValueVisitorFactory.Create(allEnums, func);
+        public IParseTreeValueVisitor CreateParseTreeValueVisitor(
+            IReadOnlyList<QualifiedContext<VBAParser.EnumerationStmtContext>> allEnums,
+            Func<ParserRuleContext, (bool success, IdentifierReference idRef)> func)
+        {
+            var enums = allEnums.Select(item => item.Context).ToList();
+            return _parseTreeValueVisitorFactory.Create(enums, func);
+        }
 
-        //Method is used as a delegate to avoid propagating RubberduckParserState beyond this class
-        private (bool success, IdentifierReference idRef) GetIdentifierReferenceForContext(ParserRuleContext context)
+        private Func<QualifiedModuleName, ParserRuleContext,(bool success, IdentifierReference idRef)> GetIdentifierReferenceForContextFunction(DeclarationFinder finder)
         {
-            return GetIdentifierReferenceForContext(context, DeclarationFinderProvider);
+            return (module, context) => GetIdentifierReferenceForContext(module, context, finder);
         }
 
         //public static to support tests
         //FIXME There should not be additional public methods just for tests. This class seems to want to be split or at least reorganized.
-        public static (bool success, IdentifierReference idRef) GetIdentifierReferenceForContext(ParserRuleContext context, IDeclarationFinderProvider declarationFinderProvider)
+        public static (bool success, IdentifierReference idRef) GetIdentifierReferenceForContext(QualifiedModuleName module, ParserRuleContext context, DeclarationFinder finder)
         {
             if (context == null)
             {
                 return (false, null);
             }
 
-            //FIXME Get the declaration finder only once inside the inspection to avoid the possibility of inconsistent state due to a reparse while inspections run.
-            var finder = declarationFinderProvider.DeclarationFinder;
-            var identifierReferences = finder.MatchName(context.GetText())
-                .SelectMany(declaration => declaration.References)
+            var qualifiedSelection = new QualifiedSelection(module, context.GetSelection());
+
+            var identifierReferences = 
+                finder
+                .IdentifierReferences(qualifiedSelection)
                 .Where(reference => reference.Context == context)
                 .ToList();
 
@@ -246,19 +251,31 @@ public static (bool success, IdentifierReference idRef) GetIdentifierReferenceFo
                 : (false, null);
         }
 
-        //Method is used as a delegate to avoid propogating RubberduckParserState beyond this class
-        private string GetVariableTypeName(string variableName, ParserRuleContext ancestor)
+        private Func<string, ParserRuleContext, string> GetVariableTypeNameFunction(QualifiedModuleName module, DeclarationFinder finder)
         {
-            var descendents = ancestor.GetDescendents<VBAParser.SimpleNameExprContext>().Where(desc => desc.GetText().Equals(variableName)).ToList();
-            if (descendents.Any())
+            return (variableName, ancestor) => GetVariableTypeName(module, variableName, ancestor, finder);
+        }
+
+        private string GetVariableTypeName(QualifiedModuleName module, string variableName, ParserRuleContext ancestor, DeclarationFinder finder)
+        {
+            if (ancestor == null)
             {
-                (bool success, IdentifierReference idRef) = GetIdentifierReferenceForContext(descendents.First(), DeclarationFinderProvider);
-                if (success)
-                {
-                    return GetBaseTypeForDeclaration(idRef.Declaration);
-                }
+                return string.Empty;
+            }
+
+            var descendents = ancestor.GetDescendents<VBAParser.SimpleNameExprContext>()
+                .Where(desc => desc.GetText().Equals(variableName))
+                .ToList();
+            if (!descendents.Any())
+            {
+                return string.Empty;
             }
-            return string.Empty;
+
+            var firstDescendent = descendents.First();
+            var (success, reference) = GetIdentifierReferenceForContext(module, firstDescendent, finder);
+            return success ?
+                GetBaseTypeForDeclaration(reference.Declaration) 
+                : string.Empty;
         }
 
         private string GetBaseTypeForDeclaration(Declaration declaration)
@@ -277,8 +294,8 @@ private string GetBaseTypeForDeclaration(Declaration declaration)
         #region UnreachableCaseInspectionListeners
         public class UnreachableCaseInspectionListener : InspectionListenerBase
         {
-            private readonly IDictionary<QualifiedModuleName, List<VBAParser.EnumerationStmtContext>> _enumStmts = new Dictionary<QualifiedModuleName, List<VBAParser.EnumerationStmtContext>>();
-            public IReadOnlyList<VBAParser.EnumerationStmtContext> EnumerationStmtContexts() => _enumStmts.AllValues().ToList();
+            private readonly IDictionary<QualifiedModuleName, List<QualifiedContext<VBAParser.EnumerationStmtContext>>> _enumStmts = new Dictionary<QualifiedModuleName, List<QualifiedContext<VBAParser.EnumerationStmtContext>>>();
+            public IReadOnlyList<QualifiedContext<VBAParser.EnumerationStmtContext>> EnumerationStmtContexts() => _enumStmts.AllValues().ToList();
 
             public override void ClearContexts()
             {
@@ -305,13 +322,14 @@ public override void EnterEnumerationStmt([NotNull] VBAParser.EnumerationStmtCon
             private void SaveEnumStmt(VBAParser.EnumerationStmtContext context)
             {
                 var module = CurrentModuleName;
+                var qualifiedContext = new QualifiedContext<VBAParser.EnumerationStmtContext>(module, context);
                 if (_enumStmts.TryGetValue(module, out var stmts))
                 {
-                    stmts.Add(context);
+                    stmts.Add(qualifiedContext);
                 }
                 else
                 {
-                    _enumStmts.Add(module, new List<VBAParser.EnumerationStmtContext> { context });
+                    _enumStmts.Add(module, new List<QualifiedContext<VBAParser.EnumerationStmtContext>> { qualifiedContext });
                 }
             }
         }
diff --git a/Rubberduck.CodeAnalysis/Inspections/Concrete/UnreachableCaseInspection/UnreachableCaseInspector.cs b/Rubberduck.CodeAnalysis/Inspections/Concrete/UnreachableCaseInspection/UnreachableCaseInspector.cs
@@ -21,7 +21,8 @@ public class UnreachableCaseInspector : IUnreachableCaseInspector
         private readonly Func<string, ParserRuleContext, string> _getVariableDeclarationTypeName;
         private IParseTreeValue _selectExpressionValue;
 
-        public UnreachableCaseInspector(VBAParser.SelectCaseStmtContext selectCaseContext, 
+        public UnreachableCaseInspector(
+            VBAParser.SelectCaseStmtContext selectCaseContext, 
             IParseTreeVisitorResults inspValues, 
             IParseTreeValueFactory valueFactory,
             Func<string,ParserRuleContext,string> getVariableTypeName = null)
diff --git a/RubberduckTests/Inspections/UnreachableCase/UnreachableCaseInspectionTests.cs b/RubberduckTests/Inspections/UnreachableCase/UnreachableCaseInspectionTests.cs
@@ -13,7 +13,11 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading;
+using Antlr4.Runtime.Tree;
 using Moq;
+using Rubberduck.Parsing.VBA.Parsing;
+using Rubberduck.VBEditor;
+using Rubberduck.VBEditor.Extensions;
 
 namespace RubberduckTests.Inspections.UnreachableCase
 {
@@ -2461,8 +2465,8 @@ End Sub
                 var factoryProvider = SpecialValueDeclarationEvaluatorFactoryProvider(TestGetValuedDeclaration);
                 var inspection = new UnreachableCaseInspection(state, factoryProvider);
 
-                var inspector = InspectionsHelper.GetInspector(inspection);
-                actualResults = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
+                WalkTrees(inspection, state);
+                actualResults = inspection.GetInspectionResults(CancellationToken.None);
             }
 
             var actualUnreachable = actualResults.Where(ar => ar.Description.Equals(Rubberduck.Resources.Inspections.InspectionResults.UnreachableCaseInspection_Unreachable));
@@ -2513,8 +2517,8 @@ End Sub
                 var factoryProvider = SpecialValueDeclarationEvaluatorFactoryProvider(TestGetValuedDeclaration);
                 var inspection = new UnreachableCaseInspection(state, factoryProvider);
 
-                var inspector = InspectionsHelper.GetInspector(inspection);
-                actualResults = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
+                WalkTrees(inspection, state);
+                actualResults = inspection.GetInspectionResults(CancellationToken.None);
             }
 
             var actualUnreachable = actualResults.Where(ar => ar.Description.Equals(Rubberduck.Resources.Inspections.InspectionResults.UnreachableCaseInspection_Unreachable));
@@ -2631,14 +2635,14 @@ private IParseTreeVisitorResults GetParseTreeValueResults(string inputCode, out
             var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputCode, out var _);
             using (var state = MockParser.CreateAndParse(vbe.Object))
             {
-                var firstParserRuleContext = (ParserRuleContext)state.ParseTrees
-                    .First(pt => pt.Value is ParserRuleContext)
-                    .Value;
-                selectStmt = firstParserRuleContext.GetDescendent<VBAParser.SelectCaseStmtContext>();
+                var finder = state.DeclarationFinder;
+                var (parseTreeModule, moduleParseTree) = state.ParseTrees
+                    .First(pt => pt.Value is ParserRuleContext);
+                selectStmt = ((ParserRuleContext)moduleParseTree).GetDescendent<VBAParser.SelectCaseStmtContext>();
                 var visitor = ParseTreeValueVisitorFactory.Create(
                     new List<VBAParser.EnumerationStmtContext>(),
-                    context => UnreachableCaseInspection.GetIdentifierReferenceForContext(context, state));
-                valueResults = selectStmt.Accept(visitor);
+                    (context) => UnreachableCaseInspection.GetIdentifierReferenceForContext(parseTreeModule, context, finder));
+                valueResults = visitor.VisitChildren(selectStmt);
             }
             return valueResults;
         }
