diff --git a/documentation-support/pom.xml b/documentation-support/pom.xml
index 373b2ce900..05d1746adc 100644
--- a/documentation-support/pom.xml
+++ b/documentation-support/pom.xml
@@ -33,6 +33,10 @@
error_prone_test_helpers
test
+
+ ${project.groupId}
+ error-prone-utils
+
com.fasterxml.jackson.core
jackson-annotations
@@ -72,6 +76,11 @@
assertj-core
test
+
+ org.jetbrains
+ annotations
+ test
+
org.jspecify
jspecify
diff --git a/documentation-support/src/main/java/tech/picnic/errorprone/documentation/BugPatternTestExtractor.java b/documentation-support/src/main/java/tech/picnic/errorprone/documentation/BugPatternTestExtractor.java
index 7ca51f683a..0810f624a5 100644
--- a/documentation-support/src/main/java/tech/picnic/errorprone/documentation/BugPatternTestExtractor.java
+++ b/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 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');
}
}
diff --git a/documentation-support/src/test/java/tech/picnic/errorprone/documentation/Compilation.java b/documentation-support/src/test/java/tech/picnic/errorprone/documentation/Compilation.java
index 452b5d2df5..8e1273e70e 100644
--- a/documentation-support/src/test/java/tech/picnic/errorprone/documentation/Compilation.java
+++ b/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 options, JavaFileObject javaFileObject) {
diff --git a/error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/TestHelperSourceFormat.java b/error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/TestHelperSourceFormat.java
index c06a8387ab..603de6b704 100644
--- a/error-prone-guidelines/src/main/java/tech/picnic/errorprone/guidelines/bugpatterns/TestHelperSourceFormat.java
+++ b/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 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 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 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 appear that they {@link SourceVersion#supportsTextBlocks(Context) aren't}.
diff --git a/error-prone-guidelines/src/test/java/tech/picnic/errorprone/guidelines/bugpatterns/TestHelperSourceFormatTest.java b/error-prone-guidelines/src/test/java/tech/picnic/errorprone/guidelines/bugpatterns/TestHelperSourceFormatTest.java
index c0c1a11f2c..317540eb94 100644
--- a/error-prone-guidelines/src/test/java/tech/picnic/errorprone/guidelines/bugpatterns/TestHelperSourceFormatTest.java
+++ b/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())
diff --git a/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/SourceCode.java b/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/SourceCode.java
index a79310a00d..b9dc4e2958 100644
--- a/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/SourceCode.java
+++ b/error-prone-utils/src/main/java/tech/picnic/errorprone/utils/SourceCode.java
@@ -10,6 +10,7 @@
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;
@@ -17,6 +18,7 @@
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 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.
diff --git a/pom.xml b/pom.xml
index 1101e2729b..30e2e8bd68 100644
--- a/pom.xml
+++ b/pom.xml
@@ -444,6 +444,11 @@
value-annotations
2.10.1
+
+ org.jetbrains
+ annotations
+ 24.1.0
+
org.jspecify
jspecify