Java: Intention that wraps list/set/map with Collections.unmodifiable - use results of DFA, test fixed (IDEA-93154)

java/java-impl/src/com/intellij/codeInsight/intention/impl/WrapWithUnmodifiableAction.java

@@ -2,6 +2,9 @@
 package com.intellij.codeInsight.intention.impl;
 
 import com.intellij.codeInsight.CodeInsightBundle;
+import com.intellij.codeInspection.dataFlow.CommonDataflow;
+import com.intellij.codeInspection.dataFlow.DfaFactType;
+import com.intellij.codeInspection.dataFlow.Mutability;
 import com.intellij.openapi.editor.Editor;
 import com.intellij.openapi.project.Project;
 import com.intellij.psi.*;
@@ -13,6 +16,7 @@ import com.intellij.psi.util.PsiTypesUtil;
 import com.intellij.psi.util.PsiUtil;
 import com.intellij.util.IncorrectOperationException;
 import com.siyeh.ig.psiutils.CommentTracker;
+import com.siyeh.ig.psiutils.ExpectedTypeUtils;
 import com.siyeh.ig.psiutils.ExpressionUtils;
 import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
@@ -124,11 +128,11 @@ public class WrapWithUnmodifiableAction extends BaseIntentionAction {
   }
 
   private static PsiType getExpectedType(@NotNull PsiExpression expression) {
-    final PsiElement parent = PsiUtil.skipParenthesizedExprUp(expression.getParent());
-    if (parent instanceof PsiConditionalExpression) {
-      return getExpectedType((PsiConditionalExpression)parent);
+    PsiType expectedType = PsiTypesUtil.getExpectedTypeByParent(expression); // try the cheaper way first
+    if (expectedType != null) {
+      return expectedType;
     }
-    return PsiTypesUtil.getExpectedTypeByParent(expression);
+    return ExpectedTypeUtils.findExpectedType(expression, false);
   }
 
   private static boolean isInheritorChain(PsiClass psiClass,
@@ -143,6 +147,10 @@ public class WrapWithUnmodifiableAction extends BaseIntentionAction {
   }
 
   private static boolean isUnmodifiable(@NotNull PsiExpression expression) {
+    Mutability fact = CommonDataflow.getExpressionFact(expression, DfaFactType.MUTABILITY);
+    if (fact != null && fact.isUnmodifiable()) {
+      return true;
+    }
     PsiMethodCallExpression methodCall = tryCast(expression, PsiMethodCallExpression.class);
     if (isUnmodifiableCall(methodCall)) {
       return true;

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/afterListOfListAtArgument.java

@@ -0,0 +1,11 @@
+// "Wrap with unmodifiable list" "true"
+import java.util.Collections;
+import java.util.List;
+import java.util.ArrayList;
+
+class C {
+    List<List<String>> test() {
+        List<String> result = new ArrayList<>();
+        return List.of(Collections.unmodifiableList(result));
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/afterMapArgument.java

@@ -0,0 +1,13 @@
+// "Wrap with unmodifiable map" "true"
+import java.util.Collections;
+import java.util.Map;
+import java.util.HashMap;
+
+class C {
+    void test() {
+        var result = new HashMap<>();
+        foo(Collections.unmodifiableMap(result));
+    }
+
+    void foo(Map<String, Integer> map) {}
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeEmptyList.java

@@ -0,0 +1,10 @@
+// "Wrap with unmodifiable list" "false"
+import java.util.List;
+import java.util.Collections;
+
+class C {
+    List<String> test() {
+        List<String> result = Collections.emptyList();
+        return <caret>result;
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeListOfListAtArgument.java

@@ -0,0 +1,10 @@
+// "Wrap with unmodifiable list" "true"
+import java.util.List;
+import java.util.ArrayList;
+
+class C {
+    List<List<String>> test() {
+        List<String> result = new ArrayList<>();
+        return List.of(re<caret>sult);
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeListOfListAtMethod.java

@@ -0,0 +1,10 @@
+// "Wrap with unmodifiable list" "false"
+import java.util.List;
+import java.util.ArrayList;
+
+class C {
+    List<List<String>> test() {
+        List<String> result = new ArrayList<>();
+        return List.<caret>of(result);
+    }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeMapArgument.java

@@ -1,4 +1,4 @@
-// "Wrap with unmodifiable map" "false"
+// "Wrap with unmodifiable map" "true"
 import java.util.Map;
 import java.util.HashMap;
 

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeReturnArrayList.java

@@ -4,7 +4,7 @@ import java.util.ArrayList;
 
 class C {
     ArrayList<String> test() {
-        List<String> result = new ArrayList<>();
+        ArrayList<String> result = new ArrayList<>();
         return <caret>result;
     }
 }

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeUnmodifiableSetInVariable.java

@@ -0,0 +1,9 @@
+// "Wrap with unmodifiable set" "false"
+import java.util.*;
+
+class C {
+    Set<String> test() {
+        Set<String> result = Collections.unmodifiableSortedSet(new TreeSet<>());
+        return <caret>result;
+    }
+}

java/java-tests/testSrc/com/intellij/java/codeInsight/intention/WrapWithUnmodifiableTest.java

@@ -2,11 +2,21 @@
 package com.intellij.java.codeInsight.intention;
 
 import com.intellij.codeInsight.daemon.LightIntentionActionTestCase;
+import com.intellij.testFramework.LightProjectDescriptor;
+import org.jetbrains.annotations.NotNull;
+
+import static com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase.JAVA_10_ANNOTATED;
 
 /**
  * @author Pavel.Dolgov
  */
 public class WrapWithUnmodifiableTest extends LightIntentionActionTestCase {
+  @NotNull
+  @Override
+  protected LightProjectDescriptor getProjectDescriptor() {
+    return JAVA_10_ANNOTATED;
+  }
+
   @Override
   protected String getBasePath() {
     return "/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable";