Java8MapApiInspection: cr fixes (IDEA-CR-44452)

1. VariableNameGenerator instead of codeStyleManager#suggestUniqueVariableName
2. use tracker for inner comments
3. simplified references search using various util methods
4. added option to disable replaceAll suggestion

java/java-impl/src/com/intellij/codeInspection/java18api/Java8MapApiInspection.java

@@ -12,11 +12,11 @@ import com.intellij.pom.java.JavaFeature;
 import com.intellij.psi.*;
 import com.intellij.psi.codeStyle.CodeStyleManager;
 import com.intellij.psi.codeStyle.JavaCodeStyleManager;
+import com.intellij.psi.codeStyle.VariableKind;
 import com.intellij.psi.search.searches.ReferencesSearch;
 import com.intellij.psi.util.PsiTreeUtil;
-import com.intellij.util.ArrayUtil;
 import com.intellij.util.ObjectUtils;
-import com.siyeh.ig.PsiReplacementUtil;
+import com.siyeh.ig.callMatcher.CallMatcher;
 import com.siyeh.ig.psiutils.*;
 import one.util.streamex.StreamEx;
 import org.jetbrains.annotations.Nls;
@@ -35,6 +35,8 @@ import static com.siyeh.ig.psiutils.Java8MigrationUtils.MapCheckCondition.fromCo
 public class Java8MapApiInspection extends AbstractBaseJavaLocalInspectionTool {
   private static final Logger LOG = Logger.getInstance(Java8MapApiInspection.class);
   public static final String SHORT_NAME = "Java8MapApi";
+  private static final CallMatcher KEY_VALUE_GET_METHODS =
+    CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP_ENTRY, "getKey", "getValue").parameterCount(0);
 
   @SuppressWarnings("PublicField")
   public boolean mySuggestMapGetOrDefault = true;
