jlengrand/error-prone-support
1
0
Fork 0
mirror of https://github.com/jlengrand/error-prone-support.git synced 2026-03-10 08:11:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

WIP

Browse Source
This commit is contained in:
Stephan Schroevers
2023-04-12 13:58:00 +02:00
parent 87f79dae60
commit 390d84d3dd
6 changed files with 297 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

163
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RefasterExpressionTypeInvariance.java Normal file
View File

@@ -0,0 +1,163 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.LIKELY_ERROR;
import static com.google.errorprone.matchers.Matchers.hasAnnotation;
import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import com.google.auto.service.AutoService;
import com.google.common.collect.ImmutableSet;
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.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.refaster.Refaster;
import com.google.errorprone.refaster.annotation.AfterTemplate;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ClassTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.LambdaExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.ReturnTree;
import com.sun.source.tree.Tree;
import com.sun.source.util.TreeScanner;
import com.sun.tools.javac.code.Type;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import org.jspecify.annotations.Nullable;
/**
* A {@link BugChecker} that flags Refaster expression templates for which the
* {@code @AfterTemplate} return type is not a subtype of the `@BeforeTemplate` return type.
*/
@AutoService(BugChecker.class)
@BugPattern(
summary = "Refaster templates should have invariant return types",
link = BUG_PATTERNS_BASE_URL + "RefasterExpressionTypeInvariance",
linkType = CUSTOM,
severity = WARNING,
tags = LIKELY_ERROR)
public final class RefasterExpressionTypeInvariance extends BugChecker implements ClassTreeMatcher {
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<ExpressionTree> REFASTER_ANY_OF =
staticMethod().onClass(Refaster.class.getName()).named("anyOf");
// XXX: False positive in `OptionalMap`. First write a rule to simplify such case.
@Override
public Description matchClass(ClassTree tree, VisitorState state) {
Optional<Type> afterTemplateResultType = getAfterTemplateResultType(tree, state);
if (afterTemplateResultType.isEmpty()) {
return Description.NO_MATCH;
}
ImmutableSet<Type> beforeTemplateResultTypes = getBeforeTemplateResultTypes(tree, state);
if (beforeTemplateResultTypes.isEmpty()) {
return Description.NO_MATCH;
}
Type replacementType = afterTemplateResultType.orElseThrow();
if (areAllSuperTypeOf(replacementType, beforeTemplateResultTypes, state)) {
return Description.NO_MATCH;
}
Type alternativeReplacementType =
(replacementType.isPrimitive()
? state.getTypes().boxedTypeOrType(replacementType)
: state.getTypes().unboxedType(replacementType));
boolean requiresBoxingOrUnboxing =
areAllSuperTypeOf(alternativeReplacementType, beforeTemplateResultTypes, state);
// @Considerations
// enum ConsiderationType:
// MAY_CAUSE_BOXING
// MAY_CAUSE_UNBOXING
// MAY_CAUSE_COMPILATION_ERROR\
if (requiresBoxingOrUnboxing) {
if (replacementType.isPrimitive()) {
// MAY_CAUSE_UNBOXING
// XXX: Flag this using a custom annotation.
return Description.NO_MATCH;
}
// MAY_CAUSE_
// XXX: Flag this using a custom annotation.
return Description.NO_MATCH;
}
// MAY_CAUSE_COMPILATION_ERROR
// XXX: Flag this using a custom annotation.
// XXX: This case contains false positives; maybe leave out for now?
return describeMatch(tree);
}
private static boolean areAllSuperTypeOf(
Type subType, ImmutableSet<Type> superTypes, VisitorState state) {
return superTypes.stream().allMatch(type -> state.getTypes().isSuperType(type, subType));
}
private static ImmutableSet<Type> getBeforeTemplateResultTypes(
ClassTree tree, VisitorState state) {
return tree.getMembers().stream()
.filter(m -> BEFORE_TEMPLATE_METHOD.matches(m, state))
.map(m -> getResultType((MethodTree) m, state))
.collect(toImmutableSet());
}
private static Optional<Type> getAfterTemplateResultType(ClassTree tree, VisitorState state) {
return tree.getMembers().stream()
.filter(m -> AFTER_TEMPLATE_METHOD.matches(m, state))
.map(m -> getResultType((MethodTree) m, state))
.reduce(state.getTypes()::lub);
}
// XXX: A variant of this pattern also occurs a few times inside Error Prone; contribute upstream.
// XXX: Copied from `JUnitValueSource`.
private static Type getResultType(MethodTree methodTree, VisitorState state) {
List<Type> resultTypes = new ArrayList<>();
new TreeScanner<@Nullable Void, @Nullable Void>() {
@Override
public @Nullable Void visitClass(ClassTree node, @Nullable Void unused) {
/* Ignore `return` statements inside anonymous/local classes. */
return null;
}
@Override
public @Nullable Void visitReturn(ReturnTree node, @Nullable Void unused) {
if (REFASTER_ANY_OF.matches(node.getExpression(), state)) {
for (ExpressionTree arg : ((MethodInvocationTree) node.getExpression()).getArguments()) {
resultTypes.add(ASTHelpers.getType(arg));
}
} else if (ASTHelpers.stripParentheses(node.getExpression()).getKind()
!= Tree.Kind.NULL_LITERAL) {
resultTypes.add(ASTHelpers.getType(node.getExpression()));
}
return super.visitReturn(node, unused);
}
@Override
public @Nullable Void visitLambdaExpression(
LambdaExpressionTree node, @Nullable Void unused) {
/* Ignore `return` statements inside lambda expressions. */
return null;
}
}.scan(methodTree, null);
return resultTypes.stream()
.reduce(state.getTypes()::lub)
.orElseGet(() -> state.getSymtab().botType);
}
}

