Merge pull request #3886 from BZngr/ParsingCrash_3883

retailcoder · web-flow · commit 5e918940c857 · 2018-04-05T15:55:55.000-04:00
Fixes StackOverflowException. TODO: Extract expression-evaluation logic into Rubberduck.Parsing and make it a resolver step to annotate all expressions with type info.
diff --git a/Rubberduck.Inspections/Concrete/UnreachableCaseInspection/ParseTreeValueVisitor.cs b/Rubberduck.Inspections/Concrete/UnreachableCaseInspection/ParseTreeValueVisitor.cs
@@ -241,53 +241,50 @@ private bool TryGetLExprValue(VBAParser.LExprContext lExprContext, out string ex
         {
             expressionValue = string.Empty;
             declaredTypeName = string.Empty;
-            var isMemberAccess = lExprContext.TryGetChildContext(out VBAParser.MemberAccessExprContext memberAccess);
-            var isSimpleName = lExprContext.TryGetChildContext(out VBAParser.SimpleNameExprContext smplName);
 
-            if (!(isMemberAccess || isSimpleName))
+            if (lExprContext.TryGetChildContext(out VBAParser.MemberAccessExprContext memberAccess))
             {
-                return false;
+                var member = memberAccess.GetChild<VBAParser.UnrestrictedIdentifierContext>();
+                GetContextValue(member, out declaredTypeName, out expressionValue);
+                return true;
             }
 
-            if (isMemberAccess)
+            if (lExprContext.TryGetChildContext(out VBAParser.SimpleNameExprContext smplName))
             {
-                var member = memberAccess.GetChild<VBAParser.UnrestrictedIdentifierContext>();
-
-                if (TryGetIdentifierReferenceForContext(member, out IdentifierReference idRef)
-                    && idRef.Declaration.DeclarationType.HasFlag(DeclarationType.EnumerationMember)
-                    && idRef.Declaration.Context is VBAParser.EnumerationStmt_ConstantContext)
-                {
-                    var declaration = idRef.Declaration;
-                    var theCtxt = declaration.Context;
-                    expressionValue = GetConstantDeclarationValueToken(declaration);
-                    declaredTypeName = declaration.AsTypeIsBaseType ? declaration.AsTypeName : declaration.AsTypeDeclaration.AsTypeName;
-                    return true;
-                }
+                GetContextValue(smplName, out declaredTypeName, out expressionValue);
+                return true;
             }
-            else if (isSimpleName)
+
+            return false;
+        }
+
+        private void GetContextValue(ParserRuleContext context, out string declaredTypeName, out string expressionValue)
+        {
+            expressionValue = context.GetText();
+            declaredTypeName = string.Empty;
+
+            if (TryGetIdentifierReferenceForContext(context, out IdentifierReference rangeClauseIdentifierReference))
             {
-                if (TryGetIdentifierReferenceForContext(smplName, out IdentifierReference rangeClauseIdentifierReference))
+                var declaration = rangeClauseIdentifierReference.Declaration;
+                expressionValue = rangeClauseIdentifierReference.IdentifierName;
+                declaredTypeName = GetBaseTypeForDeclaration(declaration);
+
+                if (declaration.DeclarationType.HasFlag(DeclarationType.Constant)
+                    || declaration.DeclarationType.HasFlag(DeclarationType.EnumerationMember))
                 {
-                    var declaration = rangeClauseIdentifierReference.Declaration;
-                    if (declaration.DeclarationType.HasFlag(DeclarationType.Constant)
-                        || declaration.DeclarationType.HasFlag(DeclarationType.EnumerationMember))
-                    {
-                        expressionValue = GetConstantDeclarationValueToken(declaration);
-                        declaredTypeName = declaration.AsTypeName;
-                        return true;
-                    }
+                    expressionValue = GetConstantDeclarationValueToken(declaration);
                 }
             }
-            return false;
         }
 
         private bool TryGetIdentifierReferenceForContext(ParserRuleContext context, out IdentifierReference idRef)
         {
             idRef = null;
+            var nameToMatch = context.GetText();
             var identifierReferences = (_state.DeclarationFinder.MatchName(context.GetText()).Select(dec => dec.References)).SelectMany(rf => rf);
-            if (identifierReferences.Any())
+            if (identifierReferences.Any(rf => rf.Context == context))
             {
-                idRef = identifierReferences.First(rf => rf.Context == context);
+                idRef = identifierReferences.First();
                 return true;
             }
             return false;
@@ -319,13 +316,15 @@ private string GetConstantDeclarationValueToken(Declaration constantDeclaration)
             return string.Empty;
         }
 
-        private static string GetBaseTypeForDeclaration(Declaration declaration)
+        private string GetBaseTypeForDeclaration(Declaration declaration)
         {
-            if (!declaration.AsTypeIsBaseType)
+            var localDeclaration = declaration;
+            var iterationGuard = 0;
+            while (!localDeclaration.AsTypeIsBaseType && iterationGuard++ < 5)
             {
-                return GetBaseTypeForDeclaration(declaration.AsTypeDeclaration);
+                localDeclaration = localDeclaration.AsTypeDeclaration;
             }
-            return declaration.AsTypeName;
+            return localDeclaration.AsTypeName;
         }
 
         private static bool IsBinaryMathContext<T>(T context)
diff --git a/Rubberduck.Inspections/Concrete/UnreachableCaseInspection/SelectCaseStmtContextWrapper.cs b/Rubberduck.Inspections/Concrete/UnreachableCaseInspection/SelectCaseStmtContextWrapper.cs
@@ -124,6 +124,12 @@ private string DeriveTypeFromCaseClauses(IParseTreeVisitorResults inspValues, VB
                 }
             }
 
