Add handling for NonPrinting string constants

BZngr · BZngr · commit 8cbe8f896505 · 2019-01-01T20:41:01.000-08:00
diff --git a/Rubberduck.CodeAnalysis/Inspections/Concrete/UnreachableCaseInspection/ParseTreeValue.cs b/Rubberduck.CodeAnalysis/Inspections/Concrete/UnreachableCaseInspection/ParseTreeValue.cs
@@ -1,6 +1,7 @@
 ﻿using Rubberduck.Parsing.Grammar;
 using Rubberduck.Parsing.PreProcessing;
 using System;
+using System.Collections.Generic;
 using System.Globalization;
 
 namespace Rubberduck.Inspections.Concrete.UnreachableCaseInspection
@@ -22,6 +23,31 @@ public struct ParseTreeValue : IParseTreeValue
         private StringLiteralExpression _stringConstant;
         private bool? _exceedsValueTypeRange;
 
+        private static Dictionary<string,string> ControlCharacterCompareTokens = new Dictionary<string, string>()
+        {
+            ["Chr$(8)"] = "Chr$(8)", //vbBack
+            ["Chr$(13)"] = "Chr$(13)", //vbCr
+            ["Chr$(13) + Chr$(10)"] = "Chr$(13)Chr$(10)", //vbCrLf
+            ["Chr$(10)"] = "Chr$(10)", //vbLf
+            ["Chr$(12)"] = "Chr$(12)", //vbFormFeed
+            ["Chr$(13) & Chr$(10)"] = "Chr$(13)Chr$(10)", //vbNewLine
+            ["Chr$(0)"] = "Chr$(0)", //vbNullChar
+            ["Chr$(9)"] = "Chr$(9)", //vbTab
+            ["Chr$(11)"] = "Chr$(11)", //vbVerticalTab
+            ["Chr$(13)Chr$(10)"] = "Chr$(13)Chr$(10)",
+        };
+
+        public static bool TryGetNonPrintingControlCharCompareToken(string controlCharCandidate, out string comparableToken)
+        {
+            comparableToken = controlCharCandidate;
+            if (controlCharCandidate.StartsWith(Tokens.Chr))
+            {
+                var key = controlCharCandidate.Replace("Chr(", "Chr$(");
+                return ControlCharacterCompareTokens.TryGetValue(key, out comparableToken);
+            }
+            return false;
+        }
+
         public static IParseTreeValue CreateValueType(TypeTokenPair value)
         {
             if (value.ValueType.Equals(Tokens.Date) || value.ValueType.Equals(Tokens.String))
@@ -89,6 +115,11 @@ public ParseTreeValue(TypeTokenPair valuePair)
                 _stringConstant = new StringLiteralExpression(new ConstantExpression(new StringValue(_typeTokenPair.Token)));
                 ParsesToConstantValue = true;
             }
+            else if (valuePair.ValueType.Equals(Tokens.String)
+                &&  TryGetNonPrintingControlCharCompareToken(valuePair.Token, out _))
+            {
+                ParsesToConstantValue = true;
+            }
         }
 
         public string ValueType => _typeTokenPair.ValueType;
diff --git a/Rubberduck.CodeAnalysis/Inspections/Concrete/UnreachableCaseInspection/ParseTreeValueFactory.cs b/Rubberduck.CodeAnalysis/Inspections/Concrete/UnreachableCaseInspection/ParseTreeValueFactory.cs
@@ -72,6 +72,12 @@ public IParseTreeValue CreateDeclaredType(string expression, string declaredType
                 throw new ArgumentNullException();
             }
 
+            if (ParseTreeValue.TryGetNonPrintingControlCharCompareToken(expression, out string comparableToken))
+            {
+                var charConversion = new TypeTokenPair(Tokens.String, comparableToken);
+                return ParseTreeValue.CreateValueType(charConversion);
+            }
+
             var goalTypeTokenPair = new TypeTokenPair(declaredTypeName, null);
             var typeToken = TypeTokenPair.ConformToType(declaredTypeName, expression);
             if (typeToken.HasValue)
