Skip to content

Commit f105933

Browse files
bclothierbclothier
authored
Merge pull request #4675 from BZngr/NRE_4659
Avoid NREs in UnreachableCaseInspection
2 parents d04b41c + e913bd0 commit f105933

File tree

6 files changed

+274
-24
lines changed

6 files changed

+274
-24
lines changed

‎Rubberduck.CodeAnalysis/Inspections/Concrete/UnreachableCaseInspection/ParseTreeValue.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using Rubberduck.Parsing.Grammar;
22
using Rubberduck.Parsing.PreProcessing;
33
using System;
4+
using System.Collections.Generic;
45
using System.Globalization;
56

67
namespace Rubberduck.Inspections.Concrete.UnreachableCaseInspection
@@ -22,6 +23,31 @@ public struct ParseTreeValue : IParseTreeValue
2223
private StringLiteralExpression _stringConstant;
2324
private bool? _exceedsValueTypeRange;
2425

26+
private static Dictionary<string,string> ControlCharacterCompareTokens = new Dictionary<string, string>()
27+
{
28+
["Chr$(8)"] = "Chr$(8)", //vbBack
29+
["Chr$(13)"] = "Chr$(13)", //vbCr
30+
["Chr$(13) + Chr$(10)"] = "Chr$(13)Chr$(10)", //vbCrLf
31+
["Chr$(10)"] = "Chr$(10)", //vbLf
32+
["Chr$(12)"] = "Chr$(12)", //vbFormFeed
33+
["Chr$(13) & Chr$(10)"] = "Chr$(13)Chr$(10)", //vbNewLine
34+
["Chr$(0)"] = "Chr$(0)", //vbNullChar
35+
["Chr$(9)"] = "Chr$(9)", //vbTab
36+
["Chr$(11)"] = "Chr$(11)", //vbVerticalTab
37+
["Chr$(13)Chr$(10)"] = "Chr$(13)Chr$(10)",
38+
};
39+
40+
public static bool TryGetNonPrintingControlCharCompareToken(string controlCharCandidate, out string comparableToken)
41+
{
42+
comparableToken = controlCharCandidate;
43+
if (controlCharCandidate.StartsWith(Tokens.Chr))
44+
{
45+
var key = controlCharCandidate.Replace("Chr(", "Chr$(");
46+
return ControlCharacterCompareTokens.TryGetValue(key, out comparableToken);
47+
}
48+
return false;
49+
}
50+
2551
public static IParseTreeValue CreateValueType(TypeTokenPair value)
2652
{
2753
if (value.ValueType.Equals(Tokens.Date) || value.ValueType.Equals(Tokens.String))
@@ -89,6 +115,11 @@ public ParseTreeValue(TypeTokenPair valuePair)
89115
_stringConstant = new StringLiteralExpression(new ConstantExpression(new StringValue(_typeTokenPair.Token)));
90116
ParsesToConstantValue = true;
91117
}
118+
else if (valuePair.ValueType.Equals(Tokens.String)
119+
&& TryGetNonPrintingControlCharCompareToken(valuePair.Token, out _))
120+
{
121+
ParsesToConstantValue = true;
122+
}
92123
}
93124

94125
public string ValueType => _typeTokenPair.ValueType;

‎Rubberduck.CodeAnalysis/Inspections/Concrete/UnreachableCaseInspection/ParseTreeValueFactory.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ public IParseTreeValue CreateDeclaredType(string expression, string declaredType
7272
throw new ArgumentNullException();
7373
}
7474

75+
if (ParseTreeValue.TryGetNonPrintingControlCharCompareToken(expression, out string comparableToken))
76+
{
77+
var charConversion = new TypeTokenPair(Tokens.String, comparableToken);
78+
return ParseTreeValue.CreateValueType(charConversion);
79+
}
80+
7581
var goalTypeTokenPair = new TypeTokenPair(declaredTypeName, null);
7682
var typeToken = TypeTokenPair.ConformToType(declaredTypeName, expression);
7783
if (typeToken.HasValue)

‎Rubberduck.CodeAnalysis/Inspections/Concrete/UnreachableCaseInspection/ParseTreeValueVisitor.cs

Lines changed: 72 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,12 @@ public interface IParseTreeValueVisitor : IParseTreeVisitor<IParseTreeVisitorRes
1414
event EventHandler<ValueResultEventArgs> OnValueResultCreated;
1515
}
1616

