-
Notifications
You must be signed in to change notification settings - Fork 727
/
UnusedReturnValueMatcher.java
334 lines (298 loc) · 14.2 KB
/
UnusedReturnValueMatcher.java
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
/*
* Copyright 2012 The Error Prone Authors.
*
* 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.matchers;
import static com.google.errorprone.matchers.Matchers.allOf;
import static com.google.errorprone.matchers.Matchers.anyOf;
import static com.google.errorprone.matchers.Matchers.enclosingMethod;
import static com.google.errorprone.matchers.Matchers.enclosingNode;
import static com.google.errorprone.matchers.Matchers.expressionStatement;
import static com.google.errorprone.matchers.Matchers.instanceMethod;
import static com.google.errorprone.matchers.Matchers.isLastStatementInBlock;
import static com.google.errorprone.matchers.Matchers.isThrowingFunctionalInterface;
import static com.google.errorprone.matchers.Matchers.kindIs;
import static com.google.errorprone.matchers.Matchers.methodCallInDeclarationOfThrowingRunnable;
import static com.google.errorprone.matchers.Matchers.nextStatement;
import static com.google.errorprone.matchers.Matchers.parentNode;
import static com.google.errorprone.matchers.Matchers.previousStatement;
import static com.google.errorprone.matchers.Matchers.staticMethod;
import static com.google.errorprone.util.ASTHelpers.findEnclosingNode;
import static com.google.errorprone.util.ASTHelpers.getReceiver;
import static com.google.errorprone.util.ASTHelpers.getResultType;
import static com.google.errorprone.util.ASTHelpers.getSymbol;
import static com.google.errorprone.util.ASTHelpers.getType;
import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
import static com.google.errorprone.util.ASTHelpers.isSameType;
import static com.google.errorprone.util.ASTHelpers.isVoidType;
import static javax.lang.model.element.Modifier.ABSTRACT;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.errorprone.VisitorState;
import com.google.errorprone.annotations.CheckReturnValue;
import com.google.errorprone.util.ASTHelpers;
import com.google.errorprone.util.MoreAnnotations;
import com.sun.source.tree.BlockTree;
import com.sun.source.tree.ExpressionTree;
import com.sun.source.tree.LambdaExpressionTree;
import com.sun.source.tree.MemberReferenceTree;
import com.sun.source.tree.MemberReferenceTree.ReferenceMode;
import com.sun.source.tree.MethodTree;
import com.sun.source.tree.StatementTree;
import com.sun.source.tree.Tree;
import com.sun.source.tree.Tree.Kind;
import com.sun.tools.javac.code.Symbol;
import com.sun.tools.javac.code.Symbol.ClassSymbol;
import com.sun.tools.javac.code.Symbol.MethodSymbol;
import com.sun.tools.javac.code.Type;
import com.sun.tools.javac.code.Type.MethodType;
import com.sun.tools.javac.tree.JCTree.JCFieldAccess;
import com.sun.tools.javac.tree.JCTree.JCMethodInvocation;
import java.util.stream.Stream;
import javax.lang.model.type.TypeKind;
/**
* Matches expressions that invoke or reference a non-void method or constructor and which do not
* use their return value and are not in a context where non-use of the return value is allowed.
*/
@CheckReturnValue
public final class UnusedReturnValueMatcher implements Matcher<ExpressionTree> {
private static final ImmutableMap<AllowReason, Matcher<ExpressionTree>> ALLOW_MATCHERS =
ImmutableMap.of(
AllowReason.MOCKING_CALL,
UnusedReturnValueMatcher::mockitoInvocation,
AllowReason.EXCEPTION_TESTING,
UnusedReturnValueMatcher::exceptionTesting,
AllowReason.RETURNS_JAVA_LANG_VOID,
UnusedReturnValueMatcher::returnsJavaLangVoid,
// TODO(kak): this exclusion really doesn't belong here, since the context of the calling
// code doesn't matter; known builder setters are _always_ treated as CIRV, and the
// surrounding context doesn't matter.
AllowReason.KNOWN_BUILDER_SETTER,
UnusedReturnValueMatcher::isKnownBuilderSetter);
private static final ImmutableSet<AllowReason> DISALLOW_EXCEPTION_TESTING =
Sets.immutableEnumSet(
Sets.filter(ALLOW_MATCHERS.keySet(), k -> !k.equals(AllowReason.EXCEPTION_TESTING)));
/** Gets an instance of this matcher. */
public static UnusedReturnValueMatcher get(boolean allowInExceptionThrowers) {
return new UnusedReturnValueMatcher(
allowInExceptionThrowers ? ALLOW_MATCHERS.keySet() : DISALLOW_EXCEPTION_TESTING);
}
private final ImmutableSet<AllowReason> validAllowReasons;
private UnusedReturnValueMatcher(ImmutableSet<AllowReason> validAllowReasons) {
this.validAllowReasons = validAllowReasons;
}
@Override
public boolean matches(ExpressionTree tree, VisitorState state) {
return isReturnValueUnused(tree, state) && !isAllowed(tree, state);
}
private static boolean isVoidMethod(MethodSymbol symbol) {
return !symbol.isConstructor() && isVoid(symbol.getReturnType());
}
private static boolean isVoid(Type type) {
return type.getKind() == TypeKind.VOID;
}
private static boolean implementsVoidMethod(ExpressionTree tree, VisitorState state) {
return isVoid(state.getTypes().findDescriptorType(getType(tree)).getReturnType());
}
/**
* Returns {@code true} if and only if the given {@code tree} is an invocation of or reference to
* a constructor or non-{@code void} method for which the return value is considered unused.
*/
public static boolean isReturnValueUnused(ExpressionTree tree, VisitorState state) {
Symbol sym = getSymbol(tree);
if (!(sym instanceof MethodSymbol) || isVoidMethod((MethodSymbol) sym)) {
return false;
}
if (tree instanceof MemberReferenceTree) {
// Runnable r = foo::getBar;
return implementsVoidMethod(tree, state);
}
Tree parent = state.getPath().getParentPath().getLeaf();
return parent instanceof LambdaExpressionTree
// Runnable r = () -> foo.getBar();
? implementsVoidMethod((LambdaExpressionTree) parent, state)
// foo.getBar();
: parent.getKind() == Kind.EXPRESSION_STATEMENT;
}
/**
* Returns {@code true} if the given expression is allowed to have an unused return value based on
* its context.
*/
public boolean isAllowed(ExpressionTree tree, VisitorState state) {
return getAllowReasons(tree, state).findAny().isPresent();
}
/**
* Returns a stream of reasons the given expression is allowed to have an unused return value
* based on its context.
*/
public Stream<AllowReason> getAllowReasons(ExpressionTree tree, VisitorState state) {
return validAllowReasons.stream()
.filter(reason -> ALLOW_MATCHERS.get(reason).matches(tree, state));
}
/**
* Returns {@code true} if the given tree is a method call to an abstract setter method inside of
* a known builder class.
*/
private static boolean isKnownBuilderSetter(ExpressionTree tree, VisitorState state) {
Symbol symbol = getSymbol(tree);
if (!(symbol instanceof MethodSymbol)) {
return false;
}
// Avoid matching static Builder factory methods, like `static Builder newBuilder()`
if (!symbol.getModifiers().contains(ABSTRACT)) {
return false;
}
MethodSymbol method = (MethodSymbol) symbol;
ClassSymbol enclosingClass = method.enclClass();
// Setters always have exactly 1 param
if (method.getParameters().size() != 1) {
return false;
}
// If the enclosing class is not a known builder type, return false.
if (!hasAnnotation(enclosingClass, "com.google.auto.value.AutoValue.Builder", state)
&& !hasAnnotation(enclosingClass, "com.google.auto.value.AutoBuilder", state)) {
return false;
}
// If the method return type is not the same as the enclosing type (the builder itself),
// e.g., the abstract `build()` method on the Builder, return false.
if (!isSameType(method.getReturnType(), enclosingClass.asType(), state)) {
return false;
}
return true;
}
private static boolean returnsJavaLangVoid(ExpressionTree tree, VisitorState state) {
return tree instanceof MemberReferenceTree
? returnsJavaLangVoid((MemberReferenceTree) tree, state)
: isVoidType(getResultType(tree), state);
}
private static boolean returnsJavaLangVoid(MemberReferenceTree tree, VisitorState state) {
if (tree.getMode() == ReferenceMode.NEW) {
// constructors can't return java.lang.Void
return false;
}
// We need to do this to get the correct return type for things like future::get when future
// is a Future<Void>.
// - The Type of the method reference is the functional interface type it's implementing.
// - The Symbol is the declared method symbol, i.e. V get().
// So we resolve the symbol (V get()) as a member of the qualifier type (Future<Void>) to get
// the method type (Void get()) and then look at the return type of that.
Type type =
state.getTypes().memberType(getType(tree.getQualifierExpression()), getSymbol(tree));
// TODO(cgdecker): There are probably other types than MethodType that we could resolve here
return type instanceof MethodType && isVoidType(type.getReturnType(), state);
}
private static boolean exceptionTesting(ExpressionTree tree, VisitorState state) {
return tree instanceof MemberReferenceTree
? isThrowingFunctionalInterface(getType(tree), state)
: expectedExceptionTest(state);
}
private static final Matcher<ExpressionTree> FAIL_METHOD =
anyOf(
instanceMethod().onDescendantOf("com.google.common.truth.AbstractVerb").named("fail"),
instanceMethod()
.onDescendantOf("com.google.common.truth.StandardSubjectBuilder")
.named("fail"),
staticMethod().onClass("org.junit.Assert").named("fail"),
staticMethod().onClass("junit.framework.Assert").named("fail"),
staticMethod().onClass("junit.framework.TestCase").named("fail"));
private static final Matcher<StatementTree> EXPECTED_EXCEPTION_MATCHER =
anyOf(
// expectedException.expect(Foo.class); me();
allOf(
isLastStatementInBlock(),
previousStatement(
expressionStatement(
anyOf(instanceMethod().onExactClass("org.junit.rules.ExpectedException"))))),
// try { me(); fail(); } catch (Throwable t) {}
allOf(enclosingNode(kindIs(Kind.TRY)), nextStatement(expressionStatement(FAIL_METHOD))),
// assertThrows(Throwable.class, () => { me(); })
allOf(
anyOf(isLastStatementInBlock(), parentNode(kindIs(Kind.LAMBDA_EXPRESSION))),
// Within the context of a ThrowingRunnable/Executable:
(t, s) -> methodCallInDeclarationOfThrowingRunnable(s)),
// @Test(expected = FooException.class) void bah() { me(); }
allOf(
UnusedReturnValueMatcher::isOnlyStatementInBlock,
enclosingMethod(UnusedReturnValueMatcher::isTestExpectedExceptionMethod)));
private static boolean isTestExpectedExceptionMethod(MethodTree tree, VisitorState state) {
if (!JUnitMatchers.wouldRunInJUnit4.matches(tree, state)) {
return false;
}
return getSymbol(tree).getAnnotationMirrors().stream()
.filter(am -> am.type.tsym.getQualifiedName().contentEquals("org.junit.Test"))
.findFirst()
.flatMap(testAm -> MoreAnnotations.getAnnotationValue(testAm, "expected"))
.flatMap(MoreAnnotations::asTypeValue)
.filter(tv -> !tv.toString().equals("org.junit.Test.None"))
.isPresent();
}
private static boolean isOnlyStatementInBlock(StatementTree t, VisitorState s) {
BlockTree parentBlock = ASTHelpers.findEnclosingNode(s.getPath(), BlockTree.class);
return parentBlock != null
&& parentBlock.getStatements().size() == 1
&& parentBlock.getStatements().get(0) == t;
}
/** Allow return values to be ignored in tests that expect an exception to be thrown. */
public static boolean expectedExceptionTest(VisitorState state) {
// Allow unused return values in tests that check for thrown exceptions, e.g.:
//
// try {
// Foo.newFoo(-1);
// fail();
// } catch (IllegalArgumentException expected) {
// }
//
StatementTree statement = findEnclosingNode(state.getPath(), StatementTree.class);
return statement != null && EXPECTED_EXCEPTION_MATCHER.matches(statement, state);
}
private static final Matcher<ExpressionTree> MOCKITO_MATCHER =
anyOf(
staticMethod().onClass("org.mockito.Mockito").named("verify"),
instanceMethod().onDescendantOf("org.mockito.stubbing.Stubber").named("when"),
instanceMethod().onDescendantOf("org.mockito.InOrder").named("verify"));
/**
* Don't match the method that is invoked through {@code Mockito.verify(t)} or {@code
* doReturn(val).when(t)}.
*/
public static boolean mockitoInvocation(Tree tree, VisitorState state) {
if (!(tree instanceof JCMethodInvocation)) {
return false;
}
JCMethodInvocation invocation = (JCMethodInvocation) tree;
if (!(invocation.getMethodSelect() instanceof JCFieldAccess)) {
return false;
}
ExpressionTree receiver = getReceiver(invocation);
return MOCKITO_MATCHER.matches(receiver, state);
}
/**
* Enumeration of known reasons that an unused return value may be allowed because of the context
* in which the method is used. Suppression is not considered here; these are reasons that don't
* have anything to do with specific checkers.
*/
public enum AllowReason {
/**
* The context is one in which the method is probably being called to test for an exception it
* throws.
*/
EXCEPTION_TESTING,
/** The context is a mocking call such as in {@code verify(foo).getBar();}. */
MOCKING_CALL,
/** The method returns {@code java.lang.Void} at this use-site. */
RETURNS_JAVA_LANG_VOID,
/** The method is a known Builder setter method (that always returns "this"). */
KNOWN_BUILDER_SETTER,
}
}