diff --git a/Rubberduck.CodeAnalysis/Inspections/Concrete/UnreachableCaseInspection/ParseTreeValueVisitor.cs b/Rubberduck.CodeAnalysis/Inspections/Concrete/UnreachableCaseInspection/ParseTreeValueVisitor.cs
@@ -16,8 +16,6 @@ public interface IParseTreeValueVisitor : IParseTreeVisitor<IParseTreeVisitorRes
 
     public interface ITestParseTreeVisitor
     {
-        bool IsVBStringConstant(string token, out string literalValue);
-        bool IsNonPrintingControlCharacter(string token);
         void InjectValuedDeclarationEvaluator(Func<Declaration, (bool, string, string)> func);
     }
 
@@ -41,20 +39,6 @@ public EnumMember(VBAParser.EnumerationStmt_ConstantContext constContext, long i
         private List<VBAParser.EnumerationStmtContext> _enumStmtContexts;
         private List<EnumMember> _enumMembers;
 
-        private static Dictionary<string, string> _vbStringConstants = new Dictionary<string, string>()
-        {
-            [Tokens.vbBack] = ((char)8).ToString(),
-            [Tokens.vbCr] = ((char)13).ToString(),
-            [Tokens.vbCrLf] = ((char)13).ToString() + ((char)10).ToString(),
-            [Tokens.vbLf] = ((char)10).ToString(),
-            [Tokens.vbFormFeed] = ((char)12).ToString(),
-            [Tokens.vbNewLine] = Environment.NewLine,
-            [Tokens.vbNullChar] = ((char)0).ToString(),
-            [Tokens.vbNullString] = null,
-            [Tokens.vbTab] =((char)9).ToString(),
-            [Tokens.vbVerticalTab] =((char)11).ToString(),
-        };
-
         public ParseTreeValueVisitor(IParseTreeValueFactory valueFactory, List<VBAParser.EnumerationStmtContext> allEnums, Func<ParserRuleContext, (bool success, IdentifierReference idRef)> idRefRetriever)
         {
             _inspValueFactory = valueFactory;
@@ -291,6 +275,14 @@ private bool TryGetLExprValue(VBAParser.LExprContext lExprContext, out string ex
                 return true;
             }
 
+            if (lExprContext.TryGetChildContext(out VBAParser.IndexExprContext idxExpr)
+                && ParseTreeValue.TryGetNonPrintingControlCharCompareToken(idxExpr.GetText(), out string comparableToken))
+            {
+                declaredTypeName = Tokens.String;
+                expressionValue = comparableToken;
+                return true;
+            }
+
             return false;
         }
 
@@ -336,14 +328,9 @@ private void GetContextValue(ParserRuleContext context, out string declaredTypeN
                     expressionValue = ExpressionValue;
                     declaredTypeName = TypeName;
 
-                    if (IsVBStringConstant(expressionValue, out string constLiteral))
-                    {
-                        declaredTypeName = Tokens.String;
-                        expressionValue = constLiteral;
-                        return;
-                    }
-                    else if (IsNonPrintingControlCharacter(expressionValue))
+                    if (ParseTreeValue.TryGetNonPrintingControlCharCompareToken(expressionValue, out string resolvedValue))
                     {
+                        expressionValue = resolvedValue;
                         declaredTypeName = Tokens.String;
                         return;
                     }
@@ -450,12 +437,6 @@ private static bool IsBinaryOpEvaluationContext<T>(T context)
             return false;
         }
 
-        public bool IsVBStringConstant(string candidate, out string literal)
-            => _vbStringConstants.TryGetValue(candidate, out literal);
-
-        public bool IsNonPrintingControlCharacter(string controlChar)
-            => controlChar != null && _vbStringConstants.ContainsValue(controlChar);
-
         public void InjectValuedDeclarationEvaluator( Func<Declaration, (bool, string, string)> func)
             => ValuedDeclarationEvaluator = func;
 
diff --git a/RubberduckTests/Inspections/UnreachableCase/UnreachableCaseInspectionTests.cs b/RubberduckTests/Inspections/UnreachableCase/UnreachableCaseInspectionTests.cs
@@ -2441,44 +2441,6 @@ End Sub
             Assert.AreEqual(expectedMsg, actualMsg);
         }
 
