Skip to content

Commit

Permalink
UnusedVariable: subject the parameters of @Inject &c methods to analy…
Browse files Browse the repository at this point in the history
…sis.

These should be used, or they should be removed. Removing them should be safe, given only the framework should be calling them.

I used "should" a lot there; they could be manually invoked, but that's a problem unto itself.

PiperOrigin-RevId: 499443898
  • Loading branch information
graememorgan authored and Error Prone Team committed Jan 4, 2023
1 parent 819218d commit 836caf2
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import static com.google.errorprone.util.ASTHelpers.getStartPosition;
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.isStatic;
import static com.google.errorprone.util.ASTHelpers.isSubtype;
import static com.google.errorprone.util.ASTHelpers.shouldKeep;
Expand Down Expand Up @@ -141,6 +142,16 @@ public final class UnusedVariable extends BugChecker implements CompilationUnitT
"org.apache.beam.sdk.transforms.DoFn.TimerId",
"org.apache.beam.sdk.transforms.DoFn.StateId");

// TODO(ghm): Find a sensible place to dedupe this with UnnecessarilyVisible.
private static final ImmutableSet<String> ANNOTATIONS_INDICATING_PARAMETERS_SHOULD_BE_CHECKED =
ImmutableSet.of(
"com.google.inject.Inject",
"com.google.inject.Provides",
"com.google.inject.multibindings.ProvidesIntoMap",
"com.google.inject.multibindings.ProvidesIntoSet",
"dagger.Provides",
"javax.inject.Inject");

private final ImmutableSet<String> methodAnnotationsExemptingParameters;

/** The set of types exempting a type that is extending or implementing them. */
Expand Down Expand Up @@ -640,11 +651,16 @@ private boolean isParameterSubjectToAnalysis(Symbol sym) {
Symbol enclosingMethod = sym.owner;

for (String annotationName : methodAnnotationsExemptingParameters) {
if (ASTHelpers.hasAnnotation(enclosingMethod, annotationName, state)) {
if (hasAnnotation(enclosingMethod, annotationName, state)) {
return false;
}
}

if (ANNOTATIONS_INDICATING_PARAMETERS_SHOULD_BE_CHECKED.stream()
.anyMatch(a -> hasAnnotation(enclosingMethod, a, state))) {
return true;
}

return enclosingMethod.getModifiers().contains(Modifier.PRIVATE);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,21 @@ public void unusedInject() {
.doTest();
}

@Test
public void unusedInjectConstructorParameter() {
helper
.addSourceLines(
"Test.java",
"package unusedvars;",
"import javax.inject.Inject;",
"public class Test {",
" @Inject Test(",
" // BUG: Diagnostic contains:",
" String foo) {}",
"}")
.doTest();
}

@Test
public void unusedInject_notByDefault() {
helper
Expand Down

0 comments on commit 836caf2

Please sign in to comment.