Support test names by comment

conformance-test-framework/src/main/java/org/jspecify/conformance/ConformanceTestReport.java

@@ -130,7 +130,7 @@ private boolean isUnexpected(ReportedFact reportedFact) {
 
   private static void writeFact(Formatter report, Fact fact, String status) {
     report.format(
-        "%s: %s:%d %s%n", status, fact.getFile(), fact.getLineNumber(), fact.getFactText());
+        "%s: %s:%s:%s%n", status, fact.getFile(), fact.getIdentifier(), fact.getFactText());
   }
 
   /** A builder for {@link ConformanceTestReport}s. */
@@ -191,6 +191,7 @@ private Stream<ImmutableMap.Entry<ExpectedFact, ReportedFact>> matchFacts(
 
     /** Builds the report. */
     ConformanceTestReport build() {
+      expectedFactReader.checkErrors();
       return new ConformanceTestReport(
           files.build(),
           index(expectedFacts.build(), Fact::getFile),

conformance-test-framework/src/main/java/org/jspecify/conformance/ExpectedFact.java

@@ -15,17 +15,23 @@
 package org.jspecify.conformance;
 
 import static com.google.common.base.MoreObjects.toStringHelper;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap;
 import static java.util.stream.Collectors.joining;
 
+import com.google.common.base.CharMatcher;
+import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import java.nio.file.Path;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.ListIterator;
 import java.util.Map;
 import java.util.Objects;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import org.jspecify.annotations.Nullable;
 
 /**
  * An assertion that the tool behaves in a way consistent with a specific fact about a line in the
@@ -47,11 +53,14 @@ public final class ExpectedFact extends Fact {
           Pattern.compile("test:irrelevant-annotation:\\S+"),
           Pattern.compile("test:sink-type:[^:]+:.*"));
 
+  private final @Nullable String testName;
   private final String factText;
   private final long factLineNumber;
 
-  ExpectedFact(Path file, long lineNumber, String factText, long factLineNumber) {
+  ExpectedFact(
+      Path file, long lineNumber, @Nullable String testName, String factText, long factLineNumber) {
     super(file, lineNumber);
+    this.testName = testName;
     this.factText = factText;
     this.factLineNumber = factLineNumber;
   }
@@ -73,6 +82,11 @@ long getFactLineNumber() {
     return factLineNumber;
   }
 
+  @Override
+  public String getIdentifier() {
+    return testName == null ? super.getIdentifier() : testName;
+  }
+
   @Override
   public boolean equals(Object o) {
     if (this == o) {
@@ -84,19 +98,21 @@ public boolean equals(Object o) {
     ExpectedFact that = (ExpectedFact) o;
     return this.getFile().equals(that.getFile())
         && this.getLineNumber() == that.getLineNumber()
+        && Objects.equals(this.testName, that.testName)
         && this.factText.equals(that.factText)
         && this.factLineNumber == that.factLineNumber;
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(getFile(), getLineNumber(), factText);
+    return Objects.hash(getFile(), getLineNumber(), testName, factText);
   }
 
   @Override
   public String toString() {
     return toStringHelper(this)
         .add("file", getFile())
+        .add("testName", testName)
         .add("lineNumber", getLineNumber())
         .add("factText", factText)
         .add("factLineNumber", factLineNumber)
@@ -108,35 +124,96 @@ static class Reader {
 
     private static final Pattern EXPECTATION_COMMENT =
         Pattern.compile(
-            "\\s*// "
+            "\\s*// ("
+                + "(test:name:(?<testName>.*))"
+                + "|"
                 + ("(?<fact>"
                     + FACT_PATTERNS.stream().map(Pattern::pattern).collect(joining("|"))
                     + ")")
-                + "\\s*");
+                + ")\\s*");
+
+    private static final CharMatcher ASCII_DIGIT = CharMatcher.inRange('0', '9');
+
+    private final Map<Long, String> facts = new HashMap<>();
+    private final List<String> errors = new ArrayList<>();
+
+    private Path file;
+    private @Nullable String testName;
+    private long lineNumber;
 
     /** Reads expected facts from lines in a file. */
     ImmutableList<ExpectedFact> readExpectedFacts(Path file, List<String> lines) {
+      this.file = file;
       ImmutableList.Builder<ExpectedFact> expectedFacts = ImmutableList.builder();
-      Map<Long, String> facts = new HashMap<>();
       ListIterator<String> i = lines.listIterator();
       while (i.hasNext()) {
         String line = i.next();
-        long lineNumber = i.nextIndex();
+        lineNumber = i.nextIndex();
         Matcher matcher = EXPECTATION_COMMENT.matcher(line);
         if (matcher.matches()) {
+          setTestName(matcher.group("testName"));
           String fact = matcher.group("fact");
           if (fact != null) {
             facts.put(lineNumber, fact.trim());
           }
         } else {
+          if (testName != null) {
+            check(!facts.isEmpty(), "no expected facts for test named %s", testName);
+          }
           facts.forEach(
               (factLineNumber, factText) ->
-                  expectedFacts.add(new ExpectedFact(file, lineNumber, factText, factLineNumber)));
+                  expectedFacts.add(
+                      new ExpectedFact(file, lineNumber, testName, factText, factLineNumber)));
           facts.clear();
+          testName = null;
         }
       }
-      // TODO(netdpb): Report an error if facts is not empty.
-      return expectedFacts.build();
+      return checkUniqueTestNames(expectedFacts.build());
+    }
+
+    private void setTestName(@Nullable String testName) {
+      if (testName == null) {
+        return;
+      }
+      check(this.testName == null, "test name already set");
+      check(facts.isEmpty(), "test name must come before assertions for a line");
+      this.testName = checkTestName(testName.trim());
+    }
+
+    private boolean check(boolean test, String format, Object... args) {
+      if (!test) {
+        errors.add(String.format("  %s:%d: %s", file, lineNumber, String.format(format, args)));
+      }
+      return test;
+    }
+
+    private String checkTestName(String testName) {
+      if (check(!testName.isEmpty(), "test name cannot be empty")) {
+        check(!testName.contains(":"), "test name cannot contain a colon: %s", testName);
+        check(!ASCII_DIGIT.matchesAllOf(testName), "test name cannot be an integer: %s", testName);
+      }
+      return testName;
+    }
+
+    private ImmutableList<ExpectedFact> checkUniqueTestNames(
+        ImmutableList<ExpectedFact> expectedFacts) {
+      expectedFacts.stream()
+          .filter(ef -> ef.testName != null)
+          .collect(toImmutableSetMultimap(ef -> ef.testName, ExpectedFact::getLineNumber))
+          .asMap()
+          .forEach(
+              (testName, lineNumbers) ->
+                  check(
+                      lineNumbers.size() == 1,
+                      "test name not unique: test '%s' appears on tests of lines %s",
+                      testName,
+                      lineNumbers));
+      return expectedFacts;
+    }
+
+    /** Throws if there were any errors encountered while reading expected facts. */
+    void checkErrors() {
+      checkArgument(errors.isEmpty(), "errors in test inputs:\n%s", Joiner.on('\n').join(errors));
     }
   }
 }

