Introduce IsEmpty matcher for use by Refaster templates (#744)

While there, generalize a number of Refaster rules using this new matcher.
This commit is contained in:
Stephan Schroevers
2023-10-23 09:07:47 +02:00
committed by GitHub
parent 2ff3095fca
commit fa026de336
10 changed files with 703 additions and 225 deletions

View File

@@ -55,6 +55,11 @@
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</dependency>
<dependency>
<groupId>io.projectreactor</groupId>
<artifactId>reactor-core</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>

View File

@@ -0,0 +1,151 @@
package tech.picnic.errorprone.refaster.matchers;
import static com.google.common.base.Verify.verify;
import static com.google.errorprone.matchers.FieldMatchers.staticField;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.argumentCount;
import static com.google.errorprone.matchers.Matchers.isPrimitiveType;
import static com.google.errorprone.matchers.Matchers.isSameType;
import static com.google.errorprone.matchers.Matchers.isSubtypeOf;
import static com.google.errorprone.matchers.Matchers.staticMethod;
import static com.google.errorprone.matchers.Matchers.toType;
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMultimap;
import com.google.errorprone.VisitorState;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.NewArrayTree;
import com.sun.source.tree.NewClassTree;
import com.sun.source.tree.Tree;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.Stack;
import java.util.TreeMap;
import java.util.TreeSet;
import java.util.Vector;
import java.util.regex.Pattern;
import java.util.stream.Stream;
/**
* A matcher of expressions that are guaranteed to yield "empty" instances, as defined by their
* respective types.
*/
// XXX: Also match (effectively) final variables that reference provably-empty objects.
// XXX: Also handle `#copyOf(someEmptyInstance)`, `#sortedCopyOf(someEmptyInstance)`,
// `Sets.immutableEnumSet(emptyIterable)` (and other `Sets` methods), `EnumSet.noneOf(...)`,
// `emptyCollection.stream()`, `emptyStream.collect(...)`, `emptyMap.{keySet,values,entrySet}()`,
// etc.
// XXX: Also recognize null-hostile "container" expression types that can only reference empty
// instances, such as `ImmutableCollection<Void>` and `Flux<Void>`.
// XXX: Also recognize empty instances of `Optional`, `OptionalInt`, `OptionalLong`, and
// `OptionalDouble`.
// XXX: Also recognize empty builders and `emptyBuilder.build()` invocations.
public final class IsEmpty implements Matcher<ExpressionTree> {
private static final long serialVersionUID = 1L;
private static final Pattern EMPTY_INSTANCE_FACTORY_METHOD_PATTERN = Pattern.compile("empty.*");
private static final Matcher<Tree> EMPTY_COLLECTION_CONSTRUCTOR_ARGUMENT =
anyOf(isPrimitiveType(), isSubtypeOf(Comparator.class));
// XXX: Extend this list to include additional JDK collection types with a public constructor.
private static final Matcher<ExpressionTree> MUTABLE_COLLECTION_TYPE =
anyOf(
isSameType(ArrayList.class),
isSameType(HashMap.class),
isSameType(HashSet.class),
isSameType(LinkedHashMap.class),
isSameType(LinkedHashSet.class),
isSameType(LinkedList.class),
isSameType(Stack.class),
isSameType(TreeMap.class),
isSameType(TreeSet.class),
isSameType(Vector.class));
private static final Matcher<ExpressionTree> EMPTY_INSTANCE_FACTORY =
anyOf(
staticField(Collections.class.getName(), "EMPTY_LIST"),
staticField(Collections.class.getName(), "EMPTY_MAP"),
staticField(Collections.class.getName(), "EMPTY_SET"),
toType(
MethodInvocationTree.class,
allOf(
argumentCount(0),
anyOf(
staticMethod()
.onClass(Collections.class.getName())
.withNameMatching(EMPTY_INSTANCE_FACTORY_METHOD_PATTERN),
staticMethod()
.onDescendantOfAny(
ImmutableCollection.class.getName(),
ImmutableMap.class.getName(),
ImmutableMultimap.class.getName(),
List.class.getName(),
Map.class.getName(),
Set.class.getName(),
Stream.class.getName())
.named("of"),
staticMethod()
.onClassAny(
Stream.class.getName(),
"reactor.core.publisher.Flux",
"reactor.core.publisher.Mono",
"reactor.util.context.Context")
.named("empty"),
staticMethod()
.onDescendantOf("reactor.core.publisher.Flux")
.named("just")))));
/** Instantiates a new {@link IsEmpty} instance. */
public IsEmpty() {}
@Override
public boolean matches(ExpressionTree tree, VisitorState state) {
return isEmptyArrayCreation(tree)
|| EMPTY_INSTANCE_FACTORY.matches(tree, state)
|| isEmptyCollectionConstructor(tree, state);
}
private boolean isEmptyCollectionConstructor(ExpressionTree tree, VisitorState state) {
if (!(tree instanceof NewClassTree) || !MUTABLE_COLLECTION_TYPE.matches(tree, state)) {
return false;
}
List<? extends ExpressionTree> arguments = ((NewClassTree) tree).getArguments();
if (arguments.stream().allMatch(a -> EMPTY_COLLECTION_CONSTRUCTOR_ARGUMENT.matches(a, state))) {
/*
* This is a default constructor, or a constructor that creates an empty collection using
* custom (re)size/load factor parameters and/or a custom `Comparator`.
*/
return true;
}
/*
* This looks like a copy constructor, in which case the resultant collection is empty if its
* argument is.
*/
verify(arguments.size() == 1, "Unexpected %s-ary constructor", arguments.size());
return matches(arguments.get(0), state);
}
private static boolean isEmptyArrayCreation(ExpressionTree tree) {
if (!(tree instanceof NewArrayTree)) {
return false;
}
NewArrayTree newArray = (NewArrayTree) tree;
return (!newArray.getDimensions().isEmpty()
&& ASTHelpers.constValue(newArray.getDimensions().get(0), Integer.class) == 0)
|| (newArray.getInitializers() != null && newArray.getInitializers().isEmpty());
}
}

View File

@@ -0,0 +1,443 @@
package tech.picnic.errorprone.refaster.matchers;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import com.google.errorprone.BugPattern;
import com.google.errorprone.CompilationTestHelper;
import com.google.errorprone.bugpatterns.BugChecker;
import org.junit.jupiter.api.Test;
final class IsEmptyTest {
@Test
void matches() {
CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass())
.addSourceLines(
"A.java",
"import com.google.common.collect.ImmutableList;",
"import com.google.common.collect.ImmutableMap;",
"import com.google.common.collect.ImmutableSet;",
"import com.google.common.collect.ImmutableSetMultimap;",
"import java.util.ArrayList;",
"import java.util.Collections;",
"import java.util.Comparator;",
"import java.util.HashMap;",
"import java.util.HashSet;",
"import java.util.LinkedHashMap;",
"import java.util.LinkedHashSet;",
"import java.util.LinkedList;",
"import java.util.List;",
"import java.util.Map;",
"import java.util.Random;",
"import java.util.Set;",
"import java.util.Stack;",
"import java.util.TreeMap;",
"import java.util.TreeSet;",
"import java.util.Vector;",
"import java.util.stream.Stream;",
"import reactor.core.publisher.Flux;",
"import reactor.core.publisher.Mono;",
"import reactor.util.context.Context;",
"",
"class A {",
" int[] negative1() {",
" return new int[1];",
" }",
"",
" int[][] negative2() {",
" return new int[1][0];",
" }",
"",
" int[] negative3() {",
" return new int[] {0};",
" }",
"",
" int[][] negative4() {",
" return new int[][] {{0}};",
" }",
"",
" Random negative5() {",
" return new Random();",
" }",
"",
" List<Integer> negative6() {",
" return new ArrayList<>(ImmutableList.of(1));",
" }",
"",
" Map<Integer, Integer> negative7() {",
" return new HashMap<>(ImmutableMap.of(1, 2));",
" }",
"",
" Set<Integer> negative8() {",
" return new HashSet<>(ImmutableList.of(1));",
" }",
"",
" Map<Integer, Integer> negative9() {",
" return new LinkedHashMap<>(ImmutableMap.of(1, 2));",
" }",
"",
" Set<Integer> negative10() {",
" return new LinkedHashSet<>(ImmutableList.of(1));",
" }",
"",
" List<Integer> negative11() {",
" return new LinkedList<>(ImmutableList.of(1));",
" }",
"",
" Map<Integer, Integer> negative12() {",
" return new HashMap<>(ImmutableMap.of(1, 2));",
" }",
"",
" Set<Integer> negative13() {",
" return new HashSet<>(ImmutableList.of(1));",
" }",
"",
" List<Integer> negative14() {",
" return new Vector<>(ImmutableList.of(1));",
" }",
"",
" ImmutableList<Integer> negative15() {",
" return ImmutableList.of(1);",
" }",
"",
" ImmutableSet<Integer> negative16() {",
" return ImmutableSet.of(1);",
" }",
"",
" ImmutableMap<Integer, Integer> negative17() {",
" return ImmutableMap.of(1, 2);",
" }",
"",
" ImmutableSetMultimap<Integer, Integer> negative18() {",
" return ImmutableSetMultimap.of(1, 2);",
" }",
"",
" List<Integer> negative19() {",
" return List.of(1);",
" }",
"",
" Map<Integer, Integer> negative20() {",
" return Map.of(1, 2);",
" }",
"",
" Set<Integer> negative21() {",
" return Set.of(1);",
" }",
"",
" Stream<Integer> negative22() {",
" return Stream.of(1);",
" }",
"",
" int[] positive1() {",
" // BUG: Diagnostic contains:",
" return new int[0];",
" }",
"",
" int[][] positive2() {",
" // BUG: Diagnostic contains:",
" return new int[0][1];",
" }",
"",
" int[] positive3() {",
" // BUG: Diagnostic contains:",
" return new int[] {};",
" }",
"",
" int[][] positive4() {",
" // BUG: Diagnostic contains:",
" return new int[][] {};",
" }",
"",
" List<Integer> positive5() {",
" // BUG: Diagnostic contains:",
" return new ArrayList<>();",
" }",
"",
" List<String> positive6() {",
" // BUG: Diagnostic contains:",
" return new ArrayList<>(1);",
" }",
"",
" List<String> positive7() {",
" // BUG: Diagnostic contains:",
" return new ArrayList<>(",
" // BUG: Diagnostic contains:",
" ImmutableList.of());",
" }",
"",
" Map<String, String> positive8() {",
" // BUG: Diagnostic contains:",
" return new HashMap<>();",
" }",
"",
" Map<String, String> positive9() {",
" // BUG: Diagnostic contains:",
" return new HashMap<>(1);",
" }",
"",
" Map<String, String> positive10() {",
" // BUG: Diagnostic contains:",
" return new HashMap<>(1, 1.0F);",
" }",
"",
" Map<String, String> positive11() {",
" // BUG: Diagnostic contains:",
" return new HashMap<>(",
" // BUG: Diagnostic contains:",
" ImmutableMap.of());",
" }",
"",
" Set<Integer> positive12() {",
" // BUG: Diagnostic contains:",
" return new HashSet<>();",
" }",
"",
" Set<Integer> positive13() {",
" // BUG: Diagnostic contains:",
" return new HashSet<>(1);",
" }",
"",
" Set<Integer> positive14() {",
" // BUG: Diagnostic contains:",
" return new HashSet<>(1, 1.0F);",
" }",
"",
" Set<Integer> positive15() {",
" // BUG: Diagnostic contains:",
" return new HashSet<>(",
" // BUG: Diagnostic contains:",
" ImmutableList.of());",
" }",
"",
" Map<String, String> positive16() {",
" // BUG: Diagnostic contains:",
" return new LinkedHashMap<>();",
" }",
"",
" Map<String, String> positive17() {",
" // BUG: Diagnostic contains:",
" return new LinkedHashMap<>(1);",
" }",
"",
" Map<String, String> positive18() {",
" // BUG: Diagnostic contains:",
" return new LinkedHashMap<>(1, 1.0F);",
" }",
"",
" Map<String, String> positive19() {",
" // BUG: Diagnostic contains:",
" return new LinkedHashMap<>(1, 1.0F, false);",
" }",
"",
" Map<String, String> positive20() {",
" // BUG: Diagnostic contains:",
" return new LinkedHashMap<>(",
" // BUG: Diagnostic contains:",
" ImmutableMap.of());",
" }",
"",
" Set<Integer> positive21() {",
" // BUG: Diagnostic contains:",
" return new LinkedHashSet<>();",
" }",
"",
" Set<Integer> positive22() {",
" // BUG: Diagnostic contains:",
" return new LinkedHashSet<>(1);",
" }",
"",
" Set<Integer> positive23() {",
" // BUG: Diagnostic contains:",
" return new LinkedHashSet<>(1, 1.0F);",
" }",
"",
" Set<Integer> positive24() {",
" // BUG: Diagnostic contains:",
" return new LinkedHashSet<>(",
" // BUG: Diagnostic contains:",
" ImmutableList.of());",
" }",
"",
" List<Integer> positive25() {",
" // BUG: Diagnostic contains:",
" return new LinkedList<>();",
" }",
"",
" List<String> positive26() {",
" // BUG: Diagnostic contains:",
" return new LinkedList<>(",
" // BUG: Diagnostic contains:",
" ImmutableList.of());",
" }",
"",
" List<Integer> positive27() {",
" // BUG: Diagnostic contains:",
" return new Stack<>();",
" }",
"",
" Map<String, String> positive28() {",
" // BUG: Diagnostic contains:",
" return new TreeMap<>();",
" }",
"",
" Map<String, String> positive29() {",
" // BUG: Diagnostic contains:",
" return new TreeMap<>(Comparator.naturalOrder());",
" }",
"",
" Map<String, String> positive30() {",
" // BUG: Diagnostic contains:",
" return new TreeMap<>(",
" // BUG: Diagnostic contains:",
" ImmutableMap.of());",
" }",
"",
" Set<Integer> positive31() {",
" // BUG: Diagnostic contains:",
" return new TreeSet<>();",
" }",
"",
" Set<Integer> positive32() {",
" // BUG: Diagnostic contains:",
" return new TreeSet<>(Comparator.naturalOrder());",
" }",
"",
" Set<Integer> positive33() {",
" // BUG: Diagnostic contains:",
" return new TreeSet<>(",
" // BUG: Diagnostic contains:",
" ImmutableList.of());",
" }",
"",
" List<Integer> positive34() {",
" // BUG: Diagnostic contains:",
" return new Vector<>();",
" }",
"",
" List<Integer> positive35() {",
" // BUG: Diagnostic contains:",
" return new Vector<>(1);",
" }",
"",
" List<Integer> positive36() {",
" // BUG: Diagnostic contains:",
" return new Vector<>(1, 2);",
" }",
"",
" List<Integer> positive37() {",
" // BUG: Diagnostic contains:",
" return new Vector<>(",
" // BUG: Diagnostic contains:",
" ImmutableList.of());",
" }",
"",
" List<Integer> positive38() {",
" // BUG: Diagnostic contains:",
" return Collections.EMPTY_LIST;",
" }",
"",
" Map<String, String> positive39() {",
" // BUG: Diagnostic contains:",
" return Collections.EMPTY_MAP;",
" }",
"",
" Set<Integer> positive40() {",
" // BUG: Diagnostic contains:",
" return Collections.EMPTY_SET;",
" }",
"",
" List<Integer> positive41() {",
" // BUG: Diagnostic contains:",
" return Collections.emptyList();",
" }",
"",
" Map<String, String> positive42() {",
" // BUG: Diagnostic contains:",
" return Collections.emptyMap();",
" }",
"",
" Set<Integer> positive43() {",
" // BUG: Diagnostic contains:",
" return Collections.emptySet();",
" }",
"",
" ImmutableList<Integer> positive44() {",
" // BUG: Diagnostic contains:",
" return ImmutableList.of();",
" }",
"",
" ImmutableSet<Integer> positive45() {",
" // BUG: Diagnostic contains:",
" return ImmutableSet.of();",
" }",
"",
" ImmutableMap<String, Integer> positive46() {",
" // BUG: Diagnostic contains:",
" return ImmutableMap.of();",
" }",
"",
" ImmutableSetMultimap<String, Integer> positive47() {",
" // BUG: Diagnostic contains:",
" return ImmutableSetMultimap.of();",
" }",
"",
" List<Integer> positive48() {",
" // BUG: Diagnostic contains:",
" return List.of();",
" }",
"",
" Map<String, String> positive49() {",
" // BUG: Diagnostic contains:",
" return Map.of();",
" }",
"",
" Set<Integer> positive50() {",
" // BUG: Diagnostic contains:",
" return Set.of();",
" }",
"",
" Stream<Integer> positive51() {",
" // BUG: Diagnostic contains:",
" return Stream.of();",
" }",
"",
" Stream<Integer> positive52() {",
" // BUG: Diagnostic contains:",
" return Stream.empty();",
" }",
"",
" Flux<Integer> positive53() {",
" // BUG: Diagnostic contains:",
" return Flux.empty();",
" }",
"",
" Mono<Integer> positive54() {",
" // BUG: Diagnostic contains:",
" return Mono.empty();",
" }",
"",
" Context positive55() {",
" // BUG: Diagnostic contains:",
" return Context.empty();",
" }",
"",
" Flux<Integer> positive56() {",
" // BUG: Diagnostic contains:",
" return Flux.just();",
" }",
"}")
.doTest();
}
/** A {@link BugChecker} that simply delegates to {@link IsEmpty}. */
@BugPattern(summary = "Flags expressions matched by `IsEmpty`", severity = ERROR)
public static final class MatcherTestChecker extends AbstractMatcherTestChecker {
private static final long serialVersionUID = 1L;
// XXX: This is a false positive reported by Checkstyle. See
// https://github.com/checkstyle/checkstyle/issues/10161#issuecomment-1242732120.
@SuppressWarnings("RedundantModifier")
public MatcherTestChecker() {
super(new IsEmpty());
}
}
}