Another round

refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/CodeTransformers.java

@@ -1,25 +1,18 @@
 package tech.picnic.errorprone.refaster.runner;
 
 import com.google.common.base.Suppliers;
-import com.google.common.collect.ImmutableClassToInstanceMap;
 import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.reflect.ClassPath;
 import com.google.common.reflect.ClassPath.ResourceInfo;
 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.IOException;
 import java.io.InputStream;
 import java.io.ObjectInputStream;
 import java.io.UncheckedIOException;
-import java.lang.annotation.Annotation;
 import java.util.NoSuchElementException;
 import java.util.Optional;
 import java.util.function.Supplier;
-import tech.picnic.errorprone.refaster.annotation.TemplateCollection;
 
 /**
  * Scans the classpath for {@value #REFASTER_RULE_SUFFIX} files and loads them as {@link
@@ -113,50 +106,4 @@ public final class CodeTransformers {
       throw new IllegalStateException("Can't load `CodeTransformer` from " + resource, e);
     }
   }
-
-  // XXX: Move to separate file?
-  // XXX: Can we find a better class name?
-  private static final class CodeTransformerDescriptionAdapter implements CodeTransformer {
-    private final String name;
-    private final CodeTransformer delegate;
-
-    private CodeTransformerDescriptionAdapter(String name, CodeTransformer delegate) {
-      this.name = name;
-      this.delegate = delegate;
-    }
-
-    @Override
-    public void apply(TreePath path, Context context, DescriptionListener listener) {
-      TemplateCollection coll = annotations().getInstance(TemplateCollection.class);
-
-      if (coll != null) {
-        coll.linkPattern();
-      }
-
-      delegate.apply(
-          path, context, description -> listener.onDescribed(augmentDescription(description)));
-    }
-
-    @Override
-    public ImmutableClassToInstanceMap<Annotation> annotations() {
-      return delegate.annotations();
-    }
-
-    private Description augmentDescription(Description description) {
-      // XXX: Make this configurable based on a Refaster annotation. (E.g. by allowing users to
-      // specify an optional URL pattern.)
-      // XXX: Replace only the first `$`.
-      // XXX: Review URL format. Currently produced format:
-      // https://error-prone.picnic.tech/refastertemplates/OptionalTemplates#OptionalOrElseThrow
-      // XXX: Test this.
-      return Description.builder(
-              description.position,
-              description.checkName,
-              "https://error-prone.picnic.tech/refastertemplates/" + name.replace('$', '#'),
-              description.severity,
-              "Refactoring opportunity")
-          .addAllFixes(description.fixes)
-          .build();
-    }
-  }
 }

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

@@ -1,12 +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;
 
+// XXX: By grouping multiple properties this annotation does not lend itself to overriding. Perhaps
+// have separate annotations for each?
+// XXX: ^ Additional argument: the current setup "requires" defaults, which then causes duplication
+// with `AnnotatedCompositeCodeTransformer`.
+// XXX: The name `TemplateCollection` isn't appropriate if used directly on a Refaster template.
+// Find a more neutral name.
 @Target(ElementType.TYPE)
 @Retention(RetentionPolicy.SOURCE)
 public @interface TemplateCollection {
-  String linkPattern();
+  // XXX: This default is Error Prone Support-specific. Appropriate?
+  String linkPattern() default "https://error-prone.picnic.tech/refastertemplates/%s";
+
+  SeverityLevel severity() default SeverityLevel.SUGGESTION;
+
+  String description() default "Refactoring opportunity";
 }

refaster-test-support/pom.xml

@@ -43,6 +43,7 @@
         <dependency>
             <groupId>${project.groupId}</groupId>
             <artifactId>refaster-support</artifactId>
+            <scope>test</scope>
         </dependency>
         <dependency>
             <groupId>com.google.auto.service</groupId>

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

@@ -1,5 +1,7 @@
 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;
@@ -8,8 +10,8 @@ import tech.picnic.errorprone.refaster.annotation.TemplateCollection;
 final class MatchInWrongMethodRules {
   private MatchInWrongMethodRules() {}
 
-  // XXX: Test merging/overriding.
-  //  @TemplateCollection(linkPattern = "XXX")
+  // XXX: Demo: nesting overrides.
+  @TemplateCollection(linkPattern = "YYY", severity = ERROR, description = "Foo")
   static final class StringIsEmpty {
     @BeforeTemplate
     boolean before(String string) {