WIP

documentation-support/pom.xml

@@ -33,6 +33,10 @@
             <artifactId>error_prone_test_helpers</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>${project.groupId}</groupId>
+            <artifactId>error-prone-utils</artifactId>
+        </dependency>
         <dependency>
             <groupId>com.fasterxml.jackson.core</groupId>
             <artifactId>jackson-annotations</artifactId>
@@ -72,6 +76,11 @@
             <artifactId>assertj-core</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.jetbrains</groupId>
+            <artifactId>annotations</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>org.jspecify</groupId>
             <artifactId>jspecify</artifactId>

documentation-support/src/main/java/tech/picnic/errorprone/documentation/BugPatternTestExtractor.java

@@ -28,6 +28,7 @@
 import java.util.Optional;
 import org.jspecify.annotations.Nullable;
 import tech.picnic.errorprone.documentation.BugPatternTestExtractor.TestCases;
+import tech.picnic.errorprone.utils.SourceCode;
 
 /**
  * An {@link Extractor} that describes how to extract data from classes that test a {@code
@@ -189,21 +190,10 @@ private static void extractReplacementTestCases(
       }
     }
 
-    // XXX: This logic is duplicated in `ErrorProneTestSourceFormat`. Can we do better?
     private static Optional<String> getSourceCode(MethodInvocationTree tree) {
-      List<? extends ExpressionTree> sourceLines =
-          tree.getArguments().subList(1, tree.getArguments().size());
-      StringBuilder source = new StringBuilder();
-
-      for (ExpressionTree sourceLine : sourceLines) {
-        String value = ASTHelpers.constValue(sourceLine, String.class);
-        if (value == null) {
-          return Optional.empty();
-        }
-        source.append(value).append('\n');
-      }
-
-      return Optional.of(source.toString());
+      return SourceCode.joinConstantSourceCodeLines(
+              tree.getArguments().subList(1, tree.getArguments().size()))
+          .map(s -> s + '\n');
     }
   }
 

documentation-support/src/test/java/tech/picnic/errorprone/documentation/Compilation.java

@@ -14,6 +14,7 @@
 import javax.tools.Diagnostic;
 import javax.tools.JavaCompiler;
 import javax.tools.JavaFileObject;
+import org.intellij.lang.annotations.Language;
 
 // XXX: Generalize and move this class so that it can also be used by `refaster-compiler`.
 // XXX: This class is supported by the `ErrorProneTestHelperSourceFormat` check, but until that
@@ -23,12 +24,12 @@ public final class Compilation {
   private Compilation() {}
 
   public static void compileWithDocumentationGenerator(
-      Path outputDirectory, String path, String... lines) {
-    compileWithDocumentationGenerator(outputDirectory.toAbsolutePath().toString(), path, lines);
+      Path outputDirectory, String path, @Language("JAVA") String source) {
+    compileWithDocumentationGenerator(outputDirectory.toAbsolutePath().toString(), path, source);
   }
 
   public static void compileWithDocumentationGenerator(
-      String outputDirectory, String path, String... lines) {
+      String outputDirectory, String path, @Language("JAVA") String source) {
     /*
      * The compiler options specified here largely match those used by Error Prone's
      * `CompilationTestHelper`. A key difference is the stricter linting configuration. When
@@ -49,7 +50,7 @@ public static void compileWithDocumentationGenerator(
             "-Xplugin:DocumentationGenerator -XoutputDirectory=" + outputDirectory,
             "-XDdev",
             "-XDcompilePolicy=simple"),
-        FileObjects.forSourceLines(path, lines));
+        FileObjects.forSourceLines(path, source));
   }
 
   private static void compile(ImmutableList<String> options, JavaFileObject javaFileObject) {

error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/TestHelperSourceFormat.java

@@ -53,27 +53,20 @@
  * are not able to) remove imports that become obsolete as a result of applying their suggested
  * fix(es).
  */
-// XXX: The check does not flag well-formatted text blocks with insufficient indentation. Cover this
-// using an generic check or wait for Google Java Format support (see
+// XXX: The check does not flag well-formatted text blocks with insufficient or excess indentation.
+// Cover this using an generic check or wait for Google Java Format support (see
 // https://github.com/google/google-java-format/issues/883#issuecomment-1404336418).
-// XXX: The check does not flag well-formatted text blocks with excess indentation.
-// XXX: ^ Validate this claim.
-// XXX: GJF guesses the line separator to be used by inspecting the source. When using text blocks
-// this may cause the current unconditional use of `\n` not to be sufficient when building on
-// Windows; TBD.
-// XXX: Forward compatibility: ignore "malformed" code in tests that, based on an
-// `@DisabledForJreRange` or `@EnableForJreRange` annotation, target a Java runtime greater than the
-// current runtime.
 @AutoService(BugChecker.class)
 @BugPattern(
     summary =
-        "Test code should follow the Google Java style (and when targeting JDK 15+ be "
-            + "specified using a single text block)",
+        """
+        Test code should follow the Google Java style (and when targeting JDK
+        15+ be specified using a single text block)""",
     link = BUG_PATTERNS_BASE_URL + "TestHelperSourceFormat",
     linkType = CUSTOM,
     severity = SUGGESTION,
     tags = STYLE)
