Skip to content

Commit 27884db

Browse files
electrumelectrum
committed
[UnusedMethod] Handle reflective method references
Handles Class.getMethod, getDeclaredMethod, getConstructor, getDeclaredConstructor, and MethodHandles.Lookup.findStatic, findVirtual, findSpecial, findConstructor. Only suppresses findings when the method name, owner class, and parameter types can all be statically resolved.
1 parent c7ae7e1 commit 27884db

2 files changed

Lines changed: 429 additions & 0 deletions

File tree

‎core/src/main/java/com/google/errorprone/bugpatterns/UnusedMethod.java‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,19 @@
2626
import static com.google.errorprone.fixes.SuggestedFix.emptyFix;
2727
import static com.google.errorprone.fixes.SuggestedFixes.replaceIncludingComments;
2828
import static com.google.errorprone.matchers.Matchers.SERIALIZATION_METHODS;
29+
import static com.google.errorprone.matchers.method.MethodMatchers.instanceMethod;
30+
import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
2931
import static com.google.errorprone.suppliers.Suppliers.typeFromString;
3032
import static com.google.errorprone.util.ASTHelpers.canBeRemoved;
33+
import static com.google.errorprone.util.ASTHelpers.constValue;
3134
import static com.google.errorprone.util.ASTHelpers.getEnclosedElements;
35+
import static com.google.errorprone.util.ASTHelpers.getReceiver;
3236
import static com.google.errorprone.util.ASTHelpers.getSymbol;
3337
import static com.google.errorprone.util.ASTHelpers.getType;
3438
import static com.google.errorprone.util.ASTHelpers.hasAnnotation;
3539
import static com.google.errorprone.util.ASTHelpers.isGeneratedConstructor;
3640
import static com.google.errorprone.util.ASTHelpers.isRecord;
41+
import static com.google.errorprone.util.ASTHelpers.isSameType;
3742
import static com.google.errorprone.util.ASTHelpers.isSubtype;
3843
import static com.google.errorprone.util.MoreAnnotations.asStrings;
3944
import static com.google.errorprone.util.MoreAnnotations.getAnnotationValue;
@@ -43,13 +48,15 @@
4348
import static javax.lang.model.element.Modifier.FINAL;
4449

4550
import com.google.common.base.Ascii;
51+
import com.google.common.collect.ImmutableList;
4652
import com.google.common.collect.ImmutableListMultimap;
4753
import com.google.common.collect.ImmutableSet;
4854
import com.google.errorprone.BugPattern;
4955
import com.google.errorprone.VisitorState;
5056
import com.google.errorprone.bugpatterns.BugChecker.CompilationUnitTreeMatcher;
5157
import com.google.errorprone.fixes.SuggestedFix;
5258
import com.google.errorprone.matchers.Description;
59+
import com.google.errorprone.matchers.Matcher;
5360
import com.google.errorprone.suppliers.Supplier;
5461
import com.google.errorprone.suppliers.Suppliers;
5562
import com.google.errorprone.util.ASTHelpers;
@@ -58,6 +65,7 @@
5865
import com.sun.source.tree.ClassTree;
5966
import com.sun.source.tree.CompilationUnitTree;
6067
import com.sun.source.tree.DeconstructionPatternTree;
68+
import com.sun.source.tree.ExpressionTree;
6169
import com.sun.source.tree.IdentifierTree;
6270
import com.sun.source.tree.MemberReferenceTree;
6371
import com.sun.source.tree.MemberSelectTree;
@@ -71,6 +79,7 @@
7179
import com.sun.tools.javac.code.Symbol;
7280
import com.sun.tools.javac.code.Symbol.ClassSymbol;
7381
import com.sun.tools.javac.code.Symbol.MethodSymbol;
82+
import com.sun.tools.javac.code.Symbol.VarSymbol;
7483
import com.sun.tools.javac.code.Type;
7584
import com.sun.tools.javac.tree.JCTree.JCAnnotation;
7685
import com.sun.tools.javac.tree.JCTree.JCAssign;
@@ -84,6 +93,7 @@
8493
import javax.inject.Inject;
8594
import javax.lang.model.element.Modifier;
8695
import javax.lang.model.element.Name;
96+
import org.jspecify.annotations.Nullable;
8797

