Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX S2252 FP: Rule raises issue in case of non integer value in for loop. #5523

Merged
merged 3 commits into from
Apr 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public sealed class ForLoopConditionAlwaysFalse : SonarDiagnosticAnalyzer
private const string DiagnosticId = "S2252";
private const string MessageFormat = "This loop will never execute.";

private static readonly CSharpExpressionNumericConverter ExpressionNumericConverter = new CSharpExpressionNumericConverter();
private static readonly CSharpExpressionNumericConverter ExpressionNumericConverter = new();

private static readonly ISet<SyntaxKind> ConditionsToCheck = new HashSet<SyntaxKind>
{
Expand Down Expand Up @@ -82,23 +82,23 @@ private static bool IsConditionFalseAtInitialization(ForStatementSyntax forNode)
return false;
}

var loopVariableDeclarationMapping = GetVariableDeclarationMapping(forNode.Declaration);
var loopInitializerMapping = GetLoopInitializerMapping(forNode.Initializers);
var loopVariableDeclarationMapping = VariableDeclarationMapping(forNode.Declaration);
var loopInitializerMapping = LoopInitializerMapping(forNode.Initializers);

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

var binaryCondition = (BinaryExpressionSyntax)condition;
if (GetIntValue(variableNameToIntegerMapping, binaryCondition.Left, out var leftValue)
&& GetIntValue(variableNameToIntegerMapping, 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, int leftValue, int rightValue) =>
private static bool ConditionIsTrue(SyntaxKind syntaxKind, decimal leftValue, decimal rightValue) =>
syntaxKind switch
{
SyntaxKind.GreaterThanExpression => leftValue > rightValue,
Expand All @@ -120,72 +120,63 @@ private static bool IsLogicalNot(ExpressionSyntax expression, out PrefixUnaryExp
&& prefixUnaryExpression.OperatorToken.IsKind(SyntaxKind.ExclamationToken);
}

/// <summary>
/// We try to retrieve the integer value of an expression. If the expression is an integer literal, we return its value, otherwise if
/// the expression is an identifier, we attempt to retrieve the integer value the variable was initialized with if it exists.
/// </summary>
/// <param name="variableNameToIntegerValue">A dictionary mapping variable names to the integer value they were initialized with if it exists</param>
/// <param name="expression">The expression for which we want to retrieve the integer value</param>
/// <param name="intValue">The output parameter that will hold the integer value if it is found</param>
/// <returns>true if an integer value was found for the expression, false otherwise</returns>
private static bool GetIntValue(IDictionary<string, int> variableNameToIntegerValue, ExpressionSyntax expression, out int intValue)
private static bool DecimalValue(IDictionary<string, decimal> variableNameToDecimalValue, ExpressionSyntax expression, out decimal parsedValue)
{
if (ExpressionNumericConverter.TryGetConstantIntValue(expression, out intValue)
if (ExpressionNumericConverter.TryGetConstantDecimalValue(expression, out parsedValue)
|| (expression is SimpleNameSyntax simpleName
&& variableNameToIntegerValue.TryGetValue(simpleName.Identifier.ValueText, out intValue)))
&& variableNameToDecimalValue.TryGetValue(simpleName.Identifier.ValueText, out parsedValue)))
{
return true;
}

intValue = default(int);
parsedValue = default;
return false;
}

/// <summary>
/// Retrieves the mapping of variable names to their integer 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, int> GetVariableDeclarationMapping(VariableDeclarationSyntax variableDeclarationSyntax)
private static IDictionary<string, decimal> VariableDeclarationMapping(VariableDeclarationSyntax variableDeclarationSyntax)
{
var loopInitializerValues = new Dictionary<string, int>();
var loopInitializerValues = new Dictionary<string, decimal>();
if (variableDeclarationSyntax != null)
{
foreach (var variableDeclaration in variableDeclarationSyntax.Variables)
{
if (variableDeclaration.Initializer is EqualsValueClauseSyntax initializer
&& ExpressionNumericConverter.TryGetConstantIntValue(initializer.Value, out var intValue))
if (variableDeclaration.Initializer is { } initializer
&& ExpressionNumericConverter.TryGetConstantDecimalValue(initializer.Value, out var decimalValue))
{
loopInitializerValues.Add(variableDeclaration.Identifier.ValueText, intValue);
loopInitializerValues.Add(variableDeclaration.Identifier.ValueText, decimalValue);
}
}
}
return loopInitializerValues;
}

/// <summary>
/// Retrieves the mapping of variable names to their integer 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, int> GetLoopInitializerMapping(IEnumerable<ExpressionSyntax> initializers)
private static IDictionary<string, decimal> LoopInitializerMapping(IEnumerable<ExpressionSyntax> initializers)
{
var loopInitializerValues = new Dictionary<string, int>();
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.TryGetConstantIntValue(assignment.Right, out var intValue))
&& initializer is AssignmentExpressionSyntax { Left: SimpleNameSyntax simpleName } assignment
&& ExpressionNumericConverter.TryGetConstantDecimalValue(assignment.Right, out var decimalValue))
{
loopInitializerValues.Add(simpleName.Identifier.ValueText, intValue);
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 @@ -29,6 +29,15 @@ public class ForLoopConditionAlwaysFalseTest
{
[TestMethod]
public void ForLoopConditionAlwaysFalse() =>
OldVerifier.VerifyAnalyzer(@"TestCases\ForLoopConditionAlwaysFalse.cs", new 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 @@ -56,9 +56,17 @@ void LoopTest(int x, int y)
for (; z < 0; z++) { } // FN - we only check for initializers inside the loop statement

// Reproducer for https://github.com/SonarSource/sonar-dotnet/issues/5428
for (float n = 0.0F; n > -0.1F; n -= 0.005F) { } // Noncompliant FP
for (double n = 0.0; n > -0.1; n -= 0.005) { } // Noncompliant FP
for (var n = 0.0; n > -0.1; n -= 0.005) { } // Noncompliant FP
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
costin-zaharia-sonarsource marked this conversation as resolved.
Show resolved Hide resolved
for (decimal n = 0.0M; n <= -0.1M; n -= 0.005M) { } // Noncompliant

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