Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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<? extends VariableTree> 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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
44 changes: 44 additions & 0 deletions docs/bugpattern/NonApiType.md
Original file line number Diff line number Diff line change
@@ -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.
Loading