+            if (TryDetermineEvaluationTypeFromTypes(caseClauseTypeNames, out _evalTypeName))
+            {
+                return _evalTypeName;
+            }
+
+            caseClauseTypeNames.Remove(Tokens.Variant);
             if (TryDetermineEvaluationTypeFromTypes(caseClauseTypeNames, out _evalTypeName))
             {
                 return _evalTypeName;
@@ -141,7 +147,7 @@ private static bool TryDetermineEvaluationTypeFromTypes(IEnumerable<string> type
             {
                 return false;
             }
-            typeList.All(tn => new string[] { typeList.First() }.Contains(tn));
+
             //If all match, the typeName is easy...This is the only way to return "String" or "Currency".
             if (typeList.All(tn => new string[] { typeList.First() }.Contains(tn)))
             {
diff --git a/RubberduckTests/Inspections/UnreachableCaseInspectionTests.cs b/RubberduckTests/Inspections/UnreachableCaseInspectionTests.cs
@@ -4,6 +4,7 @@
 using Rubberduck.Parsing;
 using Rubberduck.Parsing.Grammar;
 using Rubberduck.Parsing.Inspections.Resources;
+using Rubberduck.VBEditor.SafeComWrappers;
 using RubberduckTests.Mocks;
 using System;
 using System.Collections.Generic;
@@ -362,12 +363,6 @@ public void UciUnit_LogicBinaryConstants(string operands, string expected)
             Assert.IsTrue(result.ParsesToConstantValue);
         }
 
-        //        Dim A, B, C, D, MyCheck
-        //A = 10: B = 8: C = 6: D = Null    ' Initialize variables.
-        //MyCheck = A > B Eqv B > C    ' Returns True.
-        //MyCheck = B > A Eqv B > C    ' Returns False.
-        //MyCheck = A > B Eqv B > D    ' Returns Null.
-        //MyCheck = A Eqv B    ' Returns -3 (bitwise comparison).
         [TestCase("True_Eqv_True", "True")]
         [TestCase("False_Eqv_True", "False")]
         [TestCase("True_Eqv_False", "False")]
@@ -698,7 +693,7 @@ public void UciUnit_Extents(string typeName)
          * stored as variable RelationalOp expressions.
         */
         [TestCase("Is < True", "Single=False")] //Always False
-        [TestCase("Is <= True", "RelOp=Is <= True")] //Result depends on Select Case value
+        [TestCase("Is <= True", "RelOp=Is <= True")]
         [TestCase("Is > True", "RelOp=Is > True")]
         [TestCase("Is >= True", "Single=True")] //Always True
         [TestCase("Is = True", "RelOp=Is = True")]
@@ -2247,7 +2242,205 @@ End Select
             CheckActualResultsEqualsExpected(inputCode, unreachable: 1);
         }
 
+        //Issue# 3885
+        //this test only proves that the Select Statement is not inspected
+        [Test]
+        [Category("Inspections")]
+        public void UciFunctional_BuiltInMember()
+        {
+            string inputCode =
+@"
+Option Explicit
+
+Sub FooCount(x As Long)
+
+    Select Case err.Number
+        Case ""5903""
+            'OK
+        Case 5900 + 3
+            'Unreachable - but undetected by unit tests, 
+        Case 5
+            'Unreachable - but undetected by unit tests, 
+        Case 4 + 1
+            'Unreachable - but undetected by unit tests, 
+    End Select
+
+    Select Case x
+        Case ""5""
+            MsgBox ""Foo""
+        Case 2 + 3
+            'Unreachable - just to make sure the test finds something 
+            MsgBox ""Bar""
+    End Select
+End Sub
+";
+
+            CheckActualResultsEqualsExpected(inputCode, unreachable: 1);
+        }
+
+        [Test]
+        [Category("Inspections")]
+        public void UciFunctional_BuiltInMemberInCaseClause()
+        {
+            string inputCode =
+@"
+Option Explicit
+
+Sub FooCount(x As Long)
+
+    Select Case x
+        Case 5900 + 3
+            'OK
+        Case err.Number
+            'OK - not evaluated
+        Case 5903
+            'Unreachable
+        Case 5900 + 2 + 1
+            'Unreachable
+    End Select
+End Sub
+";
+
+            CheckActualResultsEqualsExpected(inputCode, unreachable: 2);
+        }
+
+        //Issue# 3885 - replicates with UDT rather than a built-in
+        [TestCase("Long")]
+        [TestCase("Variant")]
+        [Category("Inspections")]
+        public void UciFunctional_MemberAccessor(string propertyType)
+        {
+            string inputCode =
+@"
+Option Explicit
+
+Sub AddVariable(testClass As Class1)
+    Select Case testClass.AValue
+        Case 5903
+            'OK
+        Case 5900 + 3
+            'unreachable
+        Case Else
+            Exit Sub
+    End Select
+End Sub";
+
+            string inputClassCode =
+@"
+Option Explicit
+
+Private myVal As <propertyType>
+
+Public Property Set AValue(val As <propertyType>)
+    myVal = val
+End Property
+
+Public Property Get AValue() As <propertyType>
+    AValue = myVal
+End Property
+";
+            inputClassCode = inputClassCode.Replace("<propertyType>", propertyType);
+            var components = new List<Tuple<string, string>>()
+            {
+                new Tuple<string, string>("TestModule1",inputCode),
+                new Tuple<string, string>("Class1", inputClassCode)
+            };
+
+            CheckActualResultsEqualsExpected(components, unreachable: 1);
+        }
+
+        [TestCase("Long")]
+        [TestCase("Variant")]
+        [Category("Inspections")]
+        public void UciFunctional_MemberAccessorInCaseClause(string propertyType)
+        {
+            string inputCode =
+@"
+Option Explicit
+
+Sub AddVariable(x As Long)
+    Select Case x
+        Case 300
+            'OK
+        Case testClass.AValue
+            'OK - variable, not value
+        Case 150 + 150
+            'OK
+        Case 3 * 100
+            'OK
+    End Select
+End Sub";
+
+            string inputClassCode =
+@"
+Option Explicit
+
+Private myVal As <propertyType>
+
+Public Property Set AValue(val As <propertyType>)
+    myVal = val
+End Property
+
+Public Property Get AValue() As <propertyType>
+    AValue = myVal
+End Property
+";
+            inputClassCode = inputClassCode.Replace("<propertyType>", propertyType);
+            var components = new List<Tuple<string, string>>()
+            {
+                new Tuple<string, string>("TestModule1",inputCode),
+                new Tuple<string, string>("Class1", inputClassCode)
+            };
+
+            CheckActualResultsEqualsExpected(components, unreachable: 2);
+        }
+
+        [TestCase("Long = 300")]
+        [Category("Inspections")]
+        public void UciFunctional_ConstanInOtherModule(string propertyType)
+        {
+            string inputCode =
+@"
+Option Explicit
+
+Sub AddVariable(x As Variant)
+    Select Case x
+        Case TestModule2.My_CONSTANT
+            'OK
+        Case 300
+            'unreachable
+        Case Else
+            Exit Sub
+    End Select
+End Sub";
+
+            string inputModule2Code =
+@"
+Option Explicit
+
+Public Const MY_CONSTANT As <propertyTypeAndAssignment> 
+";
+            inputModule2Code = inputModule2Code.Replace("<propertyTypeAndAssignment>", propertyType);
+            var components = new List<Tuple<string, string>>()
+            {
+                new Tuple<string, string>("TestModule1",inputCode),
+                new Tuple<string, string>("TestModule2", inputModule2Code)
+            };
+
+            CheckActualResultsEqualsExpected(components, unreachable: 1);
+        }
+
         private static void CheckActualResultsEqualsExpected(string inputCode, int unreachable = 0, int mismatch = 0, int caseElse = 0)
+        {
+            var components = new List<Tuple<string, string>>()
+            {
+                new Tuple<string, string>("TestModule1", inputCode)
+            };
+
+            CheckActualResultsEqualsExpected(components, unreachable, mismatch, caseElse);
+        }
+
+        private static void CheckActualResultsEqualsExpected(List<Tuple<string, string>> inputCode, int unreachable = 0, int mismatch = 0, int caseElse = 0)
         {
             var expected = new Dictionary<string, int>
             {
@@ -2256,7 +2449,12 @@ private static void CheckActualResultsEqualsExpected(string inputCode, int unrea
                 { InspectionsUI.UnreachableCaseInspection_CaseElse, caseElse },
             };
 
-            var vbe = MockVbeBuilder.BuildFromSingleStandardModule(inputCode, out var _);
+            var builder = new MockVbeBuilder();
+            var project = builder.ProjectBuilder("VBAProject", ProjectProtection.Unprotected);
+            inputCode.ForEach(input => project.AddComponent(input.Item1, NameToComponentType(input.Item1), input.Item2));
+            builder = builder.AddProject(project.Build());
+            var vbe = builder.Build();
+
             IEnumerable<Rubberduck.Parsing.Inspections.Abstract.IInspectionResult> actualResults;
             using (var state = MockParser.CreateAndParse(vbe.Object))
             {
@@ -2274,6 +2472,15 @@ private static void CheckActualResultsEqualsExpected(string inputCode, int unrea
             Assert.AreEqual(expectedMsg, actualMsg);
         }
 
+        private static ComponentType NameToComponentType(string name)
+        {
+            if (name.StartsWith("Class"))
+            {
+                return ComponentType.ClassModule;
+            }
+            return ComponentType.StandardModule;
+        }
+
         private static string BuildResultString(int unreachableCount, int mismatchCount, int caseElseCount)
         {
             return  $"Unreachable={unreachableCount}, Mismatch={mismatchCount}, CaseElse={caseElseCount}";

Original file line number	Diff line number	Diff line change
`@@ -124,6 +124,12 @@ private string DeriveTypeFromCaseClauses(IParseTreeVisitorResults inspValues, VB`
`124`	`124`	`}`
`125`	`125`	`}`
`126`	`126`
	`127`	`+ if (TryDetermineEvaluationTypeFromTypes(caseClauseTypeNames, out _evalTypeName))`
	`128`	`+ {`
	`129`	`+ return _evalTypeName;`
	`130`	`+ }`
	`131`	`+`
	`132`	`+ caseClauseTypeNames.Remove(Tokens.Variant);`
`127`	`133`	`if (TryDetermineEvaluationTypeFromTypes(caseClauseTypeNames, out _evalTypeName))`
`128`	`134`	`{`
`129`	`135`	`return _evalTypeName;`
`@@ -141,7 +147,7 @@ private static bool TryDetermineEvaluationTypeFromTypes(IEnumerable<string> type`
`141`	`147`	`{`
`142`	`148`	`return false;`
`143`	`149`	`}`
`144`		`- typeList.All(tn => new string[] { typeList.First() }.Contains(tn));`
	`150`	`+`
`145`	`151`	`//If all match, the typeName is easy...This is the only way to return "String" or "Currency".`
`146`	`152`	`if (typeList.All(tn => new string[] { typeList.First() }.Contains(tn)))`
`147`	`153`	`{`