Descend into VariableTrees when looking for variables to check.

graememorgan · Error Prone Team · commit da7be27b3c4a · 2023-12-14T07:15:43.000-08:00
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
diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java b/core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java
@@ -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();
@@ -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:
@@ -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)) {
@@ -675,7 +680,6 @@ && exemptedFieldBySuperType(getType(variableTree), state)) {
         default:
           break;
       }
-      return null;
     }
 
     private boolean exemptedFieldBySuperType(Type type, VisitorState state) {
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java
@@ -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
