Skip to content

Commit

Permalink
Better annotation support, simplify setup
Browse files Browse the repository at this point in the history
  • Loading branch information
Stephan202 committed Sep 23, 2022
1 parent 3b232c3 commit 3e47909
Show file tree
Hide file tree
Showing 12 changed files with 189 additions and 140 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}

Expand Down Expand Up @@ -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")
Expand Down
4 changes: 4 additions & 0 deletions refaster-compiler/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
<description>Java compiler plugin which identifies and compiles Refaster templates, storing them as resource files on the classpath.</description>

<dependencies>
<dependency>
<groupId>${groupId.error-prone}</groupId>
<artifactId>error_prone_annotation</artifactId>
</dependency>
<dependency>
<groupId>${groupId.error-prone}</groupId>
<artifactId>error_prone_annotations</artifactId>
Expand Down
Original file line number Diff line number Diff line change
@@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 0 additions & 11 deletions refaster-runner/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +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>
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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();
}
Original file line number Diff line number Diff line change
@@ -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";
}
Original file line number Diff line number Diff line change
@@ -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();
}

This file was deleted.

1 change: 1 addition & 0 deletions refaster-test-support/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down

0 comments on commit 3e47909

Please sign in to comment.