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.

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

Ban extending @AutoValue.Builder and @AutoBuilder classes #4645

Merged
merged 1 commit into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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
Loading