Skip to content

Commit

Permalink
Detect calls to scalb that should be using the double overload in…
Browse files Browse the repository at this point in the history
…stead

PiperOrigin-RevId: 711799309
  • Loading branch information
cushon authored and Error Prone Team committed Jan 3, 2025
1 parent 20b1329 commit 0d17234
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -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<ExpressionTree> 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) "));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -963,6 +964,7 @@ public static ScannerSupplier warningChecks() {
Inliner.class,
InputStreamSlowMultibyteRead.class,
InstanceOfAndCastMatchWrongType.class,
IntFloatConversion.class,
IntLongMath.class,
InvalidBlockTag.class,
InvalidInlineTag.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}
}
20 changes: 20 additions & 0 deletions docs/bugpattern/IntFloatConversion.md
Original file line number Diff line number Diff line change
@@ -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);
```

0 comments on commit 0d17234

Please sign in to comment.