Skip to content

Commit

Permalink
Review update
Browse files Browse the repository at this point in the history
  • Loading branch information
csaba-sagi-sonarsource committed Mar 30, 2022
1 parent 3b16806 commit 64beaf6
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,20 +85,20 @@ private static bool IsConditionFalseAtInitialization(ForStatementSyntax forNode)
var loopVariableDeclarationMapping = VariableDeclarationMapping(forNode.Declaration);
var loopInitializerMapping = LoopInitializerMapping(forNode.Initializers);

var variableNameToDoubleMapping = loopVariableDeclarationMapping
var variableNameToDecimalMapping = loopVariableDeclarationMapping
.Union(loopInitializerMapping)
.ToDictionary(d => d.Key, d => d.Value);

var binaryCondition = (BinaryExpressionSyntax)condition;
if (DoubleValue(variableNameToDoubleMapping, binaryCondition.Left, out var leftValue)
&& DoubleValue(variableNameToDoubleMapping, binaryCondition.Right, out var rightValue))
if (DecimalValue(variableNameToDecimalMapping, binaryCondition.Left, out var leftValue)
&& DecimalValue(variableNameToDecimalMapping, binaryCondition.Right, out var rightValue))
{
return !ConditionIsTrue(condition.Kind(), leftValue, rightValue);
}
return false;
}

