Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement automatic fixes for ImmutablesStyle #1846

Merged
merged 3 commits into from
Jul 23, 2021
Merged

Implement automatic fixes for ImmutablesStyle #1846

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
80e36e5
Implement automatic fixes for `ImmutablesStyle`
carterkozak Jul 22, 2021
087c464
Add generated changelog entries
svc-changelog Jul 23, 2021
fa8a16a
suppress top-level class ImmutablesStyle findings automatically
carterkozak Jul 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 96 additions & 16 deletions baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/ImmutablesStyle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,31 @@
package com.palantir.baseline.errorprone;

import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.BugPattern;
import com.google.errorprone.BugPattern.LinkType;
import com.google.errorprone.BugPattern.SeverityLevel;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.inject.ElementPredicates;
import com.google.errorprone.fixes.SuggestedFix;
import com.google.errorprone.fixes.SuggestedFixes;
import com.google.errorprone.matchers.AnnotationMatcherUtils;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.Matchers;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.AnnotationTree;
import com.sun.source.tree.ClassTree;
import com.sun.tools.javac.code.Symbol.ClassSymbol;
import com.sun.source.tree.ExpressionTree;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Type;
import java.lang.annotation.Annotation;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.util.List;
import javax.annotation.Nullable;
import org.immutables.value.Value;

@AutoService(BugChecker.class)
Expand All @@ -46,24 +59,91 @@ public final class ImmutablesStyle extends BugChecker implements BugChecker.Clas

