From a86e28bd53d3b95f799a286ec6cacfaf6542de91 Mon Sep 17 00:00:00 2001 From: Kurt Alfred Kluever Date: Wed, 14 Jun 2023 12:18:13 -0700 Subject: [PATCH] Warn when on `Stream` parameters and `Iterator` return types. #klippy4apis PiperOrigin-RevId: 540344813 --- .../errorprone/bugpatterns/NonApiType.java | 37 +++++++++++++------ .../bugpatterns/NonApiTypeTest.java | 28 ++++++++++++++ 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/NonApiType.java b/core/src/main/java/com/google/errorprone/bugpatterns/NonApiType.java index 763146ea7ee..571d8d52834 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/NonApiType.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/NonApiType.java @@ -19,8 +19,10 @@ import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; import static com.google.errorprone.matchers.Description.NO_MATCH; import static com.google.errorprone.predicates.TypePredicates.anyOf; +import static com.google.errorprone.predicates.TypePredicates.anything; import static com.google.errorprone.predicates.TypePredicates.isDescendantOf; import static com.google.errorprone.predicates.TypePredicates.isExactType; +import static com.google.errorprone.predicates.TypePredicates.not; import static com.google.errorprone.util.ASTHelpers.getSymbol; import static com.google.errorprone.util.ASTHelpers.getType; import static com.google.errorprone.util.ASTHelpers.isSameType; @@ -33,7 +35,6 @@ import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher; import com.google.errorprone.matchers.Description; import com.google.errorprone.predicates.TypePredicate; -import com.google.errorprone.predicates.TypePredicates; import com.sun.source.tree.MethodTree; import com.sun.source.tree.Tree; import com.sun.tools.javac.code.Type; @@ -54,12 +55,13 @@ public final class NonApiType extends BugChecker implements MethodTreeMatcher { private static final String INTERFACES_NOT_IMPLS_LINK = ""; private static final String PRIMITIVE_ARRAYS_LINK = ""; private static final String PROTO_TIME_SERIALIZATION_LINK = ""; + private static final String ITERATOR_LINK = ""; + private static final String STREAM_LINK = ""; private static final String OPTIONAL_AS_PARAM_LINK = ""; private static final String PREFER_JDK_OPTIONAL_LINK = ""; - private static final TypePredicate GRAPH_WRAPPER = - TypePredicates.not( - TypePredicates.isDescendantOf("com.google.apps.framework.producers.GraphWrapper")); + private static final TypePredicate NON_GRAPH_WRAPPER = + not(isDescendantOf("com.google.apps.framework.producers.GraphWrapper")); private static final ImmutableSet NON_API_TYPES = ImmutableSet.of( @@ -96,23 +98,23 @@ public final class NonApiType extends BugChecker implements MethodTreeMatcher { // ImmutableFoo as params withPublicVisibility( isExactType("com.google.common.collect.ImmutableCollection"), - GRAPH_WRAPPER, + NON_GRAPH_WRAPPER, "Consider accepting a java.util.Collection or Iterable instead. " + TYPE_GENERALITY_LINK, ApiElementType.PARAMETER), withPublicVisibility( isExactType("com.google.common.collect.ImmutableList"), - GRAPH_WRAPPER, + NON_GRAPH_WRAPPER, "Consider accepting a java.util.List or Iterable instead. " + TYPE_GENERALITY_LINK, ApiElementType.PARAMETER), withPublicVisibility( isExactType("com.google.common.collect.ImmutableSet"), - GRAPH_WRAPPER, + NON_GRAPH_WRAPPER, "Consider accepting a java.util.Set or Iterable instead. " + TYPE_GENERALITY_LINK, ApiElementType.PARAMETER), withPublicVisibility( isExactType("com.google.common.collect.ImmutableMap"), - GRAPH_WRAPPER, + NON_GRAPH_WRAPPER, "Consider accepting a java.util.Map instead. " + TYPE_GENERALITY_LINK, ApiElementType.PARAMETER), @@ -136,6 +138,20 @@ public final class NonApiType extends BugChecker implements MethodTreeMatcher { "Prefer a java.util.Map instead. " + INTERFACES_NOT_IMPLS_LINK, ApiElementType.ANY), + // Iterators + withPublicVisibility( + isDescendantOf("java.util.Iterator"), + "Prefer returning a Stream (or collecting to an ImmutableList/ImmutableSet) instead. " + + ITERATOR_LINK, + ApiElementType.RETURN_TYPE), + // TODO(b/279464660): consider also warning on an Iterator as a ApiElementType.PARAMETER + + // Streams + withPublicVisibility( + isDescendantOf("java.util.stream.Stream"), + "Prefer accepting an Iterable or Collection instead. " + STREAM_LINK, + ApiElementType.PARAMETER), + // ProtoTime withPublicVisibility( isExactType("com.google.protobuf.Duration"), @@ -242,8 +258,7 @@ enum ApiVisibility { private static TypeToCheck withPublicVisibility( TypePredicate typePredicate, String failureMessage, ApiElementType elementType) { - return withPublicVisibility( - typePredicate, TypePredicates.anything(), failureMessage, elementType); + return withPublicVisibility(typePredicate, anything(), failureMessage, elementType); } private static TypeToCheck withPublicVisibility( @@ -258,7 +273,7 @@ private static TypeToCheck withPublicVisibility( private static TypeToCheck withAnyVisibility( TypePredicate typePredicate, String failureMessage, ApiElementType elementType) { return new AutoValue_NonApiType_TypeToCheck( - typePredicate, TypePredicates.anything(), failureMessage, ApiVisibility.ANY, elementType); + typePredicate, anything(), failureMessage, ApiVisibility.ANY, elementType); } @AutoValue diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/NonApiTypeTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/NonApiTypeTest.java index faa5fb8fc69..ffc95d05014 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/NonApiTypeTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/NonApiTypeTest.java @@ -235,4 +235,32 @@ public void normalApisAreNotFlagged() { } // Tests below are copied from FloggerPassedAroundTest (so it can be deleted) + + @Test + public void streams() { + helper + .addSourceLines( + "Test.java", + "import java.util.stream.Stream;", + "public class Test {", + " // BUG: Diagnostic contains: NonApiType", + " public Test(Stream iterator) {}", + " // BUG: Diagnostic contains: NonApiType", + " public void methodParam(Stream iterator) {}", + "}") + .doTest(); + } + + @Test + public void iterators() { + helper + .addSourceLines( + "Test.java", + "import java.util.Iterator;", + "public class Test {", + " // BUG: Diagnostic contains: NonApiType", + " public Iterator returnType() { return null; }", + "}") + .doTest(); + } }