Skip to content

Commit 4529efb

Browse files
committed
Addressed some comments
1 parent 5e91894 commit 4529efb

File tree

6 files changed

+192
-52
lines changed

6 files changed

+192
-52
lines changed

Rubberduck.Inspections/Concrete/UnreachableCaseInspection/ParseTreeExpressionEvaluator.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,11 @@ internal static class MathSymbols
153153
private static string _plus;
154154
private static string _minusSign;
155155
private static string _exponent;
156+
private static string _integerDivide;
156157

157158
public static string MULTIPLY => _multiply ?? LoadSymbols(VBAParser.MULT);
158159
public static string DIVIDE => _divide ?? LoadSymbols(VBAParser.DIV);
159-
public static string INTEGER_DIVIDE => @"\";
160+
public static string INTEGER_DIVIDE => _integerDivide ?? LoadSymbols(VBAParser.INTDIV);
160161
public static string PLUS => _plus ?? LoadSymbols(VBAParser.PLUS);
161162
public static string MINUS => _minusSign ?? LoadSymbols(VBAParser.MINUS);
162163
public static string ADDITIVE_INVERSE => MINUS;
@@ -167,6 +168,7 @@ private static string LoadSymbols(int target)
167168
{
168169
_multiply = VBAParser.DefaultVocabulary.GetLiteralName(VBAParser.MULT).Replace("'", "");
169170
_divide = VBAParser.DefaultVocabulary.GetLiteralName(VBAParser.DIV).Replace("'", "");
171+
_integerDivide = VBAParser.DefaultVocabulary.GetLiteralName(VBAParser.INTDIV).Replace("'", "");
170172
_plus = VBAParser.DefaultVocabulary.GetLiteralName(VBAParser.PLUS).Replace("'", "");
171173
_minusSign = VBAParser.DefaultVocabulary.GetLiteralName(VBAParser.MINUS).Replace("'", "");
172174
_exponent = VBAParser.DefaultVocabulary.GetLiteralName(VBAParser.POW).Replace("'", "");

Rubberduck.Inspections/Concrete/UnreachableCaseInspection/ParseTreeValueVisitor.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,6 @@ private bool TryGetLExprValue(VBAParser.LExprContext lExprContext, out string ex
241241
{
242242
expressionValue = string.Empty;
243243
declaredTypeName = string.Empty;
244-
245244
if (lExprContext.TryGetChildContext(out VBAParser.MemberAccessExprContext memberAccess))
246245
{
247246
var member = memberAccess.GetChild<VBAParser.UnrestrictedIdentifierContext>();
@@ -320,11 +319,13 @@ private string GetBaseTypeForDeclaration(Declaration declaration)
320319
{
321320
var localDeclaration = declaration;
322321
var iterationGuard = 0;
323-
while (!localDeclaration.AsTypeIsBaseType && iterationGuard++ < 5)
322+
while (!(localDeclaration is null)
323+
&& !localDeclaration.AsTypeIsBaseType
324+
&& iterationGuard++ < 5)
324325
{
325326
localDeclaration = localDeclaration.AsTypeDeclaration;
326327
}
327-
return localDeclaration.AsTypeName;
328+
return localDeclaration is null ? declaration.AsTypeName : localDeclaration.AsTypeName;
328329
}
329330

330331
private static bool IsBinaryMathContext<T>(T context)

Rubberduck.Inspections/Concrete/UnreachableCaseInspection/RangeClauseFilter.cs

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ public class RangeClauseFilter<T> : IRangeClauseFilter, IRangeClauseFilterTestSu
4444
private bool _hasExtents;
4545
private T _minExtent;
4646
private T _maxExtent;
47+
private string _cachedDescriptor;
48+
private bool _descriptorIsDirty;
4749

4850
public RangeClauseFilter(string typeName, IParseTreeValueFactory valueFactory, IRangeClauseFilterFactory filterFactory, TryConvertParseTreeValue<T> tConverter)
4951
{
@@ -61,6 +63,8 @@ public RangeClauseFilter(string typeName, IParseTreeValueFactory valueFactory, I
6163
_falseValue = ConvertToContainedGeneric(false);
6264
_trueValue = ConvertToContainedGeneric(true);
6365
TypeName = typeName;
66+
_cachedDescriptor = string.Empty;
67+
_descriptorIsDirty = true;
6468
}
6569

6670
private List<Tuple<T, T>> RangeValues => _ranges;
@@ -83,6 +87,10 @@ public bool FiltersAllValues
8387
{
8488
get
8589
{
90+
if (ContainsBooleans && CoversTrueFalse())
91+
{
92+
return true;
93+
}
8694

8795
var coversAll = false;
8896
var hasLTFilter = TryGetIsLTValue(out T ltValue);
@@ -101,10 +109,6 @@ public bool FiltersAllValues
101109
coversAll = gt - lt + 1 <= RangesValuesCount() + SingleValues.Count()
102110
|| gt == lt && RangesFilterValue(ltValue);
103111
}
104-
else if (ContainsBooleans && !coversAll)
105-
{
106-
coversAll = SingleValues.Contains(_trueValue);
107-
}
108112
return coversAll;
109113
}
110114
}
@@ -115,9 +119,9 @@ public bool ContainsFilters
115119
{
116120
return _ranges.Any() || _variableRanges.Any()
117121
|| _singleValues.Any() || _variableSingles.Any()
122+
|| _relationalOps.Any()
118123
|| TryGetIsLTValue(out T isLT) && isLT.CompareTo(_minExtent) != 0
119-
|| TryGetIsGTValue(out T isGT) && isGT.CompareTo(_maxExtent) != 0
120-
|| _relationalOps.Any();
124+
|| TryGetIsGTValue(out T isGT) && isGT.CompareTo(_maxExtent) != 0;
121125
}
122126
}
123127

@@ -152,6 +156,7 @@ public void Add(IRangeClauseFilter filter)
152156
{
153157
AddSingleValueImpl(val);
154158
}
159+
_descriptorIsDirty = true;
155160
}
156161

157162
public void AddIsClause(IParseTreeValue value, string opSymbol)
@@ -168,6 +173,7 @@ public void AddIsClause(IParseTreeValue value, string opSymbol)
168173
{
169174
AddRelationalOpImpl(value.ValueText);
170175
}
176+
_descriptorIsDirty = true;
171177
}
172178

173179
public void AddRelationalOp(IParseTreeValue value)
@@ -184,6 +190,7 @@ public void AddRelationalOp(IParseTreeValue value)
184190
{
185191
AddRelationalOpImpl(value.ValueText);
186192
}
193+
_descriptorIsDirty = true;
187194
}
188195

