Improvements to NamedParameterChecker

Provide a more descriptor diagnostic message that includes the
comment and formal parameter name, to be more helpful in
contexts where the suggested fix isn't surfaced.

Also accept all comment styles as long as they match the formal
parameter name, instead of reporting findings to e.g. rewrite
`/*foo*/ foo` to `/* foo= */ foo`.

MOE_MIGRATED_REVID=163704182

core/src/main/java/com/google/errorprone/bugpatterns/argumentselectiondefects/NamedParameterChecker.java

@@ -105,8 +105,9 @@ private Description matchNewClassOrMethodInvocation(
           addComment(labelledArgument, fixBuilder);
           incorrectParameterDescriptions.add(
               String.format(
-                  "%s (comment does not conform to required style)",
-                  labelledArgument.parameterName()));
+                  "`%s` should be `%s <arg>`",
+                  labelledArgument.matchedComment().comment().getText(),
+                  NamedParameterComment.toCommentText(labelledArgument.parameterName())));
           break;
         case BAD_MATCH:
           Comment badLabel = labelledArgument.matchedComment().comment();
@@ -137,8 +138,8 @@ private Description matchNewClassOrMethodInvocation(
           }
           incorrectParameterDescriptions.add(
               String.format(
-                  "%s (comment does not match formal parameter name)",
-                  labelledArgument.parameterName()));
+                  "`%s` does not match formal parameter name `%s`",
+                  badLabel.getText(), labelledArgument.parameterName()));
           break;
       }
     }

core/src/main/java/com/google/errorprone/bugpatterns/argumentselectiondefects/NamedParameterComment.java

@@ -147,7 +147,16 @@ static MatchedComment match(Commented<ExpressionTree> actual, String formal) {
             .findFirst();
 
     if (approximateMatchComment.isPresent()) {
-      return MatchedComment.create(approximateMatchComment.get(), MatchType.APPROXIMATE_MATCH);
+      // Report EXACT_MATCH for comments that don't use the recommended style (e.g. `/*foo*/`
+      // instead of `/* foo= */`), but which match the formal parameter name exactly, since it's
+      // a style nit rather than a possible correctness issue.
+      // TODO(cushon): revisit this if we standardize on the recommended comment style.
+      String text =
+          CharMatcher.anyOf("=:")
+              .trimTrailingFrom(Comments.getTextFromComment(approximateMatchComment.get()).trim());
+      return MatchedComment.create(
+          approximateMatchComment.get(),
+          text.equals(formal) ? MatchType.EXACT_MATCH : MatchType.APPROXIMATE_MATCH);
     }
 
     return MatchedComment.notAnnotated();

core/src/test/java/com/google/errorprone/bugpatterns/argumentselectiondefects/NamedParameterCheckerTest.java

@@ -108,44 +108,58 @@ public void namedParametersChecker_reformatsComment_onRequiredNamesMethod() {
   }
 
   @Test
-  public void namedParametersChecker_reformatsComment_withNoEquals() {
+  public void namedParametersChecker_tolerateComment_withNoEquals() {
     compilationHelper
         .addSourceLines(
             "Test.java",
             "abstract class Test {",
             "  abstract void target(Object param);",
             "  void test(Object arg) {",
-            "    // BUG: Diagnostic contains: target(/* param= */arg)",
             "    target(/*param*/arg);",
             "  }",
             "}")
         .doTest();
   }
 
   @Test
-  public void namedParametersChecker_reformatsComment_blockAfter() {
+  public void namedParametersChecker_toleratesMatchingComment_blockAfter() {
     compilationHelper
         .addSourceLines(
             "Test.java",
             "abstract class Test {",
             "  abstract void target(Object param);",
             "  void test(Object arg) {",
-            "    // BUG: Diagnostic contains: target(/* param= */arg)",
             "    target(arg/*param*/);",
             "  }",
             "}")
         .doTest();
   }
 
   @Test
-  public void namedParametersChecker_reformatsMatchingComment_lineAfter() {
+  public void namedParametersChecker_refactorsComment_blockAfter() {
+    compilationHelper
+        .addSourceLines(
+            "Test.java",
+            "abstract class Test {",
+            "  abstract void target(Object param);",
+            "  void test(Object arg) {",
+            "    // BUG: Diagnostic contains:",
+            "    // target(/* param= */arg)",
+            "    // `/*imprecise match for param*/` should be `/* param= */ <arg>`",
+            "    target(arg/*imprecise match for param*/);",
+            "  }",
+            "}")
+        .doTest();
+  }
+
+  @Test
+  public void namedParametersChecker_toleratesMatchingComment_lineAfter() {
     compilationHelper
         .addSourceLines(
             "Test.java",
             "abstract class Test {",
             "  abstract void target(Object param);",
             "  void test(Object arg) {",
-            "    // BUG: Diagnostic contains: target(/* param= */arg)",
             "    target(arg); //param",
             "  }",
             "}")
@@ -219,7 +233,9 @@ public void namedParametersChecker_suggestsChangeComment_whenNoMatchingNames() {
             "abstract class Test {",
             "  abstract void target(Object param1, Object param2);",
             "  void test(Object arg1, Object arg2) {",
-            "    // BUG: Diagnostic contains: target(/* param1= */arg1, arg2)",
+            "    // BUG: Diagnostic contains:",
+            "    // target(/* param1= */arg1, arg2)",
+            "    // `/* notMatching= */` does not match formal parameter name `param1`",
             "    target(/* notMatching= */arg1, arg2);",
             "  }",
             "}")