Introduce ReturnValueUnused matcher for Refaster use

WIP:
1. As-is this isn't actually useful, because currently there's no way to
   express that a certain matcher should apply to a whole expression
   (rather than just a "free variable"/template parameter).
2. There are some XXXes.
3. Assuming (1) is resolved somehow, we should add at least one Refaster
   template demonstrating how this is used. Example transformation we'd
   want to be able to do in case the expression's return value is
   ignored: `flux.blockLast()` -> `flux.then().block()`.
This commit is contained in:
Stephan Schroevers
2021-11-20 23:02:56 +01:00
parent da3ec2ce90
commit a1ca5b2505
3 changed files with 124 additions and 0 deletions

View File

@@ -0,0 +1,42 @@
package tech.picnic.errorprone.refaster.matchers;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.kindIs;
import static com.google.errorprone.matchers.Matchers.parentNode;
import com.google.errorprone.VisitorState;
import com.google.errorprone.matchers.Matcher;
import com.google.errorprone.util.ASTHelpers;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.LambdaExpressionTree;
import com.sun.source.tree.Tree;
import javax.lang.model.type.TypeKind;
/**
* A matcher of expressions of which the result (if any) is unused, for use with Refaster's
* {@code @Matches} annotation.
*/
// XXX: Review whether other parts of Error Prone's `AbstractReturnValueIgnored` should be ported to
// this class.
public final class ReturnValueUnused implements Matcher<ExpressionTree> {
private static final long serialVersionUID = 1L;
private static final Matcher<ExpressionTree> DELEGATE =
parentNode(
anyOf(
ReturnValueUnused::isVoidReturningLambdaExpression,
kindIs(Tree.Kind.EXPRESSION_STATEMENT)));
/** Instantiates a new {@link ReturnValueUnused} instance. */
public ReturnValueUnused() {}
@Override
public boolean matches(ExpressionTree tree, VisitorState state) {
return DELEGATE.matches(tree, state);
}
private static boolean isVoidReturningLambdaExpression(Tree tree, VisitorState state) {
return tree instanceof LambdaExpressionTree
&& state.getTypes().findDescriptorType(ASTHelpers.getType(tree)).getReturnType().getKind()
== TypeKind.VOID;
}
}

View File

@@ -41,6 +41,7 @@ abstract class AbstractMatcherTestChecker extends BugChecker implements Compilat
TreePath path = new TreePath(getCurrentPath(), tree);
ExpressionTree expressionTree = (ExpressionTree) tree;
if (!isMethodSelect(expressionTree, path)
&& state.getSourceForNode(expressionTree) != null
&& delegate.matches(expressionTree, state.withPath(path))) {
state.reportMatch(describeMatch(tree));
}

View File

@@ -0,0 +1,81 @@
package tech.picnic.errorprone.refaster.matchers;
import static com.google.errorprone.BugPattern.SeverityLevel.ERROR;
import com.google.errorprone.BugPattern;
import com.google.errorprone.CompilationTestHelper;
import com.google.errorprone.bugpatterns.BugChecker;
import org.junit.jupiter.api.Test;
final class ReturnValueUnusedTest {
@Test
void matches() {
CompilationTestHelper.newInstance(MatcherTestChecker.class, getClass())
.addSourceLines(
"A.java",
"import java.util.function.Consumer;",
"",
"class A {",
" String negative1() {",
" return toString();",
" }",
"",
" void negative2() {",
" String s = toString();",
" }",
"",
" String negative3() {",
" return toString().toString();",
" }",
"",
// XXX: The `valueOf` result is ignored, but `String::valueOf` itself isn't. Review.
" Object negative4() {",
" return sink(String::valueOf);",
" }",
"",
" void positive1() {",
" // BUG: Diagnostic contains:",
" toString();",
" }",
"",
" void positive2() {",
" // BUG: Diagnostic contains:",
" toString().toString();",
" }",
"",
" void positive3() {",
" // BUG: Diagnostic contains:",
" new Object();",
" }",
"",
" Object positive4() {",
" // BUG: Diagnostic contains:",
" return sink(v -> toString());",
" }",
"",
" Object positive5() {",
" // BUG: Diagnostic contains:",
" return sink(v -> new Object());",
" }",
"",
" private <T, S> S sink(Consumer<T> consumer) {",
" return null;",
" }",
"}")
.doTest();
}
/** A {@link BugChecker} that simply delegates to {@link ReturnValueUnused}. */
@BugPattern(summary = "Flags expressions matched by `ReturnValueUnused`", severity = ERROR)
@SuppressWarnings({"RedundantModifier", "serial"})
public static final class MatcherTestChecker extends AbstractMatcherTestChecker {
private static final long serialVersionUID = 1L;
// XXX: This is a false positive reported by Checkstyle. See
// https://github.com/checkstyle/checkstyle/issues/10161#issuecomment-1242732120.
@SuppressWarnings("RedundantModifier")
public MatcherTestChecker() {
super(new ReturnValueUnused());
}
}
}