-
Notifications
You must be signed in to change notification settings - Fork 742
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Discourage public Foo stream() where Foo does _not_ end with "Stream"
PiperOrigin-RevId: 390684609
- Loading branch information
1 parent
ba12995
commit 4aad239
Showing
3 changed files
with
192 additions
and
0 deletions.
There are no files selected for viewing
71 changes: 71 additions & 0 deletions
71
...c/main/java/com/google/errorprone/bugpatterns/PublicApiNamedStreamShouldReturnStream.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 [email protected] (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); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
119 changes: 119 additions & 0 deletions
119
...st/java/com/google/errorprone/bugpatterns/PublicApiNamedStreamShouldReturnStreamTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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(); | ||
} | ||
} |