diff --git a/java-checks-test-sources/default/src/main/java/checks/RedundantTypeCastCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/RedundantTypeCastCheckSample.java index 6c8537e7c9a..9cf199d59e4 100644 --- a/java-checks-test-sources/default/src/main/java/checks/RedundantTypeCastCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/RedundantTypeCastCheckSample.java @@ -55,6 +55,20 @@ List foo2() { return null; } + void lossyWideningCasts(int intVar, long longVar) { + float f1 = (float) intVar; // Compliant: documents potential precision loss, required by S9395 + float f2 = (float) longVar; // Compliant + double d1 = (double) longVar; // Compliant + float f3 = (float) (intVar); // Compliant + double d2 = (double) intVar; // Noncompliant {{Remove this unnecessary cast to "double".}} + float f4 = (float) 42; // Noncompliant {{Remove this unnecessary cast to "float".}} + double d3 = (double) 42L; // Noncompliant {{Remove this unnecessary cast to "double".}} + float f5 = (float) (intVar * intVar); // Noncompliant {{Remove this unnecessary cast to "float".}} + float f6 = (float) (intVar + intVar); // Noncompliant + double d4 = (double) (longVar / longVar); // Noncompliant + float f7 = (float) intVar * intVar; // Compliant + } + private static int charConversion(char c) { return (char) ((c | 0x20) - 'a'); // Compliant } diff --git a/java-checks-test-sources/default/src/main/java/checks/S9395CheckSample.java b/java-checks-test-sources/default/src/main/java/checks/S9395CheckSample.java index e045ce55a79..3f457143a1f 100644 --- a/java-checks-test-sources/default/src/main/java/checks/S9395CheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/S9395CheckSample.java @@ -161,9 +161,51 @@ void lambdaReturnDoesNotAffect(int intVar) { }; } - void intArithmeticToFloat(int a, int b) { - float f = a + b; // Noncompliant -// ^^^^^ + void intArithmeticToFloat(int a, int b, long c) { + float f1 = a + b; // compliant: reported by S2184, the fix is to cast an operand + float f2 = a / b; // compliant: reported by S2184, casting the result would not fix the integer division + float f3 = a * b; // compliant: reported by S2184 + double d1 = c - a; // compliant: reported by S2184 + float f4 = (a / b); // Noncompliant {{Cast one of the operands of this "int" operation to "float" to document potential precision loss.}} +// ^^^^^ + float f5 = a % b; // Noncompliant +// ^^^^^ + float f6 = (float) a / b; // compliant + } + + float field; + + void intArithmeticNotReportedByS2184(int a, int b, long c, float fv, double dv) { + float f1 = (a * b); // Noncompliant {{Cast one of the operands of this "int" operation to "float" to document potential precision loss.}} +// ^^^^^ + float f2 = ((a + b)); // Noncompliant +// ^^^^^ + fv = (a - b); // Noncompliant +// ^^^^^ + fv += a * b; // Noncompliant +// ^^^^^ + dv -= c + a; // Noncompliant {{Cast one of the operands of this "long" operation to "double" to document potential precision loss.}} +// ^^^^^ + fv += a / b; // compliant: reported by S2184 + fv *= (a / b); // compliant: reported by S2184 + field = a / b; // compliant: reported by S2184 + takeFloatVarargs(a * b, "x", "y"); // Noncompliant +// ^^^^^ + takeFloatVarargs(a / b); // Noncompliant +// ^^^^^ + takeFloatVarargs(a * b, "x"); // compliant: reported by S2184 + takeFloat((a * b)); // Noncompliant +// ^^^^^ + takeFloat(a * b); // compliant: reported by S2184 + } + + float returnParenthesizedArithmetic(int a, int b) { + return (a * b); // Noncompliant +// ^^^^^ + } + + float returnArithmetic(int a, int b) { + return a * b; // compliant: reported by S2184 } void hexAndBinaryLiterals() { diff --git a/java-checks/src/main/java/org/sonar/java/checks/RedundantTypeCastCheck.java b/java-checks/src/main/java/org/sonar/java/checks/RedundantTypeCastCheck.java index 94079774068..ad2f59e5b9f 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/RedundantTypeCastCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/RedundantTypeCastCheck.java @@ -81,7 +81,9 @@ public static Tree skipParentheses(Tree tree) { } private boolean isUnnecessaryCast(TypeCastTree typeCastTree) { - if (NumericConversionUtils.isPotentiallyLossyWideningConversion(typeCastTree.expression(), typeCastTree.type().symbolType())) { + ExpressionTree castOperand = ExpressionUtils.skipParentheses(typeCastTree.expression()); + if (!castOperand.is(Tree.Kind.PLUS, Tree.Kind.MINUS, Tree.Kind.MULTIPLY, Tree.Kind.DIVIDE) + && NumericConversionUtils.isPotentiallyLossyWideningConversion(castOperand, typeCastTree.type().symbolType())) { return false; } if (skipParentheses(typeCastTree.expression()).is(Tree.Kind.NULL_LITERAL)) { diff --git a/java-checks/src/main/java/org/sonar/java/checks/S9395Check.java b/java-checks/src/main/java/org/sonar/java/checks/S9395Check.java index ea9c1c87bc7..fe5fe05545a 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/S9395Check.java +++ b/java-checks/src/main/java/org/sonar/java/checks/S9395Check.java @@ -43,6 +43,8 @@ @Rule(key = "S9395") public class S9395Check extends BaseTreeVisitor implements JavaFileScanner { + private static final Tree.Kind[] ARITHMETIC_KINDS = {Tree.Kind.PLUS, Tree.Kind.MINUS, Tree.Kind.MULTIPLY, Tree.Kind.DIVIDE}; + private JavaFileScannerContext context; private final Deque floatingPointReturnTypes = new LinkedList<>(); @@ -57,7 +59,7 @@ public void scanFile(JavaFileScannerContext context) { @Override public void visitVariable(VariableTree tree) { - checkExpression(tree.type().symbolType(), tree.initializer()); + checkExpression(tree.type().symbolType(), tree.initializer(), true); super.visitVariable(tree); } @@ -65,7 +67,7 @@ public void visitVariable(VariableTree tree) { public void visitAssignmentExpression(AssignmentExpressionTree tree) { Type targetType = tree.variable().symbolType(); ExpressionTree rhs = tree.expression(); - checkExpression(targetType, rhs); + checkExpression(targetType, rhs, tree.is(Tree.Kind.ASSIGNMENT)); super.visitAssignmentExpression(tree); } @@ -85,7 +87,7 @@ public void visitNewClass(NewClassTree tree) { public void visitReturnStatement(ReturnStatementTree tree) { Type returnType = floatingPointReturnTypes.peek(); if (returnType != null) { - checkExpression(returnType, tree.expression()); + checkExpression(returnType, tree.expression(), true); } super.visitReturnStatement(tree); } @@ -117,25 +119,51 @@ private void checkArguments(Arguments arguments, Symbol.MethodSymbol symbol) { } else { return; } + boolean checkedByS2184 = arguments.size() == parameterTypes.size(); for (int i = 0; i < fixedCount && i < arguments.size(); i++) { - checkExpression(parameterTypes.get(i), arguments.get(i)); + checkExpression(parameterTypes.get(i), arguments.get(i), checkedByS2184); } } } - private void checkExpression(Type targetType, @Nullable ExpressionTree expr) { + /** + * Reports implicit lossy conversions of {@code expr} to {@code targetType}. + * Arithmetic expressions already reported by S2184 are skipped: S2184 checks non-parenthesized arithmetic when + * {@code checkedByS2184} is true, and integer divisions nested in a floating-point expression in any context. + */ + private void checkExpression(Type targetType, @Nullable ExpressionTree expr, boolean checkedByS2184) { if (expr == null) { return; } ExpressionTree unwrapped = ExpressionUtils.skipParentheses(expr); - if (unwrapped.is(Tree.Kind.TYPE_CAST)) { + if (unwrapped.is(Tree.Kind.TYPE_CAST) || isReportedByS2184(expr, unwrapped, checkedByS2184) + || !NumericConversionUtils.isPotentiallyLossyWideningConversion(unwrapped, targetType)) { return; } - if (NumericConversionUtils.isPotentiallyLossyWideningConversion(unwrapped, targetType)) { - Type sourceType = unwrapped.symbolType(); + String sourceType = unwrapped.symbolType().name(); + if (unwrapped.is(ARITHMETIC_KINDS)) { + context.reportIssue(this, unwrapped, + "Cast one of the operands of this \"" + sourceType + "\" operation to \"" + targetType.name() + "\" to document potential precision loss."); + } else { context.reportIssue(this, unwrapped, - "Explicitly cast this \"" + sourceType.name() + "\" to \"" + targetType.name() + "\" to document potential precision loss."); + "Explicitly cast this \"" + sourceType + "\" to \"" + targetType.name() + "\" to document potential precision loss."); + } + } + + private static boolean isReportedByS2184(ExpressionTree expr, ExpressionTree unwrapped, boolean checkedByS2184) { + return (checkedByS2184 && expr.is(ARITHMETIC_KINDS)) + || (unwrapped.is(Tree.Kind.DIVIDE) && hasFloatingPointParentExpression(unwrapped)); + } + + private static boolean hasFloatingPointParentExpression(ExpressionTree expr) { + Tree parent = expr.parent(); + while (parent instanceof ExpressionTree parentExpression) { + if (isFloatingPoint(parentExpression.symbolType())) { + return true; + } + parent = parentExpression.parent(); } + return false; } private static boolean isFloatingPoint(Type type) { diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9395.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9395.html index b92c1a5e8ea..b562ada93dc 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9395.html +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9395.html @@ -31,34 +31,42 @@

Debugging Difficulty

Silent precision loss is hard to detect and diagnose. The code compiles without warnings, and the loss may only manifest under specific input values, making bugs intermittent and difficult to reproduce.

How to fix it

-

When converting int to float, use an explicit cast to document awareness of potential precision loss for large values -(beyond ±16,777,216).

+

The right fix depends on whether the precision loss is acceptable:

+ +

Simply adding a cast may hide a bug, which is why no automatic fix is proposed.

+

For integer arithmetic expressions such as a / b converted to a floating-point type, casting the result would not fix the +integer division or overflow. Instead, one of the operands should be cast before the operation, as described by rule {rule:java:S2184}. Such +expressions are not reported by this rule when {rule:java:S2184} already reports them.

Code examples

Noncompliant code example

 public class DataProcessor {
-    public void processValue(int id) {
+    public void processId(int id) {
         float floatId = id; // Noncompliant - implicit conversion may lose precision
         System.out.println(floatId);
     }
 
-    public float calculateRatio(int numerator, int denominator) {
-        float ratio = numerator / denominator; // Noncompliant - implicit conversion
-        return ratio;
+    public void processTimestamp(long timestamp) {
+        double value = timestamp; // Noncompliant - implicit conversion may lose precision
+        System.out.println(value);
     }
 }

Compliant solution

 public class DataProcessor {
-    public void processValue(int id) {
-        float floatId = (float) id; // Explicit cast shows intent
-        System.out.println(floatId);
+    public void processId(int id) {
+        double doubleId = id; // A double can represent every int exactly
+        System.out.println(doubleId);
     }
 
-    public float calculateRatio(int numerator, int denominator) {
-        float ratio = (float) (numerator / denominator); // Clear conversion
-        return ratio;
+    public void processTimestamp(long timestamp) {
+        double value = (double) timestamp; // Explicit cast documents that the precision loss is acceptable
+        System.out.println(value);
     }
 }
diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9395.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9395.json index c7acfb697b5..c40bb38663f 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9395.json +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9395.json @@ -15,7 +15,7 @@ "ruleSpecification": "RSPEC-9395", "sqKey": "S9395", "scope": "All", - "quickfix": "unknown", + "quickfix": "infeasible", "code": { "impacts": { "MAINTAINABILITY": "MEDIUM"