diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/IntFloatConversion.java b/core/src/main/java/com/google/errorprone/bugpatterns/IntFloatConversion.java new file mode 100644 index 00000000000..2956a4e58b1 --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/IntFloatConversion.java @@ -0,0 +1,71 @@ +/* + * Copyright 2024 The Error Prone Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.errorprone.bugpatterns; + +import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; +import static com.google.errorprone.fixes.SuggestedFix.prefixWith; +import static com.google.errorprone.matchers.Description.NO_MATCH; +import static com.google.errorprone.util.ASTHelpers.getType; +import static com.google.errorprone.util.ASTHelpers.targetType; + +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.matchers.method.MethodMatchers; +import com.google.errorprone.util.ASTHelpers.TargetType; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.Tree; +import com.sun.tools.javac.code.TypeTag; + +/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */ +@BugPattern( + summary = + "Conversion from int to float may lose precision; use an explicit cast to float if this" + + " was intentional", + severity = WARNING) +public class IntFloatConversion extends BugChecker implements MethodInvocationTreeMatcher { + + /** + * int to float conversions aren't always problematic, this specific issue is that there are float + * and double overloads, and when passing an int the float will be resolved in situations where + * double precision may be desired. + */ + private static final Matcher MATCHER = + MethodMatchers.staticMethod() + .onClass("java.lang.Math") + .named("scalb") + .withParameters("float", "int"); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (!MATCHER.matches(tree, state)) { + return NO_MATCH; + } + Tree arg = tree.getArguments().get(0); + if (!getType(arg).hasTag(TypeTag.INT)) { + return NO_MATCH; + } + TargetType targetType = targetType(state); + if (targetType == null || !targetType.type().hasTag(TypeTag.DOUBLE)) { + return NO_MATCH; + } + return describeMatch(arg, prefixWith(arg, "(double) ")); + } +} diff --git a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java index e07059e67ef..9ddd5dc4930 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -190,6 +190,7 @@ import com.google.errorprone.bugpatterns.InputStreamSlowMultibyteRead; import com.google.errorprone.bugpatterns.InsecureCipherMode; import com.google.errorprone.bugpatterns.InstanceOfAndCastMatchWrongType; +import com.google.errorprone.bugpatterns.IntFloatConversion; import com.google.errorprone.bugpatterns.IntLongMath; import com.google.errorprone.bugpatterns.InterfaceWithOnlyStatics; import com.google.errorprone.bugpatterns.InterruptedExceptionSwallowed; @@ -963,6 +964,7 @@ public static ScannerSupplier warningChecks() { Inliner.class, InputStreamSlowMultibyteRead.class, InstanceOfAndCastMatchWrongType.class, + IntFloatConversion.class, IntLongMath.class, InvalidBlockTag.class, InvalidInlineTag.class, diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/IntFloatConversionTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/IntFloatConversionTest.java new file mode 100644 index 00000000000..71c548a49bf --- /dev/null +++ b/core/src/test/java/com/google/errorprone/bugpatterns/IntFloatConversionTest.java @@ -0,0 +1,60 @@ +/* + * Copyright 2024 The Error Prone Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.errorprone.bugpatterns; + +import com.google.errorprone.CompilationTestHelper; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class IntFloatConversionTest { + + private final CompilationTestHelper testHelper = + CompilationTestHelper.newInstance(IntFloatConversion.class, getClass()); + + @Test + public void positive() { + testHelper + .addSourceLines( + "Test.java", + """ + class Test { + double f(int x) { + // BUG: Diagnostic contains: + return Math.scalb(x, 2); + } + } + """) + .doTest(); + } + + @Test + public void negative() { + testHelper + .addSourceLines( + "Test.java", + """ + class Test { + float f(int x) { + return Math.scalb(x, 2); + } + } + """) + .doTest(); + } +} diff --git a/docs/bugpattern/IntFloatConversion.md b/docs/bugpattern/IntFloatConversion.md new file mode 100644 index 00000000000..7c0b35a68ca --- /dev/null +++ b/docs/bugpattern/IntFloatConversion.md @@ -0,0 +1,20 @@ +Implicit conversions from `int` to `float` may lose precision when when calling +methods with overloads that accept both`float` and `double`. + +For example, `Math.scalb` has overloads +[`Math.scalb(float, int)`](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/Math.html#scalb\(float,int\)) +and +[`Math.scalb(double, int)`](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/Math.html#scalb\(double,int\)). + +When passing an `int` as the first argument, `Math.scalb(float, int)` will be +selected. If the result of `Math.scalb(float, int)` is then used as a `double`, +this may result in a loss of precision. + +To avoid this, an explicit cast to `double` can be used to call the +`Match.scalb(double, int)` overload: + +```java +int x = ... +int y = ... +double f = Math.scalb((double) x, 2); +```