189196
public void AddSingleValue(IParseTreeValue value)
@@ -200,6 +207,7 @@ public void AddSingleValue(IParseTreeValue value)
200207
{
201208
VariableSingleValues.Add(value.ValueText);
202209
}
210+
_descriptorIsDirty = true;
203211
}
204212

205213
public void AddValueRange(IParseTreeValue inputStartVal, IParseTreeValue inputEndVal)
@@ -225,6 +233,7 @@ public void AddValueRange(IParseTreeValue inputStartVal, IParseTreeValue inputEn
225233
{
226234
AddVariableRangeImpl(inputStartVal.ValueText, inputEndVal.ValueText);
227235
}
236+
_descriptorIsDirty = true;
228237
}
229238

230239
public IRangeClauseFilter FilterUnreachableClauses(IRangeClauseFilter filter)
@@ -309,6 +318,11 @@ public override bool Equals(object obj)
309318

310319
public override string ToString()
311320
{
321+
if (!_descriptorIsDirty)
322+
{
323+
return _cachedDescriptor;
324+
}
325+
312326
var descriptors = new HashSet<string>
313327
{
314328
GetIsClausesDescriptor(LogicSymbols.LT),
@@ -329,7 +343,9 @@ public override string ToString()
329343
}
330344
descriptor.Append(descriptors.ElementAt(idx));
331345
}
332-
return descriptor.ToString();
346+
_cachedDescriptor = descriptor.ToString();
347+
_descriptorIsDirty = false;
348+
return _cachedDescriptor;
333349
}
334350

335351
public override int GetHashCode()
@@ -357,8 +373,7 @@ private bool FiltersAllRelationalOps
357373
{
358374
if (ContainsBooleans)
359375
{
360-
return _singleValues.Contains(_trueValue) && _singleValues.Contains(_falseValue)
361-
|| RangesFilterValue(_trueValue) && RangesFilterValue(_falseValue);
376+
return CoversTrueFalse();
362377
}
363378
return _singleValues.Contains(_trueValue) && _singleValues.Contains(_falseValue)
364379
|| RangesFilterValue(_trueValue) && RangesFilterValue(_falseValue)
@@ -367,6 +382,12 @@ private bool FiltersAllRelationalOps
367382
}
368383
}
369384

385+
private bool CoversTrueFalse()
386+
{
387+
return _singleValues.Contains(_trueValue) && _singleValues.Contains(_falseValue)
388+
|| RangesFilterValue(_trueValue) && RangesFilterValue(_falseValue);
389+
}
390+
370391
private void RemoveIsLTClause() => RemoveIsClauseImpl(LogicSymbols.LT);
371392

372393
private void RemoveIsGTClause() => RemoveIsClauseImpl(LogicSymbols.GT);

Rubberduck.Inspections/Concrete/UnreachableCaseInspection/SelectCaseStmtContextWrapper.cs

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ public class SelectCaseStmtContextWrapper : ContextWrapperBase, ISelectCaseStmtC
2222
private List<ParserRuleContext> _unreachableResults;
2323
private List<ParserRuleContext> _mismatchResults;
2424
private List<ParserRuleContext> _caseElseResults;
25-
private string _evalTypeName;
2625

