CompilationTestHelper: a failure cannot say which of several identical cases lost its diagnostic

[vlsi]

When a CompilationTestHelper test fails in CI, I cannot tell from the log which case in the source failed. A missing diagnostic is reported by its line number and the expected text, and a test source often holds several cases whose expected text and code line are both identical. To learn which case broke, I have to open the test file at the right commit and count lines inside a text block.

What the log shows today

Error Prone 2.50.0 (error_prone_core and error_prone_test_helpers), JUnit 4.13.2, OpenJDK 21.0.9, Maven 3.9.16. DiagnosticTestHelper on master is unchanged since 2.50.0.

package repro;

import com.google.errorprone.CompilationTestHelper;
import com.google.errorprone.bugpatterns.DeadException;
import org.junit.Test;

public class MarkerLabelTest {
  private final CompilationTestHelper helper =
      CompilationTestHelper.newInstance(DeadException.class, getClass());

  @Test
  public void identicalLinesInTwoHolders() {
    helper
        .addSourceLines(
            "Test.java",
            """
            class Test {
              static class Plain {
                void create() {
                  // BUG: Diagnostic contains: Exception created but not thrown
                  new RuntimeException();
                }
              }
              static class Shadowed {
                static class RuntimeException {}
                void create() {
                  // BUG: Diagnostic contains: Exception created but not thrown
                  new RuntimeException();
                }
              }
            }
            """)
        .doTest();
  }
}

mvn test prints:

com.google.common.truth.AssertionErrorWithFacts: 
Did not see an error on line 12 matching Exception created but not thrown. All errors:
/Test.java:5: error: [DeadException] Exception created but not thrown
      new RuntimeException();
      ^
    (see https://errorprone.info/bugpattern/DeadException)
  Did you mean 'throw new RuntimeException();'?
expected to be true

The expected text is the same on both markers. The only source line in the log is line 5, which passed, and it is textually identical to line 12, which failed. What distinguishes the two cases is the declaration of RuntimeException three lines up, and nothing in the log points at it.

The same shape in a real test: WildcardTests.wildcardCaptureLocals in NullAway has three markers reading // BUG: Diagnostic contains: dereferenced expression 'x' is @Nullable, on source lines 11, 21, and 26. All three mark x.hashCode();, and so does line 16, a control that expects no diagnostic. The cases differ only in the parameter type of the enclosing method, declared three lines above each call (Foo<? extends @Nullable Object>, Foo<? super @Nullable String>, Foo<? super String>). wildcardCaptureParameters and wildcardCaptureReturns in the same file have the same structure. A failure on line 26 tells a reader of the log that the Foo<? super String> case broke only after they open the file and count.

Relation to #6116 and #6117

#6116 reports that the helper names only the first mismatched line, and #6117 makes it name every mismatched line. After #6117, each mismatch is still identified by its line number. An unexpected diagnostic gets its source line through All errors:; a missing one does not. I am preparing a separate change that prints the source line of a missing diagnostic. That change is complementary to this request, not a substitute: in both examples above, the source line of the failing case is identical to the source line of a case that passed.

What // BUG: Diagnostic matches: offers

// BUG: Diagnostic matches: KEY together with expectErrorMessage(KEY, predicate) puts a name in the source, but the failure prints the predicate, not the key. With a lambda:

helper
    .addSourceLines(
        "Test.java",
        """
        class Test {
          static class Shadowed {
            static class RuntimeException {}
            void create() {
              // BUG: Diagnostic matches: shadowed-exception-type
              new RuntimeException();
            }
          }
        }
        """)
    .expectErrorMessage(
        "shadowed-exception-type", s -> s.contains("Exception created but not thrown"))
    .doTest();

Did not see an error on line 6 matching repro.MarkerLabelTest$$Lambda/0x0000030001392000@75459c75. There were no errors.

The key does reach the log if the predicate's toString() includes it:

private static Predicate<String> labeled(String label, String text) {
  return new Predicate<>() {
    @Override
    public boolean test(String message) {
      return message.contains(text);
    }

    @Override
    public String toString() {
      return "[" + label + "] " + text;
    }
  };
}

Did not see an error on line 6 matching [shadowed-exception-type] Exception created but not thrown. There were no errors.

That works today, and it is what I would use until something better exists. It moves the expected text out of the source and into the test method, it needs one expectErrorMessage call per label, and it gives nothing to existing Diagnostic contains: markers unless each is rewritten into this form. NullAway's test sources alone have 1533 lines containing BUG: Diagnostic.

What I am asking for

A way to give a // BUG: Diagnostic contains: marker a short name that the failure message prints next to the line number. Acceptance: for a marker named shadowed-exception-type on the case above, the failure message contains the string shadowed-exception-type on the same line as line 6. For symmetry, the same name on a Diagnostic matches: marker would be printed too.

Out of scope: markers without a name keep their current meaning and messages, and the name does not change which diagnostics match.

One possible syntax

The syntax is yours to choose; this is one shape that the current parser does not misread:

// BUG[shadowed-exception-type]: Diagnostic contains: Exception created but not thrown
new RuntimeException();

Did not see an error on line 6 [shadowed-exception-type] matching Exception created but not thrown. There were no errors.

DiagnosticTestHelper recognizes a marker with line.contains("// BUG: Diagnostic contains:") and line.contains("// BUG: Diagnostic matches:"), so a line that starts with // BUG[ matches neither. Text after the marker cannot carry the name, because it is the expected text, and the // lines that follow are further expected strings. Neither Error Prone's nor NullAway's sources contain // BUG[ today.

On 2.50.0, this form is treated as an ordinary comment. The next line is then expected to have no diagnostic, so the test fails where the diagnostic is reported. For a source holding only a Plain case with this marker:

java.lang.AssertionError: 
Saw unexpected error on line 4. All errors:
/Test.java:4: error: [DeadException] Exception created but not thrown
    new RuntimeException();
    ^
    (see https://errorprone.info/bugpattern/DeadException)
  Did you mean 'throw new RuntimeException();'?

Where the diagnostic is absent, the older helper passes the test without checking anything: a source holding only the Shadowed case, marked this way, gives Tests run: 1, Failures: 0 on 2.50.0. Anything that looks for the literal marker string would need to learn the new form as well; the MisformattedTestData change in #6131, for example, matches // BUG: Diagnostic contains: and // BUG: Diagnostic matches: literally.

A shape that keeps the literal marker intact is a name before it on the same line, such as /* shadowed-exception-type */ // BUG: Diagnostic contains: …. The current helper accepts that line as a marker and drops the prefix; on 2.50.0 its failure reads Did not see an error on line 6 matching Exception created but not thrown. There were no errors. A helper that printed the prefix would be backward compatible in both directions, at the cost of a less obvious syntax.

I found no earlier request for this; I searched the tracker, open and closed, for CompilationTestHelper, DiagnosticTestHelper, expectErrorMessage, "Diagnostic contains", "BUG: Diagnostic", and "Did not see an error on line".