Fixes #5553 : Skip UnnecessaryOptionalGet when lambda parameter is unnamed

`UnnecessaryOptionalGet` suggested replacing `.get()` with the lambda parameter name, but the parameter `_` (unnamed variable, Java 22+) produces uncompilable code since `_` cannot be referenced.

Changes:
- Added an early return when the lambda parameter source is `_`
- Added a test guarded by `assume().that(Runtime.version().feature()).isAtLeast(22)`

Fixes #5553

Fixes #5649

COPYBARA_INTEGRATE_REVIEW=#5649 from mirkoalicastro:mirkoalicastro/fix-unnecessary-optional-get-unnamed-variable bdcd042
PiperOrigin-RevId: 899107954

Author: mirkoalicastro

check_api/src/main/java/com/google/errorprone/fixes/SuggestedFixes.java

@@ -122,6 +122,7 @@
 import java.util.Comparator;
 import java.util.Deque;
 import java.util.EnumSet;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.ListIterator;
@@ -743,11 +744,20 @@ public static SuggestedFix renameVariable(
     // and a tree without an end position for earlier versions.
     int typeEndPos = tree.getType() != null ? state.getEndPosition(tree.getType()) : -1;
     int searchOffset = typeEndPos == -1 ? 0 : (typeEndPos - startPos);
-    int pos = startPos + state.getSourceForNode(tree).indexOf(name, searchOffset);
-    return SuggestedFix.builder()
-        .replace(pos, pos + name.length(), replacement)
-        .merge(renameVariableUsages(tree, replacement, state))
-        .build();
+    SuggestedFix.Builder fix = SuggestedFix.builder();
+    state.getOffsetTokens(startPos + searchOffset, state.getEndPosition(tree)).stream()
+        .filter(
+            token ->
+                switch (token.kind()) {
+                  // VariableTree#getName is empty for unnamed _ variables
+                  case UNDERSCORE -> name.isEmpty();
+                  case IDENTIFIER -> token.name().contentEquals(name);
+                  default -> false;
+                })
+        .findFirst()
+        .ifPresent(token -> fix.replace(token.pos(), token.endPos(), replacement));
+    fix.merge(renameVariableUsages(tree, replacement, state));
+    return fix.build();
   }
 
   /**
@@ -1861,5 +1871,30 @@ public static Visibility from(Tree tree) {
     }
   }
 
+  public static VariableNamer variableNamer(VisitorState state) {
+    return new VariableNamer(state);
+  }
+
+  /** Helper class for avoiding variable name shadowing. */
+  public static class VariableNamer {
+    private final Set<String> idents;
+
+    private VariableNamer(VisitorState state) {
+      this.idents =
+          FindIdentifiers.findAllIdents(state).stream()
+              .map(s -> s.getSimpleName().toString())
+              .collect(toCollection(HashSet::new));
+    }
+
+    public String avoidShadowing(String name) {
+      for (int i = 1; ; i++) {
+        String n = i == 1 ? name : (name + i);
+        if (idents.add(n)) {
+          return n;
+        }
+      }
+    }
+  }
+
   private SuggestedFixes() {}
 }

core/src/main/java/com/google/errorprone/bugpatterns/AssertThrowsMinimizer.java

@@ -31,7 +31,6 @@
 import static com.google.errorprone.util.ASTHelpers.getType;
 import static com.google.errorprone.util.ASTHelpers.isCheckedExceptionType;
 import static com.google.errorprone.util.ASTHelpers.isSubtype;
-import static java.util.stream.Collectors.toCollection;
 
 import com.google.common.base.CaseFormat;
 import com.google.common.collect.ImmutableList;
@@ -47,7 +46,6 @@
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
 import com.google.errorprone.predicates.TypePredicates;
-import com.google.errorprone.util.FindIdentifiers;
 import com.sun.source.tree.BlockTree;
 import com.sun.source.tree.ExpressionStatementTree;
 import com.sun.source.tree.ExpressionTree;