private static bool ConditionIsTrue(SyntaxKind syntaxKind, double leftValue, double rightValue) =>
private static bool ConditionIsTrue(SyntaxKind syntaxKind, decimal leftValue, decimal rightValue) =>
syntaxKind switch
{
SyntaxKind.GreaterThanExpression => leftValue > rightValue,
Expand All @@ -120,11 +120,11 @@ private static bool IsLogicalNot(ExpressionSyntax expression, out PrefixUnaryExp
&& prefixUnaryExpression.OperatorToken.IsKind(SyntaxKind.ExclamationToken);
}

private static bool DoubleValue(IDictionary<string, double> variableNameToDoubleValue, ExpressionSyntax expression, out double parsedValue)
private static bool DecimalValue(IDictionary<string, decimal> variableNameToDecimalValue, ExpressionSyntax expression, out decimal parsedValue)
{
if (ExpressionNumericConverter.TryGetConstantDoubleValue(expression, out parsedValue)
if (ExpressionNumericConverter.TryGetConstantDecimalValue(expression, out parsedValue)
|| (expression is SimpleNameSyntax simpleName
&& variableNameToDoubleValue.TryGetValue(simpleName.Identifier.ValueText, out parsedValue)))
&& variableNameToDecimalValue.TryGetValue(simpleName.Identifier.ValueText, out parsedValue)))
{
return true;
}
Expand All @@ -134,50 +134,49 @@ private static bool DoubleValue(IDictionary<string, double> variableNameToDouble
}

/// <summary>
/// Retrieves the mapping of variable names to their double value from the variable declaration part of a for loop.
/// Retrieves the mapping of variable names to their decimal value from the variable declaration part of a for loop.
/// This will find the mapping for such cases:
/// <code>
/// for (var i = 0;;) {}
/// </code>
/// </summary>
private static IDictionary<string, double> VariableDeclarationMapping(VariableDeclarationSyntax variableDeclarationSyntax)
private static IDictionary<string, decimal> VariableDeclarationMapping(VariableDeclarationSyntax variableDeclarationSyntax)
{
var loopInitializerValues = new Dictionary<string, double>();
var loopInitializerValues = new Dictionary<string, decimal>();
if (variableDeclarationSyntax != null)
{
foreach (var variableDeclaration in variableDeclarationSyntax.Variables)
{
if (variableDeclaration.Initializer is EqualsValueClauseSyntax initializer
&& ExpressionNumericConverter.TryGetConstantDoubleValue(initializer.Value, out var doubleValue))
if (variableDeclaration.Initializer is { } initializer
&& ExpressionNumericConverter.TryGetConstantDecimalValue(initializer.Value, out var decimalValue))
{
loopInitializerValues.Add(variableDeclaration.Identifier.ValueText, doubleValue);
loopInitializerValues.Add(variableDeclaration.Identifier.ValueText, decimalValue);
}
}
}
return loopInitializerValues;
}

/// <summary>
/// Retrieves the mapping of variable names to their double value from the initializer part of a for loop.
/// Retrieves the mapping of variable names to their decimal value from the initializer part of a for loop.
/// This will find the mapping for such cases:
/// <code>
/// int i;
/// for (i = 0;;) {}
/// </code>
/// </summary>
private static IDictionary<string, double> LoopInitializerMapping(IEnumerable<ExpressionSyntax> initializers)
private static IDictionary<string, decimal> LoopInitializerMapping(IEnumerable<ExpressionSyntax> initializers)
{
var loopInitializerValues = new Dictionary<string, double>();
var loopInitializerValues = new Dictionary<string, decimal>();
if (initializers != null)
{
foreach (var initializer in initializers)
{
if (initializer.IsKind(SyntaxKind.SimpleAssignmentExpression)
&& initializer is AssignmentExpressionSyntax assignment
&& assignment.Left is SimpleNameSyntax simpleName
&& ExpressionNumericConverter.TryGetConstantDoubleValue(assignment.Right, out var doubleValue))
&& initializer is AssignmentExpressionSyntax { Left: SimpleNameSyntax simpleName } assignment
&& ExpressionNumericConverter.TryGetConstantDecimalValue(assignment.Right, out var decimalValue))
{
loopInitializerValues.Add(simpleName.Identifier.ValueText, doubleValue);
loopInitializerValues.Add(simpleName.Identifier.ValueText, decimalValue);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ public bool TryGetConstantIntValue(SyntaxNode expression, out int value) =>
public bool TryGetConstantDoubleValue(SyntaxNode expression, out double value) =>
TryGetConstantValue(null, expression, Convert.ToDouble, (multiplier, v) => multiplier * v, out value);

public bool TryGetConstantDecimalValue(SyntaxNode expression, out decimal value) =>
TryGetConstantValue(null, expression, Convert.ToDecimal, (multiplier, v) => multiplier * v, out value);

private bool TryGetConstantValue<T>(SemanticModel semanticModel, SyntaxNode expression, Func<object, T> converter, Func<int, T, T> multiplierCalculator, out T value)
where T : struct
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,14 @@ public class ForLoopConditionAlwaysFalseTest
[TestMethod]
public void ForLoopConditionAlwaysFalse() =>
new VerifierBuilder<ForLoopConditionAlwaysFalse>().AddPaths("ForLoopConditionAlwaysFalse.cs").Verify();

#if NET

[TestMethod]
public void ForLoopConditionAlwaysFalse_CSharp9() =>
new VerifierBuilder<ForLoopConditionAlwaysFalse>().AddPaths("ForLoopConditionAlwaysFalse.CSharp9.cs").WithOptions(ParseOptionsHelper.FromCSharp9).Verify();

#endif

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
using System;

namespace Tests.Diagnostics
{
class Program
{
void LoopTest(int x, int y)
{
for (nint i = 9; i >= 5;) { } // Compliant
for (nuint i = 9; i >= 5;) { } // Compliant

for (nint a = 0, b = 1; b < 1;) { } // Noncompliant
for (nuint a = 0, b = 1; b < 1;) { } // Noncompliant
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,12 @@ void LoopTest(int x, int y)
for (float n = 0.0F; n > -0.1F; n -= 0.005F) { } // Compliant
for (double n = 0.0; n > -0.1; n -= 0.005) { } // Compliant
for (var n = 0.0; n > -0.1; n -= 0.005) { } // Compliant
for (decimal n = 0.0M; n > -0.1M; n -= 0.005M) { } // Compliant

for (float n = -0.16F; n == -0.23F;) { } // Noncompliant
for (double n = -0.42; n != -0.42;) { } // Noncompliant
for (var n = 0.0; n <= -0.1; n -= 0.005) { } // Noncompliant
for (decimal n = 0.0M; n <= -0.1M; n -= 0.005M) { } // Noncompliant

for (var i = 9; i < 4 - 2;) { } // FN
}
Expand Down

0 comments on commit 64beaf6

Please sign in to comment.