conformance-test-framework/src/main/java/org/jspecify/conformance/Fact.java

@@ -39,4 +39,9 @@ final long getLineNumber() {
 
   /** The text form of the fact. */
   protected abstract String getFactText();
+
+  /** Returns an object that helps to identify this fact within a file. */
+  public String getIdentifier() {
+    return String.valueOf(getLineNumber());
+  }
 }

conformance-test-framework/src/test/java/org/jspecify/conformance/ExpectedFactTest.java

@@ -17,6 +17,7 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static java.util.Arrays.asList;
+import static org.junit.Assert.assertThrows;
 
 import com.google.common.collect.ImmutableList;
 import java.nio.file.Path;
@@ -50,12 +51,113 @@ public void readExpectedFacts() {
                 "// test:irrelevant-annotation:@Nullable ",
                 "yet another line under test"))
         .containsExactly(
-            new ExpectedFact(FILE, 3, "jspecify_nullness_mismatch", 1),
-            new ExpectedFact(FILE, 3, "jspecify_nullness_mismatch plus_other_stuff", 2),
-            new ExpectedFact(FILE, 7, "test:cannot-convert:type1 to type2", 4),
-            new ExpectedFact(FILE, 7, "test:expression-type:type2:expr2", 5),
-            new ExpectedFact(FILE, 7, "test:sink-type:type1:expr1", 6),
-            new ExpectedFact(FILE, 9, "test:irrelevant-annotation:@Nullable", 8))
+            new ExpectedFact(FILE, 3, null, "jspecify_nullness_mismatch", 1),
+            new ExpectedFact(FILE, 3, null, "jspecify_nullness_mismatch plus_other_stuff", 2),
+            new ExpectedFact(FILE, 7, null, "test:cannot-convert:type1 to type2", 4),
+            new ExpectedFact(FILE, 7, null, "test:expression-type:type2:expr2", 5),
+            new ExpectedFact(FILE, 7, null, "test:sink-type:type1:expr1", 6),
+            new ExpectedFact(FILE, 9, null, "test:irrelevant-annotation:@Nullable", 8))
         .inOrder();