@@ -67,10 +65,7 @@
 import com.sun.tools.javac.code.Symbol.MethodSymbol;
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.Types;
-import java.util.HashSet;
 import java.util.Optional;
-import java.util.Set;
-import java.util.stream.IntStream;
 import java.util.stream.Stream;
 import javax.inject.Inject;
 import javax.lang.model.element.ElementKind;
@@ -197,8 +192,9 @@ public Description matchMethod(MethodTree tree, VisitorState state) {
     }
 
     // update the tree path so VariableName considers the method parameters
-    VariableNamer variableNamer =
-        new VariableNamer(state.withPath(new TreePath(state.getPath(), toFix.getFirst().runnable)));
+    SuggestedFixes.VariableNamer variableNamer =
+        SuggestedFixes.variableNamer(
+            state.withPath(new TreePath(state.getPath(), toFix.getFirst().runnable)));
     for (AssertThrows current : toFix) {
       StringBuilder hoistedVariables = new StringBuilder();
       for (Hoist hoist : current.toHoist) {
@@ -367,25 +363,4 @@ private static boolean isCheckedException(Type exception, VisitorState state) {
               .forClass(
                   TypePredicates.isDescendantOf(
                       "com.google.android.gms.tagmanager.internal.type.AbstractType")));
-
-  private static class VariableNamer {
-    private final Set<String> idents;
-
-    VariableNamer(VisitorState state) {
-      this.idents =
-          FindIdentifiers.findAllIdents(state).stream()
-              .map(s -> s.getSimpleName().toString())
-              .collect(toCollection(HashSet::new));
-    }
-
-    // Stolen from PatternMatchingInstanceof
-    // TODO: cushon - add to SuggestedFixes?
-    private String avoidShadowing(String name) {
-      return IntStream.iterate(1, i -> i + 1)
-          .mapToObj(i -> i == 1 ? name : (name + i))
-          .filter(n -> idents.add(n))
-          .findFirst()
-          .get();
-    }
-  }
 }

core/src/main/java/com/google/errorprone/bugpatterns/PatternMatchingInstanceof.java

@@ -17,7 +17,6 @@
 package com.google.errorprone.bugpatterns;
 
 import static com.google.common.base.Ascii.toLowerCase;
-import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.common.collect.Streams.stream;
 import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
 import static com.google.errorprone.fixes.SuggestedFix.mergeFixes;
@@ -36,8 +35,8 @@
 import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions;
 import com.google.errorprone.bugpatterns.threadsafety.ConstantExpressions.ConstantExpression;
 import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.fixes.SuggestedFixes;
 import com.google.errorprone.matchers.Description;
-import com.google.errorprone.util.FindIdentifiers;
 import com.google.errorprone.util.Reachability;
 import com.sun.source.tree.BinaryTree;
 import com.sun.source.tree.BlockTree;
@@ -56,7 +55,6 @@
 import com.sun.tools.javac.code.Type;
 import com.sun.tools.javac.code.TypeTag;
 import java.util.HashSet;
-import java.util.stream.IntStream;
 import java.util.stream.Stream;
 import javax.inject.Inject;
 import javax.lang.model.SourceVersion;
@@ -167,24 +165,12 @@ private static String generateVariableName(Type targetType, VisitorState state)
     String simpleName = IdentifierNames.fixInitialisms(targetType.tsym.getSimpleName().toString());
     String lowerFirstLetter = toLowerCase(String.valueOf(simpleName.charAt(0)));
     String camelCased = lowerFirstLetter + simpleName.substring(1);
+    SuggestedFixes.VariableNamer variableNamer = SuggestedFixes.variableNamer(state);
     if (SourceVersion.isKeyword(camelCased)
         || (unboxed != null && unboxed.getTag() != TypeTag.NONE)) {
-      return avoidShadowing(lowerFirstLetter, state);
+      return variableNamer.avoidShadowing(lowerFirstLetter);
     }
-    return avoidShadowing(camelCased, state);
-  }
-
-  // TODO: cushon - add to SuggestedFixes?
-  private static String avoidShadowing(String name, VisitorState state) {
-    var idents =
-        FindIdentifiers.findAllIdents(state).stream()
-            .map(s -> s.getSimpleName().toString())
-            .collect(toImmutableSet());
-    return IntStream.iterate(1, i -> i + 1)
-        .mapToObj(i -> i == 1 ? name : (name + i))
-        .filter(n -> !idents.contains(n))
-        .findFirst()
-        .get();
+    return variableNamer.avoidShadowing(camelCased);
   }
 
   /** Finds trees which are implied by the {@code instanceOfTree}. */

