Skip to content

Commit

Permalink
Ban extending @AutoValue.Builder and @autobuilder classes
Browse files Browse the repository at this point in the history 
This addresses the internal AndroidLint checker, and the external-but-disabled-internally errorprone.

This was TGP'ed. There were only 3 offending cases. I suppressed 2 and refactored 1.

I reworded "an @AutoValue/@AutoOneOf class" to a broader "AutoValue-like classes", but I'm open to better ideas.

PiperOrigin-RevId: 686061210
  • Loading branch information
@java-team-github-bot
java-team-github-bot authored and Error Prone Team committed Oct 29, 2024
1 parent e71db1f commit c60ecb0
Show file tree
Hide file tree
Showing 2 changed files with 157 additions and 19 deletions.
62 changes: 49 additions & 13 deletions core/src/main/java/com/google/errorprone/bugpatterns/ExtendsAutoValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,47 +16,83 @@

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 {

private static final Supplier<ImmutableSet<Name>> AUTOS =
VisitorState.memoize(
s ->
ImmutableSet.of(
s.getName("com.google.auto.value.AutoOneOf"),
s.getName("com.google.auto.value.AutoValue")));

private static final Supplier<ImmutableSet<Name>> 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<ImmutableSet<Name>> 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<Tree> 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) {
Expand Down
114 changes: 108 additions & 6 deletions core/src/test/java/com/google/errorprone/bugpatterns/ExtendsAutoValueTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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 {}
}
""")
Expand All @@ -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();
Expand Down Expand Up @@ -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 {}
}
""")
Expand Down

0 comments on commit c60ecb0

Please sign in to comment.