Skip to content

Commit c5218ed

Browse files
java-team-github-botError Prone Team
authored and
Error Prone Team
committed
Add suggested fixes for OptionalOfRedundantMethod
Flume hits: unknown commit PiperOrigin-RevId: 386373157
1 parent 91416d5 commit c5218ed

File tree

2 files changed

+404
-3
lines changed

2 files changed

+404
-3
lines changed

core/src/main/java/com/google/errorprone/bugpatterns/OptionalOfRedundantMethod.java

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,20 @@
2222
import static com.google.errorprone.matchers.Matchers.instanceMethod;
2323
import static com.google.errorprone.matchers.Matchers.staticMethod;
2424

25+
import com.google.common.collect.ImmutableList;
2526
import com.google.errorprone.BugPattern;
2627
import com.google.errorprone.VisitorState;
2728
import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
29+
import com.google.errorprone.fixes.SuggestedFix;
30+
import com.google.errorprone.fixes.SuggestedFixes;
2831
import com.google.errorprone.matchers.Description;
2932
import com.google.errorprone.matchers.Matcher;
3033
import com.google.errorprone.util.ASTHelpers;
34+
import com.sun.source.tree.ExpressionStatementTree;
3135
import com.sun.source.tree.ExpressionTree;
3236
import com.sun.source.tree.MethodInvocationTree;
37+
import com.sun.source.tree.Tree;
38+
import com.sun.tools.javac.util.Name;
3339

3440
/**
3541
* Checks if {@code Optional#of} is chained with a redundant method.
@@ -58,10 +64,11 @@
5864
severity = ERROR)
5965
public class OptionalOfRedundantMethod extends BugChecker implements MethodInvocationTreeMatcher {
6066

67+
private static final Matcher<ExpressionTree> GUAVA_OPTIONAL_OF_MATCHER =
68+
staticMethod().onClass("com.google.common.base.Optional").named("of");
69+
6170
private static final Matcher<ExpressionTree> OPTIONAL_OF_MATCHER =
62-
anyOf(
63-
staticMethod().onClass("java.util.Optional").named("of"),
64-
staticMethod().onClass("com.google.common.base.Optional").named("of"));
71+
anyOf(staticMethod().onClass("java.util.Optional").named("of"), GUAVA_OPTIONAL_OF_MATCHER);
6572

6673
private static final Matcher<ExpressionTree> REDUNDANT_METHOD_MATCHER =
6774
anyOf(
@@ -90,6 +97,47 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
9097
"Optional.of() always returns a non-empty Optional. Using '%s' method on it is"
9198
+ " unnecessary and most probably a bug.",
9299
methodName))
100+
.addAllFixes(getSuggestedFixes(tree, state))
93101
.build();
94102
}
103+
104+
private ImmutableList<SuggestedFix> getSuggestedFixes(
105+
MethodInvocationTree tree, VisitorState state) {
106+
MethodInvocationTree optionalOfInvocationTree =
107+
(MethodInvocationTree) ASTHelpers.getReceiver(tree);
108+
String nullableMethodName =
109+
GUAVA_OPTIONAL_OF_MATCHER.matches(optionalOfInvocationTree, state)
110+
? "fromNullable"
111+
: "ofNullable";
112+
113+
ImmutableList.Builder<SuggestedFix> fixesBuilder = ImmutableList.builder();
114+
fixesBuilder.add(
115+
SuggestedFixes.renameMethodInvocation(optionalOfInvocationTree, nullableMethodName, state));
116+
117+
if (state.getPath().getParentPath().getLeaf() instanceof ExpressionStatementTree) {
118+
return fixesBuilder.build();
119+
}
120+
121+
Name methodSimpleName = ASTHelpers.getSymbol(tree).getSimpleName();
122+
if (methodSimpleName.contentEquals("orElse")
123+
|| methodSimpleName.contentEquals("orElseGet")
124+
|| methodSimpleName.contentEquals("orElseThrow")
125+
|| methodSimpleName.contentEquals("or")
126+
|| methodSimpleName.contentEquals("orNull")) {
127+
Tree argument = optionalOfInvocationTree.getArguments().get(0);
128+
SuggestedFix.Builder fixBuilder =
129+
SuggestedFix.builder().replace(tree, state.getSourceForNode(argument));
130+
fixBuilder.setShortDescription("Simplify expression.");
131+
if (methodSimpleName.contentEquals("orElse")) {
132+
fixBuilder.setShortDescription(
133+
"Simplify expression. Note that this may change semantics if arguments have side"
134+
+ " effects");
135+
}
136+
fixesBuilder.add(fixBuilder.build());
137+
} else if (methodSimpleName.contentEquals("isPresent")) {
138+
fixesBuilder.add(SuggestedFix.builder().replace(tree, "true").build());
139+
}
140+
// TODO(b/192550897): Add suggested fix for ifpresent
141+
return fixesBuilder.build();
142+
}
95143
}

0 commit comments

Comments
 (0)