Look for infinite recursion in the first statement of multi-statement…

… methods. PiperOrigin-RevId: 500163360
google · Jan 6, 2023 · c06c7b8 · c06c7b8
1 parent 0f5753f
commit c06c7b8
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 3 deletions.
diff --git 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,19 +37,37 @@
 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. */
 @BugPattern(
     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<MethodInvocationTree, Void>() {

diff --git 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