diff --git a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ZeroWarmupRateLimiter.java b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ZeroWarmupRateLimiter.java index 2476d9e3b..5601517dc 100644 --- a/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ZeroWarmupRateLimiter.java +++ b/baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ZeroWarmupRateLimiter.java @@ -35,6 +35,8 @@ import com.sun.tools.javac.code.Symbol; import com.sun.tools.javac.code.Symbol.VarSymbol; import java.time.Duration; +import java.util.List; +import java.util.concurrent.TimeUnit; @AutoService(BugChecker.class) @BugPattern( @@ -46,22 +48,29 @@ + "Tracked at https://github.com/google/guava/issues/2730") public final class ZeroWarmupRateLimiter extends BugChecker implements BugChecker.MethodInvocationTreeMatcher { - private static final Matcher MATCHER = Matchers.methodInvocation( + private static final Matcher DURATION_ZERO_MATCHER = Matchers.methodInvocation( MethodMatchers.staticMethod() .onClass(RateLimiter.class.getName()) .named("create") .withParameters("double", Duration.class.getName()), MatchType.LAST, ZeroWarmupRateLimiter::isDurationZero); + private static final Matcher INT_LITERAL_ZERO_MATCHER = Matchers.allOf( + MethodMatchers.staticMethod() + .onClass(RateLimiter.class.getName()) + .named("create") + .withParameters("double", "long", TimeUnit.class.getName()), + Matchers.argument(1, ZeroWarmupRateLimiter::isIntLiteralZero)); + private static final Matcher MATCHER = + Matchers.anyOf(DURATION_ZERO_MATCHER, INT_LITERAL_ZERO_MATCHER); @Override public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + List args = tree.getArguments(); if (MATCHER.matches(tree, state)) { return buildDescription(tree) .addFix(SuggestedFix.replace( - state.getEndPosition(tree.getArguments().get(0)), - state.getEndPosition(tree.getArguments().get(1)), - "")) + state.getEndPosition(args.get(0)), state.getEndPosition(args.get(args.size() - 1)), "")) .build(); } return Description.NO_MATCH; @@ -81,4 +90,9 @@ private static boolean isDurationZero(ExpressionTree expressionTree, VisitorStat } return false; } + + private static boolean isIntLiteralZero(ExpressionTree expressionTree, VisitorState state) { + Number warmupTime = ASTHelpers.constValue(expressionTree, Number.class); + return warmupTime != null && (warmupTime.equals(0) || warmupTime.equals(0L)); + } } diff --git a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/ZeroWarmupRateLimiterTest.java b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/ZeroWarmupRateLimiterTest.java index e6bc5b0dc..2c9ccf0a0 100644 --- a/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/ZeroWarmupRateLimiterTest.java +++ b/baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/ZeroWarmupRateLimiterTest.java @@ -66,6 +66,52 @@ public void should_remove_duration_zero_static_import() { .doTest(); } + @Test + public void should_remove_int_literal_zero() { + fix().addInputLines( + "Test.java", + "import com.google.common.util.concurrent.RateLimiter;", + "import java.util.concurrent.TimeUnit;", + "class Test {", + " void f() {", + " RateLimiter.create(10, 0, TimeUnit.SECONDS);", + " }", + "}") + .addOutputLines( + "Test.java", + "import com.google.common.util.concurrent.RateLimiter;", + "import java.util.concurrent.TimeUnit;", + "class Test {", + " void f() {", + " RateLimiter.create(10);", + " }", + "}") + .doTest(); + } + + @Test + public void should_remove_long_literal_zero() { + fix().addInputLines( + "Test.java", + "import com.google.common.util.concurrent.RateLimiter;", + "import java.util.concurrent.TimeUnit;", + "class Test {", + " void f() {", + " RateLimiter.create(10, 0L, TimeUnit.SECONDS);", + " }", + "}") + .addOutputLines( + "Test.java", + "import com.google.common.util.concurrent.RateLimiter;", + "import java.util.concurrent.TimeUnit;", + "class Test {", + " void f() {", + " RateLimiter.create(10);", + " }", + "}") + .doTest(); + } + @Test public void should_not_modify_existing_uses() { fix().addInputLines( @@ -81,6 +127,36 @@ public void should_not_modify_existing_uses() { .doTest(); } + @Test + public void should_not_modify_existing_uses_int_literal() { + fix().addInputLines( + "Test.java", + "import com.google.common.util.concurrent.RateLimiter;", + "import java.util.concurrent.TimeUnit;", + "class Test {", + " void f() {", + " RateLimiter.create(10, 100, TimeUnit.SECONDS);", + " }", + "}") + .expectUnchanged() + .doTest(); + } + + @Test + public void should_not_modify_existing_int_literal_zero_permits() { + fix().addInputLines( + "Test.java", + "import com.google.common.util.concurrent.RateLimiter;", + "import java.util.concurrent.TimeUnit;", + "class Test {", + " void f() {", + " RateLimiter.create(0, 100, TimeUnit.SECONDS);", + " }", + "}") + .expectUnchanged() + .doTest(); + } + private RefactoringValidator fix() { return RefactoringValidator.of(ZeroWarmupRateLimiter.class, getClass()); } diff --git a/changelog/@unreleased/pr-1960.v2.yml b/changelog/@unreleased/pr-1960.v2.yml new file mode 100644 index 000000000..56555e93c --- /dev/null +++ b/changelog/@unreleased/pr-1960.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Improve Zero Warmup Rate Limiter to Catch Int Literals + links: + - https://github.com/palantir/gradle-baseline/pull/1960