Suggestions

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

@@ -5,6 +5,7 @@
 import static java.util.Objects.requireNonNull;
 
 import com.google.auto.common.AnnotationMirrors;
+import com.google.auto.service.AutoService;
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
 import com.google.errorprone.BugPattern;
@@ -16,36 +17,43 @@
 import com.sun.source.tree.ClassTree;
 import com.sun.tools.javac.code.Attribute;
 import com.sun.tools.javac.code.Symbol.ClassSymbol;
+import java.util.Optional;
 import javax.lang.model.element.AnnotationValue;
 import tech.picnic.errorprone.documentation.BugPatternExtractor.BugPatternDocumentation;
 
 /**
  * An {@link Extractor} that describes how to extract data from a {@code @BugPattern} annotation.
  */
 @Immutable
-final class BugPatternExtractor implements Extractor<BugPatternDocumentation> {
+@AutoService(Extractor.class)
+public final class BugPatternExtractor implements Extractor<BugPatternDocumentation> {
   @Override
-  public BugPatternDocumentation extract(ClassTree tree, VisitorState state) {
-    ClassSymbol symbol = ASTHelpers.getSymbol(tree);
-    BugPattern annotation = symbol.getAnnotation(BugPattern.class);
-    requireNonNull(annotation, "BugPattern annotation must be present");
-
-    return new AutoValue_BugPatternExtractor_BugPatternDocumentation(
-        symbol.getQualifiedName().toString(),
-        annotation.name().isEmpty() ? tree.getSimpleName().toString() : annotation.name(),
-        ImmutableList.copyOf(annotation.altNames()),
-        annotation.link(),
-        ImmutableList.copyOf(annotation.tags()),
-        annotation.summary(),
-        annotation.explanation(),
-        annotation.severity(),
-        annotation.disableable(),
-        annotation.documentSuppression() ? getSuppressionAnnotations(tree) : ImmutableList.of());
+  public String identifier() {
+    return "bugpattern";
   }
 
   @Override
-  public boolean canExtract(ClassTree tree, VisitorState state) {
-    return ASTHelpers.hasDirectAnnotationWithSimpleName(tree, BugPattern.class.getSimpleName());
+  public Optional<BugPatternDocumentation> tryExtract(ClassTree tree, VisitorState state) {
+    ClassSymbol symbol = ASTHelpers.getSymbol(tree);
+    BugPattern annotation = symbol.getAnnotation(BugPattern.class);
+    if (annotation == null) {
+      return Optional.empty();
+    }
+
+    return Optional.of(
+        new AutoValue_BugPatternExtractor_BugPatternDocumentation(
+            symbol.getQualifiedName().toString(),
+            annotation.name().isEmpty() ? tree.getSimpleName().toString() : annotation.name(),
+            ImmutableList.copyOf(annotation.altNames()),
+            annotation.link(),
+            ImmutableList.copyOf(annotation.tags()),
+            annotation.summary(),
+            annotation.explanation(),
+            annotation.severity(),
+            annotation.disableable(),
+            annotation.documentSuppression()
+                ? getSuppressionAnnotations(tree)
+                : ImmutableList.of()));
   }
 
   /**

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

@@ -9,6 +9,7 @@
 import static com.google.errorprone.matchers.Matchers.toType;
 import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
 
+import com.google.auto.service.AutoService;
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
 import com.google.errorprone.VisitorState;
@@ -31,11 +32,12 @@
 import tech.picnic.errorprone.documentation.BugPatternTestExtractor.BugPatternTestDocumentation;
 
 /**
- * An {@link Extractor} that describes how to extract data from a test that tests a {@code
+ * An {@link Extractor} that describes how to extract data from classes that test a {@code
  * BugChecker}.
  */
 @Immutable
-final class BugPatternTestExtractor implements Extractor<BugPatternTestDocumentation> {
+@AutoService(Extractor.class)
+public final class BugPatternTestExtractor implements Extractor<BugPatternTestDocumentation> {
   private static final Pattern TEST_CLASS_NAME_PATTERN = Pattern.compile("(.*)Test");
   private static final Matcher<Tree> JUNIT_TEST_METHOD =
       toType(MethodTree.class, hasAnnotation("org.junit.jupiter.api.Test"));
@@ -48,34 +50,30 @@ final class BugPatternTestExtractor implements Extractor<BugPatternTestDocumenta
               .named("newInstance"),
           argument(0, classLiteral(anything())));
 
-  // XXX: Improve support for correctly extracting multiple sources from a single
-  // `{BugCheckerRefactoring,Compilation}TestHelper` test.
   @Override
-  public BugPatternTestDocumentation extract(ClassTree tree, VisitorState state) {
-    CollectBugPatternTests scanner = new CollectBugPatternTests();
-
-    for (Tree m : tree.getMembers()) {
-      if (JUNIT_TEST_METHOD.matches(m, state)) {
-        scanner.scan(m, state);
-      }
-    }
-
-    String bugPatternName =
-        getClassUnderTest(tree)
-            .orElseThrow(
-                () ->
-                    new IllegalArgumentException(
-                        String.format(
-                            "Name of given class does not match '%s'", TEST_CLASS_NAME_PATTERN)));
-    return new AutoValue_BugPatternTestExtractor_BugPatternTestDocumentation(
-        bugPatternName, scanner.getIdentificationTests(), scanner.getReplacementTests());
+  public String identifier() {
+    return "bugpattern-test";
   }
 
+  // XXX: Improve support for correctly extracting multiple sources from a single
+  // `{BugCheckerRefactoring,Compilation}TestHelper` test.
   @Override
-  public boolean canExtract(ClassTree tree, VisitorState state) {
+  public Optional<BugPatternTestDocumentation> tryExtract(ClassTree tree, VisitorState state) {
     return getClassUnderTest(tree)
         .filter(bugPatternName -> testsBugPattern(bugPatternName, tree, state))
-        .isPresent();
+        .map(
+            bugPatternName -> {
+              CollectBugPatternTests scanner = new CollectBugPatternTests();
+
+              for (Tree m : tree.getMembers()) {
+                if (JUNIT_TEST_METHOD.matches(m, state)) {
+                  scanner.scan(m, state);
+                }
+              }
+
+              return new AutoValue_BugPatternTestExtractor_BugPatternTestDocumentation(
+                  bugPatternName, scanner.getIdentificationTests(), scanner.getReplacementTests());
+            });
   }
 
   private static boolean testsBugPattern(

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

@@ -5,6 +5,7 @@
 import com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility;
 import com.fasterxml.jackson.annotation.PropertyAccessor;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.collect.ImmutableList;
 import com.google.errorprone.VisitorState;
 import com.sun.source.tree.ClassTree;
 import com.sun.source.tree.CompilationUnitTree;
@@ -22,6 +23,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.ServiceLoader;
 import javax.tools.JavaFileObject;
 
 /**
@@ -30,6 +32,10 @@
  */
 // XXX: Find a better name for this class; it doesn't generate documentation per se.
 final class DocumentationGeneratorTaskListener implements TaskListener {
+  @SuppressWarnings({"rawtypes", "unchecked"})
+  private static final ImmutableList<Extractor<?>> EXTRACTORS =
+      (ImmutableList) ImmutableList.copyOf(ServiceLoader.load(Extractor.class));
+
   private static final ObjectMapper OBJECT_MAPPER =
       new ObjectMapper().setVisibility(PropertyAccessor.FIELD, Visibility.ANY);
 
@@ -65,13 +71,14 @@ public void finished(TaskEvent taskEvent) {
         VisitorState.createForUtilityPurposes(context)
             .withPath(new TreePath(new TreePath(compilationUnit), classTree));
 
-    ExtractorType.findMatchingType(classTree, state)
-        .ifPresent(
-            extractorType ->
-                writeToFile(
-                    extractorType.getIdentifier(),
-                    getSimpleClassName(sourceFile.toUri()),
-                    extractorType.getExtractor().extract(classTree, state)));
+    for (Extractor<?> extractor : EXTRACTORS) {
+      extractor
+          .tryExtract(classTree, state)
+          .ifPresent(
+              data ->
+                  writeToFile(
+                      extractor.identifier(), getSimpleClassName(sourceFile.toUri()), data));
+    }
   }
 
   private void createDocsDirectory() {

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

@@ -3,36 +3,31 @@
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.annotations.Immutable;
 import com.sun.source.tree.ClassTree;
+import java.util.Optional;
 
 /**
  * Interface implemented by classes that define how to extract data of some type {@link T} from a
  * given {@link ClassTree}.
  *
  * @param <T> The type of data that is extracted.
  */
-// XXX: Here and in the implementations, either:
-// 1. Swap `canExtract` and `extract`.
-// 2. Combine the methods into a single `Optional<T> tryExtract`.
 @Immutable
 interface Extractor<T> {
   /**
-   * Extracts and returns an instance of {@link T} using the provided arguments.
+   * Returns the unique identifier of this extractor.
    *
-   * @param tree The {@link ClassTree} to analyze and from which to extract instances of {@link T}.
-   * @param state A {@link VisitorState} describing the context in which the given {@link ClassTree}
-   *     is found.
-   * @return A non-null instance of {@link T}.
+   * @return A non-{@code null} string.
    */
-  T extract(ClassTree tree, VisitorState state);
+  String identifier();
 
   /**
-   * Tells whether this {@link Extractor} can extract documentation content from the given {@link
-   * ClassTree}.
+   * Attempts to extract an instance of type {@link T} using the provided arguments.
    *
-   * @param tree The {@link ClassTree} of interest.
+   * @param tree The {@link ClassTree} to analyze and from which to extract an instance of type
+   *     {@link T}.
    * @param state A {@link VisitorState} describing the context in which the given {@link ClassTree}
    *     is found.
-   * @return {@code true} iff data extraction is supported.
+   * @return An instance of type {@link T}, if possible.
    */
-  boolean canExtract(ClassTree tree, VisitorState state);
+  Optional<T> tryExtract(ClassTree tree, VisitorState state);
 }

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

@@ -3,7 +3,6 @@
 import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import com.google.common.io.Resources;
 import com.google.errorprone.BugPattern;
@@ -14,7 +13,6 @@
 import com.google.errorprone.matchers.Description;
 import com.sun.source.tree.ClassTree;
 import java.io.IOException;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
@@ -120,7 +118,8 @@ void bugPatternAnnotationIsAbsent() {
 
   private static void verifyFileMatchesResource(
       Path outputDirectory, String fileName, String resourceName) throws IOException {
-    assertThat(Files.readString(outputDirectory.resolve(fileName)))
+    assertThat(outputDirectory.resolve(fileName))
+        .content(UTF_8)
         .isEqualToIgnoringWhitespace(getResource(resourceName));
   }
 
@@ -139,14 +138,10 @@ public static final class TestChecker extends BugChecker implements ClassTreeMat
 
     @Override
     public Description matchClass(ClassTree tree, VisitorState state) {
-      BugPatternExtractor extractor = new BugPatternExtractor();
-
-      assertThatThrownBy(() -> extractor.extract(tree, state))
-          .isInstanceOf(NullPointerException.class)
-          .hasMessage("BugPattern annotation must be present");
-
       return buildDescription(tree)
-          .setMessage(String.format("Can extract: %s", extractor.canExtract(tree, state)))
+          .setMessage(
+              String.format(
+                  "Can extract: %s", new BugPatternExtractor().tryExtract(tree, state).isPresent()))
           .build();
     }
   }

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

@@ -5,7 +5,6 @@
 
 import com.google.common.io.Resources;
 import java.io.IOException;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
@@ -338,7 +337,8 @@ void bugPatternTestMultipleIdentificationAndReplacement(@TempDir Path outputDire
 
   private static void verifyFileMatchesResource(
       Path outputDirectory, String fileName, String resourceName) throws IOException {
-    assertThat(Files.readString(outputDirectory.resolve(fileName)))
+    assertThat(outputDirectory.resolve(fileName))
+        .content(UTF_8)
         .isEqualToIgnoringWhitespace(getResource(resourceName));
   }