Create ErrorProne to discourage nesting Modules.combine() inside Guic…

…e.createInjector()

PiperOrigin-RevId: 629831969

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

@@ -0,0 +1,67 @@
+/*
+ * Copyright 2024 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.method.MethodMatchers.staticMethod;
+import static java.util.stream.Collectors.joining;
+
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.MethodInvocationTree;
+import java.util.List;
+
+/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */
+@BugPattern(
+    summary = "Nesting Modules.combine() inside Guice.createInjector() is unnecessary.",
+    severity = WARNING)
+public class GuiceCreateInjectorWithCombineRefactor extends BugChecker
+    implements MethodInvocationTreeMatcher {
+  private static final Matcher<ExpressionTree> GUICE_CREATE_INJECTOR_METHOD =
+      staticMethod().onClass("com.google.inject.Guice").named("createInjector");
+
+  private static final Matcher<ExpressionTree> MODULES_COMBINE_METHOD =
+      staticMethod().onClass("com.google.inject.util.Modules").named("combine");
+
+  /** Matches if Guice.createInjector() is called with a Modules.combine() argument. */
+  @Override
+  public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+    if (!GUICE_CREATE_INJECTOR_METHOD.matches(tree, state)) {
+      return Description.NO_MATCH;
+    }
+
+    SuggestedFix.Builder suggestedFixBuilder = SuggestedFix.builder();
+    for (ExpressionTree arg : tree.getArguments()) {
+      if (MODULES_COMBINE_METHOD.matches(arg, state)) {
+        MethodInvocationTree combineInvocation = (MethodInvocationTree) arg;
+        List<? extends ExpressionTree> subModules = combineInvocation.getArguments();
+        String replacement =
+            subModules.stream()
+                .map(module -> state.getSourceForNode(module))
+                .collect(joining(", "));
+        suggestedFixBuilder.replace(arg, replacement);
+      }
+    }
+    var fix = suggestedFixBuilder.build();
+    return fix.isEmpty() ? Description.NO_MATCH : describeMatch(tree, fix);
+  }
+}

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

@@ -157,6 +157,7 @@
 import com.google.errorprone.bugpatterns.GetClassOnAnnotation;
 import com.google.errorprone.bugpatterns.GetClassOnClass;
 import com.google.errorprone.bugpatterns.GetClassOnEnum;
+import com.google.errorprone.bugpatterns.GuiceCreateInjectorWithCombineRefactor;
 import com.google.errorprone.bugpatterns.HashtableContains;
 import com.google.errorprone.bugpatterns.HidingField;
 import com.google.errorprone.bugpatterns.ICCProfileGetInstance;
@@ -916,6 +917,7 @@ public static ScannerSupplier warningChecks() {
           FragmentNotInstantiable.class,
           FutureReturnValueIgnored.class,
           GetClassOnEnum.class,
+          GuiceCreateInjectorWithCombineRefactor.class,
           HidingField.class,
           ICCProfileGetInstance.class,
           IdentityHashMapUsage.class,

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

@@ -0,0 +1,112 @@
+/*
+ * Copyright 2024 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;
+
+@RunWith(JUnit4.class)
+public final class GuiceCreateInjectorWithCombineRefactorTest {
+  private final BugCheckerRefactoringTestHelper refactoringTestHelper =
+      BugCheckerRefactoringTestHelper.newInstance(
+          GuiceCreateInjectorWithCombineRefactor.class, getClass());
+
+  @Test
+  public void positive() {
+    refactoringTestHelper
+        .addInputLines(
+            "Test.java",
+            "import com.google.inject.AbstractModule;",
+            "import com.google.inject.Guice;",
+            "import com.google.inject.util.Modules;",
+            "class Test {",
+            "  private class ModuleA extends AbstractModule {}",
+            "  private class ModuleB extends AbstractModule {}",
+            "  private class ModuleC extends AbstractModule {}",
+            "  public void test() {",
+            "    Guice.createInjector(",
+            "      new ModuleA(),",
+            "      Modules.combine(new ModuleB(), new ModuleC()));",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Test.java",
+            "import com.google.inject.AbstractModule;",
+            "import com.google.inject.Guice;",
+            "import com.google.inject.util.Modules;",
+            "class Test {",
+            "  private class ModuleA extends AbstractModule {}",
+            "  private class ModuleB extends AbstractModule {}",
+            "  private class ModuleC extends AbstractModule {}",
+            "  public void test() {",
+            "    Guice.createInjector(",
+            "      new ModuleA(),",
+            "      new ModuleB(),",
+            "      new ModuleC());",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void negative_whenNoCombine() {
+    refactoringTestHelper
+        .addInputLines(
+            "Test.java",
+            "import com.google.inject.AbstractModule;",
+            "import com.google.inject.Guice;",
+            "import com.google.inject.util.Modules;",
+            "class Test {",
+            "  private class ModuleA extends AbstractModule {}",
+            "  private class ModuleB extends AbstractModule {}",
+            "  private class ModuleC extends AbstractModule {}",
+            "  public void test() {",
+            "    Guice.createInjector(",
+            "      new ModuleA(),",
+            "      new ModuleB(),",
+            "      new ModuleC());",
+            "  }",
+            "}")
+        .expectUnchanged()
+        .doTest();
+  }
+
+  @Test
+  public void negative_whenNoCreateInjector() {
+    refactoringTestHelper
+        .addInputLines(
+            "Test.java",
+            "import com.google.inject.AbstractModule;",
+            "import com.google.inject.Module;",
+            "import com.google.inject.util.Modules;",
+            "class Test {",
+            "  private class ModuleA extends AbstractModule {}",
+            "  private class ModuleB extends AbstractModule {}",
+            "  private class ModuleC extends AbstractModule {}",
+            "  public void test() {",
+            "    Module extraModule = Modules.combine(",
+            "      new ModuleA(),",
+            "      new ModuleB(),",
+            "      new ModuleC());",
+            "  }",
+            "}")
+        .expectUnchanged()
+        .doTest();
+  }
+}