0

Reland "Android: Add Error Prone warning against Java stream() apis"

This reverts commit 8ff33dbc1d.

Reason for reland: Internal fix landed

Original change's description:
> Revert "Android: Add Error Prone warning against Java stream() apis"
>
> This reverts commit e910d272c0.
>
> Reason for revert: crbug.com/344943957#comment13
>
> Original change's description:
> > Android: Add Error Prone warning against Java stream() apis
> >
> > And minor clarifying tweaks to style guide wrt streams
> >
> > Bug: 344943957
> > Change-Id: I90b976866c8f3f71826459b7d3097692e1f533b4
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6042674
> > Auto-Submit: Andrew Grieve <agrieve@chromium.org>
> > Commit-Queue: Andrew Grieve <agrieve@chromium.org>
> > Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1388426}
>
> Bug: 344943957
> Change-Id: Ib9b3d8db9f47383eaced5192a15e7cac6427da35
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6051419
> Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
> Auto-Submit: Keigo Oka <oka@chromium.org>
> Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Reviewed-by: Andrew Grieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1388467}

Bug: 344943957
Change-Id: I6668c182aa455972684e6863925c498f494fd11f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6055458
Auto-Submit: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1389041}
This commit is contained in:
Andrew Grieve
2024-11-27 21:00:13 +00:00
committed by Chromium LUCI CQ
parent babffaac09
commit 31928c9844
6 changed files with 72 additions and 8 deletions
build/android/gyp
chrome/browser/tabmodel/android/java/src/org/chromium/chrome/browser/tabmodel
styleguide/java
tools/android/errorprone_plugin
BUILD.gnOWNERS
src
org
chromium
tools
errorprone

@ -38,6 +38,8 @@ ERRORPRONE_CHECKS_TO_APPLY = []
TESTONLY_ERRORPRONE_WARNINGS_TO_DISABLE = [
# Too much effort to enable.
'UnusedVariable',
# These are allowed in tests.
'NoStreams',
]
# Full list of checks: https://errorprone.info/bugpatterns
@ -148,6 +150,7 @@ ERRORPRONE_WARNINGS_TO_ENABLE = [
'UnnecessaryStaticImport',
'UseBinds',
'WildcardImport',
'NoStreams',
]
@ -354,6 +357,7 @@ def _OnStaleMd5(changes, options, javac_cmd, javac_args, java_files, kt_files):
stamp_file=options.jar_path,
force=options.use_build_server,
experimental=options.experimental_build_server)):
logging.info('Using build server')
return
if options.enable_kythe_annotations:

@ -105,6 +105,7 @@ class TabGroup {
}
/** Returns the ID of the last tab in the group. */
@SuppressWarnings("NoStreams") // No better way to do this while using LinkedHashSet.
int getTabIdOfLastTab() {
return mTabIds.stream().skip(mTabIds.size() - 1).findFirst().get();
}

@ -261,16 +261,32 @@ Log.d(TAG, "There are %d cats", countCats()); // countCats() not stripped.
### Streams
Most uses of [Java streams] are discouraged. If you can write your code as an
explicit loop, then do so. The primary reason for this guidance is because the
lambdas (and method references) needed for streams almost always result in
larger binary size ([example](https://chromium-review.googlesource.com/c/chromium/src/+/4329952).
Using [Java streams] outside of tests is strongly discouraged. If you can write
your code as an explicit loop, then do so. The primary reason for this guidance
is because the lambdas and method references needed for streams almost always
result in larger binary size than their loop equivalents (see
[crbug.com/344943957] for examples).
The `parallel()` and `parallelStream()` APIs are simpler than their loop
equivalents, but are are currently banned due to a lack of a compelling use case
in Chrome. If you find one, please discuss on `java@chromium.org`.
equivalents, but are banned due to a lack of a compelling use case in Chrome.
If you find one, please discuss on `java@chromium.org`.
Use of `stream()` without a lambda / method reference is allowed. E.g.:
```java
@SuppressWarnings("NoStreams")
private static List<Integer> boxInts(int[] arr) {
return Arrays.stream(arr).boxed().collect(Collectors.toList());
}
@SuppressWarnings("NoStreams")
private static List<String> readLines(BufferedReader bufferedReader) {
return bufferedReader.lines().collect(Collectors.toList());
}
```
[Java streams]: https://docs.oracle.com/javase/8/docs/api/java/util/stream/package-summary.html
[crbug.com/344943957]: https://crbug.com/344943957
### AndroidX Annotations {#annotations}

@ -17,6 +17,7 @@ java_binary("errorprone_plugin") {
"src/org/chromium/tools/errorprone/plugin/NoDynamicStringsInTraceEventCheck.java",
"src/org/chromium/tools/errorprone/plugin/NoRedundantFieldInitCheck.java",
"src/org/chromium/tools/errorprone/plugin/NoResourcesGetColor.java",
"src/org/chromium/tools/errorprone/plugin/NoStreams.java",
"src/org/chromium/tools/errorprone/plugin/NoSynchronizedMethodCheck.java",
"src/org/chromium/tools/errorprone/plugin/NoSynchronizedThisCheck.java",
"src/org/chromium/tools/errorprone/plugin/TestClassNameCheck.java",

@ -1,3 +1,2 @@
agrieve@chromium.org
file://build/android/OWNERS
nyquist@chromium.org
wnwen@chromium.org

@ -0,0 +1,43 @@
// Copyright 2024 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.tools.errorprone.plugin;
import static com.google.errorprone.matchers.Matchers.instanceMethod;
import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.predicates.TypePredicates;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MethodInvocationTree;
import org.chromium.build.annotations.ServiceImpl;
/** Checks for calls to .stream(). See //styleguide/java/java.md for rationale. */
@ServiceImpl(BugChecker.class)
@BugPattern(
name = "NoStreams",
summary = "Prefer loops over stream().",
severity = BugPattern.SeverityLevel.WARNING,
linkType = BugPattern.LinkType.CUSTOM,
link =
"https://chromium.googlesource.com/chromium/src/+/main/styleguide/java/java.md#Streams")
public class NoStreams extends BugChecker implements BugChecker.MethodInvocationTreeMatcher {
private static final Matcher<ExpressionTree> MATCHER =
instanceMethod().onClass(TypePredicates.isDescendantOf("java.util.stream.Stream"));
@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState visitorState) {
if (!MATCHER.matches(tree, visitorState)) {
return Description.NO_MATCH;
}
return buildDescription(tree)
.setMessage("Using Java stream APIs is strongly discouraged. Use loops instead.")
.build();
}
}