Don't allow @InlineMe on methods with params names matching `arg[0-…

…9]+`. #inlineme PiperOrigin-RevId: 673502523
google · Sep 11, 2024 · fcd3939 · fcd3939
1 parent 655bdc4
commit fcd3939
Show file tree

Hide file tree

Showing 5 changed files with 83 additions and 8 deletions.
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/inlineme/InlinabilityResult.java b/core/src/main/java/com/google/errorprone/bugpatterns/inlineme/InlinabilityResult.java
@@ -92,7 +92,7 @@ static InlinabilityResult inlinable(ExpressionTree body) {
 
   boolean isValidForSuggester() {
     return isValidForValidator()
-        || error() == InlineValidationErrorReason.METHOD_CAN_BE_OVERIDDEN_BUT_CAN_BE_FIXED;
+        || error() == InlineValidationErrorReason.METHOD_CAN_BE_OVERRIDDEN_BUT_CAN_BE_FIXED;
   }
 
   boolean isValidForValidator() {
@@ -111,20 +111,21 @@ enum InlineValidationErrorReason {
     BODY_WOULD_EVALUATE_DIFFERENTLY(
         "Inlining this method will result in a change in evaluation timing for one or more"
             + " arguments to this method."),
-    METHOD_CAN_BE_OVERIDDEN_AND_CANT_BE_FIXED(
+    METHOD_CAN_BE_OVERRIDDEN_AND_CANT_BE_FIXED(
         "Methods that are inlined should not be overridable, as the implementation of an overriding"
             + " method may be different than the inlining"),
 
     // Technically an error in the case where an existing @InlineMe annotation is applied, but could
     // be fixed while suggesting
-    METHOD_CAN_BE_OVERIDDEN_BUT_CAN_BE_FIXED(
+    METHOD_CAN_BE_OVERRIDDEN_BUT_CAN_BE_FIXED(
         "Methods that are inlined should not be overridable, as the implementation of an overriding"
             + " method may be different than the inlining"),
     VARARGS_USED_UNSAFELY(
         "When using a varargs parameter, it must only be passed in the last position of a method"
             + " call to another varargs method"),
     EMPTY_VOID("InlineMe cannot yet be applied to no-op void methods"),
-    REUSE_OF_ARGUMENTS("Implementations cannot use an argument more than once:");
+    REUSE_OF_ARGUMENTS("Implementations cannot use an argument more than once:"),
+    PARAM_NAMES_ARE_NAMED_ARGN("Method parameter names cannot match `arg[0-9]+`.");
 
     private final @Nullable String errorMessage;
 
@@ -137,6 +138,10 @@ String getErrorMessage() {
     }
   }
 
+  static boolean matchesArgN(String paramName) {
+    return paramName.matches("arg[0-9]+");
+  }
+
   static InlinabilityResult forMethod(MethodTree tree, VisitorState state) {
     if (tree.getBody() == null) {
       return fromError(InlineValidationErrorReason.NO_BODY);
@@ -179,6 +184,12 @@ static InlinabilityResult forMethod(MethodTree tree, VisitorState state) {
       return fromError(InlineValidationErrorReason.VARARGS_USED_UNSAFELY, body);
     }
 
+    for (VarSymbol param : methSymbol.params()) {
+      if (matchesArgN(param.name.toString())) {
+        return fromError(InlineValidationErrorReason.PARAM_NAMES_ARE_NAMED_ARGN);
+      }
+    }
+
     // TODO(kak): declare a list of all the types we don't want to allow (e.g., ClassTree) and use
     // contains
     if (body.toString().contains("{") || body.getKind() == Kind.CONDITIONAL_EXPRESSION) {
@@ -209,8 +220,8 @@ static InlinabilityResult forMethod(MethodTree tree, VisitorState state) {
       // overridden due to having no publicly-accessible constructors.
       return fromError(
           methSymbol.isDefault()
-              ? InlineValidationErrorReason.METHOD_CAN_BE_OVERIDDEN_AND_CANT_BE_FIXED
-              : InlineValidationErrorReason.METHOD_CAN_BE_OVERIDDEN_BUT_CAN_BE_FIXED,
+              ? InlineValidationErrorReason.METHOD_CAN_BE_OVERRIDDEN_AND_CANT_BE_FIXED
+              : InlineValidationErrorReason.METHOD_CAN_BE_OVERRIDDEN_BUT_CAN_BE_FIXED,
           body);
     }
 

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/inlineme/Inliner.java b/core/src/main/java/com/google/errorprone/bugpatterns/inlineme/Inliner.java
@@ -239,7 +239,7 @@ && stringContainsComments(state.getSourceForNode(tree), state.context)) {
       String varName = varNames.get(i);
 
       // If the parameter names are missing (b/365094947), don't perform the inlining.
-      if (varName.matches("arg[0-9]+")) {
+      if (InlinabilityResult.matchesArgN(varName)) {
         return Description.NO_MATCH;
       }
 

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/inlineme/Suggester.java b/core/src/main/java/com/google/errorprone/bugpatterns/inlineme/Suggester.java
@@ -86,7 +86,7 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
                 InlineMeData.buildExpectedInlineMeAnnotation(state, inlinabilityResult.body())
                     .buildAnnotation());
     if (inlinabilityResult.error()
-        == InlineValidationErrorReason.METHOD_CAN_BE_OVERIDDEN_BUT_CAN_BE_FIXED) {
+        == InlineValidationErrorReason.METHOD_CAN_BE_OVERRIDDEN_BUT_CAN_BE_FIXED) {
       SuggestedFixes.addModifiers(tree, state, Modifier.FINAL).ifPresent(fixBuilder::merge);
     }
     return describeMatch(tree, fixBuilder.build());

diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/inlineme/SuggesterTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/inlineme/SuggesterTest.java
@@ -1046,4 +1046,49 @@ public void importStatic_getsIncorrectlySuggestedAsImportsInsteadOfStaticImports
             "}")
         .doTest();
   }
+
+  @Test
+  public void noInlineMeSuggestionWhenParameterNamesAreArgN() {
+    refactoringTestHelper
+        .addInputLines(
+            "Client.java",
+            "public final class Client {",
+            "  @Deprecated",
+            "  public void before(int arg0, int arg1) {",
+            "    after(arg0, arg1);",
+            "  }",
+            "  public void after(int arg0, int arg1) {",
+            "  }",
+            "}")
+        .expectUnchanged()
+        .doTest();
+  }
+
+  @Test
+  public void inlineMeSuggestionWhenParameterNamesAreNotArgN() {
+    refactoringTestHelper
+        .addInputLines(
+            "Client.java",
+            "public final class Client {",
+            "  @Deprecated",
+            "  public void before(int int0, int int1) {",
+            "    after(int0, int1);",
+            "  }",
+            "  public void after(int int0, int int1) {",
+            "  }",
+            "}")
+        .addOutputLines(
+            "Client.java",
+            "import com.google.errorprone.annotations.InlineMe;",
+            "public final class Client {",
+            "  @InlineMe(replacement = \"this.after(int0, int1)\")",
+            "  @Deprecated",
+            "  public void before(int int0, int int1) {",
+            "    after(int0, int1);",
+            "  }",
+            "  public void after(int int0, int int1) {",
+            "  }",
+            "}")
+        .doTest();
+  }
 }
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/inlineme/ValidatorTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/inlineme/ValidatorTest.java
@@ -795,6 +795,25 @@ public void customInlineMe() {
         .doTest();
   }
 
+  @Test
+  public void validationFailsWhenParameterNamesAreArgN() {
+    helper
+        .addSourceLines(
+            "Client.java",
+            "import com.google.errorprone.annotations.InlineMe;",
+            "public final class Client {",
+            "  @Deprecated",
+            "  @InlineMe(replacement = \"this.after(arg0, arg1)\")",
+            "  // BUG: Diagnostic contains: `arg[0-9]+`",
+            "  public void before(int arg0, int arg1) {",
+            "    after(arg0, arg1);",
+            "  }",
+            "  public void after(int arg0, int arg1) {",
+            "  }",
+            "}")
+        .doTest();
+  }
+
   private BugCheckerRefactoringTestHelper getHelperInCleanupMode() {
     return BugCheckerRefactoringTestHelper.newInstance(Validator.class, getClass())
         .setArgs("-XepOpt:" + Validator.CLEANUP_INLINE_ME_FLAG + "=true");