core/src/main/java/com/google/errorprone/bugpatterns/UnnecessaryOptionalGet.java

@@ -25,6 +25,7 @@
 import com.google.errorprone.VisitorState;
 import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
 import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.fixes.SuggestedFixes;
 import com.google.errorprone.matchers.Description;
 import com.google.errorprone.matchers.Matcher;
 import com.sun.source.tree.ExpressionTree;
@@ -83,14 +84,22 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState
       return Description.NO_MATCH;
     }
     VariableTree arg = getOnlyElement(lambdaExpressionTree.getParameters());
+    SuggestedFixes.VariableNamer variableNamer = SuggestedFixes.variableNamer(state);
     SuggestedFix.Builder fix = SuggestedFix.builder();
+    String replacement;
+    if (arg.getName().isEmpty()) {
+      replacement = variableNamer.avoidShadowing("value");
+      fix.merge(SuggestedFixes.renameVariable(arg, replacement, state));
+    } else {
+      replacement = arg.getName().toString();
+    }
     new TreeScanner<Void, VisitorState>() {
       @Override
       public Void visitMethodInvocation(
           MethodInvocationTree methodInvocationTree, VisitorState visitorState) {
         if (OPTIONAL_GET.matches(methodInvocationTree, visitorState)
             && sameVariable(getReceiver(tree), getReceiver(methodInvocationTree))) {
-          fix.replace(methodInvocationTree, state.getSourceForNode(arg));
+          fix.replace(methodInvocationTree, replacement);
         }
         return super.visitMethodInvocation(methodInvocationTree, visitorState);
       }

core/src/test/java/com/google/errorprone/bugpatterns/UnnecessaryOptionalGetTest.java

@@ -16,6 +16,8 @@
 
 package com.google.errorprone.bugpatterns;
 
+import static com.google.common.truth.TruthJUnit.assume;
+
 import com.google.errorprone.BugCheckerRefactoringTestHelper;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -77,6 +79,7 @@ public class Test {
               private void home() {
                 Optional<String> op = Optional.of("hello");
                 op.transform(x -> Long.parseLong(op.get()));
+                op.transform((String x) -> Long.parseLong(op.get()));
               }
             }
             """)
@@ -89,6 +92,7 @@ public class Test {
               private void home() {
                 Optional<String> op = Optional.of("hello");
                 op.transform(x -> Long.parseLong(x));
+                op.transform((String x) -> Long.parseLong(x));
               }
             }
             """)
@@ -388,4 +392,37 @@ private void home() {
             """)
         .doTest();
   }
+
+  @Test
+  public void unnamedVariable_withGet_warning() {
+    assume().that(Runtime.version().feature()).isAtLeast(22);
+    refactoringTestHelper
+        .addInputLines(
+            "Test.java",
+            """
+            import java.util.Optional;
+
+            public class Test {
+              private void home() {
+                Optional<String> op = Optional.of("hello");
+                op.ifPresent(_ -> System.out.println(op.get()));
+                op.ifPresent((String _) -> System.out.println(op.get()));
+              }
+            }
+            """)
+        .addOutputLines(
+            "Test.java",
+            """
+            import java.util.Optional;
+
+            public class Test {
+              private void home() {
+                Optional<String> op = Optional.of("hello");
+                op.ifPresent(value -> System.out.println(value));
+                op.ifPresent((String value) -> System.out.println(value));
+              }
+            }
+            """)
+        .doTest();
+  }
 }