From 564bb424de6ab9842e4014aa1bdec54bab1b590b Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Mon, 25 Feb 2019 15:37:43 +0300 Subject: [PATCH] Java: Intention that wraps list/set/map with Collections.unmodifiable - use results of DFA, test fixed (IDEA-93154) --- .../impl/WrapWithUnmodifiableAction.java | 16 ++++++++++++---- .../afterListOfListAtArgument.java | 11 +++++++++++ .../wrapWithUnmodifiable/afterMapArgument.java | 13 +++++++++++++ .../wrapWithUnmodifiable/beforeEmptyList.java | 10 ++++++++++ .../beforeListOfListAtArgument.java | 10 ++++++++++ .../beforeListOfListAtMethod.java | 10 ++++++++++ .../wrapWithUnmodifiable/beforeMapArgument.java | 2 +- .../beforeReturnArrayList.java | 2 +- .../beforeUnmodifiableSetInVariable.java | 9 +++++++++ .../intention/WrapWithUnmodifiableTest.java | 10 ++++++++++ 10 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/afterListOfListAtArgument.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/afterMapArgument.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeEmptyList.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeListOfListAtArgument.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeListOfListAtMethod.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeUnmodifiableSetInVariable.java diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/WrapWithUnmodifiableAction.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/WrapWithUnmodifiableAction.java index dce1aa586b60..b3c2885ca142 100644 --- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/WrapWithUnmodifiableAction.java +++ b/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; diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/afterListOfListAtArgument.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/afterListOfListAtArgument.java new file mode 100644 index 000000000000..004ac088d819 --- /dev/null +++ b/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> test() { + List result = new ArrayList<>(); + return List.of(Collections.unmodifiableList(result)); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/afterMapArgument.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/afterMapArgument.java new file mode 100644 index 000000000000..377836408e35 --- /dev/null +++ b/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 map) {} +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeEmptyList.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeEmptyList.java new file mode 100644 index 000000000000..09ed0114c44c --- /dev/null +++ b/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 test() { + List result = Collections.emptyList(); + return result; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeListOfListAtArgument.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeListOfListAtArgument.java new file mode 100644 index 000000000000..17c7e3c8967f --- /dev/null +++ b/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> test() { + List result = new ArrayList<>(); + return List.of(result); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeListOfListAtMethod.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeListOfListAtMethod.java new file mode 100644 index 000000000000..d9786e314c30 --- /dev/null +++ b/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> test() { + List result = new ArrayList<>(); + return List.of(result); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeMapArgument.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeMapArgument.java index 27a8e0b92321..85e96fd77251 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeMapArgument.java +++ b/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; diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeReturnArrayList.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeReturnArrayList.java index 56372bb1bef3..cbf72522c83f 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeReturnArrayList.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeReturnArrayList.java @@ -4,7 +4,7 @@ import java.util.ArrayList; class C { ArrayList test() { - List result = new ArrayList<>(); + ArrayList result = new ArrayList<>(); return result; } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeUnmodifiableSetInVariable.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/wrapWithUnmodifiable/beforeUnmodifiableSetInVariable.java new file mode 100644 index 000000000000..aed07ef2640d --- /dev/null +++ b/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 test() { + Set result = Collections.unmodifiableSortedSet(new TreeSet<>()); + return result; + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/intention/WrapWithUnmodifiableTest.java b/java/java-tests/testSrc/com/intellij/java/codeInsight/intention/WrapWithUnmodifiableTest.java index 34b3ac23ed19..0798d701b62d 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/intention/WrapWithUnmodifiableTest.java +++ b/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";