2726
public SelectCaseStmtContextWrapper(VBAParser.SelectCaseStmtContext selectCaseContext, IParseTreeVisitorResults inspValues, IUnreachableCaseInspectionFactoryProvider factoryFactory)
2827
: base(selectCaseContext, inspValues, factoryFactory)
@@ -31,12 +30,11 @@ public SelectCaseStmtContextWrapper(VBAParser.SelectCaseStmtContext selectCaseCo
3130
_unreachableResults = new List<ParserRuleContext>();
3231
_mismatchResults = new List<ParserRuleContext>();
3332
_caseElseResults = new List<ParserRuleContext>();
34-
_evalTypeName = null;
3533
_inspectionRangeFactory = factoryFactory.CreateIRangeClauseContextWrapperFactory();
34+
SetEvaluationTypeName(Context, ParseTreeValueResults, FilterFactory);
3635
}
3736

38-
public string EvaluationTypeName => _evalTypeName
39-
?? DetermineSelectCaseEvaluationTypeName(Context, ParseTreeValueResults, FilterFactory);
37+
public string EvaluationTypeName { private set; get; }
4038

4139
public void InspectForUnreachableCases()
4240
{
@@ -80,28 +78,43 @@ public void InspectForUnreachableCases()
8078

8179
public List<ParserRuleContext> UnreachableCaseElseCases => _caseElseResults;
8280

83-
private static bool InspectionCanEvaluateTypeName(string typeName) => !(typeName == string.Empty || typeName == Tokens.Variant);
84-
85-
private string DetermineSelectCaseEvaluationTypeName(ParserRuleContext context, IParseTreeVisitorResults inspValues, IRangeClauseFilterFactory factory)
81+
private static List<string> InspectableTypes = new List<string>()
82+
{
83+
Tokens.Byte,
84+
Tokens.Integer,
85+
Tokens.Int,
86+
Tokens.Long,
87+
Tokens.LongLong,
88+
Tokens.Single,
89+
Tokens.Double,
90+
Tokens.Decimal,
91+
Tokens.Currency,
92+
Tokens.Boolean,
93+
Tokens.String
94+
};
95+
96+
private static bool InspectionCanEvaluateTypeName(string typeName) => InspectableTypes.Contains(typeName);
97+
98+
private void SetEvaluationTypeName(ParserRuleContext context, IParseTreeVisitorResults inspValues, IRangeClauseFilterFactory factory)
8699
{
87100
var selectStmt = (VBAParser.SelectCaseStmtContext)context;
88-
if (TryDetectTypeHint(selectStmt.selectExpression().GetText(), out _evalTypeName))
101+
if (TryDetectTypeHint(selectStmt.selectExpression().GetText(), out string evalTypName))
89102
{
90-
return _evalTypeName;
103+
EvaluationTypeName = evalTypName;
104+
return;
91105
}
92106

93107
var typeName = string.Empty;
94108
if (inspValues.TryGetValue(selectStmt.selectExpression(), out IParseTreeValue result))
95109
{
96-
_evalTypeName = result.TypeName;
110+
EvaluationTypeName = result.TypeName;
97111
}
98112

99-
if (InspectionCanEvaluateTypeName(_evalTypeName))
113+
if (InspectionCanEvaluateTypeName(EvaluationTypeName))
100114
{
101-
return _evalTypeName;
115+
return;
102116
}
103-
_evalTypeName = DeriveTypeFromCaseClauses(inspValues, selectStmt);
104-
return _evalTypeName;
117+
EvaluationTypeName = DeriveTypeFromCaseClauses(inspValues, selectStmt);
105118
}
106119

107120
private string DeriveTypeFromCaseClauses(IParseTreeVisitorResults inspValues, VBAParser.SelectCaseStmtContext selectStmt)
@@ -118,21 +131,16 @@ private string DeriveTypeFromCaseClauses(IParseTreeVisitorResults inspValues, VB
118131
else
119132
{
120133
var inspRange = _inspectionRangeFactory.Create(range, inspValues);
121-
var types = inspRange.ResultContexts.Select(rc => inspValues.GetTypeName(rc));
134+
var types = inspRange.ResultContexts.Select(rc => inspValues.GetTypeName(rc))
135+
.Where(tp => InspectableTypes.Contains(tp));
122136
caseClauseTypeNames.AddRange(types);
123137
}
124138
}
125139
}
126140

127-
if (TryDetermineEvaluationTypeFromTypes(caseClauseTypeNames, out _evalTypeName))
128-
{
129-
return _evalTypeName;
130-
}
131-
132-
caseClauseTypeNames.Remove(Tokens.Variant);
133-
if (TryDetermineEvaluationTypeFromTypes(caseClauseTypeNames, out _evalTypeName))
141+
if (TryDetermineEvaluationTypeFromTypes(caseClauseTypeNames, out string evalTypeName))
134142
{
135-
return _evalTypeName;
143+
return evalTypeName;
136144
}
137145
return string.Empty;
138146
}

Rubberduck.Inspections/Concrete/UnreachableCaseInspection/UnreachableCaseInspection.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ public UnreachableCaseInspection(RubberduckParserState state) : base(state)
4949

5050
protected override IEnumerable<IInspectionResult> DoGetInspectionResults()
5151
{
52+
InspectionResults = new List<IInspectionResult>();
5253
var qualifiedSelectCaseStmts = Listener.Contexts
5354
.Where(result => !IsIgnoringInspectionResultFor(result.ModuleName, result.Context.Start.Line));
5455

0 commit comments

Comments
 (0)