Skip to content

Commit

Permalink
Introduce RefasterRuleTestExtractor for documentation generation
Browse files Browse the repository at this point in the history
This new `Extractor` implementation collects Refaster example input and
output code from rule collection tests.

This change also introduces explicit compilation steps for the test
code. As a side-effect this produces faster feedback in case of invalid
input or output code.
  • Loading branch information
Stephan202 committed Aug 31, 2024
1 parent f6e9dbb commit c8c5107
Show file tree
Hide file tree
Showing 11 changed files with 455 additions and 46 deletions.
9 changes: 9 additions & 0 deletions documentation-support/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,15 @@
<artifactId>error_prone_test_helpers</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>error-prone-utils</artifactId>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>refaster-test-support</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-annotations</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import java.util.List;
import java.util.Optional;
import org.jspecify.annotations.Nullable;
import tech.picnic.errorprone.documentation.BugPatternTestExtractor.TestCases;
import tech.picnic.errorprone.documentation.BugPatternTestExtractor.BugPatternTestCases;

/**
* An {@link Extractor} that describes how to extract data from classes that test a {@code
Expand All @@ -40,7 +40,7 @@
@Immutable
@AutoService(Extractor.class)
@SuppressWarnings("rawtypes" /* See https://github.com/google/auto/issues/870. */)
public final class BugPatternTestExtractor implements Extractor<TestCases> {
public final class BugPatternTestExtractor implements Extractor<BugPatternTestCases> {
/** Instantiates a new {@link BugPatternTestExtractor} instance. */
public BugPatternTestExtractor() {}

Expand All @@ -50,7 +50,7 @@ public String identifier() {
}

@Override
public Optional<TestCases> tryExtract(ClassTree tree, VisitorState state) {
public Optional<BugPatternTestCases> tryExtract(ClassTree tree, VisitorState state) {
BugPatternTestCollector collector = new BugPatternTestCollector();

collector.scan(tree, state);
Expand All @@ -59,7 +59,7 @@ public Optional<TestCases> tryExtract(ClassTree tree, VisitorState state) {
.filter(not(ImmutableList::isEmpty))
.map(
tests ->
new AutoValue_BugPatternTestExtractor_TestCases(
new AutoValue_BugPatternTestExtractor_BugPatternTestCases(
state.getPath().getCompilationUnit().getSourceFile().toUri(),
ASTHelpers.getSymbol(tree).className(),
tests));
Expand Down Expand Up @@ -95,10 +95,10 @@ private static final class BugPatternTestCollector
.onDescendantOf("com.google.errorprone.BugCheckerRefactoringTestHelper.ExpectOutput")
.namedAnyOf("addOutputLines", "expectUnchanged");

private final List<TestCase> collectedTestCases = new ArrayList<>();
private final List<BugPatternTestCase> collectedBugPatternTestCases = new ArrayList<>();

private ImmutableList<TestCase> getCollectedTests() {
return ImmutableList.copyOf(collectedTestCases);
private ImmutableList<BugPatternTestCase> getCollectedTests() {
return ImmutableList.copyOf(collectedBugPatternTestCases);
}

@Override
Expand All @@ -110,14 +110,14 @@ private ImmutableList<TestCase> getCollectedTests() {
classUnderTest -> {
List<TestEntry> entries = new ArrayList<>();
if (isReplacementTest) {
extractReplacementTestCases(node, entries, state);
extractReplacementBugPatternTestCases(node, entries, state);
} else {
extractIdentificationTestCases(node, entries, state);
extractIdentificationBugPatternTestCases(node, entries, state);
}

if (!entries.isEmpty()) {
collectedTestCases.add(
new AutoValue_BugPatternTestExtractor_TestCase(
collectedBugPatternTestCases.add(
new AutoValue_BugPatternTestExtractor_BugPatternTestCase(
classUnderTest, ImmutableList.copyOf(entries).reverse()));
}
});
Expand All @@ -140,7 +140,7 @@ private static Optional<String> getClassUnderTest(
: Optional.empty();
}

private static void extractIdentificationTestCases(
private static void extractIdentificationBugPatternTestCases(
MethodInvocationTree tree, List<TestEntry> sink, VisitorState state) {
if (IDENTIFICATION_SOURCE_LINES.matches(tree, state)) {
String path = ASTHelpers.constValue(tree.getArguments().get(0), String.class);
Expand All @@ -155,11 +155,11 @@ private static void extractIdentificationTestCases(

ExpressionTree receiver = ASTHelpers.getReceiver(tree);
if (receiver instanceof MethodInvocationTree methodInvocation) {
extractIdentificationTestCases(methodInvocation, sink, state);
extractIdentificationBugPatternTestCases(methodInvocation, sink, state);
}
}

private static void extractReplacementTestCases(
private static void extractReplacementBugPatternTestCases(
MethodInvocationTree tree, List<TestEntry> sink, VisitorState state) {
if (REPLACEMENT_OUTPUT_SOURCE_LINES.matches(tree, state)) {
/*
Expand All @@ -185,7 +185,7 @@ private static void extractReplacementTestCases(

ExpressionTree receiver = ASTHelpers.getReceiver(tree);
if (receiver instanceof MethodInvocationTree methodInvocation) {
extractReplacementTestCases(methodInvocation, sink, state);
extractReplacementBugPatternTestCases(methodInvocation, sink, state);
}
}

Expand All @@ -208,24 +208,26 @@ private static Optional<String> getSourceCode(MethodInvocationTree tree) {
}

@AutoValue
@JsonDeserialize(as = AutoValue_BugPatternTestExtractor_TestCases.class)
abstract static class TestCases {
static TestCases create(URI source, String testClass, ImmutableList<TestCase> testCases) {
return new AutoValue_BugPatternTestExtractor_TestCases(source, testClass, testCases);
@JsonDeserialize(as = AutoValue_BugPatternTestExtractor_BugPatternTestCases.class)
abstract static class BugPatternTestCases {
static BugPatternTestCases create(
URI source, String testClass, ImmutableList<BugPatternTestCase> testCases) {
return new AutoValue_BugPatternTestExtractor_BugPatternTestCases(
source, testClass, testCases);
}

abstract URI source();

abstract String testClass();

abstract ImmutableList<TestCase> testCases();
abstract ImmutableList<BugPatternTestCase> testCases();
}

@AutoValue
@JsonDeserialize(as = AutoValue_BugPatternTestExtractor_TestCase.class)
abstract static class TestCase {
static TestCase create(String classUnderTest, ImmutableList<TestEntry> entries) {
return new AutoValue_BugPatternTestExtractor_TestCase(classUnderTest, entries);
@JsonDeserialize(as = AutoValue_BugPatternTestExtractor_BugPatternTestCase.class)
abstract static class BugPatternTestCase {
static BugPatternTestCase create(String classUnderTest, ImmutableList<TestEntry> entries) {
return new AutoValue_BugPatternTestExtractor_BugPatternTestCase(classUnderTest, entries);
}

abstract String classUnderTest();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
package tech.picnic.errorprone.documentation;

import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.errorprone.matchers.Matchers.isSubtypeOf;
import static java.util.stream.Collectors.joining;

import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.google.auto.service.AutoService;
import com.google.auto.value.AutoValue;
import com.google.common.base.Splitter;
import com.google.common.base.Supplier;
import com.google.common.base.VerifyException;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.VisitorState;
import com.google.errorprone.annotations.FormatMethod;
import com.google.errorprone.annotations.Immutable;
import com.google.errorprone.matchers.Matcher;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.MethodTree;
import java.net.URI;
import java.util.Optional;
import java.util.regex.Pattern;
import tech.picnic.errorprone.documentation.RefasterRuleCollectionTestExtractor.RefasterTestCases;
import tech.picnic.errorprone.utils.SourceCode;

/**
* An {@link Extractor} that describes how to extract data from Refaster rule input and output test
* classes.
*/
// XXX: Drop this extractor if/when the Refaster test framework is reimlemented such that tests can
// be located alongside rules, rather than in two additional resource files as currently required by
// `RefasterRuleCollection`.
@Immutable
@AutoService(Extractor.class)
@SuppressWarnings("rawtypes" /* See https://github.com/google/auto/issues/870. */)
public final class RefasterRuleCollectionTestExtractor implements Extractor<RefasterTestCases> {
private static final Matcher<ClassTree> IS_REFASTER_RULE_COLLECTION_TEST_CASE =
isSubtypeOf("tech.picnic.errorprone.refaster.test.RefasterRuleCollectionTestCase");
private static final Pattern TEST_CLASS_NAME_PATTERN = Pattern.compile("(.*)Test");
private static final Pattern TEST_CLASS_FILE_NAME_PATTERN =
Pattern.compile(".*(Input|Output)\\.java");
private static final Pattern TEST_METHOD_NAME_PATTERN = Pattern.compile("test(.*)");
private static final String LINE_SEPARATOR = "\n";
private static final Splitter LINE_SPLITTER = Splitter.on(LINE_SEPARATOR);

/** Instantiates a new {@link RefasterRuleCollectionTestExtractor} instance. */
public RefasterRuleCollectionTestExtractor() {}

@Override
public String identifier() {
return "refaster-rule-collection-test";
}

@Override
public Optional<RefasterTestCases> tryExtract(ClassTree tree, VisitorState state) {
if (!IS_REFASTER_RULE_COLLECTION_TEST_CASE.matches(tree, state)) {
return Optional.empty();
}

URI sourceFile = state.getPath().getCompilationUnit().getSourceFile().toUri();
return Optional.of(
RefasterTestCases.create(
sourceFile,
getRuleCollectionName(tree),
isInputFile(sourceFile),
getRefasterTestCases(tree, state)));
}

private static String getRuleCollectionName(ClassTree tree) {
String className = tree.getSimpleName().toString();

// XXX: Instead of throwing an error here, it'd be nicer to have a bug checker validate key
// aspects of `RefasterRuleCollectionRefasterTestCase` subtypes.
return tryExtractPatternGroup(className, TEST_CLASS_NAME_PATTERN)
.orElseThrow(
violation(
"Refaster rule collection test class name '%s' does not match '%s'",
className, TEST_CLASS_NAME_PATTERN));
}

private static boolean isInputFile(URI sourceFile) {
String path = sourceFile.getPath();

// XXX: Instead of throwing an error here, it'd be nicer to have a bug checker validate key
// aspects of `RefasterRuleCollectionRefasterTestCase` subtypes.
return "Input"
.equals(
tryExtractPatternGroup(path, TEST_CLASS_FILE_NAME_PATTERN)
.orElseThrow(
violation(
"Refaster rule collection test file name '%s' does not match '%s'",
path, TEST_CLASS_FILE_NAME_PATTERN)));
}

private static ImmutableList<RefasterTestCase> getRefasterTestCases(
ClassTree tree, VisitorState state) {
return tree.getMembers().stream()
.filter(MethodTree.class::isInstance)
.map(MethodTree.class::cast)
.flatMap(m -> tryExtractRefasterTestCase(m, state).stream())
.collect(toImmutableList());
}

private static Optional<RefasterTestCase> tryExtractRefasterTestCase(
MethodTree method, VisitorState state) {
return tryExtractPatternGroup(method.getName().toString(), TEST_METHOD_NAME_PATTERN)
.map(name -> RefasterTestCase.create(name, getFormattedSource(method, state)));
}

/**
* Returns the source code for the specified method.
*
* @implNote This operation attempts to trim leading whitespace, such that the start and end of
* the method declaration are aligned. The implemented heuristic assumes that the code is
* formatted using Google Java Format.
*/
// XXX: Leading Javadoc and other comments are currently not extracted. Consider fixing this.
private static String getFormattedSource(MethodTree method, VisitorState state) {
String source = SourceCode.treeToString(method, state);
int finalNewline = source.lastIndexOf(LINE_SEPARATOR);
if (finalNewline < 0) {

Check warning on line 121 in documentation-support/src/main/java/tech/picnic/errorprone/documentation/RefasterRuleCollectionTestExtractor.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 121 without causing a test to fail

changed conditional boundary (covered by 2 tests ConditionalsBoundaryMutator)
return source;
}

int indentation = source.substring(finalNewline).lastIndexOf(' ');
String prefixToStrip = " ".repeat(indentation);

Check warning on line 126 in documentation-support/src/main/java/tech/picnic/errorprone/documentation/RefasterRuleCollectionTestExtractor.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 126 without causing a test to fail

removed call to repeat (covered by 2 tests RemoveChainedCallsMutator)

return LINE_SPLITTER
.splitToStream(source)
.map(line -> line.startsWith(prefixToStrip) ? line.substring(indentation) : line)
.collect(joining(LINE_SEPARATOR));
}

private static Optional<String> tryExtractPatternGroup(String input, Pattern pattern) {
java.util.regex.Matcher matcher = pattern.matcher(input);
return matcher.matches() ? Optional.of(matcher.group(1)) : Optional.empty();
}

@FormatMethod
private static Supplier<VerifyException> violation(String format, Object... args) {
return () -> new VerifyException(String.format(format, args));
}

@AutoValue
@JsonDeserialize(as = AutoValue_RefasterRuleCollectionTestExtractor_RefasterTestCases.class)
abstract static class RefasterTestCases {
static RefasterTestCases create(
URI source,
String ruleCollection,
boolean isInput,
ImmutableList<RefasterTestCase> testCases) {
return new AutoValue_RefasterRuleCollectionTestExtractor_RefasterTestCases(
source, ruleCollection, isInput, testCases);
}

abstract URI source();

abstract String ruleCollection();

abstract boolean isInput();

abstract ImmutableList<RefasterTestCase> testCases();
}

@AutoValue
@JsonDeserialize(as = AutoValue_RefasterRuleCollectionTestExtractor_RefasterTestCase.class)
abstract static class RefasterTestCase {
static RefasterTestCase create(String name, String content) {
return new AutoValue_RefasterRuleCollectionTestExtractor_RefasterTestCase(name, content);

Check warning on line 169 in documentation-support/src/main/java/tech/picnic/errorprone/documentation/RefasterRuleCollectionTestExtractor.java

View workflow job for this annotation

GitHub Actions / pitest

A change can be made to line 169 without causing a test to fail

swapped parameters 1 and 2 in call to <init> (covered by 2 tests ParamSwapMutator)
}

abstract String name();

abstract String content();
}
}
Loading

0 comments on commit c8c5107

Please sign in to comment.