Better annotation support, simplify setup

error-prone-contrib/src/main/java/tech/picnic/errorprone/refastertemplates/OptionalTemplates.java

@@ -17,8 +17,11 @@
 import java.util.function.Supplier;
 import java.util.stream.Stream;
 import javax.annotation.Nullable;
+import tech.picnic.errorprone.refaster.annotation.Description;
+import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
 
 /** Refaster templates related to expressions dealing with {@link Optional}s. */
+@OnlineDocumentation
 final class OptionalTemplates {
   private OptionalTemplates() {}
 
@@ -63,7 +66,7 @@ boolean after(Optional<T> optional) {
     }
   }
 
-  /** Prefer {@link Optional#orElseThrow()} over the less explicit {@link Optional#get()}. */
+  @Description("Prefer `Optional#orElseThrow()` over the less explicit `Optional#get()`")
   static final class OptionalOrElseThrow<T> {
     @BeforeTemplate
     @SuppressWarnings("NullAway")

refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/AnnotatedCompositeCodeTransformer.java

@@ -0,0 +1,96 @@
+package tech.picnic.errorprone.refaster.plugin;
+
+import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
+
+import com.google.common.collect.ImmutableClassToInstanceMap;
+import com.google.common.collect.ImmutableList;
+import com.google.errorprone.BugPattern.SeverityLevel;
+import com.google.errorprone.CodeTransformer;
+import com.google.errorprone.DescriptionListener;
+import com.google.errorprone.matchers.Description;
+import com.sun.source.util.TreePath;
+import com.sun.tools.javac.util.Context;
+import java.io.Serializable;
+import java.lang.annotation.Annotation;
+import java.util.Optional;
+import java.util.function.Function;
+import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
+import tech.picnic.errorprone.refaster.annotation.Severity;
+
+// XXX: Rename? Use `@AutoValue`?
+// XXX: Should the name _also_ be derived from an annotation? Upshot is limited.
+// XXX: ^ Name could be dropped if we derive it from `description.checkName`, with the assumption
+// that package and class names follow idiomatic Java naming convention. Kinda icky...
+final class AnnotatedCompositeCodeTransformer implements CodeTransformer, Serializable {
+  private static final long serialVersionUID = 1L;
+
+  private final String name;
+  private final ImmutableList<CodeTransformer> transformers;
+  private final ImmutableClassToInstanceMap<Annotation> annotations;
+
+  AnnotatedCompositeCodeTransformer(
+      String name,
+      ImmutableList<CodeTransformer> transformers,
+      ImmutableClassToInstanceMap<Annotation> annotations) {
+    this.name = name;
+    this.transformers = transformers;
+    this.annotations = annotations;
+  }
+
+  @Override
+  public ImmutableClassToInstanceMap<Annotation> annotations() {
+    return annotations;
+  }
+
+  @Override
+  public void apply(TreePath path, Context context, DescriptionListener listener) {
+    for (CodeTransformer transformer : transformers) {
+      transformer.apply(
+          path,
+          context,
+          description -> listener.onDescribed(augmentDescription(description, transformer)));
+    }
+  }
+
+  private Description augmentDescription(Description description, CodeTransformer delegate) {
+    // XXX: Replace only the first `$`.
+    // XXX: Test this.
+    return Description.builder(
+            description.position,
+            description.checkName,
+            String.format(getLinkPattern(delegate), name.replace('$', '#')),
+            getSeverity(delegate),
+            getDescription(delegate))
+        .addAllFixes(description.fixes)
+        .build();
+  }
+
+  private String getLinkPattern(CodeTransformer delegate) {
+    return getAnnotationValue(OnlineDocumentation.class, OnlineDocumentation::value, delegate, "");
+  }
+
+  private SeverityLevel getSeverity(CodeTransformer delegate) {
+    return getAnnotationValue(Severity.class, Severity::value, delegate, SUGGESTION);
+  }
+
+  private String getDescription(CodeTransformer delegate) {
+    return getAnnotationValue(
+        tech.picnic.errorprone.refaster.annotation.Description.class,
+        tech.picnic.errorprone.refaster.annotation.Description::value,
+        delegate,
+        "Refactoring opportunity");
+  }
+
+  private <A extends Annotation, T> T getAnnotationValue(
+      Class<A> annotation, Function<A, T> extractor, CodeTransformer delegate, T defaultValue) {
+    return getAnnotationValue(delegate, annotation)
+        .or(() -> getAnnotationValue(this, annotation))
+        .map(extractor)
+        .orElse(defaultValue);
+  }
+
+  private static <A extends Annotation> Optional<A> getAnnotationValue(
+      CodeTransformer codeTransformer, Class<A> annotation) {
+    return Optional.ofNullable(codeTransformer.annotations().getInstance(annotation));
+  }
+}

refaster-compiler/src/main/java/tech/picnic/errorprone/refaster/plugin/RefasterRuleCompilerTaskListener.java

@@ -32,7 +32,6 @@
 import javax.tools.FileObject;
 import javax.tools.JavaFileManager;
 import javax.tools.StandardLocation;
-import tech.picnic.errorprone.refaster.AnnotatedCompositeCodeTransformer;
 
 /**
  * A variant of {@code com.google.errorprone.refaster.RefasterRuleCompilerAnalyzer} which stores

refaster-runner/pom.xml

@@ -32,18 +32,6 @@
         <dependency>
             <groupId>${project.groupId}</groupId>
             <artifactId>refaster-compiler</artifactId>
-            <!-- This dependency is declared only as a hint to Maven that
-            compilation depends on it; see the `maven-compiler-plugin`'s
-            `annotationProcessorPaths` configuration below. -->
-            <scope>provided</scope>
-        </dependency>
-        <!-- XXX: Maybe it's better if we move
-        `AnnotatedCompositeCodeTransformer` to `refaster-compiler` and drop the
-        `provided` above. -->
-        <dependency>
-            <groupId>${project.groupId}</groupId>
-            <artifactId>refaster-support</artifactId>
-            <scope>runtime</scope>
         </dependency>
         <dependency>
             <groupId>com.google.auto.service</groupId>

refaster-support/src/main/java/tech/picnic/errorprone/refaster/annotation/Description.java

@@ -0,0 +1,22 @@
+package tech.picnic.errorprone.refaster.annotation;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Describes the intent of a Refaster template or group of Refaster templates.
+ *
+ * <p>Annotations on nested classes override the description associated with any enclosing class.
+ */
+@Target(ElementType.TYPE)
+@Retention(RetentionPolicy.SOURCE)
+public @interface Description {
+  /**
+   * A description of the annotated Refaster template(s).
+   *
+   * @return A non-{@code null} string.
+   */
+  String value();
+}

refaster-support/src/main/java/tech/picnic/errorprone/refaster/annotation/OnlineDocumentation.java

@@ -0,0 +1,33 @@
+package tech.picnic.errorprone.refaster.annotation;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Signals that a Refaster template or group of Refaster templates comes with online documentation.
+ *
+ * <p>The provided value may be a full URL, or a URL pattern containing a single {@code %s}
+ * placeholder. If a placeholder is present, then it will be replaced with the name of the
+ * associated Refaster template any time the URL is rendered.
+ *
+ * <p>By default it is assumed that the Refaster template(s) are documented on the Error Prone
+ * Support website. Annotations on nested classes override the documentation URL associated with any
+ * enclosing class.
+ */
+// XXX: Is the `%s` protocol sufficiently generic for non-Picnic use cases?
+// XXX: The documentation is misleading, in that the generated anchor isn't mentioned.
+@Target(ElementType.TYPE)
+@Retention(RetentionPolicy.SOURCE)
+public @interface OnlineDocumentation {
+  /**
+   * The URL or URL pattern of the website at which the annotated Refaster template(s) are
+   * documented.
+   *
+   * @return A non-{@code null} string.
+   */
+  // XXX: This default is Error Prone Support-specific. Appropriate? (The alternative is to repeat
+  // this URL pattern in many places.) If we drop this, also update the class documentation.
+  String value() default "https://error-prone.picnic.tech/refastertemplates/%s";
+}

refaster-support/src/main/java/tech/picnic/errorprone/refaster/annotation/Severity.java

@@ -0,0 +1,24 @@
+package tech.picnic.errorprone.refaster.annotation;
+
+import com.google.errorprone.BugPattern.SeverityLevel;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Describes the severity of a Refaster template or group of Refaster templates.
+ *
+ * <p>The default severity is {@link SeverityLevel#SUGGESTION}. Annotations on nested classes
+ * override the severity associated with any enclosing class.
+ */
+@Target(ElementType.TYPE)
+@Retention(RetentionPolicy.SOURCE)
+public @interface Severity {
+  /**
+   * The expected severity of any match of the annotated Refaster template(s).
+   *
+   * @return An Error Prone severity level.
+   */
+  SeverityLevel value();
+}

refaster-test-support/pom.xml

@@ -40,6 +40,7 @@
             <groupId>${project.groupId}</groupId>
             <artifactId>refaster-runner</artifactId>
         </dependency>
+        <!-- XXX: Review need after code finalization. -->
         <dependency>
             <groupId>${project.groupId}</groupId>
             <artifactId>refaster-support</artifactId>

refaster-test-support/src/test/java/tech/picnic/errorprone/refaster/test/MatchInWrongMethodTemplates.java

@@ -1,20 +1,20 @@
 package tech.picnic.errorprone.refaster.test;
 
-import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
-
 import com.google.errorprone.refaster.annotation.AfterTemplate;
 import com.google.errorprone.refaster.annotation.BeforeTemplate;
-import tech.picnic.errorprone.refaster.annotation.TemplateCollection;
+import tech.picnic.errorprone.refaster.annotation.OnlineDocumentation;
 
 /**
  * Refaster template collection to validate reporting of a match occurring in an unexpected place.
  */
-@TemplateCollection(linkPattern = "XXX")
+@OnlineDocumentation
 final class MatchInWrongMethodTemplates {
   private MatchInWrongMethodTemplates() {}
 
   // XXX: Demo: nesting overrides.
-  @TemplateCollection(linkPattern = "YYY", severity = ERROR, description = "Foo")
+  //  @Website("YYY")
+  //  @Severity(ERROR)
+  //  @Description("Fooo!")
   static final class StringIsEmpty {
     @BeforeTemplate
     boolean before(String string) {