-// XXX: Drop suppression if/when the `avoidTextBlocks` field is dropped.
+// XXX: Drop this suppression if/when the `avoidTextBlocks` field is dropped.
 @SuppressWarnings("java:S2160" /* Super class equality definition suffices. */)
 public final class TestHelperSourceFormat extends BugChecker
     implements MethodInvocationTreeMatcher {
@@ -91,6 +84,8 @@ public final class TestHelperSourceFormat extends BugChecker
               .named("addInputLines"),
           // XXX: Add tests for `Compilation.compileWithDocumentationGenerator`. Until done, make
           // sure to update this matcher if that method's class or name is changed/moved.
+          // XXX: Alternatively, match any invocation of a method whose last argument is annotated
+          // `@Language("JAVA")`.
           staticMethod()
               .onClass("tech.picnic.errorprone.documentation.Compilation")
               .named("compileWithDocumentationGenerator"));
@@ -100,10 +95,6 @@ public final class TestHelperSourceFormat extends BugChecker
           .named("addOutputLines");
   private static final Supplier<Boolean> IS_JABEL_ENABLED =
       VisitorState.memoize(TestHelperSourceFormat::isJabelEnabled);
-  // XXX: Proper name for this?
-  // XXX: Something about tabs.
-  private static final String TEXT_BLOCK_MARKER = "\"\"\"";
-  private static final String TEXT_BLOCK_LINE_SEPARATOR = "\n";
   private static final String DEFAULT_TEXT_BLOCK_INDENTATION = " ".repeat(12);
   private static final String METHOD_SELECT_ARGUMENT_RELATIVE_INDENTATION = " ".repeat(8);
 
@@ -139,7 +130,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
     }
 
     /* Attempt to format the source code only if it fully consists of constant expressions. */
-    return getConstantSourceCode(sourceLines)
+    return SourceCode.joinConstantSourceCodeLines(sourceLines)
         .map(source -> flagFormattingIssues(sourceLines, source, isOutputSource, state))
         .orElse(Description.NO_MATCH);
   }
@@ -222,18 +213,18 @@ private static String toTextBlockExpression(
     String indentation = suggestTextBlockIndentation(tree, state);
 
     // XXX: Verify trailing """ on new line.
-    return TEXT_BLOCK_MARKER
+    return SourceCode.TEXT_BLOCK_DELIMITER
         + System.lineSeparator()
         + indentation
         + source
-            .replace(TEXT_BLOCK_LINE_SEPARATOR, System.lineSeparator() + indentation)
+            .replace(SourceCode.TEXT_BLOCK_LINE_SEPARATOR, System.lineSeparator() + indentation)
             .replace("\\", "\\\\")
-            .replace(TEXT_BLOCK_MARKER, "\"\"\\\"")
-        + TEXT_BLOCK_MARKER;
+            .replace(SourceCode.TEXT_BLOCK_DELIMITER, "\"\"\\\"")
+        + SourceCode.TEXT_BLOCK_DELIMITER;
   }
 
   private static String toLineEnumeration(String source, VisitorState state) {
-    return Splitter.on(TEXT_BLOCK_LINE_SEPARATOR)
+    return Splitter.on(SourceCode.TEXT_BLOCK_LINE_SEPARATOR)
         .splitToStream(source)
         .map(state::getConstantExpression)
         .collect(joining(", "));
@@ -268,7 +259,7 @@ private static Optional<String> getIndentation(Tree tree, String source) {
       return Optional.empty();
     }
 
-    return Optional.of(source.substring(finalNewLine + 1, startPos))
+    return Optional.of(source.substring(finalNewLine + System.lineSeparator().length(), startPos))
         .filter(CharMatcher.whitespace()::matchesAllOf);
   }
 
@@ -282,27 +273,6 @@ private static String formatSourceCode(String source, boolean retainUnusedImport
     return FORMATTER.formatSource(withOptionallyRemovedImports);
   }
 