78
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/RefasterParameterTypeGenerics.java Normal file
View File

@@ -0,0 +1,78 @@
package tech.picnic.errorprone.bugpatterns;
import static com.google.errorprone.BugPattern.LinkType.CUSTOM;
import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION;
import static com.google.errorprone.BugPattern.StandardTags.STYLE;
import static com.google.errorprone.matchers.Matchers.hasAnnotation;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
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.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.refaster.annotation.BeforeTemplate;
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 com.sun.source.tree.VariableTree;
import com.sun.tools.javac.code.Type;
// XXX: The plan towards canonical Refaster templates:
// 1. Replace wildcard types.
// 2. Enforce rule naming conventions.
// 3. Split rules with incompatible parameter types.
// 4. (Un)annotate templates with incompatible result expressions.
//
// Also:
// 0. Canonicalize template method order (@PlaceHolder, @BeforeTemplate, @AfterTemplate).
// ^ There may be logic by which to break ties; TBD.
// 1. Canonicalize template method parameter order.
// ^ See https://github.com/PicnicSupermarket/error-prone-support/pull/775.
// 2. Canonicalize template type parameter order following a similar approach.
// 3. Canonicalize template method parameter names by deriving them from the corresponding parameter
// names of the code invoked in the `@AfterTemplate` method.
// 4. Canonicalize type parameter names following a similar approach.
/**
* A {@link BugChecker} that flags Refaster expression templates parameters with wildcard types, and
* suggests simplifying these, possibly by declaring a corresponding class type parameter.
*/
// XXX: Given `? extends T`/`? super T`, suggest just dropping the wildcard if `T` is not referenced
// elsewhere in the same `@BeforeTemplate` signature.
// XXX: The type parameters can also be defined at the method level. Such cases should be moved to
// the class level.
// ^ Relatedly, if not already disallowed, constrain all Refaster method modifiers. Should not be
// `static`, etc.
@AutoService(BugChecker.class)
@BugPattern(
summary = "Refaster template parameter types should not contain wildcards",
link = BUG_PATTERNS_BASE_URL + "RefasterParameterTypeGenerics",
linkType = CUSTOM,
severity = SUGGESTION,
tags = {SIMPLIFICATION, STYLE})
public final class RefasterParameterTypeGenerics extends BugChecker implements ClassTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Matcher<Tree> BEFORE_TEMPLATE_METHOD = hasAnnotation(BeforeTemplate.class);
@Override
public Description matchClass(ClassTree tree, VisitorState state) {
for (Tree member : tree.getMembers()) {
if (BEFORE_TEMPLATE_METHOD.matches(member, state)) {
MethodTree method = (MethodTree) member;
for (VariableTree parameter : method.getParameters()) {
Type type = ASTHelpers.getType(parameter);
if (type != null && type.isParameterized()) {
return describeMatch(parameter);
}
}
}
}
return Description.NO_MATCH;
}
}

4
error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/OptionalRules.java
View File

@@ -427,12 +427,12 @@ final class OptionalRules {
// dropped in favour of `StreamMapFirst` and `OptionalIdentity`.
static final class OptionalMap<S, T> {
@BeforeTemplate
Optional<? extends T> before(Optional<S> optional, Function<? super S, ? extends T> function) {
Optional<? extends T> before(Optional<S> optional, Function<? super S, T> function) {
return optional.stream().map(function).findAny();
}
@AfterTemplate
Optional<? extends T> after(Optional<S> optional, Function<? super S, ? extends T> function) {
Optional<? extends T> after(Optional<S> optional, Function<? super S, T> function) {
return optional.map(function);
}
}

28
error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RefasterExpressionTypeInvarianceTest.java Normal file
View File

@@ -0,0 +1,28 @@
package tech.picnic.errorprone.bugpatterns;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;
final class RefasterExpressionTypeInvarianceTest {
@Test
void identification() {
CompilationTestHelper.newInstance(RefasterExpressionTypeInvariance.class, getClass())
.addSourceLines(
"A.java",
"import com.google.errorprone.refaster.annotation.BeforeTemplate;",
"",
"class A {",
" @BeforeTemplate",
" Object before(String str) {",
" if (str == null) {",
" return null;",
" }",
" if (str != null) {",
" return (CharSequence) null;",
" }",
" return str;",
" }",
"}")
.doTest();
}
}

25
error-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/RefasterRefasterParameterTypeGenericsTest.java Normal file
View File

@@ -0,0 +1,25 @@
package tech.picnic.errorprone.bugpatterns;
import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.Test;
final class RefasterRefasterParameterTypeGenericsTest {
// XXX: Rename inner class.
@Test
void identification() {
CompilationTestHelper.newInstance(RefasterParameterTypeGenerics.class, getClass())
.addSourceLines(
"A.java",
"import com.google.errorprone.refaster.annotation.BeforeTemplate;",
"import java.util.function.Function;",
"",
"class A {",
" static class B<I, O> {",
" @BeforeTemplate",
" // BUG: Diagnostic contains:",
" void before(Function<? super I, ? extends O> function) {}",
" }",
"}")
.doTest();
}
}

1
refaster-support/src/main/java/tech/picnic/errorprone/refaster/annotation/Description.java
View File

@@ -10,6 +10,7 @@ import java.lang.annotation.Target;
*
* <p>Annotations on nested classes override the description associated with any enclosing class.
*/
// XXX: This becomes easier to use once we can use text blocks.
@Target(ElementType.TYPE)
@Retention(RetentionPolicy.SOURCE)
public @interface Description {