+    reader.checkErrors();
+  }
+
+  @Test
+  public void readExpectedFacts_name() {
+    assertThat(
+            readExpectedFacts(
+                "// test:name: test 1 ",
+                "// jspecify_nullness_mismatch ",
+                "// jspecify_nullness_mismatch plus_other_stuff ",
+                "line under test",
+                "// test:name: test 2 ",
+                "// test:cannot-convert:type1 to type2 ",
+                "// test:expression-type:type2:expr2 ",
+                "// test:sink-type:type1:expr1 ",
+                "another line under test",
+                "// test:name: test 3 ",
+                "// test:irrelevant-annotation:@Nullable ",
+                "yet another line under test"))
+        .containsExactly(
+            new ExpectedFact(FILE, 4, "test 1", "jspecify_nullness_mismatch", 2),
+            new ExpectedFact(FILE, 4, "test 1", "jspecify_nullness_mismatch plus_other_stuff", 3),
+            new ExpectedFact(FILE, 9, "test 2", "test:cannot-convert:type1 to type2", 6),
+            new ExpectedFact(FILE, 9, "test 2", "test:expression-type:type2:expr2", 7),
+            new ExpectedFact(FILE, 9, "test 2", "test:sink-type:type1:expr1", 8),
+            new ExpectedFact(FILE, 12, "test 3", "test:irrelevant-annotation:@Nullable", 11))
+        .inOrder();
+    reader.checkErrors();
+  }
+
+  @Test
+  public void readExpectedFacts_name_empty_throws() {
+    ImmutableList<ExpectedFact> unused = readExpectedFacts("// test:name:  ");
+    assertThat(assertThrows(IllegalArgumentException.class, () -> reader.checkErrors()))
+        .hasMessageThat()
+        .contains("test name cannot be empty");
+  }
+
+  @Test
+  public void readExpectedFacts_name_allDigits_throws() {
+    ImmutableList<ExpectedFact> unused = readExpectedFacts("// test:name: 1234 ");
+    assertThat(assertThrows(IllegalArgumentException.class, () -> reader.checkErrors()))
+        .hasMessageThat()
+        .contains("test name cannot be an integer");
+  }
+
+  @Test
+  public void readExpectedFacts_name_colon_throws() {
+    ImmutableList<ExpectedFact> unused = readExpectedFacts("// test:name:has a : colon");
+    assertThat(assertThrows(IllegalArgumentException.class, () -> reader.checkErrors()))
+        .hasMessageThat()
+        .contains("test name cannot contain a colon");
+  }
+
+  @Test
+  public void readExpectedFacts_name_notFirst_throws() {
+    ImmutableList<ExpectedFact> unused =
+        readExpectedFacts(
+            "// test:expression-type:type:expr", //
+            "// test:name:testName",
+            "line under test");
+    assertThat(assertThrows(IllegalArgumentException.class, () -> reader.checkErrors()))
+        .hasMessageThat()
+        .contains("test name must come before assertions for a line");
+  }
+
+  @Test
+  public void readExpectedFacts_name_noFact_throws() {
+    ImmutableList<ExpectedFact> unused =
+        readExpectedFacts(
+            "// test:name:testName", //
+            "line under test");
+    assertThat(assertThrows(IllegalArgumentException.class, () -> reader.checkErrors()))
+        .hasMessageThat()
+        .contains("no expected facts");
+  }
+
+  @Test
+  public void readExpectedFacts_name_second_name() {
+    ImmutableList<ExpectedFact> unused =
+        readExpectedFacts(
+            "// test:name:testName1", //
+            "// test:name:testName2");
+    assertThat(assertThrows(IllegalArgumentException.class, () -> reader.checkErrors()))
+        .hasMessageThat()
+        .contains("test name already set");
+  }
+
+  @Test
+  public void readExpectedFacts_name_notUnique() {
+    ImmutableList<ExpectedFact> unused =
+        readExpectedFacts(
+            "// test:name:testName",
+            " // test:expression-type:type1:expr1",
+            "line 1 under test",
+            "// test:name:testName",
+            " // test:expression-type:type2:expr2",
+            "line 2 under test");
+    assertThat(assertThrows(IllegalArgumentException.class, () -> reader.checkErrors()))
+        .hasMessageThat()
+        .contains("test name not unique: test 'testName' appears on tests of lines [3, 6]");
   }
 }

