mirror of
https://github.com/jlengrand/error-prone-support.git
synced 2026-03-10 08:11:25 +00:00
Suggest canonical modifier usage for Refaster template definitions (#254)
This commit is contained in:
Stephan Schroevers
committed by
GitHub
GitHub
parent
2ba7bf9f46
commit
397f9c3df7
@@ -0,0 +1,111 @@
|
||||
package tech.picnic.errorprone.bugpatterns;
|
||||
|
||||
import static com.google.errorprone.BugPattern.LinkType.NONE;
|
||||
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
|
||||
import static com.google.errorprone.BugPattern.StandardTags.STYLE;
|
||||
import static com.google.errorprone.matchers.Matchers.anyOf;
|
||||
import static com.google.errorprone.matchers.Matchers.hasAnnotation;
|
||||
|
||||
import com.google.auto.service.AutoService;
|
||||
import com.google.errorprone.BugPattern;
|
||||
import com.google.errorprone.VisitorState;
|
||||
import com.google.errorprone.bugpatterns.BugChecker;
|
||||
import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher;
|
||||
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
|
||||
import com.google.errorprone.fixes.SuggestedFix;
|
||||
import com.google.errorprone.fixes.SuggestedFixes;
|
||||
import com.google.errorprone.matchers.Description;
|
||||
import com.google.errorprone.matchers.Matcher;
|
||||
import com.google.errorprone.refaster.annotation.AfterTemplate;
|
||||
import com.google.errorprone.refaster.annotation.BeforeTemplate;
|
||||
import com.google.errorprone.refaster.annotation.Placeholder;
|
||||
import com.google.errorprone.util.ASTHelpers;
|
||||
import com.sun.source.tree.ClassTree;
|
||||
import com.sun.source.tree.MethodTree;
|
||||
import com.sun.source.tree.Tree;
|
||||
import java.util.EnumSet;
|
||||
import java.util.Set;
|
||||
import javax.lang.model.element.Modifier;
|
||||
|
||||
/**
|
||||
* A {@link BugChecker} which suggests a canonical set of modifiers for Refaster class and method
|
||||
* definitions.
|
||||
*/
|
||||
@AutoService(BugChecker.class)
|
||||
@BugPattern(
|
||||
summary = "Refaster class and method definitions should specify a canonical set of modifiers",
|
||||
linkType = NONE,
|
||||
severity = SUGGESTION,
|
||||
tags = STYLE)
|
||||
public final class RefasterTemplateModifiers extends BugChecker
|
||||
implements ClassTreeMatcher, MethodTreeMatcher {
|
||||
private static final long serialVersionUID = 1L;
|
||||
private static final Matcher<Tree> BEFORE_TEMPLATE_METHOD = hasAnnotation(BeforeTemplate.class);
|
||||
private static final Matcher<Tree> AFTER_TEMPLATE_METHOD = hasAnnotation(AfterTemplate.class);
|
||||
private static final Matcher<Tree> PLACEHOLDER_METHOD = hasAnnotation(Placeholder.class);
|
||||
private static final Matcher<Tree> REFASTER_METHOD =
|
||||
anyOf(BEFORE_TEMPLATE_METHOD, AFTER_TEMPLATE_METHOD, PLACEHOLDER_METHOD);
|
||||
|
||||
@Override
|
||||
public Description matchClass(ClassTree tree, VisitorState state) {
|
||||
if (!hasMatchingMember(tree, BEFORE_TEMPLATE_METHOD, state)) {
|
||||
/* This class does not contain a Refaster template. */
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
|
||||
SuggestedFix fix = suggestCanonicalModifiers(tree, state);
|
||||
return fix.isEmpty() ? Description.NO_MATCH : describeMatch(tree, fix);
|
||||
}
|
||||
|
||||
@Override
|
||||
public Description matchMethod(MethodTree tree, VisitorState state) {
|
||||
if (!REFASTER_METHOD.matches(tree, state)) {
|
||||
return Description.NO_MATCH;
|
||||
}
|
||||
|
||||
return SuggestedFixes.removeModifiers(
|
||||
tree,
|
||||
state,
|
||||
Modifier.FINAL,
|
||||
Modifier.PRIVATE,
|
||||
Modifier.PROTECTED,
|
||||
Modifier.PUBLIC,
|
||||
Modifier.STATIC,
|
||||
Modifier.SYNCHRONIZED)
|
||||
.map(fix -> describeMatch(tree, fix))
|
||||
.orElse(Description.NO_MATCH);
|
||||
}
|
||||
|
||||
private static SuggestedFix suggestCanonicalModifiers(ClassTree tree, VisitorState state) {
|
||||
Set<Modifier> modifiersToAdd = EnumSet.noneOf(Modifier.class);
|
||||
Set<Modifier> modifiersToRemove =
|
||||
EnumSet.of(Modifier.PRIVATE, Modifier.PROTECTED, Modifier.PUBLIC, Modifier.SYNCHRONIZED);
|
||||
|
||||
if (!hasMatchingMember(tree, PLACEHOLDER_METHOD, state)) {
|
||||
/*
|
||||
* Templates without a `@Placeholder` method should be `final`. Note that Refaster enforces
|
||||
* that `@Placeholder` methods are `abstract`, so templates _with_ such a method will
|
||||
* naturally be `abstract` and non-`final`.
|
||||
*/
|
||||
modifiersToAdd.add(Modifier.FINAL);
|
||||
modifiersToRemove.add(Modifier.ABSTRACT);
|
||||
}
|
||||
|
||||
if (ASTHelpers.findEnclosingNode(state.getPath(), ClassTree.class) != null) {
|
||||
/* Nested classes should be `static`. */
|
||||
modifiersToAdd.add(Modifier.STATIC);
|
||||
}
|
||||
|
||||
SuggestedFix.Builder fix = SuggestedFix.builder();
|
||||
SuggestedFixes.addModifiers(tree, tree.getModifiers(), state, modifiersToAdd)
|
||||
.ifPresent(fix::merge);
|
||||
SuggestedFixes.removeModifiers(tree.getModifiers(), state, modifiersToRemove)
|
||||
.ifPresent(fix::merge);
|
||||
return fix.build();
|
||||
}
|
||||
|
||||
private static boolean hasMatchingMember(
|
||||
ClassTree tree, Matcher<Tree> matcher, VisitorState state) {
|
||||
return tree.getMembers().stream().anyMatch(member -> matcher.matches(member, state));
|
||||
}
|
||||
}
|
||||
@@ -141,8 +141,8 @@ final class ImmutableListMultimapTemplates {
|
||||
}
|
||||
|
||||
/**
|
||||
* Don't map a a stream's elements to map entries, only to subsequently collect them into an
|
||||
* {@link ImmutableListMultimap}. The collection can be performed directly.
|
||||
* Don't map stream's elements to map entries, only to subsequently collect them into an {@link
|
||||
* ImmutableListMultimap}. The collection can be performed directly.
|
||||
*/
|
||||
abstract static class StreamOfMapEntriesToImmutableListMultimap<E, K, V> {
|
||||
@Placeholder(allowsIdentity = true)
|
||||
|
||||
@@ -153,7 +153,7 @@ final class OptionalTemplates {
|
||||
* Prefer {@link Optional#filter(Predicate)} over {@link Optional#map(Function)} when converting
|
||||
* an {@link Optional} to a boolean.
|
||||
*/
|
||||
abstract static class MapOptionalToBoolean<T> {
|
||||
static final class MapOptionalToBoolean<T> {
|
||||
@BeforeTemplate
|
||||
boolean before(Optional<T> optional, Function<? super T, Boolean> predicate) {
|
||||
return optional.map(predicate).orElse(Refaster.anyOf(false, Boolean.FALSE));
|
||||
@@ -315,7 +315,7 @@ final class OptionalTemplates {
|
||||
}
|
||||
|
||||
/** Prefer {@link Optional#or(Supplier)} over more verbose alternatives. */
|
||||
abstract static class OptionalOrOtherOptional<T> {
|
||||
static final class OptionalOrOtherOptional<T> {
|
||||
@BeforeTemplate
|
||||
@SuppressWarnings("NestedOptionals" /* Auto-fix for the `NestedOptionals` check. */)
|
||||
Optional<T> before(Optional<T> optional1, Optional<T> optional2) {
|
||||
|
||||
@@ -174,7 +174,7 @@ final class WebClientTemplates {
|
||||
}
|
||||
|
||||
/** Don't unnecessarily use {@link RequestHeadersUriSpec#uri(Function)}. */
|
||||
abstract static class RequestHeadersUriSpecUri {
|
||||
static final class RequestHeadersUriSpecUri {
|
||||
@BeforeTemplate
|
||||
RequestHeadersSpec<?> before(
|
||||
RequestHeadersUriSpec<?> requestHeadersUriSpec,
|
||||
|
||||
@@ -0,0 +1,208 @@
|
||||
package tech.picnic.errorprone.bugpatterns;
|
||||
|
||||
import com.google.errorprone.BugCheckerRefactoringTestHelper;
|
||||
import com.google.errorprone.BugCheckerRefactoringTestHelper.TestMode;
|
||||
import com.google.errorprone.CompilationTestHelper;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
final class RefasterTemplateModifiersTest {
|
||||
private final CompilationTestHelper compilationTestHelper =
|
||||
CompilationTestHelper.newInstance(RefasterTemplateModifiers.class, getClass());
|
||||
private final BugCheckerRefactoringTestHelper refactoringTestHelper =
|
||||
BugCheckerRefactoringTestHelper.newInstance(RefasterTemplateModifiers.class, getClass());
|
||||
|
||||
@Test
|
||||
void identification() {
|
||||
compilationTestHelper
|
||||
.addSourceLines(
|
||||
"A.java",
|
||||
"import com.google.errorprone.refaster.annotation.BeforeTemplate;",
|
||||
"",
|
||||
"final class A {",
|
||||
" @BeforeTemplate",
|
||||
" String before(String str) {",
|
||||
" return str;",
|
||||
" }",
|
||||
"",
|
||||
" String nonRefasterMethod(String str) {",
|
||||
" return str;",
|
||||
" }",
|
||||
"",
|
||||
" static final class Inner {",
|
||||
" @BeforeTemplate",
|
||||
" String before(String str) {",
|
||||
" return str;",
|
||||
" }",
|
||||
" }",
|
||||
"}")
|
||||
.addSourceLines(
|
||||
"B.java",
|
||||
"import com.google.errorprone.refaster.annotation.BeforeTemplate;",
|
||||
"import com.google.errorprone.refaster.annotation.Placeholder;",
|
||||
"",
|
||||
"abstract class B<I, O> {",
|
||||
" @Placeholder",
|
||||
" abstract O someFunction(I input);",
|
||||
"",
|
||||
" @BeforeTemplate",
|
||||
" String before(I input) {",
|
||||
" return String.valueOf(someFunction(input));",
|
||||
" }",
|
||||
"",
|
||||
" abstract static class Inner<I, O> {",
|
||||
" @Placeholder",
|
||||
" abstract O someFunction(I input);",
|
||||
"",
|
||||
" @BeforeTemplate",
|
||||
" String before(I input) {",
|
||||
" return String.valueOf(someFunction(input));",
|
||||
" }",
|
||||
" }",
|
||||
"}")
|
||||
.addSourceLines(
|
||||
"C.java",
|
||||
"import com.google.errorprone.refaster.annotation.BeforeTemplate;",
|
||||
"",
|
||||
"// BUG: Diagnostic contains:",
|
||||
"class C {",
|
||||
" @BeforeTemplate",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" final String beforeFinal(String str) {",
|
||||
" return str;",
|
||||
" }",
|
||||
"",
|
||||
" @BeforeTemplate",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" private String beforePrivate(String str) {",
|
||||
" return str;",
|
||||
" }",
|
||||
"",
|
||||
" @BeforeTemplate",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" public String beforePublic(String str) {",
|
||||
" return str;",
|
||||
" }",
|
||||
"",
|
||||
" @BeforeTemplate",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" static String beforeStatic(String str) {",
|
||||
" return str;",
|
||||
" }",
|
||||
"",
|
||||
" @BeforeTemplate",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" synchronized String beforeSynchronized(String str) {",
|
||||
" return str;",
|
||||
" }",
|
||||
"",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" abstract static class AbstractInner {",
|
||||
" @BeforeTemplate",
|
||||
" String before(String str) {",
|
||||
" return str;",
|
||||
" }",
|
||||
" }",
|
||||
"",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" static class NonFinalInner {",
|
||||
" @BeforeTemplate",
|
||||
" String before(String str) {",
|
||||
" return str;",
|
||||
" }",
|
||||
" }",
|
||||
"",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" final class NonStaticInner {",
|
||||
" @BeforeTemplate",
|
||||
" String before(String str) {",
|
||||
" return str;",
|
||||
" }",
|
||||
" }",
|
||||
"}")
|
||||
.addSourceLines(
|
||||
"D.java",
|
||||
"import com.google.errorprone.refaster.annotation.BeforeTemplate;",
|
||||
"",
|
||||
"// BUG: Diagnostic contains:",
|
||||
"abstract class D {",
|
||||
" @BeforeTemplate",
|
||||
" // BUG: Diagnostic contains:",
|
||||
" protected String beforeProtected(String str) {",
|
||||
" return str;",
|
||||
" }",
|
||||
"}")
|
||||
.doTest();
|
||||
}
|
||||
|
||||
@Test
|
||||
void replacement() {
|
||||
refactoringTestHelper
|
||||
.addInputLines(
|
||||
"A.java",
|
||||
"import com.google.errorprone.refaster.annotation.BeforeTemplate;",
|
||||
"",
|
||||
"class A {",
|
||||
" @BeforeTemplate",
|
||||
" private static String before(String str) {",
|
||||
" return str;",
|
||||
" }",
|
||||
"}")
|
||||
.addOutputLines(
|
||||
"A.java",
|
||||
"import com.google.errorprone.refaster.annotation.BeforeTemplate;",
|
||||
"",
|
||||
"final class A {",
|
||||
" @BeforeTemplate",
|
||||
" String before(String str) {",
|
||||
" return str;",
|
||||
" }",
|
||||
"}")
|
||||
.addInputLines(
|
||||
"B.java",
|
||||
"import com.google.errorprone.refaster.annotation.BeforeTemplate;",
|
||||
"import com.google.errorprone.refaster.annotation.Placeholder;",
|
||||
"",
|
||||
"final class B {",
|
||||
" abstract class WithoutPlaceholder {",
|
||||
" @BeforeTemplate",
|
||||
" protected synchronized String before(String str) {",
|
||||
" return str;",
|
||||
" }",
|
||||
" }",
|
||||
"",
|
||||
" abstract class WithPlaceholder<I, O> {",
|
||||
" @Placeholder",
|
||||
" public abstract O someFunction(I input);",
|
||||
"",
|
||||
" @BeforeTemplate",
|
||||
" public final String before(I input) {",
|
||||
" return String.valueOf(someFunction(input));",
|
||||
" }",
|
||||
" }",
|
||||
"}")
|
||||
.addOutputLines(
|
||||
"B.java",
|
||||
"import com.google.errorprone.refaster.annotation.BeforeTemplate;",
|
||||
"import com.google.errorprone.refaster.annotation.Placeholder;",
|
||||
"",
|
||||
"final class B {",
|
||||
" static final class WithoutPlaceholder {",
|
||||
" @BeforeTemplate",
|
||||
" String before(String str) {",
|
||||
" return str;",
|
||||
" }",
|
||||
" }",
|
||||
"",
|
||||
" abstract static class WithPlaceholder<I, O> {",
|
||||
" @Placeholder",
|
||||
" abstract O someFunction(I input);",
|
||||
"",
|
||||
" @BeforeTemplate",
|
||||
" String before(I input) {",
|
||||
" return String.valueOf(someFunction(input));",
|
||||
" }",
|
||||
" }",
|
||||
"}")
|
||||
.doTest(TestMode.TEXT_MATCH);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user