diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/InfiniteRecursion.java b/core/src/main/java/com/google/errorprone/bugpatterns/InfiniteRecursion.java index 1fb495ef8ca..aae145581b1 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/InfiniteRecursion.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/InfiniteRecursion.java @@ -16,11 +16,11 @@ package com.google.errorprone.bugpatterns; -import static com.google.common.collect.Iterables.getOnlyElement; import static com.google.errorprone.BugPattern.SeverityLevel.ERROR; import static com.google.errorprone.matchers.Description.NO_MATCH; import com.google.errorprone.BugPattern; +import com.google.errorprone.ErrorProneFlags; import com.google.errorprone.VisitorState; import com.google.errorprone.matchers.Description; import com.google.errorprone.util.ASTHelpers; @@ -37,6 +37,7 @@ import com.sun.source.tree.TypeCastTree; import com.sun.source.util.SimpleTreeVisitor; import com.sun.tools.javac.code.Symbol.MethodSymbol; +import javax.inject.Inject; import org.checkerframework.checker.nullness.qual.Nullable; /** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */ @@ -44,12 +45,29 @@ summary = "This method always recurses, and will cause a StackOverflowError", severity = ERROR) public class InfiniteRecursion extends BugChecker implements BugChecker.MethodTreeMatcher { + private final boolean matchFirstOfMultipleStatements; + + @Inject + public InfiniteRecursion(ErrorProneFlags flags) { + // TODO(b/264529494): Remove flag. + matchFirstOfMultipleStatements = + flags.getBoolean("InfiniteRecursion:MatchFirstOfMultipleStatements").orElse(true); + } + @Override public Description matchMethod(MethodTree tree, VisitorState state) { - if (tree.getBody() == null || tree.getBody().getStatements().size() != 1) { + if (tree.getBody() == null || tree.getBody().getStatements().isEmpty()) { + return NO_MATCH; + } + if (!matchFirstOfMultipleStatements && tree.getBody().getStatements().size() > 1) { return NO_MATCH; } - Tree statement = getOnlyElement(tree.getBody().getStatements()); + /* + * TODO(b/264529494): Match statements after the first as long as they're executed + * unconditionally. And match more than just `return` and method-invocation expression + * statements, too. + */ + Tree statement = tree.getBody().getStatements().get(0); MethodInvocationTree invocation = statement.accept( new SimpleTreeVisitor() { diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/InfiniteRecursionTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/InfiniteRecursionTest.java index 106e6951ed3..4c65b5539c2 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/InfiniteRecursionTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/InfiniteRecursionTest.java @@ -50,6 +50,21 @@ public void positive() { .doTest(); } + @Test + public void positiveMultipleStatements() { + compilationHelper + .addSourceLines( + "Test.java", + "class Test {", + " Test f() {", + " // BUG: Diagnostic contains:", + " f();", + " return this;", + " }", + "}") + .doTest(); + } + @Test public void positiveStatic() { compilationHelper @@ -102,6 +117,22 @@ public void negative() { .doTest(); } + @Test + public void negativeForNowMultipleStatements() { + compilationHelper + .addSourceLines( + "Test.java", + "class Test {", + " Test f() {", + " new Test();", + // TODO(b/264529494): Match this. + " f();", + " return this;", + " }", + "}") + .doTest(); + } + @Test public void negativeDelegate() { compilationHelper