-
Notifications
You must be signed in to change notification settings - Fork 39
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Bugpattern for disallowing
Optional<Optional<...>>
- Loading branch information
1 parent
c89e390
commit 98174c1
Showing
2 changed files
with
103 additions
and
0 deletions.
There are no files selected for viewing
65 changes: 65 additions & 0 deletions
65
error-prone-contrib/src/main/java/tech/picnic/errorprone/bugpatterns/NestingOptionals.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,65 @@ | ||
package tech.picnic.errorprone.bugpatterns; | ||
|
||
import static com.google.errorprone.BugPattern.LinkType.NONE; | ||
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING; | ||
import static com.google.errorprone.BugPattern.StandardTags.SIMPLIFICATION; | ||
import static com.google.errorprone.matchers.method.MethodMatchers.anyMethod; | ||
|
||
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.bugpatterns.BugChecker.MethodInvocationTreeMatcher; | ||
import com.google.errorprone.matchers.Description; | ||
import com.google.errorprone.matchers.Matcher; | ||
import com.google.errorprone.util.ASTHelpers; | ||
import com.sun.source.tree.ExpressionTree; | ||
import com.sun.source.tree.MethodInvocationTree; | ||
import com.sun.source.util.TreeScanner; | ||
import com.sun.tools.javac.code.Type; | ||
import java.util.Optional; | ||
import javax.annotation.Nullable; | ||
|
||
/** A {@link BugChecker} which flags any (embedded) nesting of {@link Optional Optionals}. */ | ||
@AutoService(BugChecker.class) | ||
@BugPattern( | ||
summary = "Avoid creating nested optionals.", | ||
linkType = NONE, | ||
severity = WARNING, | ||
tags = SIMPLIFICATION) | ||
public final class NestingOptionals extends BugChecker implements MethodInvocationTreeMatcher { | ||
private static final long serialVersionUID = 1L; | ||
private static final String JAVA_OPTIONAL = "java.util.Optional"; | ||
private static final Matcher<ExpressionTree> ANY_OPTIONAL_METHOD = | ||
anyMethod().onDescendantOf(JAVA_OPTIONAL); | ||
|
||
@Override | ||
public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { | ||
if (!ANY_OPTIONAL_METHOD.matches(tree, state)) { | ||
return Description.NO_MATCH; | ||
} | ||
|
||
Boolean foundNestedOptional = tree.accept(new NestedOptionalDetector(), state); | ||
if (foundNestedOptional == null || !foundNestedOptional) { | ||
return Description.NO_MATCH; | ||
} | ||
|
||
return buildDescription(tree).build(); | ||
} | ||
|
||
private static class NestedOptionalDetector extends TreeScanner<Boolean, VisitorState> { | ||
@Nullable | ||
@Override | ||
public Boolean visitMethodInvocation(MethodInvocationTree node, VisitorState state) { | ||
if (ASTHelpers.getType(node).getTypeArguments().stream() | ||
.anyMatch(NestedOptionalDetector::typeIsJavaOptional)) { | ||
return true; | ||
} | ||
return super.visitMethodInvocation(node, state); | ||
} | ||
|
||
private static boolean typeIsJavaOptional(Type type) { | ||
return type.asElement().getQualifiedName().contentEquals(JAVA_OPTIONAL); | ||
} | ||
} | ||
} |
38 changes: 38 additions & 0 deletions
38
...-prone-contrib/src/test/java/tech/picnic/errorprone/bugpatterns/NestingOptionalsTest.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,38 @@ | ||
package tech.picnic.errorprone.bugpatterns; | ||
|
||
import com.google.errorprone.CompilationTestHelper; | ||
import org.junit.jupiter.api.Test; | ||
|
||
final class NestingOptionalsTest { | ||
private final CompilationTestHelper compilationTestHelper = | ||
CompilationTestHelper.newInstance(NestingOptionals.class, getClass()); | ||
|
||
@Test | ||
void identification() { | ||
compilationTestHelper | ||
.addSourceLines( | ||
"A.java", | ||
"import java.util.Optional;", | ||
"", | ||
"class A {", | ||
" void m() {", | ||
" // BUG: Diagnostic contains:", | ||
" Optional.of(Optional.empty());", | ||
" // BUG: Diagnostic contains:", | ||
" Optional.of(1).map(Optional::of);", | ||
" // BUG: Diagnostic contains:", | ||
" Optional.of(2).map(Optional::of).orElseThrow();", | ||
" // BUG: Diagnostic contains:", | ||
" Optional.of(3).map(value -> Optional.empty());", | ||
" // BUG: Diagnostic contains:", | ||
" Optional.of(4).map(value -> Optional.empty()).orElseThrow();", | ||
"", | ||
" Optional.of(5).map(String::valueOf);", | ||
" Optional.of(6).map(value -> value);", | ||
" Optional.of(7).flatMap(Optional::of);", | ||
" Optional.of(8).flatMap(value -> Optional.empty());", | ||
" }", | ||
"}") | ||
.doTest(); | ||
} | ||
} |