@Override
public Description matchClass(ClassTree tree, VisitorState state) {
if (!STYLE_ANNOTATION.matches(tree, state)) {
return Description.NO_MATCH;
if (STYLE_ANNOTATION.matches(tree, state)) {
switch (tree.getKind()) {
case CLASS:
case INTERFACE:
return matchStyleAnnotatedType(tree, state);
case ANNOTATION_TYPE:
return matchStyleMetaAnnotation(tree, state);
default:
break;
}
}
return Description.NO_MATCH;
}

switch (tree.getKind()) {
case CLASS:
case INTERFACE:
return describeMatch(tree);
case ANNOTATION_TYPE:
ClassSymbol classSymbol = ASTHelpers.getSymbol(tree);
if (!ElementPredicates.hasSourceRetention(classSymbol)) {
return describeMatch(tree);
}
break;
default:
break;
private Description matchStyleAnnotatedType(ClassTree tree, VisitorState state) {
if (ASTHelpers.enclosingClass(ASTHelpers.getSymbol(tree)) == null) {
// Top level class, we cannot create new files with suggested fixes, nor should we create
// additional top-level classes.
return buildDescription(tree)
.addFix(SuggestedFixes.addSuppressWarnings(
state, canonicalName(), "Automatically suppressed to unblock enforcement in new code"))
.build();
}
SuggestedFix.Builder fix = SuggestedFix.builder();
String qualifiedTarget = SuggestedFixes.qualifyType(state, fix, Target.class.getName());
String qualifiedElementType = SuggestedFixes.qualifyType(state, fix, ElementType.class.getName());
String qualifiedRetention = SuggestedFixes.qualifyType(state, fix, Retention.class.getName());
String qualifiedRetentionPolicy = SuggestedFixes.qualifyType(state, fix, RetentionPolicy.class.getName());
AnnotationTree styleAnnotationTree = getAnnotation(tree, Value.Style.class, state);
return buildDescription(tree)
.addFix(fix.prefixWith(
tree,
String.format(
"\n@%s(%s.TYPE)\n@%s(%s.SOURCE)\n%s\n@interface %sStyle {}\n@%sStyle\n",
qualifiedTarget,
qualifiedElementType,
qualifiedRetention,
qualifiedRetentionPolicy,
state.getSourceForNode(styleAnnotationTree),
tree.getSimpleName(),
tree.getSimpleName()))
.replace(styleAnnotationTree, "")
.build())
.build();
}

private Description matchStyleMetaAnnotation(ClassTree tree, VisitorState state) {
AnnotationTree retention = getAnnotation(tree, Retention.class, state);
if (retention == null) {
SuggestedFix.Builder fix = SuggestedFix.builder();
fix.prefixWith(
tree,
String.format(
"@%s(%s.SOURCE)",
SuggestedFixes.qualifyType(state, fix, Retention.class.getName()),
SuggestedFixes.qualifyType(state, fix, RetentionPolicy.class.getName())));
return buildDescription(tree).addFix(fix.build()).build();
}
ExpressionTree retentionValue = AnnotationMatcherUtils.getArgument(retention, "value");
Symbol retentionValueSymbol = ASTHelpers.getSymbol(retentionValue);
if (retentionValueSymbol == null
|| !retentionValueSymbol.getSimpleName().contentEquals("SOURCE")) {
SuggestedFix.Builder fix = SuggestedFix.builder();
fix.merge(SuggestedFixes.updateAnnotationArgumentValues(
retention,
"value",
ImmutableList.of(String.format(
"%s.SOURCE", SuggestedFixes.qualifyType(state, fix, RetentionPolicy.class.getName())))));
return buildDescription(tree).addFix(fix.build()).build();
}
return Description.NO_MATCH;
}

@Nullable
private static AnnotationTree getAnnotation(
ClassTree tree, Class<? extends Annotation> annotationType, VisitorState state) {
List<? extends AnnotationTree> annotations = tree.getModifiers().getAnnotations();
Type retention = state.getTypeFromString(annotationType.getName());
if (retention != null) {
for (AnnotationTree annotation : annotations) {
if (ASTHelpers.isSameType(ASTHelpers.getType(annotation.getAnnotationType()), retention, state)) {
return annotation;
}
}
}
return null;
}
}
125 changes: 125 additions & 0 deletions baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/ImmutablesStyleTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,49 @@ public void testInlineAnnotation() {
.doTest();
}

@Test
public void fixInlineAnnotation_topLevel() {
fix().addInputLines(
"Person.java",
"import org.immutables.value.Value;",
"@Value.Style(visibility = Value.Style.ImplementationVisibility.PUBLIC)",
"public interface Person {}")
.addOutputLines(
"Person.java",
"import org.immutables.value.Value;",
"@SuppressWarnings(\"ImmutablesStyle\")",
"@Value.Style(visibility = Value.Style.ImplementationVisibility.PUBLIC)",
"public interface Person {}")
.doTest();
}

@Test
public void fixInlineAnnotation_enclosed() {
fix().addInputLines(
"Enclosing.java",
"import org.immutables.value.Value;",
"public class Enclosing {",
" @Value.Style(visibility = Value.Style.ImplementationVisibility.PUBLIC)",
" public interface Person {}",
"}")
.addOutputLines(
"Enclosing.java",
"import java.lang.annotation.ElementType;",
"import java.lang.annotation.Retention;",
"import java.lang.annotation.RetentionPolicy;",
"import java.lang.annotation.Target;",
"import org.immutables.value.Value;",
"public class Enclosing {",
" @Target(ElementType.TYPE)",
" @Retention(RetentionPolicy.SOURCE)",
" @Value.Style(visibility = Value.Style.ImplementationVisibility.PUBLIC)",
" @interface PersonStyle {}",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be private? Or does that not work with immutables?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would fail to compile when encapsulated within an interface, in that case the annotation will be public. I’m not terribly worried about that case because it won’t have any runtime impact, only build time.

" @PersonStyle",
" public interface Person {}",
"}")
.doTest();
}

@Test
public void testMetaAnnotation_defaultRetention() {
helper().addSourceLines(
Expand All @@ -46,6 +89,30 @@ public void testMetaAnnotation_defaultRetention() {
.doTest();
}

@Test
public void fixMetaAnnotation_defaultRetention() {
fix().addInputLines(
"MyMetaAnnotation.java",
"import java.lang.annotation.ElementType;",
"import java.lang.annotation.Target;",
"import org.immutables.value.Value;",
"@Target({ElementType.PACKAGE, ElementType.TYPE})\n",
"@Value.Style(visibility = Value.Style.ImplementationVisibility.PUBLIC)\n",
"public @interface MyMetaAnnotation {}")
.addOutputLines(
"MyMetaAnnotation.java",
"import java.lang.annotation.ElementType;",
"import java.lang.annotation.Retention;",
"import java.lang.annotation.RetentionPolicy;",
"import java.lang.annotation.Target;",
"import org.immutables.value.Value;",
"@Retention(RetentionPolicy.SOURCE)",
"@Target({ElementType.PACKAGE, ElementType.TYPE})\n",
"@Value.Style(visibility = Value.Style.ImplementationVisibility.PUBLIC)\n",
"public @interface MyMetaAnnotation {}")
.doTest();
}

@Test
public void testMetaAnnotation_classRetention() {
helper().addSourceLines(
Expand All @@ -64,6 +131,33 @@ public void testMetaAnnotation_classRetention() {
.doTest();
}

@Test
public void fixMetaAnnotation_classRetention() {
fix().addInputLines(
"MyMetaAnnotation.java",
"import java.lang.annotation.ElementType;",
"import java.lang.annotation.Retention;",
"import java.lang.annotation.RetentionPolicy;",
"import java.lang.annotation.Target;",
"import org.immutables.value.Value;",
"@Target({ElementType.PACKAGE, ElementType.TYPE})",
"@Retention(RetentionPolicy.CLASS)",
"@Value.Style(visibility = Value.Style.ImplementationVisibility.PUBLIC)",
"public @interface MyMetaAnnotation {}")
.addOutputLines(
"MyMetaAnnotation.java",
"import java.lang.annotation.ElementType;",
"import java.lang.annotation.Retention;",
"import java.lang.annotation.RetentionPolicy;",
"import java.lang.annotation.Target;",
"import org.immutables.value.Value;",
"@Target({ElementType.PACKAGE, ElementType.TYPE})",
"@Retention(RetentionPolicy.SOURCE)",
"@Value.Style(visibility = Value.Style.ImplementationVisibility.PUBLIC)",
"public @interface MyMetaAnnotation {}")
.doTest();
}

@Test
public void testMetaAnnotation_runtimeRetention() {
helper().addSourceLines(
Expand All @@ -82,6 +176,33 @@ public void testMetaAnnotation_runtimeRetention() {
.doTest();
}

@Test
public void fixMetaAnnotation_runtimeRetention() {
fix().addInputLines(
"MyMetaAnnotation.java",
"import java.lang.annotation.ElementType;",
"import java.lang.annotation.Retention;",
"import java.lang.annotation.RetentionPolicy;",
"import java.lang.annotation.Target;",
"import org.immutables.value.Value;",
"@Target({ElementType.PACKAGE, ElementType.TYPE})",
"@Retention(value = RetentionPolicy.RUNTIME)",
"@Value.Style(visibility = Value.Style.ImplementationVisibility.PUBLIC)",
"public @interface MyMetaAnnotation {}")
.addOutputLines(
"MyMetaAnnotation.java",
"import java.lang.annotation.ElementType;",
"import java.lang.annotation.Retention;",
"import java.lang.annotation.RetentionPolicy;",
"import java.lang.annotation.Target;",
"import org.immutables.value.Value;",
"@Target({ElementType.PACKAGE, ElementType.TYPE})",
"@Retention(value = RetentionPolicy.SOURCE)",
"@Value.Style(visibility = Value.Style.ImplementationVisibility.PUBLIC)",
"public @interface MyMetaAnnotation {}")
.doTest();
}

@Test
public void testMetaAnnotation_sourceRetention() {
helper().addSourceLines(
Expand Down Expand Up @@ -109,4 +230,8 @@ public void testOtherAnnotation() {
private CompilationTestHelper helper() {
return CompilationTestHelper.newInstance(ImmutablesStyle.class, getClass());
}

private RefactoringValidator fix() {
return RefactoringValidator.of(ImmutablesStyle.class, getClass());
}
}
5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-1846.v2.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: improvement
improvement:
description: Implement automatic fixes for `ImmutablesStyle`
links:
- https://github.com/palantir/gradle-baseline/pull/1846
1 change: 1 addition & 0 deletions ...ne-java/src/main/groovy/com/palantir/baseline/extensions/BaselineErrorProneExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public class BaselineErrorProneExtension {
"ExecutorSubmitRunnableFutureIgnored",
"ExtendsErrorOrThrowable",
"FinalClass",
"ImmutablesStyle",
"ImplicitPublicBuilderConstructor",
"JavaTimeDefaultTimeZone",
"JavaTimeSystemDefaultTimeZone",
Expand Down