17-
public class ParseTreeValueVisitor : IParseTreeValueVisitor
17+
public interface ITestParseTreeVisitor
18+
{
19+
void InjectValuedDeclarationEvaluator(Func<Declaration, (bool, string, string)> func);
20+
}
21+
22+
public class ParseTreeValueVisitor : IParseTreeValueVisitor, ITestParseTreeVisitor
1823
{
1924
private class EnumMember
2025
{
@@ -41,8 +46,6 @@ public ParseTreeValueVisitor(IParseTreeValueFactory valueFactory, List<VBAParser
4146
_contextValues = new ParseTreeVisitorResults();
4247
OnValueResultCreated += _contextValues.OnNewValueResult;
4348
_enumStmtContexts = allEnums;
44-
_enumMembers = new List<EnumMember>();
45-
LoadEnumMemberValues();
4649
}
4750

4851
private Func<ParserRuleContext, (bool success, IdentifierReference idRef)> IdRefRetriever { set; get; } = null;
@@ -272,9 +275,41 @@ private bool TryGetLExprValue(VBAParser.LExprContext lExprContext, out string ex
272275
return true;
273276
}
274277

278+
if (lExprContext.TryGetChildContext(out VBAParser.IndexExprContext idxExpr)
279+
&& ParseTreeValue.TryGetNonPrintingControlCharCompareToken(idxExpr.GetText(), out string comparableToken))
280+
{
281+
declaredTypeName = Tokens.String;
282+
expressionValue = comparableToken;
283+
return true;
284+
}
285+
275286
return false;
276287
}
277288

289+
private Func<Declaration, (bool, string, string)> _valueDeclarationEvaluator;
290+
private Func<Declaration, (bool, string, string)> ValuedDeclarationEvaluator
291+
{
292+
set
293+
{
294+
_valueDeclarationEvaluator = value;
295+
}
296+
get
297+
{
298+
return _valueDeclarationEvaluator ?? GetValuedDeclaration;
299+
}
300+
}
301+
302+
303+
private (bool IsType, string ExpressionValue, string TypeName) GetValuedDeclaration(Declaration declaration)
304+
{
305+
if (declaration is ValuedDeclaration valuedDeclaration)
306+
{
307+
var typeName = GetBaseTypeForDeclaration(declaration);
308+
return (true, valuedDeclaration.Expression, typeName);
309+
}
310+
return (false, null, null);
311+
}
312+
278313
private void GetContextValue(ParserRuleContext context, out string declaredTypeName, out string expressionValue)
279314
{
280315
expressionValue = context.GetText();
@@ -286,6 +321,25 @@ private void GetContextValue(ParserRuleContext context, out string declaredTypeN
286321
expressionValue = rangeClauseIdentifierReference.IdentifierName;
287322
declaredTypeName = GetBaseTypeForDeclaration(declaration);
288323

324+
(bool IsValuedDeclaration, string ExpressionValue, string TypeName) = ValuedDeclarationEvaluator(declaration);
325+
326+
if( IsValuedDeclaration)
327+
{
328+
expressionValue = ExpressionValue;
329+
declaredTypeName = TypeName;
330+
331+
if (ParseTreeValue.TryGetNonPrintingControlCharCompareToken(expressionValue, out string resolvedValue))
332+
{
333+
expressionValue = resolvedValue;
334+
declaredTypeName = Tokens.String;
335+
return;
336+
}
337+
else if (long.TryParse(expressionValue, out _))
338+
{
339+
return;
340+
}
341+
}
342+
289343
if (declaration.DeclarationType.HasFlag(DeclarationType.Constant))
290344
{
291345
expressionValue = GetConstantContextValueToken(declaration.Context);
@@ -296,12 +350,12 @@ private void GetContextValue(ParserRuleContext context, out string declaredTypeN
296350
expressionValue = GetConstantContextValueToken(declaration.Context);
297351
if (expressionValue.Equals(string.Empty))
298352
{
299-
var enumValues = _enumMembers.Where(dt => dt.ConstantContext == declaration.Context);
300-
if (enumValues.Any())
353+
if (_enumMembers is null)
301354
{
302-
var enumValue = enumValues.First();
303-
expressionValue = enumValue.Value.ToString();
355+
LoadEnumMemberValues();
304356
}
357+
var enumValue = _enumMembers.SingleOrDefault(dt => dt.ConstantContext == declaration.Context);
358+
expressionValue = enumValue?.Value.ToString() ?? string.Empty;
305359
}
306360
}
307361
}
@@ -321,6 +375,11 @@ private bool TryGetIdentifierReferenceForContext(ParserRuleContext context, out
321375

322376
private string GetConstantContextValueToken(ParserRuleContext context)
323377
{
378+
if (context is null)
379+
{
380+
return string.Empty;
381+
}
382+
324383
var declarationContextChildren = context.children.ToList();
325384
var equalsSymbolIndex = declarationContextChildren.FindIndex(ch => ch.Equals(context.GetToken(VBAParser.EQ, 0)));
326385

@@ -378,8 +437,12 @@ private static bool IsBinaryOpEvaluationContext<T>(T context)
378437
return false;
379438
}
380439

440+
public void InjectValuedDeclarationEvaluator( Func<Declaration, (bool, string, string)> func)
441+
=> ValuedDeclarationEvaluator = func;
442+
381443
private void LoadEnumMemberValues()
382444
{
445+
_enumMembers = new List<EnumMember>();
383446
foreach (var enumStmt in _enumStmtContexts)
384447
{
385448
long enumAssignedValue = -1;
@@ -390,6 +453,8 @@ private void LoadEnumMemberValues()
390453
var enumMember = new EnumMember(enumConstContext, enumAssignedValue);
391454
if (enumMember.HasAssignment)
392455
{
456+
Visit(enumMember.ConstantContext);
457+
393458
var valueText = GetConstantContextValueToken(enumMember.ConstantContext);
394459
if (!valueText.Equals(string.Empty))
395460
{

‎Rubberduck.CodeAnalysis/Inspections/Concrete/UnreachableCaseInspection/UnreachableCaseInspection.cs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,11 @@ protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
5151
var qualifiedSelectCaseStmts = Listener.Contexts
5252
.Where(result => !IsIgnoringInspectionResultFor(result.ModuleName, result.Context.Start.Line));
5353

54-
var listener = (UnreachableCaseInspectionListener)Listener;
55-
var parseTreeValueVisitor = CreateParseTreeValueVisitor(_valueFactory, listener.EnumerationStmtContexts.ToList(), GetIdentifierReferenceForContext);
56-
parseTreeValueVisitor.OnValueResultCreated += ValueResults.OnNewValueResult;
54+
ParseTreeValueVisitor.OnValueResultCreated += ValueResults.OnNewValueResult;
5755

5856
foreach (var qualifiedSelectCaseStmt in qualifiedSelectCaseStmts)
5957
{
60-
qualifiedSelectCaseStmt.Context.Accept(parseTreeValueVisitor);
58+
qualifiedSelectCaseStmt.Context.Accept(ParseTreeValueVisitor);
6159
var selectCaseInspector = _unreachableCaseInspectorFactory.Create((VBAParser.SelectCaseStmtContext)qualifiedSelectCaseStmt.Context, ValueResults, _valueFactory, GetVariableTypeName);
6260

6361
selectCaseInspector.InspectForUnreachableCases();
@@ -71,6 +69,20 @@ protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
7169
return _inspectionResults;
7270
}
7371

72+
private IParseTreeValueVisitor _parseTreeValueVisitor;
73+
public IParseTreeValueVisitor ParseTreeValueVisitor
74+
{
75+
get
76+
{
77+
if (_parseTreeValueVisitor is null)
78+
{
79+
var listener = (UnreachableCaseInspectionListener)Listener;
80+
_parseTreeValueVisitor = CreateParseTreeValueVisitor(_valueFactory, listener.EnumerationStmtContexts.ToList(), GetIdentifierReferenceForContext);
81+
}
82+
return _parseTreeValueVisitor;
83+
}
84+
}
85+
7486
private void CreateInspectionResult(QualifiedContext<ParserRuleContext> selectStmt, ParserRuleContext unreachableBlock, string message)
7587
{
7688
var result = new QualifiedContextInspectionResult(this,
@@ -80,9 +92,7 @@ private void CreateInspectionResult(QualifiedContext<ParserRuleContext> selectSt
8092
}
8193

8294
public static IParseTreeValueVisitor CreateParseTreeValueVisitor(IParseTreeValueFactory valueFactory, List<VBAParser.EnumerationStmtContext> allEnums, Func<ParserRuleContext, (bool success, IdentifierReference idRef)> func)
83-
{
84-
return new ParseTreeValueVisitor(valueFactory, allEnums, func);
85-
}
95+
=> new ParseTreeValueVisitor(valueFactory, allEnums, func);
8696

8797
//Method is used as a delegate to avoid propogating RubberduckParserState beyond this class
8898
private (bool success, IdentifierReference idRef) GetIdentifierReferenceForContext(ParserRuleContext context)

‎Rubberduck.Parsing/Grammar/Tokens.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,16 @@ public static class Tokens
190190
public static readonly string Until = "Until";
191191
public static readonly string Val = "Val";
192192
public static readonly string Variant = "Variant";
193+
public static readonly string vbBack = "vbBack";
194+
public static readonly string vbCr = "vbCr";
193195
public static readonly string vbCrLf = "vbCrLf";
196+
public static readonly string vbFormFeed = "vbFormFeed";
197+
public static readonly string vbLf = "vbLf";
194198
public static readonly string vbNewLine = "vbNewLine";
199+
public static readonly string vbNullChar = "vbNullChar";
195200
public static readonly string vbNullString = "vbNullString";
201+
public static readonly string vbTab = "vbTab";
202+
public static readonly string vbVerticalTab = "vbVerticalTab";
196203
public static readonly string WeekDay = "WeekDay";
197204
public static readonly string Wend = "Wend";
198205
public static readonly string While = "While";

0 commit comments

Comments
 (0)