From 07f5f319b6f2e3e22de1d38e9e99b6d677dfdc02 Mon Sep 17 00:00:00 2001 From: Kurt Alfred Kluever Date: Sun, 2 Aug 2026 06:28:14 -0700 Subject: [PATCH] Avoid flagging primitive var-args in `NonApiType` (e.g., `int...`). PiperOrigin-RevId: 957935036 --- .../errorprone/bugpatterns/NonApiType.java | 9 +++- .../bugpatterns/NonApiTypeTest.java | 19 +++++--- docs/bugpattern/NonApiType.md | 44 +++++++++++++++++++ 3 files changed, 65 insertions(+), 7 deletions(-) create mode 100644 docs/bugpattern/NonApiType.md 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 2a23ed939ef..20ebfcf89ed 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/NonApiType.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/NonApiType.java @@ -38,9 +38,11 @@ import com.google.errorprone.predicates.TypePredicate; 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; import java.util.ArrayDeque; import java.util.Deque; +import java.util.List; /** Flags instances of non-API types from being accepted or returned in APIs. */ @BugPattern( @@ -239,8 +241,11 @@ public Description matchMethod(MethodTree tree, VisitorState state) { methodIsPublicAndNotAnOverride(symbol, state) && state.errorProneOptions().isPubliclyVisibleTarget(); - for (Tree parameter : tree.getParameters()) { - checkType(parameter, ApiElementType.PARAMETER, isPublicApi, enclosingType, state); + List parameters = tree.getParameters(); + // Avoid flagging primitive var-args parameters (e.g., int... rest) as array types. + int paramsToCheck = symbol.isVarArgs() ? parameters.size() - 1 : parameters.size(); + for (int i = 0; i < paramsToCheck; i++) { + checkType(parameters.get(i), ApiElementType.PARAMETER, isPublicApi, enclosingType, state); } checkType(tree.getReturnType(), ApiElementType.RETURN_TYPE, isPublicApi, enclosingType, state); 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 9536e6a8665..b5bbcef4d0e 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/NonApiTypeTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/NonApiTypeTest.java @@ -249,11 +249,20 @@ public void varargs() { helper .addSourceLines( "Test.java", - "import com.google.protobuf.Timestamp;", - "public class Test {", - // TODO(kak): we should _probably_ flag this too - " public void test(Timestamp... timestamps) {}", - "}") + """ + import com.google.protobuf.Timestamp; + + public class Test { + // TODO(kak): we should _probably_ flag this too? + public void testTimestamps(Timestamp... timestamps) {} + + public void withSuccessExitCodes(int first, int... rest) {} + + public void testDoubles(double... values) {} + + public void testLongs(Long... values) {} + } + """) .doTest(); } diff --git a/docs/bugpattern/NonApiType.md b/docs/bugpattern/NonApiType.md new file mode 100644 index 00000000000..ece0d7fb459 --- /dev/null +++ b/docs/bugpattern/NonApiType.md @@ -0,0 +1,44 @@ +Flags instances of non-API types from being accepted or returned in public APIs. + +### What it flags + +* **Primitive Arrays:** Methods accepting or returning primitive or + boxed-primitive arrays (`int[]`, `Integer[]`, `double[]`, `Double[]`, + `long[]`, `Long[]`). Prefer `ImmutableIntArray`, `ImmutableDoubleArray`, or + `ImmutableLongArray` instead. + * *Note:* Var-args parameters (e.g. `int... rest`) are **not** flagged, as + var-args is idiomatic Java for parameter lists. +* **Collection Implementations:** Accepting or returning concrete collection + classes (`ArrayList`, `LinkedList`, `HashSet`, `LinkedHashSet`, `TreeSet`, + `HashMap`, `LinkedHashMap`, `TreeMap`). Prefer interface types (`List`, + `Set`, `Map`). +* **Immutable Collections as Parameters:** Accepting `ImmutableCollection`, + `ImmutableList`, `ImmutableSet`, or `ImmutableMap` as method parameters. + Prefer accepting `Collection`, `List`, `Set`, `Map`, or `Iterable` for + parameter generality. +* **Optional Parameters:** Accepting `java.util.Optional` or + `com.google.common.base.Optional` as method parameters. Prefer method + overloading: creating one signature with the parameter and one without (or + use `@Nullable` parameters). +* **`com.google.common.base.Pair`:** Passing `Pair` across API boundaries. + Define a well-named class or record instead. +* **Iterators & Streams:** Returning `Iterator` (prefer `Stream` or collecting + to an `ImmutableList`/`ImmutableSet`) or accepting `Stream` as a parameter + (prefer `Iterable` or `Collection`). + * Returning stateful single-use `Iterator`s limits caller options +* **ProtoTime Types:** Using `com.google.protobuf.Duration`, `Timestamp`, or + `com.google.type.*` types across public APIs instead of standard + `java.time.*` types (`Duration`, `Instant`, `LocalDate`, etc.). +* **Flogger Loggers:** Passing `FluentLogger` or `GoogleLogger` instances + across method boundaries; this can break standard per-class logger + initialization patterns. + +### Why + +* **Type Generality & Interface Abstraction:** Methods should accept abstract + interface types (e.g., `List` rather than `ArrayList`) to give callers + flexibility in implementation details. +* **Immutability & Safety:** Primitive arrays are mutable and expose internal + array state directly to callers. Guava's `ImmutableIntArray`, + `ImmutableDoubleArray`, and `ImmutableLongArray` provide immutable, safe, + and efficient alternatives.