Skip to content

Commit

Permalink
Apply suggestions and minor improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
rickie committed Dec 31, 2022
1 parent 7947232 commit 7720e00
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 42 deletions.
2 changes: 1 addition & 1 deletion documentation-generator/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<artifactId>documentation-generator</artifactId>

<name>Picnic :: Error Prone Support :: Documentation Generation</name>
<description>Extracts data to generate the documentation</description>
<description>Extracts data to generate the documentation.</description>

<dependencies>
<dependency>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package tech.picnic.errorprone.plugin;

import static com.google.common.base.Preconditions.checkArgument;

import com.google.common.collect.ImmutableList;
import com.google.errorprone.BugPattern;
import com.google.errorprone.util.ASTHelpers;
Expand All @@ -19,6 +21,7 @@ public BugPatternExtractor() {}
public BugPatternData extract(ClassTree tree, TaskEvent taskEvent) {
ClassSymbol symbol = ASTHelpers.getSymbol(tree);
BugPattern annotation = symbol.getAnnotation(BugPattern.class);
checkArgument(annotation != null, "BugPattern annotation must be present");

return BugPatternData.create(
symbol.getQualifiedName().toString(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@
import com.sun.source.util.TaskEvent;

/**
* Interface implemented by a class that defines how to extract {@link T} from a given {@link
* Interface implemented by a classes that define how to extract {@link T} from a given {@link
* ClassTree}.
*
* @param <T> The resulting type of the data that is extracted.
*/
public interface DocumentationExtractor<T> {
interface DocumentationExtractor<T> {
/**
* Extracts and returns an instance of {@link T} using the provided arguments.
*
* @param tree The {@link ClassTree} to analyse and extract {@link T} from.
* @param tree The {@link ClassTree} to analyze and extract {@link T} from.
* @param taskEvent The {@link TaskEvent} containing information about the current state of the
* compilation.
* @return A non-null instance of {@link T}.
Expand All @@ -25,7 +25,7 @@ public interface DocumentationExtractor<T> {
* from the given {@link ClassTree tree}.
*
* @param tree The {@link ClassTree} to check whether documentation can be extracted or not.
* @return {@code true} iff documentation can be extracted
* @return {@code true} iff documentation can be extracted.
*/
// XXX: `JavaFileObject` will most likely be added as parameter to help identify other `DocType`s.
boolean canExtract(ClassTree tree);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package tech.picnic.errorprone.plugin;

import static com.google.common.base.Preconditions.checkArgument;

import com.google.auto.service.AutoService;
import com.sun.source.util.JavacTask;
import com.sun.source.util.Plugin;
import com.sun.tools.javac.api.BasicJavacTask;

/**
* A compiler {@link Plugin plugin} that analyzes and extracts data from files containing relevant
* information for documentation purposes.
* A compiler {@link Plugin plugin} that analyzes and extracts relevant information for
* documentation purposes from processed files.
*/
@AutoService(Plugin.class)
public final class DocumentationGenerator implements Plugin {
Expand All @@ -21,6 +23,7 @@ public String getName() {

@Override
public void init(JavacTask javacTask, String... args) {
checkArgument(args.length == 1, "Specify one output path");
javacTask.addTaskListener(
new DocumentationGeneratorTaskListener(((BasicJavacTask) javacTask).getContext(), args[0]));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,33 +24,36 @@
import javax.tools.JavaFileObject;

/**
* A {@link TaskListener} that identifies files that contain content relevant for in the
* documentation.
* A {@link TaskListener} that identifies and extracts relevant content for documentation and writes
* it to disk.
*/
final class DocumentationGeneratorTaskListener implements TaskListener {
private final Context context;
private final Path basePath;
private final ObjectMapper mapper =
private static final ObjectMapper MAPPER =
new ObjectMapper().setVisibility(PropertyAccessor.FIELD, Visibility.ANY);
private final Context context;
private final String path;
private Path basePath;

DocumentationGeneratorTaskListener(Context context, String path) {
this.context = context;
this.path = path;
this.basePath = Paths.get(path);
}

// XXX: Should we extract this method?
String docsPath = path.substring(path.indexOf('=') + 1) + File.separator + "docs";
try {
this.basePath = Files.createDirectories(Paths.get(docsPath));
} catch (IOException | InvalidPathException e) {
throw new IllegalStateException(
String.format("Error while creating directory with path '%s'", docsPath), e);
}
@Override
public void started(TaskEvent taskEvent) {
createDirectoriesForPath();
}

@Override
public void finished(TaskEvent taskEvent) {
if (taskEvent.getKind() != Kind.ANALYZE) {
return;
}

ClassTree classTree = JavacTrees.instance(context).getTree(taskEvent.getTypeElement());
JavaFileObject sourceFile = taskEvent.getSourceFile();
if (classTree == null || sourceFile == null || taskEvent.getKind() != Kind.ANALYZE) {
if (classTree == null || sourceFile == null) {
return;
}

Expand All @@ -59,10 +62,20 @@ public void finished(TaskEvent taskEvent) {
documentationType ->
writeToFile(
documentationType.getDocumentationExtractor().extract(classTree, taskEvent),
documentationType.getOutputFileNamePrefix(),
documentationType.getIdentifier(),
getSimpleClassName(sourceFile.toUri())));
}

private void createDirectoriesForPath() {
String docsPath = path.substring(path.indexOf('=') + 1) + File.separator + "docs";
try {
basePath = Files.createDirectories(Paths.get(docsPath));
} catch (IOException | InvalidPathException e) {
throw new IllegalStateException(
String.format("Error while creating directory with path '%s'", docsPath), e);
}
}

// XXX: `JavaFileObject` will most likely be added as parameter to help identify other `DocType`s.
private static Optional<DocumentationType> getDocumentationType(ClassTree tree) {
return stream(DocumentationType.values())
Expand All @@ -74,7 +87,7 @@ private <T> void writeToFile(T data, String fileName, String name) {
File file = basePath.resolve(String.format("%s-%s.json", fileName, name)).toFile();

try (FileWriter fileWriter = new FileWriter(file, UTF_8, /* append= */ true)) {
mapper.writeValue(fileWriter, data);
MAPPER.writeValue(fileWriter, data);
} catch (IOException e) {
throw new IllegalStateException(
String.format("Could not write to file '%s'", file.getPath()), e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,18 @@
enum DocumentationType {
BUG_PATTERN("bugpattern", new BugPatternExtractor());

private final String outputFileNamePrefix;
private final String identifier;

@SuppressWarnings("ImmutableEnumChecker" /* `DocumentationExtractor` is effectively immutable. */)
private final DocumentationExtractor<?> docExtractor;

DocumentationType(String outputFileNamePrefix, DocumentationExtractor<?> documentationExtractor) {
this.outputFileNamePrefix = outputFileNamePrefix;
DocumentationType(String identifier, DocumentationExtractor<?> documentationExtractor) {
this.identifier = identifier;
this.docExtractor = documentationExtractor;
}

String getOutputFileNamePrefix() {
return outputFileNamePrefix;
String getIdentifier() {
return identifier;
}

DocumentationExtractor<?> getDocumentationExtractor() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* A Java compiler plugin that generates documentation by extracting the relevant data from
* different source files.
* Module for the generation of documentation by extracting the relevant data from different source
* files.
*/
@com.google.errorprone.annotations.CheckReturnValue
@org.jspecify.annotations.NullMarked
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import org.junit.jupiter.api.condition.EnabledOnOs;
import org.junit.jupiter.api.io.TempDir;

final class DocumentationGeneratorBugPatternTest extends DocumentationGeneratorCompilerBasedTest {
final class DocumentationGeneratorBugPatternTest extends DocumentationGeneratorTaskListenerTest {
@EnabledOnOs(WINDOWS)
@Test
void wrongPathFailsWindows() {
Expand All @@ -25,15 +25,15 @@ void wrongPathFailsWindows() {
@DisabledOnOs(WINDOWS)
@Test
void wrongPathFailsOtherOperatingSystems() {
// Strictly speaking we are validating here that we cannot write to a RO FS.
// Strictly speaking we are validating here that we cannot write to a Read-only file system.
wrongPathFails('/');
}

private void wrongPathFails(char invalidCharacter) {
String invalidPath = invalidCharacter + "wrong-path";
assertThatThrownBy(() -> compile(invalidPath))
.isInstanceOf(IllegalStateException.class)
.hasMessage(
.hasCauseInstanceOf(IllegalStateException.class)
.hasMessageEndingWith(
"Error while creating directory with path '%s'", invalidPath + File.separator + "docs");
}

Expand All @@ -42,8 +42,7 @@ void noClass(@TempDir Path directory) {
Path outputPath = directory.resolve("pkg").toAbsolutePath();
compile(outputPath.toString(), "package pkg;");

assertThat(
outputPath.resolve("docs").resolve("bugpattern-CompilerBasedTestInput.json").toFile())
assertThat(outputPath.resolve("docs").resolve("bugpattern-TaskListenerTestInput.json").toFile())
.doesNotExist();
}

Expand All @@ -56,8 +55,7 @@ void noJsonExpected(@TempDir Path directory) {
"",
"public final class TestCheckerWithoutAnnotation extends BugChecker {}");

assertThat(
outputPath.resolve("docs").resolve("bugpattern-CompilerBasedTestInput.json").toFile())
assertThat(outputPath.resolve("docs").resolve("bugpattern-TaskListenerTestInput.json").toFile())
.doesNotExist();
}

Expand All @@ -77,7 +75,7 @@ void minimalBugPattern(@TempDir Path directory) throws IOException {

assertThat(
Files.readString(
outputPath.resolve("docs").resolve("bugpattern-CompilerBasedTestInput.json")))
outputPath.resolve("docs").resolve("bugpattern-TaskListenerTestInput.json")))
.isEqualToIgnoringWhitespace(getResource("bugpattern_example_minimal_testdata.json"));
}

Expand Down Expand Up @@ -106,7 +104,7 @@ void completeBugPattern(@TempDir Path directory) throws IOException {

assertThat(
Files.readString(
outputPath.resolve("docs").resolve("bugpattern-CompilerBasedTestInput.json")))
outputPath.resolve("docs").resolve("bugpattern-TaskListenerTestInput.json")))
.isEqualToIgnoringWhitespace(getResource("bugpattern_example_testdata.json"));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
import javax.tools.JavaCompiler;
import javax.tools.JavaFileObject;

abstract class DocumentationGeneratorCompilerBasedTest {
abstract class DocumentationGeneratorTaskListenerTest {
public void compile(String outputDirectory, String... lines) {
compile(outputDirectory, FileObjects.forSourceLines("CompilerBasedTestInput.java", lines));
compile(outputDirectory, FileObjects.forSourceLines("TaskListenerTestInput.java", lines));
}

private static void compile(String outputDirectory, JavaFileObject javaFileObject) {
Expand Down

0 comments on commit 7720e00

Please sign in to comment.