Skip to content

Commit

Permalink
Discourage public Foo stream() where Foo does _not_ end with "Stream"
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 390684609
  • Loading branch information
java-team-github-bot authored and Error Prone Team committed Aug 13, 2021
1 parent ba12995 commit 4aad239
Show file tree
Hide file tree
Showing 3 changed files with 192 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* Copyright 2021 Google 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.google.errorprone.bugpatterns;

import static com.google.errorprone.BugPattern.SeverityLevel.SUGGESTION;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.methodHasVisibility;
import static com.google.errorprone.matchers.Matchers.methodIsNamed;

import com.google.errorprone.BugPattern;
import com.google.errorprone.VisitorState;
import com.google.errorprone.bugpatterns.BugChecker.MethodTreeMatcher;
import com.google.errorprone.matchers.Description;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.matchers.MethodVisibility;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.MethodTree;
import com.sun.tools.javac.code.Type;

/**
* Checks if public APIs named "stream" returns a type whose name ends with Stream.
*
* @author sauravtiwary@google.com (Saurav Tiwary)
*/
@BugPattern(
name = "PublicApiNamedStreamShouldReturnStream",
summary =
"Public methods named stream() are generally expected to return a type whose name ends with"
+ " Stream. Consider choosing a different method name instead.",
severity = SUGGESTION)
public class PublicApiNamedStreamShouldReturnStream extends BugChecker
implements MethodTreeMatcher {
private static final String STREAM = "stream";

private static final Matcher<MethodTree> CONFUSING_PUBLIC_API_STREAM_MATCHER =
allOf(
methodIsNamed(STREAM),
methodHasVisibility(MethodVisibility.Visibility.PUBLIC),
PublicApiNamedStreamShouldReturnStream::returnTypeDoesNotEndsWithStream);

private static boolean returnTypeDoesNotEndsWithStream(
MethodTree methodTree, VisitorState state) {
Type returnType = ASTHelpers.getSymbol(methodTree).getReturnType();

// Constructors have no return type.
return returnType != null && !returnType.tsym.getSimpleName().toString().endsWith("Stream");
}

@Override
public Description matchMethod(MethodTree tree, VisitorState state) {
if (!CONFUSING_PUBLIC_API_STREAM_MATCHER.matches(tree, state)) {
return Description.NO_MATCH;
}

return describeMatch(tree);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@
import com.google.errorprone.bugpatterns.ProtoTruthMixedDescriptors;
import com.google.errorprone.bugpatterns.ProtocolBufferOrdinal;
import com.google.errorprone.bugpatterns.ProtosAsKeyOfSetOrMap;
import com.google.errorprone.bugpatterns.PublicApiNamedStreamShouldReturnStream;
import com.google.errorprone.bugpatterns.PublicConstructorForAbstractClass;
import com.google.errorprone.bugpatterns.RandomCast;
import com.google.errorprone.bugpatterns.RandomModInteger;
Expand Down Expand Up @@ -1036,6 +1037,7 @@ public static ScannerSupplier errorChecks() {
PrivateConstructorForNoninstantiableModule.class,
PrivateConstructorForUtilityClass.class,
ProtosAsKeyOfSetOrMap.class,
PublicApiNamedStreamShouldReturnStream.class,
PublicConstructorForAbstractClass.class,
QualifierWithTypeUse.class,
RedundantCondition.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/*
* Copyright 2021 Google 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.google.errorprone.bugpatterns;

import com.google.errorprone.CompilationTestHelper;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

/** Unit tests for {@link PublicApiNamedStreamShouldReturnStream}. */
@RunWith(JUnit4.class)
public class PublicApiNamedStreamShouldReturnStreamTest {

private CompilationTestHelper compilationHelper;

@Before
public void setup() {
compilationHelper =
CompilationTestHelper.newInstance(PublicApiNamedStreamShouldReturnStream.class, getClass());
}

@Test
public void abstractMethodPositiveCase() {
compilationHelper
.addSourceLines(
"in/Test.java",
"public abstract class Test {",
" // BUG: Diagnostic contains: PublicApiNamedStreamShouldReturnStream",
" public abstract int stream();",
"}")
.doTest();
}

@Test
public void regularMethodPositiveCase() {
compilationHelper
.addSourceLines(
"in/Test.java",
"public class Test {",
" // BUG: Diagnostic contains: PublicApiNamedStreamShouldReturnStream",
" public String stream() { return \"hello\";}",
"}")
.doTest();
}

@Test
public void compliantNegativeCase() {
compilationHelper
.addSourceLines(
"in/Test.java",
"import java.util.stream.Stream;",
"public abstract class Test {",
" public abstract Stream<Integer> stream();",
"}")
.doTest();
}

@Test
public void differentNameNegativeCase() {
compilationHelper
.addSourceLines(
"in/Test.java",
"public class Test {",
" public int differentMethodName() { return 0; }",
"}")
.doTest();
}

@Test
public void privateMethodNegativeCase() {
compilationHelper
.addSourceLines(
"in/Test.java",
"public class Test {",
" private String stream() { return \"hello\"; }",
"}")
.doTest();
}

@Test
public void returnTypeEndingWithStreamNegativeCase() {
compilationHelper
.addSourceLines(
"in/Test.java",
"public class Test {",
" private static class TestStream {}",
" public TestStream stream() { return new TestStream(); }",
"}")
.doTest();
}

@Test
public void returnTypeNotEndingWithStreamPositiveCase() {
compilationHelper
.addSourceLines(
"in/Test.java",
"public class Test {",
" private static class TestStreamRandomSuffix {}",
" // BUG: Diagnostic contains: PublicApiNamedStreamShouldReturnStream",
" public TestStreamRandomSuffix stream() { return new TestStreamRandomSuffix(); }",
"}")
.doTest();
}
}

0 comments on commit 4aad239

Please sign in to comment.