-  // XXX: This logic is duplicated in `BugPatternTestExtractor`. Can we do better?
-  private static Optional<String> getConstantSourceCode(
-      List<? extends ExpressionTree> sourceLines) {
-    StringBuilder source = new StringBuilder();
-
-    for (ExpressionTree sourceLine : sourceLines) {
-      if (source.length() > 0) {
-        source.append(TEXT_BLOCK_LINE_SEPARATOR);
-      }
-
-      String value = ASTHelpers.constValue(sourceLine, String.class);
-      if (value == null) {
-        return Optional.empty();
-      }
-
-      source.append(value);
-    }
-
-    return Optional.of(source.toString());
-  }
-
   /**
    * Tells whether Jabel appears to be enabled, indicating that text blocks are supported, even if
    * may <em>appear</em> that they {@link SourceVersion#supportsTextBlocks(Context) aren't}.

error-prone-guidelines/src/test/java/tech/picnic/errorprone/guidelines/bugpatterns/TestHelperSourceFormatTest.java

@@ -41,14 +41,14 @@ void m() {
                     .addSourceLines(
                         "F.java",
                         ""\"
-                           class F {}
-                           ""\")
+                        class F {}
+                        ""\")
                     // BUG: Diagnostic contains: Test code should follow the Google Java style (pay attention to
                     // trailing newlines)
                     .addSourceLines(
                         "G.java",
                         ""\"
-                           class G {}""\")
+                        class G {}""\")
                     .doTest();
 
                 BugCheckerRefactoringTestHelper.newInstance(RefasterAnyOfUsage.class, getClass())

error-prone-utils/src/main/java/tech/picnic/errorprone/utils/SourceCode.java

@@ -10,13 +10,15 @@
 import com.google.common.collect.Streams;
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.util.ASTHelpers;
 import com.google.errorprone.util.ErrorProneToken;
 import com.google.errorprone.util.ErrorProneTokens;
 import com.sun.source.tree.ExpressionTree;
 import com.sun.source.tree.MethodInvocationTree;
 import com.sun.source.tree.Tree;
 import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition;
 import com.sun.tools.javac.util.Position;
+import java.util.List;
 import java.util.Optional;
 
 /**
@@ -27,8 +29,14 @@ public final class SourceCode {
   /** The complement of {@link CharMatcher#whitespace()}. */
   private static final CharMatcher NON_WHITESPACE_MATCHER = CharMatcher.whitespace().negate();
 
-  // XXX: Proper name for this?
-  private static final String TEXT_BLOCK_MARKER = "\"\"\"";
+  /** The Java syntax that indicates the start and end of a text block. */
+  public static final String TEXT_BLOCK_DELIMITER = "\"\"\"";
+
+  /**
+   * The string separating lines in a Java text block, independently of the platform on which the
+   * code is compiled.
+   */
+  public static final String TEXT_BLOCK_LINE_SEPARATOR = "\n";
 
   private SourceCode() {}
 
@@ -48,7 +56,7 @@ public static boolean isTextBlock(ExpressionTree tree, VisitorState state) {
 
     /* If the source code is unavailable then we assume that this literal is _not_ a text block. */
     String src = state.getSourceForNode(tree);
-    return src != null && src.startsWith(TEXT_BLOCK_MARKER);
+    return src != null && src.startsWith(TEXT_BLOCK_DELIMITER);
   }
 
   /**
@@ -65,6 +73,37 @@ public static String treeToString(Tree tree, VisitorState state) {
     return src != null ? src : tree.toString();
   }
 
+  /**
+   * Constructs a multi-line string by joining the given source code lines, if they all represent
+   * compile-time constants.
+   *
+   * @param sourceLines The source code lines of interest.
+   * @return A non-empty optional iff all source code lines represent compile-time constants.
+   */
+  // XXX: Test!
+  // XXX: This method doesn't add a trailing newline for the benefit of one caller, but that's
+  // perhaps a bit awkward.
+  public static Optional<String> joinConstantSourceCodeLines(
+      List<? extends ExpressionTree> sourceLines) {
+    StringBuilder source = new StringBuilder();
+
+    for (ExpressionTree sourceLine : sourceLines) {
+      if (!source.isEmpty()) {
+        // XXX: Review whether we can/should use `System.lineSeparator()` here.
+        source.append('\n');
+      }
+
+      String value = ASTHelpers.constValue(sourceLine, String.class);
+      if (value == null) {
+        return Optional.empty();
+      }
+
+      source.append(value);
+    }
+
+    return Optional.of(source.toString());
+  }
+
   /**
    * Creates a {@link SuggestedFix} for the deletion of the given {@link Tree}, including any
    * whitespace that follows it.

pom.xml

@@ -444,6 +444,11 @@
                 <artifactId>value-annotations</artifactId>
                 <version>2.10.1</version>
             </dependency>
+            <dependency>
+                <groupId>org.jetbrains</groupId>
+                <artifactId>annotations</artifactId>
+                <version>24.1.0</version>
+            </dependency>
             <dependency>
                 <groupId>org.jspecify</groupId>
                 <artifactId>jspecify</artifactId>