diff --git a/its/ruling/src/test/resources/expected/guava/java-S1905.json b/its/ruling/src/test/resources/expected/guava/java-S1905.json index 3fd186b2fd5..607efda4232 100644 --- a/its/ruling/src/test/resources/expected/guava/java-S1905.json +++ b/its/ruling/src/test/resources/expected/guava/java-S1905.json @@ -2,13 +2,6 @@ "guava:src/com/google/common/base/Converter.java": [ 456 ], -"guava:src/com/google/common/cache/LongAdder.java": [ -186, -194 -], -"guava:src/com/google/common/collect/Maps.java": [ -200 -], "guava:src/com/google/common/collect/RegularImmutableSortedMultiset.java": [ 114 ], diff --git a/its/ruling/src/test/resources/expected/sonarqube-6.5/java-S1905.json b/its/ruling/src/test/resources/expected/sonarqube-6.5/java-S1905.json index f7a7318698f..3e94dc55421 100644 --- a/its/ruling/src/test/resources/expected/sonarqube-6.5/java-S1905.json +++ b/its/ruling/src/test/resources/expected/sonarqube-6.5/java-S1905.json @@ -3,9 +3,6 @@ 198, 210 ], -"sonarqube-6.5:src/main/java/org/sonar/server/computation/task/projectanalysis/qualitymodel/MaintainabilityMeasuresVisitor.java": [ -122 -], "sonarqube-6.5:src/main/java/org/sonar/server/computation/task/projectanalysis/step/ComputeMeasureVariationsStep.java": [ 124 ], 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 603b1851ffc..6c8537e7c9a 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 @@ -406,3 +406,93 @@ public static void paramsErrorMessage(Class clazz) { Outer.Nested r = (Outer.Nested) clazz.getAnnotation(Outer.Nested.class); // Handle cast of raw types } } + +class PrecisionLossCasts { + static final int SMALL_INT = 42; + static final long SMALL_LONG = 42L; + + 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 safeConstants() { + float zero = (float) 0; // Noncompliant + float fromInt = (float) 1; // Noncompliant + float fromLong = (float) 1L; // Noncompliant + double doubleFromLong = (double) 1L; // Noncompliant + + float namedInt = (float) SMALL_INT; // Noncompliant + float namedLong = (float) SMALL_LONG; // Noncompliant + double namedDouble = (double) SMALL_LONG; // Noncompliant + float folded = (float) (20 + 22); // Noncompliant + + float intUpperBound = (float) 16_777_216; // Noncompliant + float intLowerBound = (float) -16_777_216; // Noncompliant + float longUpperBound = (float) 16_777_216L; // Noncompliant + float longLowerBound = (float) -16_777_216L; // Noncompliant + double doubleUpperBound = (double) 9_007_199_254_740_992L; // Noncompliant + double doubleLowerBound = (double) -9_007_199_254_740_992L; // Noncompliant + + takeFloat(((float) (1))); // Noncompliant + takeFloat((float) 1L); // Noncompliant + takeDouble((double) 1L); // Noncompliant + new PrecisionLossCasts((float) 1, (float) 1L, (double) 1L); // Noncompliant 3 + } + + void lossyConstants() { + float fromInt = (float) 16_777_217; + float negativeInt = (float) -16_777_217; + float fromLong = (float) 16_777_217L; + float negativeLong = (float) -16_777_217L; + double doubleFromLong = (double) 9_007_199_254_740_993L; + double negativeDouble = (double) -9_007_199_254_740_993L; + } + + 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) {} +} 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 69a46c55fcf..94079774068 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 @@ -18,6 +18,7 @@ import java.util.Arrays; import java.util.List; import org.sonar.check.Rule; +import org.sonar.java.checks.helpers.NumericConversionUtils; import org.sonar.java.checks.helpers.QuickFixHelper; import org.sonar.java.model.ExpressionUtils; import org.sonar.java.model.JProblem; @@ -80,6 +81,9 @@ public static Tree skipParentheses(Tree tree) { } private boolean isUnnecessaryCast(TypeCastTree typeCastTree) { + if (NumericConversionUtils.isPotentiallyLossyWideningConversion(typeCastTree.expression(), typeCastTree.type().symbolType())) { + return false; + } if (skipParentheses(typeCastTree.expression()).is(Tree.Kind.NULL_LITERAL)) { Tree parentTree = skipParentheses(typeCastTree.parent()); return !parentTree.is(Tree.Kind.ARGUMENTS); 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 95bfcc29b9c..ea9c1c87bc7 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 @@ -21,6 +21,7 @@ import java.util.List; import javax.annotation.Nullable; import org.sonar.check.Rule; +import org.sonar.java.checks.helpers.NumericConversionUtils; import org.sonar.java.model.ExpressionUtils; import org.sonar.plugins.java.api.JavaFileScanner; import org.sonar.plugins.java.api.JavaFileScannerContext; @@ -42,9 +43,6 @@ @Rule(key = "S9395") public class S9395Check extends BaseTreeVisitor implements JavaFileScanner { - private static final long FLOAT_MAX_EXACT_INT = 1L << 24; - private static final long DOUBLE_MAX_EXACT_LONG = 1L << 53; - private JavaFileScannerContext context; private final Deque floatingPointReturnTypes = new LinkedList<>(); @@ -133,44 +131,13 @@ private void checkExpression(Type targetType, @Nullable ExpressionTree expr) { if (unwrapped.is(Tree.Kind.TYPE_CAST)) { return; } - Type sourceType = unwrapped.symbolType(); - if (isLossyWideningConversion(sourceType, targetType) && !isSafeConstant(unwrapped, targetType)) { + if (NumericConversionUtils.isPotentiallyLossyWideningConversion(unwrapped, targetType)) { + Type sourceType = unwrapped.symbolType(); context.reportIssue(this, unwrapped, "Explicitly cast this \"" + sourceType.name() + "\" to \"" + targetType.name() + "\" to document potential precision loss."); } } - private static boolean isLossyWideningConversion(Type sourceType, Type targetType) { - if (sourceType.isUnknown() || targetType.isUnknown()) { - return false; - } - if (sourceType.isPrimitive(Type.Primitives.INT) && targetType.isPrimitive(Type.Primitives.FLOAT)) { - return true; - } - if (sourceType.isPrimitive(Type.Primitives.LONG) && targetType.isPrimitive(Type.Primitives.FLOAT)) { - return true; - } - return sourceType.isPrimitive(Type.Primitives.LONG) && targetType.isPrimitive(Type.Primitives.DOUBLE); - } - - private static boolean isSafeConstant(ExpressionTree expr, Type targetType) { - Object constant = ExpressionUtils.resolveAsConstant(expr); - if (constant instanceof Integer intVal) { - long absValue = Math.abs((long) intVal); - return !targetType.isPrimitive(Type.Primitives.FLOAT) || absValue <= FLOAT_MAX_EXACT_INT; - } - if (constant instanceof Long longVal) { - long absValue = Math.abs(longVal); - if (targetType.isPrimitive(Type.Primitives.FLOAT)) { - return absValue <= FLOAT_MAX_EXACT_INT; - } - if (targetType.isPrimitive(Type.Primitives.DOUBLE)) { - return absValue <= DOUBLE_MAX_EXACT_LONG; - } - } - return false; - } - private static boolean isFloatingPoint(Type type) { return type.isPrimitive(Type.Primitives.FLOAT) || type.isPrimitive(Type.Primitives.DOUBLE); } diff --git a/java-checks/src/main/java/org/sonar/java/checks/helpers/NumericConversionUtils.java b/java-checks/src/main/java/org/sonar/java/checks/helpers/NumericConversionUtils.java new file mode 100644 index 00000000000..0b28dbae42d --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/helpers/NumericConversionUtils.java @@ -0,0 +1,66 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks.helpers; + +import org.sonar.java.model.ExpressionUtils; +import org.sonar.plugins.java.api.semantic.Type; +import org.sonar.plugins.java.api.tree.ExpressionTree; + +public final class NumericConversionUtils { + + private static final long FLOAT_MAX_EXACT_INT = 1L << 24; + private static final long DOUBLE_MAX_EXACT_LONG = 1L << 53; + + private NumericConversionUtils() { + } + + public static boolean isPotentiallyLossyWideningConversion(ExpressionTree expression, Type targetType) { + ExpressionTree unwrapped = ExpressionUtils.skipParentheses(expression); + return isLossyWideningConversion(unwrapped.symbolType(), targetType) && !isSafeConstant(unwrapped, targetType); + } + + private static boolean isLossyWideningConversion(Type sourceType, Type targetType) { + if (sourceType.isUnknown() || targetType.isUnknown()) { + return false; + } + if (sourceType.isPrimitive(Type.Primitives.INT) && targetType.isPrimitive(Type.Primitives.FLOAT)) { + return true; + } + if (sourceType.isPrimitive(Type.Primitives.LONG) && targetType.isPrimitive(Type.Primitives.FLOAT)) { + return true; + } + return sourceType.isPrimitive(Type.Primitives.LONG) && targetType.isPrimitive(Type.Primitives.DOUBLE); + } + + private static boolean isSafeConstant(ExpressionTree expr, Type targetType) { + Object constant = ExpressionUtils.resolveAsConstant(expr); + if (constant instanceof Integer intVal) { + long absValue = Math.abs((long) intVal); + return !targetType.isPrimitive(Type.Primitives.FLOAT) || absValue <= FLOAT_MAX_EXACT_INT; + } + if (constant instanceof Long longVal) { + long absValue = Math.abs(longVal); + if (targetType.isPrimitive(Type.Primitives.FLOAT)) { + return absValue <= FLOAT_MAX_EXACT_INT; + } + if (targetType.isPrimitive(Type.Primitives.DOUBLE)) { + return absValue <= DOUBLE_MAX_EXACT_LONG; + } + } + return false; + } +}