Skip to content

Commit 59ad04f

Browse files
committed
Addressing comments
1 parent 155adc3 commit 59ad04f

File tree

6 files changed

+61
-22
lines changed

6 files changed

+61
-22
lines changed

Rubberduck.Inspections/Concrete/UnreachableCaseInspection/ParseTreeExpressionEvaluator.cs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ namespace Rubberduck.Inspections.Concrete.UnreachableCaseInspection
88
public interface IParseTreeExpressionEvaluator
99
{
1010
IParseTreeValue Evaluate(IParseTreeValue LHS, IParseTreeValue RHS, string opSymbol);
11-
IParseTreeValue Evaluate(IParseTreeValue LHS, string opSymbol);
11+
IParseTreeValue Evaluate(IParseTreeValue LHS, string opSymbol, string requestedResultType);
1212
}
1313

1414
public class ParseTreeExpressionEvaluator : IParseTreeExpressionEvaluator
@@ -89,25 +89,34 @@ public IParseTreeValue Evaluate(IParseTreeValue LHS, IParseTreeValue RHS, string
8989
return _valueFactory.Create($"{LHS.ValueText} {opSymbol} {RHS.ValueText}", opResultTypeName);
9090
}
9191

92-
public IParseTreeValue Evaluate(IParseTreeValue value, string opSymbol)
92+
public IParseTreeValue Evaluate(IParseTreeValue value, string opSymbol, string requestedResultType)
9393
{
9494
var isMathOp = MathOpsUnary.ContainsKey(opSymbol);
9595
var isLogicOp = LogicOpsUnary.ContainsKey(opSymbol);
9696
Debug.Assert(isMathOp || isLogicOp);
9797

98-
var opResultTypeName = isMathOp ? value.TypeName : Tokens.Boolean;
9998
var operands = PrepareOperands(new string[] { value.ValueText });
10099
if (operands.Count == 1)
101100
{
102101
if (isMathOp)
103102
{
104103
var mathResult = MathOpsUnary[opSymbol](operands[0]);
105-
return _valueFactory.Create(mathResult.ToString(), opResultTypeName);
104+
return _valueFactory.Create(mathResult.ToString(), requestedResultType);
105+
}
106+
107+
//Unary Not (!) operator
108+
if (!value.TypeName.Equals(Tokens.Boolean) && ParseTreeValue.TryConvertValue(value, out long opValue))
109+
{
110+
var bitwiseComplement = ~opValue;
111+
return _valueFactory.Create(Convert.ToBoolean(bitwiseComplement).ToString(), requestedResultType);
112+
}
113+
else if (value.TypeName.Equals(Tokens.Boolean))
114+
{
115+
var logicResult = LogicOpsUnary[opSymbol](operands[0]);
116+
return _valueFactory.Create(logicResult.ToString(), requestedResultType);
106117
}
107-
var logicResult = LogicOpsUnary[opSymbol](operands[0]);
108-
return _valueFactory.Create(logicResult.ToString(), opResultTypeName);
109118
}
110-
return _valueFactory.Create($"{opSymbol} {value.ValueText}", opResultTypeName);
119+
return _valueFactory.Create($"{opSymbol} {value.ValueText}", requestedResultType);
111120
}
112121

113122
private static string DetermineMathResultType(IParseTreeValue LHS, IParseTreeValue RHS)

Rubberduck.Inspections/Concrete/UnreachableCaseInspection/ParseTreeValueVisitor.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ private void VisitUnaryOpEvaluationContext(ParserRuleContext context)
177177
return;
178178
}
179179

180-
var result = Calculator.Evaluate(operands[0], opSymbol);
180+
var result = Calculator.Evaluate(operands[0], opSymbol, operands[0].TypeName);
181181
StoreVisitResult(context, result);
182182
}
183183

