Skip to content

SONARJAVA-7227 Preserve casts documenting potential precision loss in S1905 #6391

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.

Sign up for GitHub

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

Open
nathsou wants to merge 1 commit into
base: master
Choose a base branch
from
+65 −0
Open

SONARJAVA-7227 Preserve casts documenting potential precision loss in S1905 #6391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
54 changes: 54 additions & 0 deletions java-checks-test-sources/default/src/main/java/checks/RedundantTypeCastCheckSample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,3 +406,57 @@ public static void paramsErrorMessage(Class clazz) {
Outer.Nested r = (Outer.Nested) clazz.getAnnotation(Outer.Nested.class); // Handle cast of raw types
}
}

class PrecisionLossCasts {
PrecisionLossCasts(float first, float second, double third) {}

void conversions(int intValue, long longValue) {
float fromInt = (float) intValue;
float fromLong = (float) longValue;
double doubleFromLong = (double) longValue;

fromInt = (float) intValue;
fromLong = (float) longValue;
doubleFromLong = (double) longValue;

fromInt += (float) intValue;
fromLong += (float) longValue;
doubleFromLong += (double) longValue;

takeFloat((float) intValue);
takeFloat((float) longValue);
takeDouble((double) longValue);
new PrecisionLossCasts((float) intValue, (float) longValue, (double) longValue);

takeFloat(((float) (intValue)));
takeFloat(((float) (longValue)));
takeDouble(((double) (longValue)));
}

float returnInt(int value) {
return (float) value;
}

float returnLong(long value) {
return (float) value;
}

double returnLongAsDouble(long value) {
return (double) value;
}

void redundantCasts(int intValue, long longValue, float floatValue, byte byteValue, short shortValue, char charValue) {
int sameInt = (int) intValue; // Noncompliant
long sameLong = (long) longValue; // Noncompliant
float sameFloat = (float) floatValue; // Noncompliant
long widerInteger = (long) intValue; // Noncompliant
double fromInt = (double) intValue; // Noncompliant
double fromFloat = (double) floatValue; // Noncompliant
float fromByte = (float) byteValue; // Noncompliant
float fromShort = (float) shortValue; // Noncompliant
float fromChar = (float) charValue; // Noncompliant
}

void takeFloat(float value) {}
void takeDouble(double value) {}
}
11 changes: 11 additions & 0 deletions java-checks/src/main/java/org/sonar/java/checks/RedundantTypeCastCheck.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ public static Tree skipParentheses(Tree tree) {
}

private boolean isUnnecessaryCast(TypeCastTree typeCastTree) {
if (isPotentiallyLossyWideningConversion(typeCastTree)) {
return false;
}
if (skipParentheses(typeCastTree.expression()).is(Tree.Kind.NULL_LITERAL)) {
Tree parentTree = skipParentheses(typeCastTree.parent());
return !parentTree.is(Tree.Kind.ARGUMENTS);
Expand All @@ -92,6 +95,14 @@ private boolean isUnnecessaryCast(TypeCastTree typeCastTree) {
return warnings.stream().anyMatch(warning -> matchesWarning(warning, typeCastTree));
}

private static boolean isPotentiallyLossyWideningConversion(TypeCastTree tree) {
Type sourceType = tree.expression().symbolType();
Type targetType = tree.type().symbolType();
return (sourceType.isPrimitive(Type.Primitives.INT) && targetType.isPrimitive(Type.Primitives.FLOAT))
|| (sourceType.isPrimitive(Type.Primitives.LONG)
&& (targetType.isPrimitive(Type.Primitives.FLOAT) || targetType.isPrimitive(Type.Primitives.DOUBLE)));
}
Comment on lines +98 to +104

@gitar-bot gitar-bot Bot Oct 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Edge Case: Exemption also hides redundant casts of exactly representable constants

isPotentiallyLossyWideningConversion only looks at the operand and target types. That means any int→float or long→float/double cast is now exempt, including casts of compile-time constants whose values are exactly representable, such as float f = (float) 0; or double d = (double) 1L;. S9395 never asks for those casts: S9395Check.isSafeConstant (S9395Check.java:156-171) skips integer constants with |v| ≤ 2^24 for float and long constants with |v| ≤ 2^53 for double. So these casts are still truly redundant, and S1905 stops reporting them without any rule conflict to justify it. The conversion can't lose precision there, so the cast documents nothing. To fix this, apply the same safe-constant exclusion here: keep reporting when ExpressionUtils.resolveAsConstant(tree.expression()) gives a value inside the exact range. The simplest way is to share S9395's isLossyWideningConversion/isSafeConstant logic. Also add a Noncompliant sample like float f = (float) 1;.

Skip the exemption for constants that convert exactly, matching S9395:

private static boolean isPotentiallyLossyWideningConversion(TypeCastTree tree) {
  Type sourceType = tree.expression().symbolType();
  Type targetType = tree.type().symbolType();
  boolean lossyTypes = (sourceType.isPrimitive(Type.Primitives.INT) && targetType.isPrimitive(Type.Primitives.FLOAT))
    || (sourceType.isPrimitive(Type.Primitives.LONG)
      && (targetType.isPrimitive(Type.Primitives.FLOAT) || targetType.isPrimitive(Type.Primitives.DOUBLE)));
  if (!lossyTypes) {
    return false;
  }
  Object constant = ExpressionUtils.resolveAsConstant(ExpressionUtils.skipParentheses(tree.expression()));
  if (constant instanceof Number n) {
    long abs = Math.abs(n.longValue());
    return targetType.isPrimitive(Type.Primitives.FLOAT) ? abs > (1L << 24) : abs > (1L << 53);
  }
  return true;
}
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

@rombirli rombirli Oct 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks correct to me

@gitar-bot gitar-bot Bot Oct 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked RedundantTypeCastCheck.java:98-104 against S9395Check.isSafeConstant (lines 156-172), and the finding still holds.

isPotentiallyLossyWideningConversion looks only at the source and target types. A cast like (float) 0 or (double) 1L is therefore exempted from S1905, even though S9395 would not ask for it: isSafeConstant treats those constants as exactly representable (|v| ≤ 2^24 for float, ≤ 2^53 for double). So S1905 stops reporting casts that really are redundant and S9395 doesn't require.

If you want to keep this as is, the trade-off is losing those true positives. If you'd rather fix it, I can apply the same constant check here and add a float f = (float) 1; // Noncompliant sample. Reply "fix this" and I'll make the change.


private static boolean isMethodInvocationReceiverOfGetClass(TypeCastTree typeCastTree) {
Tree parent = skipParenthesesUpwards( typeCastTree.parent());
return parent instanceof MemberSelectExpressionTree memberSelect &&
Expand Down
Loading