From 11034f337d20c0e7161f8544fb328435acb27686 Mon Sep 17 00:00:00 2001
From: Liam Miller-Cushon <cushon@google.com>
Date: Wed, 12 Jun 2024 12:37:32 -0700
Subject: [PATCH] Don't suggest removing parameters in private methods that are
 overridden within the current file

PiperOrigin-RevId: 642701853
---
 .../bugpatterns/UnusedVariable.java           | 33 +++++++++++++++++--
 .../bugpatterns/UnusedVariableTest.java       |  1 -
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java b/core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java
index cec249ef709..71e12a7f237 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java
@@ -25,6 +25,7 @@
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
 import static com.google.errorprone.matchers.Matchers.SERIALIZATION_METHODS;
 import static com.google.errorprone.util.ASTHelpers.canBeRemoved;
+import static com.google.errorprone.util.ASTHelpers.findSuperMethods;
 import static com.google.errorprone.util.ASTHelpers.getStartPosition;
 import static com.google.errorprone.util.ASTHelpers.getSymbol;
 import static com.google.errorprone.util.ASTHelpers.getType;
@@ -46,6 +47,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.ListMultimap;
@@ -120,6 +122,7 @@
 import javax.lang.model.element.Modifier;
 import javax.lang.model.element.Name;
 import javax.lang.model.type.NullType;
+import javax.tools.JavaFileObject;
 
 /** Bugpattern to detect unused declarations. */
 @BugPattern(
@@ -213,7 +216,10 @@ public Description matchCompilationUnit(CompilationUnitTree tree, VisitorState s
       return Description.NO_MATCH;
     }
 
-    VariableFinder variableFinder = new VariableFinder(state);
+    ImmutableMultimap<MethodSymbol, MethodSymbol> superMethodsToOverrides =
+        getSuperMethodsToOverrides(tree, state);
+
+    VariableFinder variableFinder = new VariableFinder(state, superMethodsToOverrides);
     variableFinder.scan(state.getPath(), null);
 
     // Map of symbols to variable declarations. Initially this is a map of all of the local variable
@@ -353,6 +359,23 @@ public Void visitMethod(MethodTree tree, Void unused) {
     return hasAnyNativeMethods.get();
   }
 
+  private static ImmutableMultimap<MethodSymbol, MethodSymbol> getSuperMethodsToOverrides(
+      CompilationUnitTree tree, VisitorState state) {
+    ImmutableMultimap.Builder<MethodSymbol, MethodSymbol> overrides = ImmutableMultimap.builder();
+    JavaFileObject sourceFile = tree.getSourceFile();
+    new TreeScanner<Void, Void>() {
+      @Override
+      public Void visitMethod(MethodTree tree, Void unused) {
+        MethodSymbol sym = getSymbol(tree);
+        findSuperMethods(getSymbol(tree), state.getTypes()).stream()
+            .filter(m -> sourceFile.equals(m.enclClass().sourcefile))
+            .forEach(m -> overrides.put(m, sym));
+        return null;
+      }
+    }.scan(tree, null);
+    return overrides.build();
+  }
+
   // https://docs.oracle.com/javase/specs/jls/se11/html/jls-14.html#jls-ExpressionStatement
   private static final ImmutableSet<Tree.Kind> TOP_LEVEL_EXPRESSIONS =
       ImmutableSet.of(
@@ -600,9 +623,12 @@ private class VariableFinder extends TreePathScanner<Void, Void> {
     private final ListMultimap<Symbol, TreePath> usageSites = ArrayListMultimap.create();
 
     private final VisitorState state;
+    private final ImmutableMultimap<MethodSymbol, MethodSymbol> superMethodsToOverrides;
 
-    private VariableFinder(VisitorState state) {
+    private VariableFinder(
+        VisitorState state, ImmutableMultimap<MethodSymbol, MethodSymbol> superMethodsToOverrides) {
       this.state = state;
+      this.superMethodsToOverrides = superMethodsToOverrides;
     }
 
     @Override
@@ -706,7 +732,8 @@ private boolean isParameterSubjectToAnalysis(Symbol sym) {
       Symbol enclosingMethod = sym.owner;
 
       if (!(enclosingMethod instanceof MethodSymbol)
-          || isAbstract((MethodSymbol) enclosingMethod)) {
+          || isAbstract((MethodSymbol) enclosingMethod)
+          || superMethodsToOverrides.containsKey(enclosingMethod)) {
         return false;
       }
 
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java
index d86028c149f..92107250de7 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java
@@ -1656,7 +1656,6 @@ public void unusedFunctionalInterfaceParameter_noFix() {
         .doTest();
   }
 
-  @Ignore("https://github.com/google/error-prone/issues/4409")
   @Test
   public void parameterUsedInOverride() {
     refactoringHelper