Skip to content

Commit

Permalink
Descend into VariableTrees when looking for variables to check.
Browse files Browse the repository at this point in the history
I think this `return null`'d on the assumption that a `VariableTree` can't contain more variables to check... but of course it can, with anonymous classes etc.

PiperOrigin-RevId: 590929955
  • Loading branch information
graememorgan authored and Error Prone Team committed Dec 14, 2023
1 parent affa37a commit da7be27
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -612,11 +612,16 @@ private VariableFinder(VisitorState state) {

@Override
public Void visitVariable(VariableTree variableTree, Void unused) {
handleVariable(variableTree);
return super.visitVariable(variableTree, null);
}

private void handleVariable(VariableTree variableTree) {
if (exemptedByName(variableTree.getName())) {
return null;
return;
}
if (isSuppressed(variableTree, state)) {
return null;
return;
}
VarSymbol symbol = getSymbol(variableTree);
var parent = getCurrentPath().getParentPath().getLeaf();
Expand All @@ -626,22 +631,22 @@ public Void visitVariable(VariableTree variableTree, Void unused) {
unusedElements.put(symbol, getCurrentPath());
usageSites.put(symbol, getCurrentPath());
}
return null;
return;
}
if (symbol.getKind() == ElementKind.FIELD
&& symbol.getSimpleName().contentEquals("CREATOR")
&& isSubtype(symbol.type, PARCELABLE_CREATOR.get(state), state)) {
return null;
return;
}
if (symbol.getKind() == ElementKind.FIELD
&& exemptedFieldBySuperType(getType(variableTree), state)) {
return null;
return;
}
super.visitVariable(variableTree, null);
// Return if the element is exempted by an annotation.
if (exemptedByAnnotation(variableTree.getModifiers().getAnnotations())
|| shouldKeep(variableTree)) {
return null;
return;
}
switch (symbol.getKind()) {
case FIELD:
Expand All @@ -658,14 +663,14 @@ && exemptedFieldBySuperType(getType(variableTree), state)) {
case PARAMETER:
// ignore the receiver parameter
if (variableTree.getName().contentEquals("this")) {
return null;
return;
}
// Ignore if parameter is part of canonical record constructor; tree does not seem
// to contain usage in that case, but parameter is always used implicitly
// For compact canonical constructor parameters don't have record flag so need to
// check constructor flags (`symbol.owner`) instead
if (hasRecordFlag(symbol.owner)) {
return null;
return;
}
unusedElements.put(symbol, getCurrentPath());
if (!isParameterSubjectToAnalysis(symbol)) {
Expand All @@ -675,7 +680,6 @@ && exemptedFieldBySuperType(getType(variableTree), state)) {
default:
break;
}
return null;
}

private boolean exemptedFieldBySuperType(Type type, VisitorState state) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1616,6 +1616,25 @@ public void unusedFunctionalInterfaceParameter() {
.doTest();
}

@Test
public void unusedWithinAnotherVariableTree() {
helper
.addSourceLines(
"Test.java",
"import java.util.Collections;",
"import java.util.Comparator;",
"import java.util.List;",
"class Test {",
" public void test(List<Integer> xs) {",
" var unusedLocal = ",
" xs.stream().sorted(",
" // BUG: Diagnostic contains: 'b' is never read",
" (a, b) -> a > a ? 1 : 0);",
" }",
"}")
.doTest();
}

@Test
public void unusedFunctionalInterfaceParameter_noFix() {
refactoringHelper
Expand Down

0 comments on commit da7be27

Please sign in to comment.