diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/ExtendsAutoValue.java b/core/src/main/java/com/google/errorprone/bugpatterns/ExtendsAutoValue.java index dca28df5d7c..7cd45ba6341 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/ExtendsAutoValue.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/ExtendsAutoValue.java @@ -16,20 +16,27 @@ package com.google.errorprone.bugpatterns; +import static com.google.errorprone.util.ASTHelpers.annotationsAmong; +import static com.google.errorprone.util.ASTHelpers.getSymbol; + import com.google.common.collect.ImmutableSet; import com.google.errorprone.BugPattern; import com.google.errorprone.BugPattern.SeverityLevel; +import com.google.errorprone.ErrorProneFlags; import com.google.errorprone.VisitorState; import com.google.errorprone.bugpatterns.BugChecker.ClassTreeMatcher; import com.google.errorprone.matchers.Description; import com.google.errorprone.suppliers.Supplier; import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.ClassTree; +import com.sun.source.tree.Tree; import com.sun.tools.javac.util.Name; +import java.util.stream.Stream; +import javax.inject.Inject; /** Makes sure that you are not extending a class that has @AutoValue as an annotation. */ @BugPattern( - summary = "Do not extend an @AutoValue/@AutoOneOf class in non-generated code.", + summary = "Do not extend an @AutoValue-like classes in non-generated code.", severity = SeverityLevel.ERROR) public final class ExtendsAutoValue extends BugChecker implements ClassTreeMatcher { @@ -37,26 +44,55 @@ public final class ExtendsAutoValue extends BugChecker implements ClassTreeMatch VisitorState.memoize( s -> ImmutableSet.of( + s.getName("com.google.auto.value.AutoOneOf"), + s.getName("com.google.auto.value.AutoValue"))); + + private static final Supplier> AUTOS_AND_BUILDERS = + VisitorState.memoize( + s -> + ImmutableSet.of( + s.getName("com.google.auto.value.AutoBuilder"), + s.getName("com.google.auto.value.AutoOneOf"), s.getName("com.google.auto.value.AutoValue"), - s.getName("com.google.auto.value.AutoOneOf"))); + s.getName("com.google.auto.value.AutoValue$Builder"))); + + private final boolean testBuilders; + private final Supplier> autos; + + @Inject + ExtendsAutoValue(ErrorProneFlags flags) { + this.testBuilders = flags.getBoolean("ExtendsAutoValue:builders").orElse(true); + this.autos = testBuilders ? AUTOS_AND_BUILDERS : AUTOS; + } @Override public Description matchClass(ClassTree tree, VisitorState state) { - if (tree.getExtendsClause() == null) { - // Doesn't extend anything, can't possibly be a violation. + if (tree.getExtendsClause() == null + && (!testBuilders || tree.getImplementsClause().isEmpty())) { return Description.NO_MATCH; } - if (!ASTHelpers.annotationsAmong( - ASTHelpers.getSymbol(tree.getExtendsClause()), AUTOS.get(state), state) - .isEmpty()) { - // Violation: one of its superclasses extends AutoValue. - if (!isInGeneratedCode(state)) { - return buildDescription(tree).build(); - } - } + Stream parents = + Stream.concat( + Stream.ofNullable(tree.getExtendsClause()), + testBuilders ? tree.getImplementsClause().stream() : Stream.empty()); - return Description.NO_MATCH; + return parents + .map(parent -> annotationsAmong(getSymbol(parent), autos.get(state), state)) + .filter(annotations -> !annotations.isEmpty()) + .findFirst() + .filter(unused -> !isInGeneratedCode(state)) + .map( + annotations -> { + String name = annotations.iterator().next().toString(); + name = name.substring(name.lastIndexOf('.') + 1); // Strip package + name = name.replace('$', '.'); // AutoValue$Builder -> AutoValue.Builder + return buildDescription(tree) + .setMessage( + String.format("Do not extend an @%s class in non-generated code.", name)) + .build(); + }) + .orElse(Description.NO_MATCH); } private static boolean isInGeneratedCode(VisitorState state) { diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/ExtendsAutoValueTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/ExtendsAutoValueTest.java index 5167ccb2a9c..b78536b2b9b 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/ExtendsAutoValueTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/ExtendsAutoValueTest.java @@ -98,12 +98,114 @@ public void extendsAutoValue_bad() { @AutoValue class AutoClass {} - // BUG: Diagnostic contains: ExtendsAutoValue + // BUG: Diagnostic contains: Do not extend an @AutoValue class in non-generated code. public class TestClass extends AutoClass {} """) .doTest(); } + @Test + public void extendsAutoValue_builder_bad() { + helper + .addSourceLines( + "TestBuilder.java", + """ +import com.google.auto.value.AutoValue; + +@AutoValue +class AutoClass { + @AutoValue.Builder + abstract static class Builder { + abstract AutoClass build(); + } +} + +// BUG: Diagnostic contains: Do not extend an @AutoValue.Builder class in non-generated code. +public class TestBuilder extends AutoClass.Builder { + AutoClass build() { + throw new RuntimeException(); + } +} +""") + .doTest(); + } + + @Test + public void implementsAutoValue_builder_bad() { + helper + .addSourceLines( + "TestBuilder.java", + """ +import com.google.auto.value.AutoValue; + +@AutoValue +class AutoClass { + @AutoValue.Builder + interface Builder { + AutoClass build(); + } +} + +// BUG: Diagnostic contains: Do not extend an @AutoValue.Builder class in non-generated code. +public class TestBuilder implements AutoClass.Builder { + public AutoClass build() { + throw new RuntimeException(); + } +} +""") + .doTest(); + } + + @Test + public void extendsAutoBuilder_bad() { + helper + .addSourceLines( + "TestBuilder.java", + """ + import com.google.auto.value.AutoBuilder; + + class MyClass { + @AutoBuilder + abstract static class Builder { + abstract MyClass build(); + } + } + + // BUG: Diagnostic contains: Do not extend an @AutoBuilder class in non-generated code. + public class TestBuilder extends MyClass.Builder { + MyClass build() { + throw new RuntimeException(); + } + } + """) + .doTest(); + } + + @Test + public void implementsAutoBuilder_bad() { + helper + .addSourceLines( + "TestBuilder.java", + """ + import com.google.auto.value.AutoBuilder; + + class MyClass { + @AutoBuilder + interface Builder { + MyClass build(); + } + } + + // BUG: Diagnostic contains: Do not extend an @AutoBuilder class in non-generated code. + public class TestBuilder implements MyClass.Builder { + public MyClass build() { + throw new RuntimeException(); + } + } + """) + .doTest(); + } + @Test public void extendsAutoOneOf_bad() { helper @@ -117,7 +219,7 @@ class AutoClass { enum Kind {} } - // BUG: Diagnostic contains: ExtendsAutoValue + // BUG: Diagnostic contains: Do not extend an @AutoOneOf class in non-generated code. public class TestClass extends AutoClass {} """) .doTest(); @@ -132,7 +234,7 @@ public void extendsAutoValue_badNoImport() { @com.google.auto.value.AutoValue class AutoClass {} - // BUG: Diagnostic contains: ExtendsAutoValue + // BUG: Diagnostic contains: Do not extend an @AutoValue class in non-generated code. public class TestClass extends AutoClass {} """) .doTest(); @@ -150,7 +252,7 @@ public class OuterClass { @AutoValue abstract static class AutoClass {} - // BUG: Diagnostic contains: ExtendsAutoValue + // BUG: Diagnostic contains: Do not extend an @AutoValue class in non-generated code. class TestClass extends AutoClass {} } """) @@ -170,7 +272,7 @@ class OuterClass { static class AutoClass {} } - // BUG: Diagnostic contains: ExtendsAutoValue + // BUG: Diagnostic contains: Do not extend an @AutoValue class in non-generated code. public class TestClass extends OuterClass.AutoClass {} """) .doTest(); @@ -205,7 +307,7 @@ public void extendsAutoValue_innerClassExtends() { class AutoClass {} public class TestClass { - // BUG: Diagnostic contains: ExtendsAutoValue + // BUG: Diagnostic contains: Do not extend an @AutoValue class in non-generated code. public class Extends extends AutoClass {} } """)