Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add error-prone rule for converting Stream to Iterator #3058

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
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
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ Safe Logging can be found at [github.com/palantir/safe-logging](https://github.c
- `InvocationTargetExceptionGetTargetException`: InvocationTargetException.getTargetException() predates the general-purpose exception chaining facility. The Throwable.getCause() method is now the preferred means of obtaining this information. [(source)](https://docs.oracle.com/en/java/javase/17/docs/api//java.base/java/lang/reflect/InvocationTargetException.html#getTargetException())
- `PreferInputStreamTransferTo`: Prefer JDK `InputStream.transferTo(OutputStream)` over utility methods such as `com.google.common.io.ByteStreams.copy(InputStream, OutputStream)`, `org.apache.commons.io.IOUtils.copy(InputStream, OutputStream)`, `org.apache.commons.io.IOUtils.copyLong(InputStream, OutputStream)`.
- `ConjureEndpointDeprecatedForRemoval`: Conjure endpoints marked with Deprecated and `forRemoval = true` should not be used as they are scheduled to be removed.
- `StreamToIterator`: Converting from Stream to Iterator can result in collecting the entire stream in some cases. Prefer sticking with one paradigm or another unless absolutely necessary. (https://bugs.openjdk.org/browse/JDK-8267359)

### Programmatic Application

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* (c) Copyright 2025 Palantir Technologies Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.palantir.baseline.errorprone;

import com.google.auto.service.AutoService;
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.matchers.method.MethodMatchers;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.MemberReferenceTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.code.Type;
import java.util.stream.BaseStream;

@AutoService(BugChecker.class)
@BugPattern(
link = "https://github.com/palantir/gradle-baseline#baseline-error-prone-checks",
linkType = BugPattern.LinkType.CUSTOM,
severity = BugPattern.SeverityLevel.WARNING,
summary =
"Converting from Stream to Iterator can result in collecting the entire stream in some cases. Prefer sticking with one paradigm or another unless absolutely necessary. (https://bugs.openjdk.org/browse/JDK-8267359)")
public final class StreamToIterator extends BugChecker
implements BugChecker.MethodInvocationTreeMatcher, BugChecker.MemberReferenceTreeMatcher {
private static final long serialVersionUID = 1L;

private static final Matcher<ExpressionTree> STREAM_TO_ITERATOR_INVOCATION = MethodMatchers.instanceMethod()
.onDescendantOf(BaseStream.class.getName())
.namedAnyOf("iterator", "spliterator")
.withNoParameters();

@Override
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
if (STREAM_TO_ITERATOR_INVOCATION.matches(tree, state)) {
return describeMatch(tree);
}
return Description.NO_MATCH;
}

@Override
public Description matchMemberReference(MemberReferenceTree tree, VisitorState state) {
if (tree.getMode() != MemberReferenceTree.ReferenceMode.INVOKE) {
return Description.NO_MATCH;
}

MethodSymbol method = ASTHelpers.getSymbol(tree);
if (!(method.getSimpleName().contentEquals("iterator")
|| method.getSimpleName().contentEquals("spliterator"))) {
return Description.NO_MATCH;
}

if (!method.getParameters().isEmpty()) {
return Description.NO_MATCH;
}

Type baseStream = state.getTypeFromString(BaseStream.class.getName());
if (!state.getTypes().isSubtype(method.owner.type, state.getTypes().erasure(baseStream))) {
return Description.NO_MATCH;
}

return describeMatch(tree);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
/*
* (c) Copyright 2025 Palantir Technologies Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.palantir.baseline.errorprone;

import com.google.errorprone.CompilationTestHelper;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

public final class StreamToIteratorTest {

private CompilationTestHelper compilationHelper;

@BeforeEach
public void before() {
compilationHelper = CompilationTestHelper.newInstance(StreamToIterator.class, getClass());
}

@Test
public void testDirectCase() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.util.stream.Stream;",
"class Test {",
" private void test() {",
" // BUG: Diagnostic contains: StreamToIterator",
" Stream.of(1, 2, 3).iterator();",
" }",
"}")
.doTest();
}

@Test
public void testIndirectStream() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.util.List;",
"import java.util.stream.Stream;",
"class Test {",
" private void test() {",
" // BUG: Diagnostic contains: StreamToIterator",
" List.of(1, 2, 3).stream().iterator();",
" }",
"}")
.doTest();
}

@Test
public void testStreamChain() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.util.stream.Stream;",
"class Test {",
" private void test() {",
" // BUG: Diagnostic contains: StreamToIterator",
" Stream.of(1, 2, 3).map(x -> x + 1).flatMap(x -> Stream.of(x)).iterator();",
" }",
"}")
.doTest();
}

@Test
public void testStreamAsParameter() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.util.stream.Stream;",
"class Test {",
" private void test(Stream<Object> stream) {",
" // BUG: Diagnostic contains: StreamToIterator",
" stream.iterator();",
" }",
"}")
.doTest();
}

@Test
public void testAsMethodReference() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.util.stream.Stream;",
"class Test {",
" private void test(Stream<Stream<Object>> stream) {",
" // BUG: Diagnostic contains: StreamToIterator",
" stream.map(Stream::iterator);",
" }",
"}")
.doTest();
}

@Test
public void testIntStream() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.util.stream.IntStream;",
"class Test {",
" private void test() {",
" // BUG: Diagnostic contains: StreamToIterator",
" IntStream.range(1, 3).iterator();",
" }",
"}")
.doTest();
}

@Test
public void testIntStreamReference() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.util.stream.IntStream;",
"import java.util.stream.Stream;",
"class Test {",
" private void test() {",
" // BUG: Diagnostic contains: StreamToIterator",
" Stream.of(IntStream.range(1, 3)).map(IntStream::iterator);",
" }",
"}")
.doTest();
}

@Test
public void testSpliterator() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.util.stream.Stream;",
"class Test {",
" private void test() {",
" // BUG: Diagnostic contains: StreamToIterator",
" Stream.of(1, 2, 3).spliterator();",
" }",
"}")
.doTest();
}

@Test
public void testSpliteratorMethodReference() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.util.stream.Stream;",
"class Test {",
" private void test(Stream<Stream<Object>> stream) {",
" // BUG: Diagnostic contains: StreamToIterator",
" stream.map(Stream::spliterator);",
" }",
"}")
.doTest();
}

@Test
public void testNoFalsePositiveOnNonStream() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.util.List;",
"class Test {",
" private void test() {",
" List.of(1, 2, 3).iterator();",
" }",
"}")
.doTest();
}

@Test
public void testNoFalsePositiveOnNonStreamReference() {
compilationHelper
.addSourceLines(
"Test.java",
"import java.util.List;",
"import java.util.stream.Stream;",
"class Test {",
" private void test() {",
" Stream.of(List.of(1, 2, 3)).map(List::iterator);",
" }",
"}")
.doTest();
}
}
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ id 'org.inferred.processors' version '3.7.0'
apply plugin: 'com.palantir.external-publish'
apply plugin: 'com.palantir.failure-reports'
apply plugin: 'com.palantir.jdks'
apply plugin: 'com.palantir.consistent-versions'
apply plugin: 'com.palantir.baseline'
apply plugin: 'com.palantir.baseline-java-versions'
apply plugin: 'com.palantir.consistent-versions'
apply plugin: 'com.palantir.jdks.latest'

allprojects {
Expand Down
5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-3058.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: improvement
improvement:
description: Add error-prone rule for converting Stream to Iterator
links:
- https://github.com/palantir/gradle-baseline/pull/3058