diff --git a/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/CodeTransformers.java b/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/CodeTransformers.java index 1eb1a736..7830b3c1 100644 --- a/refaster-runner/src/main/java/tech/picnic/errorprone/refaster/runner/CodeTransformers.java +++ b/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 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(); - } - } } diff --git a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/annotation/TemplateCollection.java b/refaster-support/src/main/java/tech/picnic/errorprone/refaster/annotation/TemplateCollection.java index a1e3f510..bf0e7026 100644 --- a/refaster-support/src/main/java/tech/picnic/errorprone/refaster/annotation/TemplateCollection.java +++ b/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"; } diff --git a/refaster-test-support/pom.xml b/refaster-test-support/pom.xml index ccc74a81..92da3e01 100644 --- a/refaster-test-support/pom.xml +++ b/refaster-test-support/pom.xml @@ -43,6 +43,7 @@ ${project.groupId} refaster-support + test com.google.auto.service diff --git a/refaster-test-support/src/test/java/tech/picnic/errorprone/refaster/test/MatchInWrongMethodRules.java b/refaster-test-support/src/test/java/tech/picnic/errorprone/refaster/test/MatchInWrongMethodRules.java index f731e1a7..196dbbcc 100644 --- a/refaster-test-support/src/test/java/tech/picnic/errorprone/refaster/test/MatchInWrongMethodRules.java +++ b/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) {