SONARJAVA-7238: Improve S9395 and resolve conflicts with S1905 and S2184

java-checks-test-sources/default/src/main/java/checks/RedundantTypeCastCheckSample.java

@@ -55,6 +55,20 @@ List<String> 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
   }

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() {

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)) {

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<Type> floatingPointReturnTypes = new LinkedList<>();
 
@@ -57,15 +59,15 @@ 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);
   }
 
   @Override
   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) {

sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9395.html

@@ -31,34 +31,42 @@ <h3>Debugging Difficulty</h3>
 <p>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.</p>
 <h2>How to fix it</h2>
-<p>When converting <code>int</code> to <code>float</code>, use an explicit cast to document awareness of potential precision loss for large values
-(beyond ±16,777,216).</p>
+<p>The right fix depends on whether the precision loss is acceptable:</p>
+<ul>
+  <li>If the exact value matters, keep it in an integer type, or use a floating-point type able to represent it exactly (for example
+  <code>double</code> instead of <code>float</code> for <code>int</code> values). For arbitrary precision, use <code>BigDecimal</code>.</li>
+  <li>If the precision loss is acceptable, add an explicit cast to document that it is intentional.</li>
+</ul>
+<p>Simply adding a cast may hide a bug, which is why no automatic fix is proposed.</p>
+<p>For integer arithmetic expressions such as <code>a / b</code> 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.</p>
 <h3>Code examples</h3>
 <h4>Noncompliant code example</h4>
 <pre data-diff-id="1" data-diff-type="noncompliant">
 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);
     }
 }
 </pre>
 <h4>Compliant solution</h4>
 <pre data-diff-id="1" data-diff-type="compliant">
 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);
     }
 }
 </pre>

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"