-        [TestCase("vbBack", "\b")]
-        [TestCase("vbCrLf", "\r\n")]
-        [TestCase("vbCr", "\r")]
-        [TestCase("vbLf", "\n")]
-        [TestCase("vbFormFeed", "\f")]
-        [TestCase("vbNewLine", "\r\n")]
-        [TestCase("vbNullChar", "\0")]
-        [TestCase("vbNullString", null)]
-        [TestCase("vbTab", "\t")]
-        [TestCase("vbVerticalTab", "\v")]
-        [Category("Inspections")]
-        public void UnreachableCaseInspection_VbStringConstantToLiteral_AreEqual(string constToken, string expected)
-        {
-            var parseTreeValueVisitor = new ParseTreeValueVisitor(null, new List<VBAParser.EnumerationStmtContext>(), null) as ITestParseTreeVisitor;
-            if (parseTreeValueVisitor.IsVBStringConstant(constToken, out string literalValue))
-            {
-                Assert.AreEqual(expected, literalValue);
-                return;
-            }
-            Assert.Fail($"TryConvertVBStringConstantToLiteral failed to convert {constToken}");
-        }
-
-        [TestCase("\r", true)]
-        [TestCase("\r\n", true)]
-        [TestCase("\b", true)]
-        [TestCase("\f", true)]
-        [TestCase("\0", true)]
-        [TestCase("\t", true)]
-        [TestCase("\v", true)]
-        [TestCase(null, false)]
-        [Category("Inspections")]
-        public void UnreachableCaseInspection_ControlCharToLiteral_AreEqual(string constToken, bool expected)
-        {
-            var parseTreeValueVisitor = new ParseTreeValueVisitor(null, new List<VBAParser.EnumerationStmtContext>(), null) as ITestParseTreeVisitor;
-            Assert.AreEqual(expected, parseTreeValueVisitor.IsNonPrintingControlCharacter(constToken));
-        }
-
-        //https://github.com/rubberduck-vba/Rubberduck/issues/4659        
         [Test]
         [Category("Inspections")]
         public void UnreachableCaseInspection_VbObjectErrorConstant()
@@ -2504,14 +2466,6 @@ Case vbObjectError + 1 'unreachable
     End Select
 End Sub
 ";
-            (bool IsType, string ExpressionValue, string TypeName) TestGetValuedDeclaration(Declaration declaration)
-            {
-                if (declaration.IdentifierName.Equals("vbObjectError"))
-                {
-                    return (true, "-2147221504", Tokens.Long);
-                }
-                return (false, null, null);
-            }
 
             var vbe = CreateStandardModuleProject(inputCode);
 
@@ -2533,26 +2487,81 @@ End Sub
         }
 
         //https://github.com/rubberduck-vba/Rubberduck/issues/4680
