Skip to content

Commit

Permalink
RELNOTES: Changed MathRoundIntLong's behavior with longs from casting…
Browse files Browse the repository at this point in the history
… to an int to using Guava's Ints.saturatedCast().

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=201705887
  • Loading branch information
seibelsabrina authored and cushon committed Jun 29, 2018
1 parent c01d43f commit 90d4d9e
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 34 deletions.
Expand Up @@ -27,7 +27,6 @@
import com.google.errorprone.BugPattern.ProvidesFix;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.fixes.Fix;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
Expand Down Expand Up @@ -58,23 +57,51 @@ public final class MathRoundIntLong extends BugChecker implements MethodInvocati
allOf(
MATH_ROUND_CALLS, argument(0, anyOf(isSameType("int"), isSameType("java.lang.Integer"))));

private static final Matcher<MethodInvocationTree> ROUND_CALLS_WITH_PRIMITIVE_LONG_ARG =
allOf(MATH_ROUND_CALLS, argument(0, isSameType("long")));

private static final Matcher<MethodInvocationTree> ROUND_CALLS_WITH_LONG_ARG =
allOf(MATH_ROUND_CALLS, argument(0, isSameType("java.lang.Long")));
allOf(MATH_ROUND_CALLS, argument(0, anyOf(isSameType("long"), isSameType("java.lang.Long"))));

private static final Matcher<MethodInvocationTree> ROUND_CALLS_WITH_INT_OR_LONG_ARG =
anyOf(
ROUND_CALLS_WITH_INT_ARG, ROUND_CALLS_WITH_PRIMITIVE_LONG_ARG, ROUND_CALLS_WITH_LONG_ARG);
anyOf(ROUND_CALLS_WITH_INT_ARG, ROUND_CALLS_WITH_LONG_ARG);

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
return ROUND_CALLS_WITH_INT_OR_LONG_ARG.matches(tree, state)
? describeMatch(tree, removeMathRoundCall(tree, state))
? removeMathRoundCall(tree, state)
: Description.NO_MATCH;
}

private Description removeMathRoundCall(MethodInvocationTree tree, VisitorState state) {
if (ROUND_CALLS_WITH_INT_ARG.matches(tree, state)) {
return describeMatch(
tree, SuggestedFix.replace(tree, state.getSourceForNode(tree.getArguments().get(0))));
} else if (ROUND_CALLS_WITH_LONG_ARG.matches(tree, state)) {
if (state.getTypeFromString("com.google.common.primitives.Ints") != null) {
return describeMatch(
tree,
SuggestedFix.builder()
.addImport("com.google.common.primitives.Ints")
.prefixWith(tree, "Ints.saturatedCast(")
.replace(tree, state.getSourceForNode(tree.getArguments().get(0)))
.postfixWith(tree, ")")
.build());
} else {
return buildDescription(tree)
.setMessage(
"Calling Math.round() when the argument is an integer or a long can result "
+ "in a loss of information because it coerces the argument to a float before "
+ "rounding back to an int. We suggest replacing Math.round() with Guava's"
+ " Ints.saturatedCast(). Another suggestion is to cast to an integer but that "
+ " may cause other errors when dealing with large numbers.")
.addFix(
SuggestedFix.builder()
.prefixWith(tree, castPrimitive(tree, state.getSourceForNode(tree)))
.build())
.build();
}
}
throw new AssertionError("Unknown argument type to round call: " + tree);
}

private static String castPrimitive(Tree tree, String sourceForNode) {
String openingBracket;
String closingBracket;
Expand All @@ -86,24 +113,4 @@ private static String castPrimitive(Tree tree, String sourceForNode) {
}
return String.format("%s%s%s", openingBracket, sourceForNode, closingBracket);
}

private static Fix removeMathRoundCall(MethodInvocationTree tree, VisitorState state) {
if (ROUND_CALLS_WITH_INT_ARG.matches(tree, state)) {
return SuggestedFix.replace(tree, state.getSourceForNode(tree.getArguments().get(0)));
} else if (ROUND_CALLS_WITH_PRIMITIVE_LONG_ARG.matches(tree, state)) {
return SuggestedFix.builder()
.replace(
tree,
castPrimitive(
tree.getArguments().get(0), state.getSourceForNode(tree.getArguments().get(0))))
.prefixWith(tree, "(int) ")
.build();
} else if (ROUND_CALLS_WITH_LONG_ARG.matches(tree, state)) {
return SuggestedFix.builder()
.replace(tree, state.getSourceForNode(tree.getArguments().get(0)))
.postfixWith(tree, ".intValue()")
.build();
}
throw new AssertionError("Unknown argument type to round call: " + tree);
}
}
Expand Up @@ -70,7 +70,7 @@ public void deleteRoundMethodIntClass() throws Exception {
}

@Test
public void deleteRoundMethodLong() throws Exception {
public void replaceRoundMethodLong() throws Exception {
helper
.addInputLines(
"Test.java", //
Expand All @@ -82,17 +82,18 @@ public void deleteRoundMethodLong() throws Exception {
"}")
.addOutputLines(
"Test.java", //
"import com.google.common.primitives.Ints;",
"class Test {",
" void f() {",
" long l = 3L;",
" int y = (int) l;",
" int y = Ints.saturatedCast(l);",
" }",
"}")
.doTest();
}

@Test
public void deleteRoundMethodLongClass() throws Exception {
public void replaceRoundMethodLongClass() throws Exception {
helper
.addInputLines(
"Test.java", //
Expand All @@ -104,10 +105,11 @@ public void deleteRoundMethodLongClass() throws Exception {
"}")
.addOutputLines(
"Test.java", //
"import com.google.common.primitives.Ints;",
"class Test {",
" void f() {",
" Long l = new Long(\"3\");",
" int y = l.intValue();",
" int y = Ints.saturatedCast(l);",
" }",
"}")
.doTest();
Expand Down Expand Up @@ -144,10 +146,11 @@ public void roundingDoubleNegative() throws Exception {
}

@Test
public void deleteRoundMethodAddParenthesis() throws Exception {
public void replaceRoundMethodAddParenthesis() throws Exception {
helper
.addInputLines(
"Test.java", //
"import com.google.common.primitives.Ints;",
"class Test {",
" void f() {",
" long l = 3L;",
Expand All @@ -157,11 +160,12 @@ public void deleteRoundMethodAddParenthesis() throws Exception {
"}")
.addOutputLines(
"Test.java", //
"import com.google.common.primitives.Ints;",
"class Test {",
" void f() {",
" long l = 3L;",
" long x = 6L;",
" int y = (int) (l/x);",
" int y = Ints.saturatedCast(l/x);",
" }",
"}")
.doTest();
Expand Down

0 comments on commit 90d4d9e

Please sign in to comment.