Create ReturnAtTheEndOfVoidFunction which requires that void functi…

…ons have no `return;` as a last statement which are a no-op.

PiperOrigin-RevId: 562545799

core/src/main/java/com/google/errorprone/bugpatterns/ReturnAtTheEndOfVoidFunction.java

@@ -0,0 +1,71 @@
+/*
+ * Copyright 2023 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.matchers.Description.NO_MATCH;
+import static com.google.errorprone.util.ASTHelpers.getType;
+
+import com.google.common.collect.Iterables;
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
+import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.matchers.Description;
+import com.sun.source.tree.BlockTree;
+import com.sun.source.tree.MethodTree;
+import com.sun.source.tree.StatementTree;
+import com.sun.tools.javac.code.Type;
+import java.util.List;
+import javax.lang.model.type.TypeKind;
+
+/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
+@BugPattern(
+    summary = "`return;` is unnecessary at the end of void methods and constructors.",
+    severity = WARNING)
+public final class ReturnAtTheEndOfVoidFunction extends BugChecker implements MethodTreeMatcher {
+
+  @Override
+  public Description matchMethod(MethodTree methodTree, VisitorState state) {
+
+    // is a constructor or has a void return type (Void should pass, as `return null;` is required)
+    Type returnType = getType(methodTree.getReturnType());
+    if (returnType != null && returnType.getKind() != TypeKind.VOID) {
+      return NO_MATCH;
+    }
+
+    // has a body (not abstract or native)
+    BlockTree body = methodTree.getBody();
+    if (body == null) {
+      return NO_MATCH;
+    }
+
+    // body is not empty
+    List<? extends StatementTree> statements = body.getStatements();
+    if (statements == null || statements.isEmpty()) {
+      return NO_MATCH;
+    }
+
+    // last statement is a return
+    StatementTree lastStatement = Iterables.getLast(statements);
+    if (lastStatement.getKind() != StatementTree.Kind.RETURN) {
+      return NO_MATCH;
+    }
+
+    return describeMatch(methodTree, SuggestedFix.delete(lastStatement));
+  }
+}

core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java

@@ -320,6 +320,7 @@
 import com.google.errorprone.bugpatterns.RequiredModifiersChecker;
 import com.google.errorprone.bugpatterns.RestrictedApiChecker;
 import com.google.errorprone.bugpatterns.RethrowReflectiveOperationExceptionAsLinkageError;
+import com.google.errorprone.bugpatterns.ReturnAtTheEndOfVoidFunction;
 import com.google.errorprone.bugpatterns.ReturnValueIgnored;
 import com.google.errorprone.bugpatterns.ReturnsNullCollection;
 import com.google.errorprone.bugpatterns.RobolectricShadowDirectlyOn;
@@ -1013,6 +1014,7 @@ public static ScannerSupplier warningChecks() {
           ReachabilityFenceUsage.class,
           ReferenceEquality.class,
           RethrowReflectiveOperationExceptionAsLinkageError.class,
+          ReturnAtTheEndOfVoidFunction.class,
           ReturnFromVoid.class,
           RobolectricShadowDirectlyOn.class,
           RxReturnValueIgnored.class,

core/src/test/java/com/google/errorprone/bugpatterns/ReturnAtTheEndOfVoidFunctionTest.java

@@ -0,0 +1,151 @@
+/*
+ * Copyright 2023 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.BugCheckerRefactoringTestHelper;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for the {@link ReturnAtTheEndOfVoidFunction}. */
+@RunWith(JUnit4.class)
+public class ReturnAtTheEndOfVoidFunctionTest {
+
+  private final BugCheckerRefactoringTestHelper helper =
+      BugCheckerRefactoringTestHelper.newInstance(ReturnAtTheEndOfVoidFunction.class, getClass());
+
+  @Test
+  public void lastReturnIsDeleted() {
+    helper
+        .addInputLines(
+            "Builder.java",
+            "package com.google.gporeba;",
+            "public final class Builder {",
+            "  public void stuff() {",
+            "    int x = 5;",
+            "    return;",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Builder.java",
+            "package com.google.gporeba;",
+            "public final class Builder {",
+            "  public void stuff() {",
+            "    int x = 5;",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void lastReturnIsNotDeleted() {
+    helper
+        .addInputLines(
+            "Builder.java",
+            "package com.google.gporeba;",
+            "public final class Builder {",
+            "  public int stuff() {",
+            "    int x = 5;",
+            "    return x;",
+            "  }",
+            "}")
+        .expectUnchanged()
+        .doTest();
+  }
+
+  @Test
+  public void returnAtDifferentPositionIsNotDeleted() {
+    helper
+        .addInputLines(
+            "Builder.java",
+            "package com.google.gporeba;",
+            "public final class Builder {",
+            "  public void stuff() {",
+            "    int x = 5;",
+            "    if(x > 2) {",
+            "     return;",
+            "    }",
+            "    int z = 2173;",
+            "  }",
+            "}")
+        .expectUnchanged()
+        .doTest();
+  }
+
+  @Test
+  public void emptyFunctionIsUnchanged() {
+    helper
+        .addInputLines(
+            "Builder.java",
+            "package com.google.gporeba;",
+            "public final class Builder {",
+            "  public void nothing() {",
+            "  }",
+            "}")
+        .expectUnchanged()
+        .doTest();
+  }
+
+  @Test
+  public void nullReturnInVoidIsUnchanged() {
+    helper
+        .addInputLines(
+            "Builder.java",
+            "package com.google.gporeba;",
+            "public final class Builder {",
+            "  public Void nothing() {",
+            "    return null;",
+            "  }",
+            "}")
+        .expectUnchanged()
+        .doTest();
+  }
+
+  @Test
+  public void constructorIsCleaned() {
+    helper
+        .addInputLines(
+            "Builder.java",
+            "package com.google.gporeba;",
+            "public final class Builder {",
+            "  public Builder() {",
+            "    return;",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Builder.java",
+            "package com.google.gporeba;",
+            "public final class Builder {",
+            "  public Builder() {",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void abstractDoesntCrash() {
+    helper
+        .addInputLines(
+            "Builder.java",
+            "package com.google.gporeba;",
+            "public abstract class Builder {",
+            "  public abstract void stuff();",
+            "}")
+        .expectUnchanged()
+        .doTest();
+  }
+}

docs/bugpattern/ReturnAtTheEndOfVoidFunction.md

@@ -0,0 +1,19 @@
+Detects no-op `return` statements in `void` functions when they occur at the end
+of the method.
+
+Instead of:
+
+```java
+public void stuff() {
+  int x = 5;
+  return;
+}
+```
+
+do:
+
+```java
+public void stuff() {
+  int x = 5;
+}
+```