Skip to content

Commit

Permalink
Introduce MoreTypes utility class
Browse files Browse the repository at this point in the history
The static methods of this class allow one to construct complex types,
against which expression types can subsequently be matched.
  • Loading branch information
Stephan202 committed Oct 2, 2022
1 parent 0561c37 commit c267b8f
Show file tree
Hide file tree
Showing 3 changed files with 325 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
import static com.google.errorprone.BugPattern.StandardTags.FRAGILE_CODE;
import static tech.picnic.errorprone.bugpatterns.util.Documentation.BUG_PATTERNS_BASE_URL;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.generic;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.raw;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.subOf;

import com.google.auto.service.AutoService;
import com.google.common.collect.Iterables;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
Expand All @@ -16,9 +18,7 @@
import com.google.errorprone.suppliers.Suppliers;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.util.List;
import java.util.Optional;

/** A {@link BugChecker} that flags nesting of {@link Optional Optionals}. */
Expand All @@ -33,21 +33,13 @@
public final class NestedOptionals extends BugChecker implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;
private static final Supplier<Type> OPTIONAL = Suppliers.typeFromClass(Optional.class);
private static final Supplier<Type> OPTIONAL_OF_OPTIONAL =
VisitorState.memoize(generic(OPTIONAL, subOf(raw(OPTIONAL))));

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
return isOptionalOfOptional(tree, state) ? describeMatch(tree) : Description.NO_MATCH;
}