tests/ConformanceTest-report.txt

@@ -1,20 +1,20 @@
 # 12 pass; 7 fail; 19 total; 63.2% score
-PASS: Basic.java:28 test:expression-type:Object?:nullable
-PASS: Basic.java:28 test:sink-type:Object!:return
-PASS: Basic.java:28 test:cannot-convert:Object? to Object!
-PASS: Basic.java:34 test:expression-type:Object!:nonNull
-PASS: Basic.java:34 test:sink-type:Object?:return
-PASS: Basic.java:41 test:sink-type:Object?:nullableObject
-PASS: Basic.java:43 test:sink-type:String!:testSinkType#nonNullString
-FAIL: Basic.java:49 test:expression-type:List!<capture of ? extends String?>:nullableStrings
+PASS: Basic.java:28:test:expression-type:Object?:nullable
+PASS: Basic.java:28:test:sink-type:Object!:return
+PASS: Basic.java:28:test:cannot-convert:Object? to Object!
+PASS: Basic.java:34:test:expression-type:Object!:nonNull
+PASS: Basic.java:34:test:sink-type:Object?:return
+PASS: Basic.java:41:test:sink-type:Object?:nullableObject
+PASS: Basic.java:43:test:sink-type:String!:testSinkType#nonNullString
+FAIL: Basic.java:49:test:expression-type:List!<capture of ? extends String?>:nullableStrings
 PASS: Basic.java: no unexpected facts
-PASS: Irrelevant.java:28 test:irrelevant-annotation:Nullable
-PASS: Irrelevant.java:34 test:irrelevant-annotation:Nullable
-FAIL: Irrelevant.java:38 test:irrelevant-annotation:NonNull
-FAIL: Irrelevant.java:43 test:irrelevant-annotation:NonNull
-FAIL: Irrelevant.java:43 test:irrelevant-annotation:Nullable
-FAIL: Irrelevant.java:47 test:irrelevant-annotation:NullMarked
-FAIL: Irrelevant.java:49 test:irrelevant-annotation:NullUnmarked
+PASS: Irrelevant.java:28:test:irrelevant-annotation:Nullable
+PASS: Irrelevant.java:34:test:irrelevant-annotation:Nullable
+FAIL: Irrelevant.java:38:test:irrelevant-annotation:NonNull
+FAIL: Irrelevant.java:43:test:irrelevant-annotation:NonNull
+FAIL: Irrelevant.java:43:test:irrelevant-annotation:Nullable
+FAIL: Irrelevant.java:47:test:irrelevant-annotation:NullMarked
+FAIL: Irrelevant.java:49:test:irrelevant-annotation:NullUnmarked
 FAIL: Irrelevant.java: no unexpected facts
-PASS: UsesDep.java:24 test:cannot-convert:null? to Dep*
+PASS: UsesDep.java:24:test:cannot-convert:null? to Dep*
 PASS: UsesDep.java: no unexpected facts