8898
/** Bugpattern to detect unused declarations. */
8999
@BugPattern(
@@ -99,6 +109,29 @@ public final class UnusedMethod extends BugChecker implements CompilationUnitTre
99109
private static final Supplier<Type> JUNIT_PARAMS_ANNOTATION_TYPE =
100110
Suppliers.typeFromString("junitparams.Parameters");
101111

112+
private static final Matcher<ExpressionTree> LOOKUP_FIND_METHOD =
113+
instanceMethod()
114+
.onDescendantOf("java.lang.invoke.MethodHandles.Lookup")
115+
.namedAnyOf("findStatic", "findVirtual", "findSpecial");
116+
117+
private static final Matcher<ExpressionTree> LOOKUP_FIND_CONSTRUCTOR =
118+
instanceMethod()
119+
.onDescendantOf("java.lang.invoke.MethodHandles.Lookup")
120+
.named("findConstructor");
121+
122+
private static final Matcher<ExpressionTree> CLASS_GET_METHOD =
123+
instanceMethod()
124+
.onDescendantOf("java.lang.Class")
125+
.namedAnyOf("getMethod", "getDeclaredMethod");
126+
127+
private static final Matcher<ExpressionTree> CLASS_GET_CONSTRUCTOR =
128+
instanceMethod()
129+
.onDescendantOf("java.lang.Class")
130+
.namedAnyOf("getConstructor", "getDeclaredConstructor");
131+
132+
private static final Matcher<ExpressionTree> METHOD_TYPE_FACTORY =
133+
staticMethod().onClass("java.lang.invoke.MethodType").named("methodType");
134+
102135
/**
103136
* Class annotations which exempt methods within the annotated class from findings.
104137
*
@@ -272,6 +305,7 @@ public Void visitMemberReference(MemberReferenceTree tree, Void unused) {
272305
@Override
273306
public Void visitMethodInvocation(MethodInvocationTree tree, Void unused) {
274307
handle(getSymbol(tree));
308+
handleReflectiveAccess(tree);
275309
return super.visitMethodInvocation(tree, null);
276310
}
277311

@@ -312,6 +346,122 @@ private void handle(Symbol symbol) {
312346
}
313347
}
314348

349+
private void handleReflectiveAccess(MethodInvocationTree tree) {
350+
if (LOOKUP_FIND_METHOD.matches(tree, state)) {
351+
// findStatic/findVirtual/findSpecial(Class<?> refc, String name, MethodType type, ...)
352+
List<? extends ExpressionTree> args = tree.getArguments();
353+
Type ownerType = extractTypeFromClassLiteral(args.get(0));
354+
String name = constValue(args.get(1), String.class);
355+
ImmutableList<Type> paramTypes = extractParamTypesFromMethodType(args.get(2));
356+
if (ownerType == null || name == null || paramTypes == null) {
357+
return;
358+
}
359+
removeReflectivelyAccessedMethod(name, ownerType, paramTypes);
360+
} else if (LOOKUP_FIND_CONSTRUCTOR.matches(tree, state)) {
361+
// findConstructor(Class<?> refc, MethodType type)
362+
List<? extends ExpressionTree> args = tree.getArguments();
363+
Type ownerType = extractTypeFromClassLiteral(args.get(0));
364+
ImmutableList<Type> paramTypes = extractParamTypesFromMethodType(args.get(1));
365+
if (ownerType == null || paramTypes == null) {
366+
return;
367+
}
368+
removeReflectivelyAccessedMethod(null, ownerType, paramTypes);
369+
} else if (CLASS_GET_METHOD.matches(tree, state)) {
370+
// getMethod/getDeclaredMethod(String name, Class<?>... parameterTypes)
371+
List<? extends ExpressionTree> args = tree.getArguments();
372+
Type ownerType = extractClassOwnerFromReceiver(tree);
373+
String name = constValue(args.get(0), String.class);
374+
ImmutableList<Type> paramTypes = extractClassLiteralVarargs(args, 1);
375+
if (ownerType == null || name == null || paramTypes == null) {
376+
return;
377+
}
378+
removeReflectivelyAccessedMethod(name, ownerType, paramTypes);
379+
} else if (CLASS_GET_CONSTRUCTOR.matches(tree, state)) {
380+
// getConstructor/getDeclaredConstructor(Class<?>... parameterTypes)
381+
List<? extends ExpressionTree> args = tree.getArguments();
382+
Type ownerType = extractClassOwnerFromReceiver(tree);
383+
ImmutableList<Type> paramTypes = extractClassLiteralVarargs(args, 0);
384+
if (ownerType == null || paramTypes == null) {
385+
return;
386+
}
387+
removeReflectivelyAccessedMethod(null, ownerType, paramTypes);
388+
}
389+
}
390+
391+
private @Nullable Type extractTypeFromClassLiteral(ExpressionTree expr) {
392+
if (!(expr instanceof MemberSelectTree select)) {
393+
return null;
394+
}
395+
if (!select.getIdentifier().contentEquals("class")) {
396+
return null;
397+
}
398+
return getType(select.getExpression());
399+
}
400+
401+
private @Nullable ImmutableList<Type> extractParamTypesFromMethodType(ExpressionTree expr) {
402+
if (!(expr instanceof MethodInvocationTree call)) {
403+
return null;
404+
}
405+
if (!METHOD_TYPE_FACTORY.matches(call, state)) {
406+
return null;
407+
}
408+
return extractClassLiteralVarargs(call.getArguments(), 1);
409+
}
410+
411+
private @Nullable Type extractClassOwnerFromReceiver(MethodInvocationTree tree) {
412+
ExpressionTree receiver = getReceiver(tree);
413+
if (receiver == null) {
414+
return null;
415+
}
416+
return extractTypeFromClassLiteral(receiver);
417+
}
418+
419+
private @Nullable ImmutableList<Type> extractClassLiteralVarargs(
420+
List<? extends ExpressionTree> args, int startIndex) {
421+
ImmutableList.Builder<Type> builder = ImmutableList.builder();
422+
for (int i = startIndex; i < args.size(); i++) {
423+
Type t = extractTypeFromClassLiteral(args.get(i));
424+
if (t == null) {
425+
return null;
426+
}
427+
builder.add(t);
428+
}
429+
return builder.build();
430+
}
431+
432+
private void removeReflectivelyAccessedMethod(
433+
@Nullable String name, Type ownerType, ImmutableList<Type> paramTypes) {
434+
unusedMethods
435+
.keySet()
436+
.removeIf(sym -> methodMatches((MethodSymbol) sym, name, ownerType, paramTypes));
437+
}
438+
439+
private boolean methodMatches(
440+
MethodSymbol sym, @Nullable String name, Type ownerType, ImmutableList<Type> paramTypes) {
441+
if (name == null) {
442+
if (!sym.isConstructor()) {
443+
return false;
444+
}
445+
} else {
446+
if (sym.isConstructor() || !sym.getSimpleName().contentEquals(name)) {
447+
return false;
448+
}
449+
}
450+
if (!isSameType(sym.owner.type, ownerType, state)) {
451+
return false;
452+
}
453+
List<VarSymbol> params = sym.params();
454+
if (params.size() != paramTypes.size()) {
455+
return false;
456+
}
457+
for (int i = 0; i < params.size(); i++) {
458+
if (!isSameType(params.get(i).type, paramTypes.get(i), state)) {
459+
return false;
460+
}
461+
}
462+
return true;
463+
}
464+
315465
@Override
316466
public Void visitMethod(MethodTree tree, Void unused) {
317467
handleMethodSource(tree);

0 commit comments

Comments
 (0)