private static boolean isOptionalOfOptional(Tree tree, VisitorState state) {
Type optionalType = OPTIONAL.get(state);
Type type = ASTHelpers.getType(tree);
if (!ASTHelpers.isSubtype(type, optionalType, state)) {
return false;
}

List<Type> typeArguments = type.getTypeArguments();
return !typeArguments.isEmpty()
&& ASTHelpers.isSubtype(Iterables.getOnlyElement(typeArguments), optionalType, state);
return state.getTypes().isSubtype(ASTHelpers.getType(tree), OPTIONAL_OF_OPTIONAL.get(state))
? describeMatch(tree)
: Description.NO_MATCH;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
package tech.picnic.errorprone.bugpatterns.util;

import static java.util.stream.Collectors.toCollection;

import com.google.errorprone.VisitorState;
import com.google.errorprone.suppliers.Supplier;
import com.google.errorprone.suppliers.Suppliers;
import com.sun.tools.javac.code.BoundKind;
import com.sun.tools.javac.code.Type;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.function.BiFunction;

/**
* A set of helper methods which together define a DSL for defining {@link Type types}.
*
* <p>These methods are meant to be statically imported. Example usage:
*
* <pre>{@code
* Supplier<Type> type =
* VisitorState.memoize(
* generic(
* type("reactor.core.publisher.Flux"),
* subOf(generic(type("org.reactivestreams.Publisher"), unbound()))));
* }</pre>
*
* This statement produces a memoized supplier of the type {@code Flux<? extends Publisher<?>>}.
*/
public final class MoreTypes {
private MoreTypes() {}

/**
* Creates a supplier of the type with the given fully qualified name.
*
* <p>This method should only be used when building more complex types in combination with other
* {@link MoreTypes} methods. In other cases prefer directly calling {@link
* Suppliers#typeFromString(String)}.
*
* @param typeName The type of interest.
* @return A supplier which returns the described type if available in the given state, and {@code
* null} otherwise.
*/
public static Supplier<Type> type(String typeName) {
return Suppliers.typeFromString(typeName);
}

/**
* Creates a supplier of the described generic type.
*
* @param type The base type of interest.
* @param typeArgs The desired type arguments.
* @return A supplier which returns the described type if available in the given state, and {@code
* null} otherwise.
*/
// XXX: The given `type` should be a generic type, so perhaps `withParams` would be a better
// method name. But the DSL wouldn't look as nice that way.
@SafeVarargs
@SuppressWarnings("varargs")
public static Supplier<Type> generic(Supplier<Type> type, Supplier<Type>... typeArgs) {
return propagateNull(
type,
(state, baseType) -> {
List<Type> params =
Arrays.stream(typeArgs).map(s -> s.get(state)).collect(toCollection(ArrayList::new));
if (params.stream().anyMatch(Objects::isNull)) {
return null;
}

return state.getType(baseType, /* isArray= */ false, params);
});
}

/**
* Creates a raw (erased, non-generic) variant of the given type.
*
* @param type The base type of interest.
* @return A supplier which returns the described type if available in the given state, and {@code
* null} otherwise.
*/
public static Supplier<Type> raw(Supplier<Type> type) {
return propagateNull(type, (state, baseType) -> baseType.tsym.erasure(state.getTypes()));
}

/**
* Creates a {@code ? super T} wildcard type, with {@code T} bound to the given type.
*
* @param type The base type of interest.
* @return A supplier which returns the described type if available in the given state, and {@code
* null} otherwise.
*/
public static Supplier<Type> supOf(Supplier<Type> type) {
return propagateNull(
type,
(state, baseType) ->
new Type.WildcardType(baseType, BoundKind.SUPER, state.getSymtab().boundClass));
}

/**
* Creates a {@code ? extends T} wildcard type, with {@code T} bound to the given type.
*
* @param type The base type of interest.
* @return A supplier which returns the described type if available in the given state, and {@code
* null} otherwise.
*/
public static Supplier<Type> subOf(Supplier<Type> type) {
return propagateNull(
type,
(state, baseType) ->
new Type.WildcardType(
type.get(state), BoundKind.EXTENDS, state.getSymtab().boundClass));
}

/**
* Creates an unbound wildcard type ({@code ?}).
*
* @return A supplier which returns the described type.
*/
public static Supplier<Type> unbound() {
return state ->
new Type.WildcardType(
state.getSymtab().objectType, BoundKind.UNBOUND, state.getSymtab().boundClass);
}

private static Supplier<Type> propagateNull(
Supplier<Type> type, BiFunction<VisitorState, Type, Type> transformer) {
return state ->
Optional.ofNullable(type.get(state)).map(t -> transformer.apply(state, t)).orElse(null);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
package tech.picnic.errorprone.bugpatterns.util;

import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.generic;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.raw;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.subOf;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.supOf;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.type;
import static tech.picnic.errorprone.bugpatterns.util.MoreTypes.unbound;

import com.google.common.collect.ImmutableSet;
import com.google.errorprone.BugPattern;
import com.google.errorprone.CompilationTestHelper;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.suppliers.Supplier;
import com.google.errorprone.util.ASTHelpers;
import com.google.errorprone.util.Signatures;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.tools.javac.code.Type;
import java.util.ArrayList;
import java.util.List;
import org.junit.jupiter.api.Test;

final class MoreTypesTest {
private static final ImmutableSet<Supplier<Type>> TYPES =
ImmutableSet.of(
type("java.lang.Nonexistent"),
type("java.lang.String"),
type("java.lang.Number"),
supOf(type("java.lang.Number")),
subOf(type("java.lang.Number")),
type("java.lang.Integer"),
supOf(type("java.lang.Integer")),
subOf(type("java.lang.Integer")),
type("java.util.Optional"),
raw(type("java.util.Optional")),
generic(type("java.util.Optional"), unbound()),
generic(type("java.util.Optional"), type("java.lang.Number")),
type("java.util.Collection"),
raw(type("java.util.Collection")),
generic(type("java.util.Collection"), unbound()),
generic(type("java.util.Collection"), type("java.lang.Number")),
generic(type("java.util.Collection"), supOf(type("java.lang.Number"))),
generic(type("java.util.Collection"), subOf(type("java.lang.Number"))),
generic(type("java.util.Collection"), type("java.lang.Integer")),
generic(type("java.util.Collection"), supOf(type("java.lang.Integer"))),
generic(type("java.util.Collection"), subOf(type("java.lang.Integer"))),
type("java.util.List"),
raw(type("java.util.List")),
generic(type("java.util.List"), unbound()),
generic(type("java.util.List"), type("java.lang.Number")),
generic(type("java.util.List"), supOf(type("java.lang.Number"))),
generic(type("java.util.List"), subOf(type("java.lang.Number"))),
generic(type("java.util.List"), type("java.lang.Integer")),
generic(type("java.util.List"), supOf(type("java.lang.Integer"))),
generic(type("java.util.List"), subOf(type("java.lang.Integer"))),
generic(
type("java.util.Map"),
type("java.lang.String"),
subOf(generic(type("java.util.Collection"), supOf(type("java.lang.Short"))))));

@Test
void matcher() {
CompilationTestHelper.newInstance(SubtypeFlagger.class, getClass())
.addSourceLines(
"/A.java",
"import java.util.Collection;",
"import java.util.List;",
"import java.util.Map;",
"import java.util.Optional;",
"import java.util.Set;",
"",
"class A<S, T> {",
" void m() {",
" Object object = factory();",
" A a = factory();",
"",
" // BUG: Diagnostic contains: [Number, ? super Number, Integer, ? super Integer]",
" int integer = factory();",
"",
" // BUG: Diagnostic contains: [String]",
" String string = factory();",
"",
" // BUG: Diagnostic contains: [Optional]",
" Optional rawOptional = factory();",
" // BUG: Diagnostic contains: [Optional, Optional<?>]",
" Optional<S> optionalOfS = factory();",
" // BUG: Diagnostic contains: [Optional, Optional<?>]",
" Optional<T> optionalOfT = factory();",
" // BUG: Diagnostic contains: [Optional, Optional<?>, Optional<Number>]",
" Optional<Number> optionalOfNumber = factory();",
" // BUG: Diagnostic contains: [Optional, Optional<?>]",
" Optional<Integer> optionalOfInteger = factory();",
"",
" // BUG: Diagnostic contains: [Collection]",
" Collection rawCollection = factory();",
" // BUG: Diagnostic contains: [Collection, Collection<?>, Collection<Number>, Collection<? super",
" // Number>, Collection<? extends Number>, Collection<? super Integer>]",
" Collection<Number> collectionOfNumber = factory();",
" // BUG: Diagnostic contains: [Collection, Collection<?>, Collection<? extends Number>,",
" // Collection<Integer>, Collection<? super Integer>, Collection<? extends Integer>]",
" Collection<Integer> collectionOfInteger = factory();",
" // BUG: Diagnostic contains: [Collection, Collection<?>, Collection<? extends Number>]",
" Collection<Short> collectionOfShort = factory();",
"",
" // BUG: Diagnostic contains: [Collection, List]",
" List rawList = factory();",
" // BUG: Diagnostic contains: [Collection, Collection<?>, Collection<Number>, Collection<? super",
" // Number>, Collection<? extends Number>, Collection<? super Integer>, List, List<?>,",
" // List<Number>, List<? super Number>, List<? extends Number>, List<? super Integer>]",
" List<Number> listOfNumber = factory();",
" // BUG: Diagnostic contains: [Collection, Collection<?>, Collection<? extends Number>,",
" // Collection<Integer>, Collection<? super Integer>, Collection<? extends Integer>, List,",
" // List<?>, List<? extends Number>, List<Integer>, List<? super Integer>, List<? extends",
" // Integer>]",
" List<Integer> listOfInteger = factory();",
" // BUG: Diagnostic contains: [Collection, Collection<?>, Collection<? extends Number>, List,",
" // List<?>, List<? extends Number>]",
" List<Short> listOfShort = factory();",
"",
" // BUG: Diagnostic contains: [Collection]",
" Set rawSet = factory();",
" // BUG: Diagnostic contains: [Collection, Collection<?>, Collection<Number>, Collection<? super",
" // Number>, Collection<? extends Number>, Collection<? super Integer>]",
" Set<Number> setOfNumber = factory();",
" // BUG: Diagnostic contains: [Collection, Collection<?>, Collection<? extends Number>,",
" // Collection<Integer>, Collection<? super Integer>, Collection<? extends Integer>]",
" Set<Integer> setOfInteger = factory();",
" // BUG: Diagnostic contains: [Collection, Collection<?>, Collection<? extends Number>]",
" Set<Short> setOfShort = factory();",
"",
" Map rawMap = factory();",
" Map<Number, Collection<Number>> mapFromNumberToCollectionOfNumber = factory();",
" Map<Number, Collection<Short>> mapFromNumberToCollectionOfShort = factory();",
" Map<Number, Collection<Integer>> mapFromNumberToCollectionOfInteger = factory();",
" // BUG: Diagnostic contains: [Map<String, ? extends Collection<? super Short>>]",
" Map<String, Collection<Number>> mapFromStringToCollectionOfNumber = factory();",
" // BUG: Diagnostic contains: [Map<String, ? extends Collection<? super Short>>]",
" Map<String, Collection<Short>> mapFromStringToCollectionOfShort = factory();",
" Map<String, Collection<Integer>> mapFromStringToCollectionOfInteger = factory();",
" // BUG: Diagnostic contains: [Map<String, ? extends Collection<? super Short>>]",
" Map<String, List<Number>> mapFromStringToListOfNumber = factory();",
" // BUG: Diagnostic contains: [Map<String, ? extends Collection<? super Short>>]",
" Map<String, List<Short>> mapFromStringToListOfShort = factory();",
" Map<String, List<Integer>> mapFromStringToListOfInteger = factory();",
" }",
"",
" private <T> T factory() {",
" return null;",
" }",
"}")
.doTest();
}

/**
* A {@link BugChecker} which flags method invocations that are a subtype of any type contained in
* {@link #TYPES}.
*/
@BugPattern(summary = "Flags invocations of methods with select return types", severity = ERROR)
public static final class SubtypeFlagger extends BugChecker
implements MethodInvocationTreeMatcher {
private static final long serialVersionUID = 1L;

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
Type treeType = ASTHelpers.getType(tree);

List<String> matches = new ArrayList<>();

for (Supplier<Type> type : TYPES) {
Type testType = type.get(state);
if (testType != null && state.getTypes().isSubtype(treeType, testType)) {
matches.add(Signatures.prettyType(testType));
}
}

return matches.isEmpty()
? Description.NO_MATCH
: buildDescription(tree).setMessage(matches.toString()).build();
}
}
}

0 comments on commit c267b8f

Please sign in to comment.