@@ -45,6 +47,8 @@ public class Java8MapApiInspection extends AbstractBaseJavaLocalInspectionTool {
   @SuppressWarnings("PublicField")
   public boolean mySuggestMapMerge = true;
   @SuppressWarnings("PublicField")
+  public boolean mySuggestMapReplaceAll = true;
+  @SuppressWarnings("PublicField")
   public boolean myTreatGetNullAsContainsKey = false;
   @SuppressWarnings("PublicField")
   public boolean mySideEffects = false;
@@ -57,6 +61,7 @@ public class Java8MapApiInspection extends AbstractBaseJavaLocalInspectionTool {
     panel.addCheckbox("Suggest conversion to Map.getOrDefault", "mySuggestMapGetOrDefault");
     panel.addCheckbox("Suggest conversion to Map.putIfAbsent", "mySuggestMapPutIfAbsent");
     panel.addCheckbox("Suggest conversion to Map.merge", "mySuggestMapMerge");
+    panel.addCheckbox("Suggest conversion to Map.replaceAll", "mySuggestMapReplaceAll");
     panel.addCheckbox("Treat 'get(k) != null' the same as 'containsKey(k)' (may change semantics)", "myTreatGetNullAsContainsKey");
     panel.addCheckbox("Suggest replacement even if lambda may have side effects", "mySideEffects");
     return panel;
@@ -99,6 +104,7 @@ public class Java8MapApiInspection extends AbstractBaseJavaLocalInspectionTool {
 
       @Override
       public void visitForeachStatement(PsiForeachStatement statement) {
+        if (!mySuggestMapReplaceAll) return;
         MapLoopCondition condition = MapLoopCondition.create(statement);
         if (condition == null) return;
         PsiMethodCallExpression putCall = condition.extractPut(statement);
@@ -116,9 +122,9 @@ public class Java8MapApiInspection extends AbstractBaseJavaLocalInspectionTool {
       }
 
       private boolean isUsedAsReference(@NotNull PsiElement value, @NotNull MapLoopCondition condition) {
-        VariableUsedAsReferenceVisitor visitor = new VariableUsedAsReferenceVisitor(condition.getIterParam(), "getValue", "getKey");
-        value.accept(visitor);
-        return visitor.isUsed;
+        return !VariableAccessUtils.getVariableReferences(condition.getIterParam(), value).stream()
+          .map(ExpressionUtils::getCallForQualifier)
+          .allMatch(KEY_VALUE_GET_METHODS);
       }
 
       private void processMerge(MapCheckCondition condition,
@@ -249,44 +255,6 @@ public class Java8MapApiInspection extends AbstractBaseJavaLocalInspectionTool {
     return nameCandidate.toLowerCase(Locale.ENGLISH);
   }
 
-  private static class VariableUsedAsReferenceVisitor extends JavaRecursiveElementWalkingVisitor {
-    private final PsiVariable myVariable;
-    private final String[] myExcludeMethodNames;
-
-    private boolean isUsed = false;
-
-    private VariableUsedAsReferenceVisitor(PsiVariable variable, String... excludeMethodNames) {
-      myVariable = variable;
-      myExcludeMethodNames = excludeMethodNames;
-    }
-
-    @Override
-    public void visitReferenceExpression(PsiReferenceExpression expression) {
-      if (isUsed) {
-        return;
-      }
-      super.visitReferenceExpression(expression);
-      if (expression.resolve() == myVariable) {
-        isUsed = true;
-      }
-    }
-
-    @Override
-    public void visitMethodCallExpression(PsiMethodCallExpression expression) {
-      if (isUsed) {
-        return;
-      }
-      if (ArrayUtil.contains(expression.getMethodExpression().getReferenceName(), myExcludeMethodNames)) {
-        PsiReferenceExpression qualifier =
-          ObjectUtils.tryCast(expression.getMethodExpression().getQualifierExpression(), PsiReferenceExpression.class);
-        if (qualifier != null && qualifier.resolve() == myVariable) {
-          return;
-        }
-      }
-      super.visitMethodCallExpression(expression);
-    }
-  }
-
   private static class ReplaceWithSingleMapOperation implements LocalQuickFix {
     private final String myMethodName;
     private final SmartPsiElementPointer<PsiMethodCallExpression> myCallPointer;
@@ -371,9 +339,8 @@ public class Java8MapApiInspection extends AbstractBaseJavaLocalInspectionTool {
       else if (myMethodName.equals("replaceAll")) {
         MapLoopCondition loopCondition = ObjectUtils.tryCast(condition, MapLoopCondition.class);
         if (loopCondition == null) return;
-        JavaCodeStyleManager styleManager = JavaCodeStyleManager.getInstance(project);
-        String kVar = suggestKeyName(styleManager, loopCondition, value);
-        String vVar = styleManager.suggestUniqueVariableName("v", value, true);
+        String kVar = suggestKeyName(loopCondition, value);
+        String vVar = new VariableNameGenerator(value, VariableKind.PARAMETER).byName("v", "value").generate(true);
         replacement = createLambdaForLoopReplacement(factory, kVar, vVar, loopCondition, value, ct);
         ct.delete(args);
       }
@@ -402,18 +369,17 @@ public class Java8MapApiInspection extends AbstractBaseJavaLocalInspectionTool {
     }
 
     @NotNull
-    private static String suggestKeyName(@NotNull JavaCodeStyleManager codeStyleManager,
-                                         @NotNull MapLoopCondition loopCondition,
-                                         @Nullable PsiElement value) {
-      String nameCandidate = "k";
+    private static String suggestKeyName(@NotNull MapLoopCondition loopCondition, @NotNull PsiElement value) {
+      VariableNameGenerator generator = new VariableNameGenerator(value, VariableKind.PARAMETER);
       if (!loopCondition.isEntrySet()) {
         String origName = loopCondition.getIterParam().getName();
         if (origName != null) {
-          nameCandidate = getNameCandidate(origName);
+          String nameCandidate = getNameCandidate(origName);
           if (origName.equals(nameCandidate)) return nameCandidate;
+          generator.byName(nameCandidate);
         }
       }
-      return codeStyleManager.suggestUniqueVariableName(nameCandidate, value, true);
+      return generator.byName("k", "key").generate(true);
     }
 
     @NotNull
@@ -425,10 +391,7 @@ public class Java8MapApiInspection extends AbstractBaseJavaLocalInspectionTool {
                                                                 @NotNull CommentTracker tracker) {
       if (!loopCondition.isEntrySet()) {
         PsiParameter param = loopCondition.getIterParam();
-        StreamEx.ofTree((PsiElement)value, e -> StreamEx.of(e.getChildren()))
-          .select(PsiReferenceExpression.class)
-          .filter(ref -> param.equals(ref.resolve()))
-          .forEach(ref -> ExpressionUtils.bindReferenceTo(ref, kVar));
+        VariableAccessUtils.getVariableReferences(param, value).forEach(ref -> ExpressionUtils.bindReferenceTo(ref, kVar));
       }
       else {
         if (value instanceof PsiMethodCallExpression) {
@@ -438,10 +401,10 @@ public class Java8MapApiInspection extends AbstractBaseJavaLocalInspectionTool {
         Collection<PsiMethodCallExpression> calls = PsiTreeUtil.collectElementsOfType(value, PsiMethodCallExpression.class);
         for (PsiMethodCallExpression call : calls) {
           if (loopCondition.isKeyAccess(call)) {
-            PsiReplacementUtil.replaceExpression(call, kVar, new CommentTracker());
+            tracker.replace(call, kVar);
           }
           else if (loopCondition.isValueAccess(call)) {
-            PsiReplacementUtil.replaceExpression(call, vVar, new CommentTracker());
+            tracker.replace(call, vVar);
           }
         }
       }

java/java-impl/src/inspectionDescriptions/Java8MapApi.html

@@ -1,7 +1,7 @@
 <html>
 <body>
 Reports calls to <code>Map.get()</code> which could be replaced with <code>getOrDefault()</code>, <code>computeIfAbsent()</code> or
-<code>putIfAbsent()</code>.
+<code>putIfAbsent()</code>. Also reports calls to <code>Map.put()</code> which could be replaced with <code>replaceAll()</code>.
 <ul>
   <li><b>Map.getOrDefault</b> method could be used to replace the code like this:
     <pre>
@@ -26,6 +26,13 @@ Reports calls to <code>Map.get()</code> which could be replaced with <code>getOr
       if (val == null) map.put(key, 1);
       else map.put(key, val + 1);</pre>
   </li>
+  <li><b>Map.replaceAll</b> method could be used to replace the code like this:
+    <pre>
+      for (Map.Entry&lt;String, String&gt; entry : map.entrySet()) {
+        map.put(entry.getKey(), transform(entry.getValue()));
+      }
+    </pre>
+  </li>
 
 </ul>
 <p>Note that replacement with <code>computeIfAbsent()</code> or <code>merge()</code> may work incorrectly for some <code>Map</code>

java/java-tests/testData/inspection/java8MapApi/afterReplaceAllEntrySetPoly.java

@@ -8,6 +8,7 @@ class Main {
     Map<String, String> map = new HashMap<>();
     String defaultValue = "42";
     map.put("foo", "bar");
-      map.replaceAll((k, v) -> v + k + defaultValue + "baz");
+    /*1*//*8*//*9*//*2*//*3*//*4*//*5*/
+      map/*6*/.replaceAll(/*7*//*10*/ (k, v) -> v +/*11*/ k +/*12*/ defaultValue + "baz");/*13*/
   }
 }

java/java-tests/testData/inspection/java8MapApi/afterReplaceAllEntrySetShadowVar.java

@@ -6,6 +6,7 @@ import java.util.Map;
 public class Main {
   public void test() {
     String k = "another var";
+    String key = "one more";
     Map<String, String> map = new HashMap<>();
     map.put("foo", "bar");
       map.replaceAll((k1, v) -> k);

java/java-tests/testData/inspection/java8MapApi/beforeReplaceAllEntrySetPoly.java

@@ -8,8 +8,8 @@ class Main {
     Map<String, String> map = new HashMap<>();
     String defaultValue = "42";
     map.put("foo", "bar");
-    for<caret> (Map.Entry<String, String> entry : map.entrySet()) {
-      map.put(entry.getKey(), entry.getValue() + entry.getKey() + defaultValue + "baz");
+    /*1*/for<caret> /*2*/(Map.Entry<String, String> /*3*/entry : map./*4*/entrySet()) {
+      /*5*/map/*6*/.put(/*7*/entry./*8*/getKey(/*9*/),/*10*/ entry.getValue() +/*11*/ entry.getKey() +/*12*/ defaultValue + "baz");/*13*/
     }
   }
 }

java/java-tests/testData/inspection/java8MapApi/beforeReplaceAllEntrySetShadowVar.java

@@ -6,6 +6,7 @@ import java.util.Map;
 public class Main {
   public void test() {
     String k = "another var";
+    String key = "one more";
     Map<String, String> map = new HashMap<>();
     map.put("foo", "bar");
     for<caret> (Map.Entry<String, String> entry : map.entrySet()) {