Skip to content

Commit da03fea

Browse files
java-team-github-botError Prone Team
authored andcommitted
Update MethodCanBeStatic to be able to detect and trigger on Guice @provides methods.
PiperOrigin-RevId: 802239934
1 parent 83e6582 commit da03fea

File tree

3 files changed

+99
-11
lines changed

3 files changed

+99
-11
lines changed

check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@
156156
import java.util.function.Predicate;
157157
import java.util.stream.Stream;
158158
import javax.lang.model.element.AnnotationMirror;
159+
import javax.lang.model.element.Element;
159160
import javax.lang.model.element.ElementKind;
160161
import javax.lang.model.element.Modifier;
161162
import javax.lang.model.type.TypeKind;
@@ -1018,29 +1019,46 @@ public static boolean hasDirectAnnotationWithSimpleName(Symbol sym, String simpl
10181019
if (sym instanceof VarSymbol varSymbol) {
10191020
return hasDirectAnnotationWithSimpleName(varSymbol, simpleName);
10201021
}
1021-
return hasDirectAnnotationWithSimpleName(sym.getAnnotationMirrors().stream(), simpleName);
1022+
return hasDirectAnnotation(
1023+
sym.getAnnotationMirrors().stream(),
1024+
element -> element.getSimpleName().contentEquals(simpleName));
10221025
}
10231026

10241027
public static boolean hasDirectAnnotationWithSimpleName(MethodSymbol sym, String simpleName) {
1025-
return hasDirectAnnotationWithSimpleName(
1028+
return hasDirectAnnotation(
10261029
Streams.concat(
10271030
sym.getAnnotationMirrors().stream(),
10281031
sym.getReturnType().getAnnotationMirrors().stream()),
1029-
simpleName);
1032+
element -> element.getSimpleName().contentEquals(simpleName));
10301033
}
10311034

10321035
public static boolean hasDirectAnnotationWithSimpleName(VarSymbol sym, String simpleName) {
1033-
return hasDirectAnnotationWithSimpleName(
1036+
return hasDirectAnnotation(
10341037
Streams.concat(
10351038
sym.getAnnotationMirrors().stream(), sym.asType().getAnnotationMirrors().stream()),
1036-
simpleName);
1039+
element -> element.getSimpleName().contentEquals(simpleName));
10371040
}
10381041

1039-
private static boolean hasDirectAnnotationWithSimpleName(
1040-
Stream<? extends AnnotationMirror> annotations, String simpleName) {
1042+
/**
1043+
* Check for the presence of an annotation with the given qualified name directly on this symbol
1044+
* or its type. (If the given symbol is a method symbol, the type searched for annotations is its
1045+
* return type.)
1046+
*
1047+
* <p>This method looks only a annotations that are directly present. It does <b>not</b> consider
1048+
* annotation inheritance (see JLS 9.6.4.3).
1049+
*/
1050+
public static boolean hasDirectAnnotation(MethodSymbol sym, String qualifiedName) {
1051+
return hasDirectAnnotation(
1052+
Streams.concat(
1053+
sym.getAnnotationMirrors().stream(),
1054+
sym.getReturnType().getAnnotationMirrors().stream()),
1055+
element -> ((Symbol) element).getQualifiedName().contentEquals(qualifiedName));
1056+
}
1057+
1058+
private static boolean hasDirectAnnotation(
1059+
Stream<? extends AnnotationMirror> annotations, Predicate<Element> matcher) {
10411060
return annotations.anyMatch(
1042-
annotation ->
1043-
annotation.getAnnotationType().asElement().getSimpleName().contentEquals(simpleName));
1061+
annotation -> matcher.test(annotation.getAnnotationType().asElement()));
10441062
}
10451063

10461064
/**

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

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,10 @@
6868
severity = SUGGESTION,
6969
documentSuppression = false)
7070
public class MethodCanBeStatic extends BugChecker implements CompilationUnitTreeMatcher {
71+
72+
private static final ImmutableSet<String> GUICE_PROVIDES_ANNOTATION_NAMES =
73+
ImmutableSet.of("com.google.inject.Provides");
74+
7175
private final FindingOutputStyle findingOutputStyle;
7276

7377
private final WellKnownKeep wellKnownKeep;
@@ -249,9 +253,17 @@ private boolean isExcluded(MethodTree tree, VisitorState state) {
249253
if (sym.isConstructor() || !disjoint(EXCLUDED_MODIFIERS, sym.getModifiers())) {
250254
return true;
251255
}
252-
if (!ASTHelpers.canBeRemoved(sym, state) || wellKnownKeep.shouldKeep(tree)) {
253-
return true;
256+
257+
boolean isGuiceProvidesMethod =
258+
GUICE_PROVIDES_ANNOTATION_NAMES.stream()
259+
.anyMatch(annotationName -> ASTHelpers.hasDirectAnnotation(sym, annotationName));
260+
261+
if (!isGuiceProvidesMethod) {
262+
if (!ASTHelpers.canBeRemoved(sym, state) || wellKnownKeep.shouldKeep(tree)) {
263+
return true;
264+
}
254265
}
266+
255267
switch (enclosingClass(sym).getNestingKind()) {
256268
case TOP_LEVEL -> {}
257269
case MEMBER -> {

core/src/test/java/com/google/errorprone/bugpatterns/MethodCanBeStaticTest.java

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,4 +731,62 @@ public int get() {
731731
""")
732732
.doTest();
733733
}
734+
735+
@Test
736+
public void guiceProvidesMethod_madeStatic() {
737+
refactoringHelper
738+
.addInputLines(
739+
"Test.java",
740+
"""
741+
import com.google.inject.AbstractModule;
742+
import com.google.inject.Provides;
743+
744+
class MadeStatic extends AbstractModule {
745+
@Provides
746+
Boolean provideBoolean() {
747+
return false;
748+
}
749+
}
750+
""")
751+
.addOutputLines(
752+
"Test.java",
753+
"""
754+
import com.google.inject.AbstractModule;
755+
import com.google.inject.Provides;
756+
757+
class MadeStatic extends AbstractModule {
758+
@Provides
759+
static Boolean provideBoolean() {
760+
return false;
761+
}
762+
}
763+
""")
764+
.doTest();
765+
}
766+
767+
@Test
768+
public void guiceProvidesMethod_withInstanceState_cannotBeMadeStatic() {
769+
testHelper
770+
.addSourceLines(
771+
"Test.java",
772+
"""
773+
import com.google.inject.AbstractModule;
774+
import com.google.inject.Provides;
775+
776+
class Test extends AbstractModule {
777+
778+
private final int value;
779+
780+
public Test(int value) {
781+
this.value = value;
782+
}
783+
784+
@Provides
785+
int provideInteger() {
786+
return value;
787+
}
788+
}
789+
""")
790+
.doTest();
791+
}
734792
}

0 commit comments

Comments
 (0)