From 78cf87d2c8a71bac00ad92ee7835aacab8ac6daa Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Fri, 8 Jun 2012 14:42:52 +0400 Subject: [PATCH] do not replace diamonds without necessity (IDEA-87172) --- .../intellij/psi/impl/PsiDiamondTypeUtil.java | 14 +++++++++++- ...troduceParameterMethodUsagesProcessor.java | 4 +++- .../afterPreserveDiamondOccurrences.java | 22 +++++++++++++++++++ .../beforePreserveDiamondOccurrences.java | 22 +++++++++++++++++++ .../refactoring/IntroduceParameterTest.java | 4 ++++ .../ReplaceIfWithConditionalIntention.java | 16 ++++++++++++-- ...placeableAssignmentsWithDiamondsLeave.java | 17 ++++++++++++++ ...bleAssignmentsWithDiamondsLeave_after.java | 12 ++++++++++ ...ReplaceIfWithConditionalIntentionTest.java | 4 ++++ 9 files changed, 111 insertions(+), 4 deletions(-) create mode 100644 java/java-tests/testData/refactoring/introduceParameter/afterPreserveDiamondOccurrences.java create mode 100644 java/java-tests/testData/refactoring/introduceParameter/beforePreserveDiamondOccurrences.java create mode 100644 plugins/IntentionPowerPak/test/com/siyeh/ipp/trivialif/replaceIfWithConditional/ReplaceableAssignmentsWithDiamondsLeave.java create mode 100644 plugins/IntentionPowerPak/test/com/siyeh/ipp/trivialif/replaceIfWithConditional/ReplaceableAssignmentsWithDiamondsLeave_after.java diff --git a/java/java-impl/src/com/intellij/psi/impl/PsiDiamondTypeUtil.java b/java/java-impl/src/com/intellij/psi/impl/PsiDiamondTypeUtil.java index 502055854d22..c5037ac91c3f 100644 --- a/java/java-impl/src/com/intellij/psi/impl/PsiDiamondTypeUtil.java +++ b/java/java-impl/src/com/intellij/psi/impl/PsiDiamondTypeUtil.java @@ -40,6 +40,18 @@ public class PsiDiamondTypeUtil { public static boolean canCollapseToDiamond(final PsiNewExpression expression, final PsiNewExpression context, final @Nullable PsiType expectedType) { + return canCollapseToDiamond(expression, context, expectedType, false); + } + + public static boolean canChangeContextForDiamond(final PsiNewExpression expression, final PsiType expectedType) { + final PsiNewExpression copy = (PsiNewExpression)expression.copy(); + return canCollapseToDiamond(copy, copy, expectedType, true); + } + + private static boolean canCollapseToDiamond(final PsiNewExpression expression, + final PsiNewExpression context, + final @Nullable PsiType expectedType, + boolean skipDiamonds) { if (PsiUtil.getLanguageLevel(context).isAtLeast(LanguageLevel.JDK_1_7)) { final PsiJavaCodeReferenceElement classReference = expression.getClassOrAnonymousClassReference(); if (classReference != null) { @@ -47,7 +59,7 @@ public class PsiDiamondTypeUtil { if (parameterList != null) { final PsiTypeElement[] typeElements = parameterList.getTypeParameterElements(); if (typeElements.length > 0) { - if (typeElements.length == 1 && typeElements[0].getType() instanceof PsiDiamondType) return false; + if (!skipDiamonds && typeElements.length == 1 && typeElements[0].getType() instanceof PsiDiamondType) return false; final PsiDiamondTypeImpl.DiamondInferenceResult inferenceResult = PsiDiamondTypeImpl.resolveInferredTypes(expression, context); if (inferenceResult.getErrorMessage() == null) { final List types = inferenceResult.getInferredTypes(); diff --git a/java/java-impl/src/com/intellij/refactoring/introduceParameter/JavaIntroduceParameterMethodUsagesProcessor.java b/java/java-impl/src/com/intellij/refactoring/introduceParameter/JavaIntroduceParameterMethodUsagesProcessor.java index 827540c785a4..3f16cfb1e0eb 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceParameter/JavaIntroduceParameterMethodUsagesProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceParameter/JavaIntroduceParameterMethodUsagesProcessor.java @@ -94,7 +94,9 @@ public class JavaIntroduceParameterMethodUsagesProcessor implements IntroducePar ExpressionConverter.getExpression(data.getParameterInitializer().getExpression(), StdLanguages.JAVA, data.getProject()); assert initializer instanceof PsiExpression; if (initializer instanceof PsiNewExpression) { - initializer = PsiDiamondTypeUtil.expandTopLevelDiamondsInside((PsiNewExpression)initializer); + if (!PsiDiamondTypeUtil.canChangeContextForDiamond((PsiNewExpression)initializer, ((PsiNewExpression)initializer).getType())) { + initializer = PsiDiamondTypeUtil.expandTopLevelDiamondsInside((PsiNewExpression)initializer); + } } substituteTypeParametersInInitializer(initializer, callExpression, argList, methodToSearchFor); ChangeContextUtil.encodeContextInfo(initializer, true); diff --git a/java/java-tests/testData/refactoring/introduceParameter/afterPreserveDiamondOccurrences.java b/java/java-tests/testData/refactoring/introduceParameter/afterPreserveDiamondOccurrences.java new file mode 100644 index 000000000000..4f62137d7950 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameter/afterPreserveDiamondOccurrences.java @@ -0,0 +1,22 @@ + +public class TestCompletion { + + public static ParallelPipeline test(T base, V newStage, T upstream, final ParallelPipeline anObject) { + if (base != null){ + return anObject; + } + else { + return new ParallelPipeline<>(upstream, newStage); + } + + } + + + void f() { + test(null, null, null, new ParallelPipeline<>(null, null)); + } + private static class ParallelPipeline { + public ParallelPipeline(T p0, V p1) { + } + } +} diff --git a/java/java-tests/testData/refactoring/introduceParameter/beforePreserveDiamondOccurrences.java b/java/java-tests/testData/refactoring/introduceParameter/beforePreserveDiamondOccurrences.java new file mode 100644 index 000000000000..2bf9a854544d --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceParameter/beforePreserveDiamondOccurrences.java @@ -0,0 +1,22 @@ + +public class TestCompletion { + + public static ParallelPipeline test(T base, V newStage, T upstream) { + if (base != null){ + return new ParallelPipeline<>(base, newStage); + } + else { + return new ParallelPipeline<>(upstream, newStage); + } + + } + + + void f() { + test(null, null, null); + } + private static class ParallelPipeline { + public ParallelPipeline(T p0, V p1) { + } + } +} diff --git a/java/java-tests/testSrc/com/intellij/refactoring/IntroduceParameterTest.java b/java/java-tests/testSrc/com/intellij/refactoring/IntroduceParameterTest.java index 4893ce72e001..0cad5462136e 100644 --- a/java/java-tests/testSrc/com/intellij/refactoring/IntroduceParameterTest.java +++ b/java/java-tests/testSrc/com/intellij/refactoring/IntroduceParameterTest.java @@ -275,6 +275,10 @@ public class IntroduceParameterTest extends LightRefactoringTestCase { doTest(IntroduceParameterRefactoring.REPLACE_FIELDS_WITH_GETTERS_ALL, true, false, true, false); } + public void testPreserveDiamondOccurrences() throws Exception { + doTest(IntroduceParameterRefactoring.REPLACE_FIELDS_WITH_GETTERS_ALL, true, false, true, false); + } + public void testSubstituteTypeParams() throws Exception { doTest(IntroduceParameterRefactoring.REPLACE_FIELDS_WITH_GETTERS_ALL, true, false, true, false); } diff --git a/plugins/IntentionPowerPak/src/com/siyeh/ipp/trivialif/ReplaceIfWithConditionalIntention.java b/plugins/IntentionPowerPak/src/com/siyeh/ipp/trivialif/ReplaceIfWithConditionalIntention.java index faa30fd258d9..fb1bbd251bb5 100644 --- a/plugins/IntentionPowerPak/src/com/siyeh/ipp/trivialif/ReplaceIfWithConditionalIntention.java +++ b/plugins/IntentionPowerPak/src/com/siyeh/ipp/trivialif/ReplaceIfWithConditionalIntention.java @@ -170,11 +170,14 @@ public class ReplaceIfWithConditionalIntention extends Intention { PsiExpression elseValue, PsiType requiredType) { condition = ParenthesesUtils.stripParentheses(condition); - thenValue = PsiDiamondTypeUtil.expandTopLevelDiamondsInside(ParenthesesUtils.stripParentheses(thenValue)); + thenValue = ParenthesesUtils.stripParentheses(thenValue); + elseValue = ParenthesesUtils.stripParentheses(elseValue); + + thenValue = expandDiamondsWhenNeeded(thenValue, requiredType); if (thenValue == null) { return null; } - elseValue = PsiDiamondTypeUtil.expandTopLevelDiamondsInside(ParenthesesUtils.stripParentheses(elseValue)); + elseValue = expandDiamondsWhenNeeded(elseValue, requiredType); if (elseValue == null) { return null; } @@ -217,6 +220,15 @@ public class ReplaceIfWithConditionalIntention extends Intention { return conditional.toString(); } + private static PsiExpression expandDiamondsWhenNeeded(PsiExpression thenValue, PsiType requiredType) { + if (thenValue instanceof PsiNewExpression) { + if (!PsiDiamondTypeUtil.canChangeContextForDiamond((PsiNewExpression)thenValue, requiredType)) { + return PsiDiamondTypeUtil.expandTopLevelDiamondsInside(thenValue); + } + } + return thenValue; + } + private static String getExpressionText(PsiExpression expression) { if (ParenthesesUtils.getPrecedence(expression) <= ParenthesesUtils.CONDITIONAL_PRECEDENCE) { diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/trivialif/replaceIfWithConditional/ReplaceableAssignmentsWithDiamondsLeave.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/trivialif/replaceIfWithConditional/ReplaceableAssignmentsWithDiamondsLeave.java new file mode 100644 index 000000000000..47b35995ff25 --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/trivialif/replaceIfWithConditional/ReplaceableAssignmentsWithDiamondsLeave.java @@ -0,0 +1,17 @@ +public class TestCompletion { + + public static ParallelPipeline test(T base, V newStage, T upstream) { + if (base != null) { + return new ParallelPipeline<>(base, newStage); + } + else { + return new ParallelPipeline<>(upstream, newStage); + } + } + + private static class ParallelPipeline { + public ParallelPipeline(T p0, V p1) { + } + } +} + diff --git a/plugins/IntentionPowerPak/test/com/siyeh/ipp/trivialif/replaceIfWithConditional/ReplaceableAssignmentsWithDiamondsLeave_after.java b/plugins/IntentionPowerPak/test/com/siyeh/ipp/trivialif/replaceIfWithConditional/ReplaceableAssignmentsWithDiamondsLeave_after.java new file mode 100644 index 000000000000..a11449b495b4 --- /dev/null +++ b/plugins/IntentionPowerPak/test/com/siyeh/ipp/trivialif/replaceIfWithConditional/ReplaceableAssignmentsWithDiamondsLeave_after.java @@ -0,0 +1,12 @@ +public class TestCompletion { + + public static ParallelPipeline test(T base, V newStage, T upstream) { + return base != null ? new ParallelPipeline<>(base, newStage) : new ParallelPipeline<>(upstream, newStage); + } + + private static class ParallelPipeline { + public ParallelPipeline(T p0, V p1) { + } + } +} + diff --git a/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/trivialif/ReplaceIfWithConditionalIntentionTest.java b/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/trivialif/ReplaceIfWithConditionalIntentionTest.java index f4a51a62f53e..9cbfe40f0d35 100644 --- a/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/trivialif/ReplaceIfWithConditionalIntentionTest.java +++ b/plugins/IntentionPowerPak/testSrc/com/siyeh/ipp/trivialif/ReplaceIfWithConditionalIntentionTest.java @@ -28,6 +28,10 @@ public class ReplaceIfWithConditionalIntentionTest extends IPPTestCase { doTest(); } + public void testReplaceableAssignmentsWithDiamondsLeave() { + doTest(); + } + @Override protected String getIntentionName() { return IntentionPowerPackBundle.message("replace.if.with.conditional.intention.name");