-        [Test]
-        [Ignore("Issue 4680")]
-        [Category("Inspections")]
-        public void UnreachableCaseInspection_VbStringConstant()
-        {
+        [TestCase("vbNewLine", "vbCr + vbLf")]
+        [TestCase("vbNewLine", "Chr(13) + Chr(10)")]
+        [TestCase("vbNewLine", "Chr$(13) + Chr$(10)")]
+        [TestCase("Chr(13) + Chr(10)", "Chr$(13) + Chr$(10)")]
+        [TestCase("vbCr + vbLf", "vbNewLine")]
+        [TestCase("vbCr + Chr(10)", "vbNewLine")]
+        [TestCase("Chr(13) + vbLf", "vbNewLine")]
+        [TestCase("Chr(0)", "vbNullChar")]
+        [TestCase("Chr$(0)", "vbNullChar")]
+        [TestCase("Chr(8)", "vbBack")]
+        [TestCase("Chr$(8)", "vbBack")]
+        [TestCase("Chr(12)", "vbFormFeed")]
+        [TestCase("Chr$(12)", "vbFormFeed")]
+        [TestCase("Chr(9)", "vbTab")]
+        [TestCase("Chr$(9)", "vbTab")]
+        [TestCase("Chr(11)", "vbVerticalTab")]
+        [TestCase("Chr$(11)", "vbVerticalTab")]
+        [Category("Inspections")]
+        public void UnreachableCaseInspection_NonPrintingControlConstants(string testCase, string equivalent)
+        {
+            var expectedUnreachableCount = 1;
             string inputCode =
-@"
+$@"
 Sub Foo(value As String)
     Select Case value
-        Case ""Hello"" + vbNewLine + ""World""
-            MsgBox ""vbNewLine version""
-        Case ""Hello"" + vbCr + vbLf + ""World"" 'unreachable
-            MsgBox ""vbCr + vbLf version""
+        Case ""Hello"" + {testCase} + ""World""
+            MsgBox ""testCase version""
+        Case ""Hello"" + {equivalent} + ""World"" 'unreachable
+            MsgBox ""equivalent version""
         Case ""Reachable""
             MsgBox ""Reachable""
     End Select
 End Sub
 ";
-            (string expectedMsg, string actualMsg) = CheckActualResultsEqualsExpected(inputCode, unreachable: 1);
-            Assert.AreEqual(expectedMsg, actualMsg);
+            var vbe = CreateStandardModuleProject(inputCode);
+
+            IEnumerable<Rubberduck.Parsing.Inspections.Abstract.IInspectionResult> actualResults;
+            using (var state = MockParser.CreateAndParse(vbe.Object))
+            {
+                var inspection = new UnreachableCaseInspection(state);
+                var parseTreeValueVisitor = inspection.ParseTreeValueVisitor as ITestParseTreeVisitor;
+
+                parseTreeValueVisitor.InjectValuedDeclarationEvaluator(TestGetValuedDeclaration);
+
+                var inspector = InspectionsHelper.GetInspector(inspection);
+                actualResults = inspector.FindIssuesAsync(state, CancellationToken.None).Result;
+            }
+
+            var actualUnreachable = actualResults.Where(ar => ar.Description.Equals(InspectionResults.UnreachableCaseInspection_Unreachable));
+
+            Assert.AreEqual(expectedUnreachableCount, actualUnreachable.Count());
+        }
+
+        private static Dictionary<string, (string, string)> _vbConstConversions = new Dictionary<string, (string, string)>()
+        {
+            ["vbNewLine"] = ("Chr$(13) & Chr$(10)", Tokens.String),
+            ["vbCr"] = ("Chr$(13)", Tokens.String),
+            ["vbLf"] = ("Chr$(10)", Tokens.String),
+            ["vbNullChar"] = ("Chr$(0)", Tokens.String),
+            ["vbBack"] = ("Chr$(8)", Tokens.String),
+            ["vbTab"] = ("Chr$(9)", Tokens.String),
+            ["vbVerticalTab"] = ("Chr$(11)", Tokens.String),
+            ["vbFormFeed"] = ("Chr$(12)", Tokens.String),
+            ["vbObjectError"] = ("-2147221504", Tokens.Long),
+        };
+
+        private static (bool IsType, string ExpressionValue, string TypeName) TestGetValuedDeclaration(Declaration declaration)
+        {
+            if (!_vbConstConversions.ContainsKey(declaration.IdentifierName))
+            {
+                return (false, null, null);
+            }
+
+            (string expressionValue, string typename) = _vbConstConversions[declaration.IdentifierName];
+            return (true, expressionValue , typename);
         }
 
         private static (string expectedMsg, string actualMsg) CheckActualResultsEqualsExpected(string inputCode, int unreachable = 0, int mismatch = 0, int caseElse = 0, int inherentlyUnreachable = 0, int overflow = 0)
