From 240dc8ab521dc78f35542c17107d4900866c4c23 Mon Sep 17 00:00:00 2001
From: Maksim Bezsaznyj <maksim@squareup.com>
Date: Tue, 19 Sep 2023 10:23:06 -0700
Subject: [PATCH] Add support for specifying exemptPrefixes/exemptNames for
 UnusedVariable via flags

In some cases projects have conventions about use of custom arguments/variables that are not meant to be deleted even if unused. One of such examples could be keeping an ORMs `Session session` argument in the method arguments signifying that the method happens within the boundaries of provided session and is doing calls to the underlying datastore.

This PR would allow to provide custom values for exemptPrefixes and exemptNames via error-prone flags.

Fixes #2753

FUTURE_COPYBARA_INTEGRATE_REVIEW=https://github.com/google/error-prone/pull/2753 from bezmax:unused-variable-parameters 36b9502a885781d19dfb424148ff91e83e220db3
PiperOrigin-RevId: 566671352
---
 .../bugpatterns/UnusedVariable.java           | 19 ++++++++++++++-----
 .../bugpatterns/UnusedMethodTest.java         |  9 ++++++++-
 .../bugpatterns/UnusedVariableTest.java       |  7 +++++++
 3 files changed, 29 insertions(+), 6 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 153c4e02971a..b80f872414da 100644
--- a/core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java
+++ b/core/src/main/java/com/google/errorprone/bugpatterns/UnusedVariable.java
@@ -125,9 +125,9 @@
     severity = WARNING,
     documentSuppression = false)
 public final class UnusedVariable extends BugChecker implements CompilationUnitTreeMatcher {
-  private static final String EXEMPT_PREFIX = "unused";
+  private final ImmutableSet<String> exemptPrefixes;
 
-  private static final ImmutableSet<String> EXEMPT_NAMES = ImmutableSet.of("ignored");
+  private final ImmutableSet<String> exemptNames;
 
   /**
    * The set of annotation full names which exempt annotated element from being reported as unused.
@@ -185,6 +185,14 @@ public final class UnusedVariable extends BugChecker implements CompilationUnitT
         .ifPresent(methodAnnotationsExemptingParameters::addAll);
     this.methodAnnotationsExemptingParameters = methodAnnotationsExemptingParameters.build();
     this.reportInjectedFields = flags.getBoolean("Unused:ReportInjectedFields").orElse(false);
+
+    ImmutableSet.Builder<String> exemptNames = ImmutableSet.<String>builder().add("ignored");
+    flags.getList("Unused:exemptNames").ifPresent(exemptNames::addAll);
+    this.exemptNames = exemptNames.build();
+
+    ImmutableSet.Builder<String> exemptPrefixes = ImmutableSet.<String>builder().add("unused");
+    flags.getSet("Unused:exemptPrefixes").ifPresent(exemptPrefixes::addAll);
+    this.exemptPrefixes = exemptPrefixes.build();
   }
 
   @Override
@@ -566,10 +574,11 @@ private static boolean exemptedByAnnotation(List<? extends AnnotationTree> annot
     return false;
   }
 
-  private static boolean exemptedByName(Name name) {
+  private boolean exemptedByName(Name name) {
     String nameString = name.toString();
-    return Ascii.toLowerCase(nameString).startsWith(EXEMPT_PREFIX)
-        || EXEMPT_NAMES.contains(nameString);
+    String nameStringLower = Ascii.toLowerCase(nameString);
+    return exemptPrefixes.stream().anyMatch(nameStringLower::startsWith)
+        || exemptNames.contains(nameString);
   }
 
   private class VariableFinder extends TreePathScanner<Void, Void> {
diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/UnusedMethodTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/UnusedMethodTest.java
index b41c55fe19b7..20a1d8a5b5d5 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/UnusedMethodTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/UnusedMethodTest.java
@@ -131,10 +131,17 @@ public void exemptedByName() {
             "Unuseds.java",
             "package unusedvars;",
             "class ExemptedByName {",
-            "  private void unused1(int a, int unusedParam) {",
+            "  private void unused1("
+                + "     int a, int unusedParam, "
+                + "     int customUnused1, int customUnused2, "
+                + "     int prefixUnused1Param, int prefixUnused2Param"
+                + "  ) {",
             "    int unusedLocal = a;",
             "  }",
             "}")
+        .setArgs(
+            "-XepOpt:Unused:exemptNames=customUnused1,customUnused2",
+            "-XepOpt:Unused:exemptPrefixes=prefixunused1,prefixunused2")
         .doTest();
   }
 
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 90abe1733e95..6f248ed024f3 100644
--- a/core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java
+++ b/core/src/test/java/com/google/errorprone/bugpatterns/UnusedVariableTest.java
@@ -434,7 +434,14 @@ public void exemptedByName() {
             "  private int unusedInt;",
             "  private static final int UNUSED_CONSTANT = 5;",
             "  private int ignored;",
+            "  private int customUnused1;",
+            "  private int customUnused2;",
+            "  private int prefixUnused1Field;",
+            "  private int prefixUnused2Field;",
             "}")
+        .setArgs(
+            "-XepOpt:Unused:exemptNames=customUnused1,customUnused2",
+            "-XepOpt:Unused:exemptPrefixes=prefixunused1,prefixunused2")
         .doTest();
   }