Skip to content

UnusedVariable: exempt @FieldSource #4838

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Strings.nullToEmpty;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.Iterables.getLast;
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
Expand All @@ -35,6 +36,8 @@
import static com.google.errorprone.util.ASTHelpers.isStatic;
import static com.google.errorprone.util.ASTHelpers.isSubtype;
import static com.google.errorprone.util.ASTHelpers.shouldKeep;
import static com.google.errorprone.util.MoreAnnotations.asStrings;
import static com.google.errorprone.util.MoreAnnotations.getAnnotationValue;
import static com.google.errorprone.util.SideEffectAnalysis.hasSideEffect;
import static com.sun.source.tree.Tree.Kind.POSTFIX_DECREMENT;
import static com.sun.source.tree.Tree.Kind.POSTFIX_INCREMENT;
Expand Down Expand Up @@ -245,7 +248,8 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s
// appropriate fixes for them.
ListMultimap<Symbol, TreePath> usageSites = variableFinder.usageSites;

FilterUsedVariables filterUsedVariables = new FilterUsedVariables(unusedElements, usageSites);
FilterUsedVariables filterUsedVariables =
new FilterUsedVariables(state, unusedElements, usageSites);
filterUsedVariables.scan(state.getPath(), null);

// Keeps track of whether a symbol was _ever_ used (between reassignments).
Expand Down Expand Up @@ -816,6 +820,8 @@ private static final class FilterUsedVariables extends TreePathScanner<Void, Voi

private TreePath currentExpressionStatement = null;

private final VisitorState state;

private final Map<Symbol, TreePath> unusedElements;

private final ListMultimap<Symbol, TreePath> usageSites;
Expand All @@ -828,7 +834,10 @@ private static final class FilterUsedVariables extends TreePathScanner<Void, Voi
private final ImmutableMap<Symbol, TreePath> declarationSites;

private FilterUsedVariables(
Map<Symbol, TreePath> unusedElements, ListMultimap<Symbol, TreePath> usageSites) {
VisitorState state,
Map<Symbol, TreePath> unusedElements,
ListMultimap<Symbol, TreePath> usageSites) {
this.state = state;
this.unusedElements = unusedElements;
this.usageSites = usageSites;
this.declarationSites = ImmutableMap.copyOf(unusedElements);
Expand Down Expand Up @@ -1066,6 +1075,40 @@ public Void visitMethodInvocation(MethodInvocationTree tree, Void unused) {
inMethodCall--;
return null;
}

@Override
public Void visitMethod(final MethodTree node, final Void unused) {
handleFieldSource(node);
return super.visitMethod(node, unused);
}

/**
* If a method is annotated with @FieldSource, the annotation value refers to a field that is
* used reflectively to supply test parameters, so that field should not be considered unused.
*/
private void handleFieldSource(MethodTree tree) {
MethodSymbol sym = getSymbol(tree);
Name name = ORG_JUNIT_JUPITER_PARAMS_PROVIDER_FIELDSOURCE.get(state);
sym.getRawAttributes().stream()
.filter(a -> a.type.tsym.getQualifiedName().equals(name))
.findAny()
// get the annotation value array as a set of Names
.flatMap(a -> getAnnotationValue(a, "value"))
.map(y -> asStrings(y).map(state::getName).map(Name::toString).collect(toImmutableSet()))
// remove all potentially unused methods referenced by the @FieldSource
.ifPresent(
referencedNames ->
unusedElements
.entrySet()
.removeIf(
e -> {
Symbol unusedSym = e.getKey();
String simpleName = unusedSym.getSimpleName().toString();
return referencedNames.contains(simpleName)
|| referencedNames.contains(
unusedSym.owner.getQualifiedName() + "#" + simpleName);
}));
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all copypasta from UnusedMethod.

}

@AutoValue
Expand Down Expand Up @@ -1103,4 +1146,9 @@ private static UnusedSpec of(

private static final Supplier<Type> PARCELABLE_CREATOR =
VisitorState.memoize(state -> state.getTypeFromString("android.os.Parcelable.Creator"));

private static final Supplier<com.sun.tools.javac.util.Name>
ORG_JUNIT_JUPITER_PARAMS_PROVIDER_FIELDSOURCE =
VisitorState.memoize(
state -> state.getName("org.junit.jupiter.params.provider.FieldSource"));
}
Original file line number Diff line number Diff line change
Expand Up @@ -1922,4 +1922,32 @@ public void foo(int xs) {}
.expectUnchanged()
.doTest();
}

@Test
public void fieldSource() {
helper
.addSourceLines(
"FieldSource.java",
"""
package org.junit.jupiter.params.provider;

public @interface FieldSource {
String[] value();
}
""")
.addSourceLines(
"Test.java",
"""
import java.util.List;
import org.junit.jupiter.params.provider.FieldSource;

class Test {
@FieldSource("parameters")
void test() {}

private static final List<String> parameters = List.of("apple", "banana");
}
""")
.doTest();
}
}