Rubberduck.Inspections/Concrete/UnreachableCaseInspection/RangeClauseContextWrapper.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public List<ParserRuleContext> ResultContexts
7575
get
7676
{
7777
var results = new List<ParserRuleContext>();
78-
if(!TryGetFirstResultContext(out ParserRuleContext resultContext))
78+
if (!TryGetFirstResultContext(out ParserRuleContext resultContext))
7979
{
8080
return results;
8181
}

Rubberduck.Inspections/Concrete/UnreachableCaseInspection/RangeClauseFilter.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -683,7 +683,7 @@ private void AddVariableRangeImpl(string inputStart, string inputEnd)
683683

684684
private void AddValueRangeImpl(T inputStart, T inputEnd)
685685
{
686-
if (ContainsBooleans)
686+
if (ContainsBooleans || inputStart.CompareTo(inputEnd) == 0)
687687
{
688688
SingleValues.Add(inputStart);
689689
SingleValues.Add(inputEnd);
@@ -846,7 +846,7 @@ private int RangesValuesCount()
846846
private T ConvertToContainedGeneric<K>(K value)
847847
{
848848
var parseTreeValue = _valueFactory.Create(value.ToString(), TypeName);
849-
if(_valueConverter(parseTreeValue, out T tValue))
849+
if (_valueConverter(parseTreeValue, out T tValue))
850850
{
851851
return tValue;
852852
}
@@ -923,7 +923,7 @@ private string GetSingleValueTypeDescriptor<K>(List<K> values, string prefix)
923923

924924
private string GetRangesDescriptor()
925925
{
926-
if(!(_ranges.Any() || _variableRanges.Any())) { return string.Empty; }
926+
if (!(_ranges.Any() || _variableRanges.Any())) { return string.Empty; }
927927

928928
StringBuilder series = new StringBuilder();
929929
foreach (var val in _ranges)

Rubberduck.Inspections/Concrete/UnreachableCaseInspection/RangeClauseFilterFactory.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ public class RangeClauseFilterFactory : IRangeClauseFilterFactory
3030
{
3131
public IRangeClauseFilter Create(string typeName, IParseTreeValueFactory valueFactory)
3232
{
33-
if(valueFactory is null) { throw new ArgumentNullException(); }
33+
if (valueFactory is null) { throw new ArgumentNullException(); }
3434

35-
if(!(IntegralNumberExtents.Keys.Contains(typeName)
35+
if (!(IntegralNumberExtents.Keys.Contains(typeName)
3636
|| typeName.Equals(Tokens.Double)
3737
|| typeName.Equals(Tokens.Single)
3838
|| typeName.Equals(Tokens.Currency)

RubberduckTests/Inspections/UnreachableCaseInspectionTests.cs

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -400,12 +400,13 @@ public void UciUnit_LogicImpOperator(string operands, string expected)
400400

401401
[TestCase("Not_False", "True")]
402402
[TestCase("Not_True", "False")]
403+
[TestCase("Not_1", "True")]
403404
[Category("Inspections")]
404405
public void UciUnit_LogicUnaryConstants(string operands, string expected)
405406
{
406407
GetUnaryOpValues(operands, out IParseTreeValue theValue, out string opSymbol);
407408

408-
var result = Calculator.Evaluate(theValue, opSymbol);
409+
var result = Calculator.Evaluate(theValue, opSymbol, Tokens.Boolean);
409410

410411
Assert.AreEqual(expected, result.ValueText);
411412
Assert.IsTrue(result.ParsesToConstantValue, "Expected IsConstantValue field to be 'True'");
@@ -415,14 +416,17 @@ public void UciUnit_LogicUnaryConstants(string operands, string expected)
415416
[TestCase("-_23.78", "-23.78")]
416417
[TestCase("-_True", "True?Boolean")]
417418
[TestCase("-_False", "False?Boolean")]
419+
[TestCase("-_True", "1?Integer")]
420+
[TestCase("-_-1", "1?Long")]
421+
[TestCase("-_0", "False?Boolean")]
418422
[TestCase("-_1?Double", "-1?Double")]
419423
[TestCase("-_-1?Double", "1?Double")]
420424
[Category("Inspections")]
421425
public void UciUnit_MinusUnaryOp(string operands, string expected)
422426
{
423427
var expectedVal = CreateInspValueFrom(expected);
424428
GetUnaryOpValues(operands, out IParseTreeValue LHS, out string opSymbol);
425-
var result = Calculator.Evaluate(LHS, opSymbol);
429+
var result = Calculator.Evaluate(LHS, opSymbol, expectedVal.TypeName);
426430

427431
Assert.AreEqual(expectedVal.ValueText, result.ValueText);
428432
Assert.IsTrue(result.ParsesToConstantValue);
@@ -440,12 +444,13 @@ public void UciUnit_MinusUnaryOp(string operands, string expected)
440444
[Category("Inspections")]
441445
public void UciUnit_ToString(string firstCase, string secondCase, string expected)
442446
{
443-
var filters = RangeDescriptorsToFilters(new string[] { firstCase, secondCase/*, expectedClauses*/ }, Tokens.Long);
447+
var filters = RangeDescriptorsToFilters(new string[] { firstCase, secondCase }, Tokens.Long);
444448
filters[0].Add(filters[1]);
445449

446450
Assert.AreEqual(expected, filters[0].ToString());
447451
}
448452

453+
[TestCase("50?Long_To_50?Long", "Long", "Single=50")]
449454
[TestCase("50?Long_To_x?Long", "Long", "Range=50:x")]
450455
[TestCase("50?Long_To_100?Long", "Long", "Range=50:100")]
451456
[TestCase("Soup?String_To_Nuts?String", "String", "Range=Nuts:Soup")]
@@ -735,12 +740,13 @@ private IParseTreeValue TestBinaryOp(string opSymbol, string operands, string ex
735740

736741
if (typeName.Equals(Tokens.Double) || typeName.Equals(Tokens.Single) || typeName.Equals(Tokens.Currency))
737742
{
738-
Assert.IsTrue(Math.Abs(double.Parse(result.ValueText) - double.Parse(expected)) < .001, $"Actual={result.ValueText} Expected={expected}");
743+
var compareLength = expected.Length > 5 ? 5 : expected.Length;
744+
Assert.IsTrue(Math.Abs(double.Parse(result.ValueText.Substring(0, compareLength)) - double.Parse(expected.Substring(0, compareLength))) <= double.Epsilon, $"Actual={result.ValueText} Expected={expected}");
739745
}
740746
else if (typeName.Equals(Tokens.String))
741747
{
742-
var toComp = expected.Length > 5 ? 5 : expected.Length;
743-
Assert.AreEqual(expected.Substring(0, toComp), result.ValueText.Substring(0, toComp));
748+
var compareLength = expected.Length > 5 ? 5 : expected.Length;
749+
Assert.AreEqual(expected.Substring(0, compareLength), result.ValueText.Substring(0, compareLength));
744750
}
745751
else
746752
{
@@ -851,7 +857,7 @@ End Select
851857
Assert.AreEqual(expected, result);
852858
}
853859

854-
[TestCase("Not x", "x As Long", "Boolean")]
860+
[TestCase("Not x", "x As Long", "Long")]
855861
[TestCase("x", "x As Long", "Long")]
856862
[TestCase("x < 5", "x As Long", "Boolean")]
857863
[TestCase("ToLong(True) * .0035", "x As Byte", "Double")]
@@ -1466,6 +1472,30 @@ End Select
14661472
CheckActualResultsEqualsExpected(inputCode, unreachable: 1);
14671473
}
14681474

1475+
[Test]
1476+
[Category("Inspections")]
1477+
public void UciFunctional_SingleValueRange()
1478+
{
1479+
const string inputCode =
1480+
@"Sub Foo( x As Long)
1481+
1482+
private const y As Double = 0.5
1483+
1484+
Select Case x
1485+
Case 55
1486+
'OK
1487+
Case 55 To 55
1488+
'Unreachable
1489+
Case 95
1490+
'OK
1491+
Case Else
1492+
'OK
1493+
End Select
1494+
1495+
End Sub";
1496+
CheckActualResultsEqualsExpected(inputCode, unreachable: 1);
1497+
}
1498+
14691499
[Test]
14701500
[Category("Inspections")]
14711501
public void UciFunctional_LongCollisionUnaryMathOperation()
@@ -1477,7 +1507,7 @@ Select Case -x
14771507
Case x > -3000
14781508
'OK
14791509
Case y > -3000
1480-
'Cannot disqualify other, or be disqualified, except by another y > ** statement
1510+
'OK
14811511
Case x < y
14821512
'OK - indeterminant
14831513
Case 95

0 commit comments

Comments
 (0)