From f836ad796d43b5fc0b126e26ab20aeb96ac29955 Mon Sep 17 00:00:00 2001 From: Dmitry Batkovich Date: Wed, 10 Feb 2016 21:28:09 +0300 Subject: [PATCH] IDEA-151190 Change class signature doesn't support complicated bounds --- .../ChangeClassSignatureFromUsageFix.java | 17 +++---------- .../psi/impl/source/PsiCodeFragmentImpl.java | 5 +++- .../impl/source/PsiTypeCodeFragmentImpl.java | 16 ++++++++++++- .../ChangeClassSignatureDialog.java | 22 +++++++++++------ .../ChangeClassSignatureProcessor.java | 4 +++- .../TypeParameterInfo.java | 2 ++ .../intellij/psi/JavaCodeFragmentFactory.java | 6 ++++- .../com/intellij/psi/PsiIntersectionType.java | 9 +++++-- .../psi/impl/source/tree/JavaElementType.java | 20 ++++++++++++---- .../refactoring/util/CanonicalTypes.java | 24 +++++++++++++++---- .../AddBoundWithIntersection.java | 9 +++++++ .../AddBoundWithIntersection.java.after | 9 +++++++ .../AddWithBound.java.after | 3 ++- .../ModifyWithBound.java.after | 3 ++- .../ChangeClassSignatureTest.java | 16 +++++++++++-- 15 files changed, 125 insertions(+), 40 deletions(-) create mode 100644 java/java-tests/testData/refactoring/changeClassSignature/AddBoundWithIntersection.java create mode 100644 java/java-tests/testData/refactoring/changeClassSignature/AddBoundWithIntersection.java.after diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/ChangeClassSignatureFromUsageFix.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/ChangeClassSignatureFromUsageFix.java index 76d449dee2f8..b566b6f2e5d5 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/ChangeClassSignatureFromUsageFix.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/ChangeClassSignatureFromUsageFix.java @@ -24,7 +24,6 @@ import com.intellij.refactoring.changeClassSignature.ChangeClassSignatureDialog; import com.intellij.refactoring.changeClassSignature.TypeParameterInfo; import com.intellij.util.IncorrectOperationException; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; import java.util.*; @@ -123,26 +122,16 @@ public class ChangeClassSignatureFromUsageFix extends BaseIntentionAction { else { suggestedName = suggester.suggestUnusedName("T"); } - final PsiTypeCodeFragment boundFragment = createBoundCodeFragment(boundType, typeElement, factory); + final PsiTypeCodeFragment boundFragment = ChangeClassSignatureDialog.createTableCodeFragment(boundType, typeElement, factory, true); result.add(new TypeParameterInfoView(new TypeParameterInfo.New(suggestedName, defaultType, null), boundFragment, boundType == null ? factory.createTypeCodeFragment(suggestedName, typeElement, true) - : createBoundCodeFragment(boundType, typeElement, factory))); + : ChangeClassSignatureDialog + .createTableCodeFragment(boundType, typeElement, factory, false))); } return result; } - private static PsiTypeCodeFragment createBoundCodeFragment(@Nullable PsiClassType boundType, - @NotNull PsiElement context, - @NotNull JavaCodeFragmentFactory factory) { - final PsiTypeCodeFragment boundFragment = - factory.createTypeCodeFragment(boundType == null ? "" : boundType.getClassName(), context, true); - if (boundType != null) { - boundFragment.addImportsFromString(boundType.getCanonicalText()); - } - return boundFragment; - } - private static boolean isAssignable(@NotNull PsiTypeParameter typeParameter, @NotNull PsiType type) { for (PsiClassType t : typeParameter.getExtendsListTypes()) { if (!t.isAssignableFrom(type)) { diff --git a/java/java-impl/src/com/intellij/psi/impl/source/PsiCodeFragmentImpl.java b/java/java-impl/src/com/intellij/psi/impl/source/PsiCodeFragmentImpl.java index 3ee8503e43d9..10a30242b606 100644 --- a/java/java-impl/src/com/intellij/psi/impl/source/PsiCodeFragmentImpl.java +++ b/java/java-impl/src/com/intellij/psi/impl/source/PsiCodeFragmentImpl.java @@ -207,7 +207,10 @@ public class PsiCodeFragmentImpl extends PsiFileImpl implements JavaCodeFragment } IElementType i = myContentElementType; - if (i == JavaElementType.TYPE_TEXT || i == JavaElementType.EXPRESSION_STATEMENT || i == JavaElementType.REFERENCE_TEXT) { + if (i == JavaElementType.TYPE_WITH_CONJUNCTIONS_TEXT || + i == JavaElementType.TYPE_WITH_DISJUNCTIONS_TEXT || + i == JavaElementType.EXPRESSION_STATEMENT || + i == JavaElementType.REFERENCE_TEXT) { return true; } else { diff --git a/java/java-impl/src/com/intellij/psi/impl/source/PsiTypeCodeFragmentImpl.java b/java/java-impl/src/com/intellij/psi/impl/source/PsiTypeCodeFragmentImpl.java index 10cec6570360..4e65c96d3e0e 100644 --- a/java/java-impl/src/com/intellij/psi/impl/source/PsiTypeCodeFragmentImpl.java +++ b/java/java-impl/src/com/intellij/psi/impl/source/PsiTypeCodeFragmentImpl.java @@ -15,6 +15,7 @@ */ package com.intellij.psi.impl.source; +import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.impl.source.tree.JavaElementType; @@ -29,8 +30,11 @@ import static com.intellij.util.BitUtil.isSet; * @author dsl */ public class PsiTypeCodeFragmentImpl extends PsiCodeFragmentImpl implements PsiTypeCodeFragment { + private final static Logger LOG = Logger.getInstance(PsiTypeCodeFragmentImpl.class); + private final boolean myAllowEllipsis; private final boolean myAllowDisjunction; + private final boolean myAllowConjunction; public PsiTypeCodeFragmentImpl(final Project project, final boolean isPhysical, @@ -38,10 +42,17 @@ public class PsiTypeCodeFragmentImpl extends PsiCodeFragmentImpl implements PsiT final CharSequence text, final int flags, PsiElement context) { - super(project, JavaElementType.TYPE_TEXT, isPhysical, name, text, context); + super(project, + isSet(flags, JavaCodeFragmentFactory.ALLOW_INTERSECTION) ? JavaElementType.TYPE_WITH_CONJUNCTIONS_TEXT : JavaElementType.TYPE_WITH_DISJUNCTIONS_TEXT, + isPhysical, + name, + text, + context); myAllowEllipsis = isSet(flags, JavaCodeFragmentFactory.ALLOW_ELLIPSIS); myAllowDisjunction = isSet(flags, JavaCodeFragmentFactory.ALLOW_DISJUNCTION); + myAllowConjunction = isSet(flags, JavaCodeFragmentFactory.ALLOW_INTERSECTION); + LOG.assertTrue(!myAllowConjunction || !myAllowDisjunction); if (isSet(flags, JavaCodeFragmentFactory.ALLOW_VOID)) { putUserData(PsiUtil.VALID_VOID_TYPE_IN_CODE_FRAGMENT, Boolean.TRUE); @@ -79,6 +90,9 @@ public class PsiTypeCodeFragmentImpl extends PsiCodeFragmentImpl implements PsiT else if (type instanceof PsiDisjunctionType && !myAllowDisjunction) { throw new TypeSyntaxException("Disjunction not allowed: " + type); } + else if (type instanceof PsiDisjunctionType && !myAllowConjunction) { + throw new TypeSyntaxException("Conjunction not allowed: " + type); + } return type; } diff --git a/java/java-impl/src/com/intellij/refactoring/changeClassSignature/ChangeClassSignatureDialog.java b/java/java-impl/src/com/intellij/refactoring/changeClassSignature/ChangeClassSignatureDialog.java index c4d7d5888805..e6b143497da7 100644 --- a/java/java-impl/src/com/intellij/refactoring/changeClassSignature/ChangeClassSignatureDialog.java +++ b/java/java-impl/src/com/intellij/refactoring/changeClassSignature/ChangeClassSignatureDialog.java @@ -20,6 +20,7 @@ import com.intellij.lang.findUsages.DescriptiveNameUtil; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; import com.intellij.psi.*; +import com.intellij.psi.util.PsiUtil; import com.intellij.refactoring.HelpID; import com.intellij.refactoring.RefactoringBundle; import com.intellij.refactoring.ui.CodeFragmentTableCellRenderer; @@ -109,11 +110,6 @@ public class ChangeClassSignatureDialog extends RefactoringDialog { init(); } - private PsiTypeCodeFragment createValueCodeFragment() { - final JavaCodeFragmentFactory factory = JavaCodeFragmentFactory.getInstance(myProject); - return factory.createTypeCodeFragment("", myClass.getLBrace(), true); - } - protected JComponent createNorthPanel() { return new JLabel(RefactoringBundle.message("changeClassSignature.class.label.text", DescriptiveNameUtil.getDescriptiveName(myClass))); } @@ -240,6 +236,16 @@ public class ChangeClassSignatureDialog extends RefactoringDialog { return null; } + public static PsiTypeCodeFragment createTableCodeFragment(@Nullable PsiClassType type, + @NotNull PsiElement context, + @NotNull JavaCodeFragmentFactory factory, + boolean allowConjunctions) { + return factory.createTypeCodeFragment(type == null ? "" : type.getCanonicalText(), + context, + true, + (allowConjunctions && PsiUtil.isLanguageLevel8OrHigher(context)) ? JavaCodeFragmentFactory.ALLOW_INTERSECTION : 0); + } + private class MyTableModel extends AbstractTableModel implements EditableModel { public int getColumnCount() { return 3; @@ -301,8 +307,10 @@ public class ChangeClassSignatureDialog extends RefactoringDialog { public void addRow() { TableUtil.stopEditing(myTable); myTypeParameterInfos.add(new TypeParameterInfo.New("", null, null)); - myBoundValueTypeCodeFragments.add(createValueCodeFragment()); - myDefaultValueTypeCodeFragments.add(createValueCodeFragment()); + JavaCodeFragmentFactory codeFragmentFactory = JavaCodeFragmentFactory.getInstance(myProject); + PsiElement context = myClass.getLBrace() != null ? myClass.getLBrace() : myClass; + myBoundValueTypeCodeFragments.add(createTableCodeFragment(null, context, codeFragmentFactory, true)); + myDefaultValueTypeCodeFragments.add(createTableCodeFragment(null, context, codeFragmentFactory, false)); final int row = myDefaultValueTypeCodeFragments.size() - 1; fireTableRowsInserted(row, row); } diff --git a/java/java-impl/src/com/intellij/refactoring/changeClassSignature/ChangeClassSignatureProcessor.java b/java/java-impl/src/com/intellij/refactoring/changeClassSignature/ChangeClassSignatureProcessor.java index ed61ae004d15..ea14c94caf0c 100644 --- a/java/java-impl/src/com/intellij/refactoring/changeClassSignature/ChangeClassSignatureProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/changeClassSignature/ChangeClassSignatureProcessor.java @@ -185,7 +185,9 @@ public class ChangeClassSignatureProcessor extends BaseRefactoringProcessor { for (final TypeParameterInfo info : myNewSignature) { newTypeParameters.add(info.getTypeParameter(originalTypeParameters, myProject)); } - ChangeSignatureUtil.synchronizeList(myClass.getTypeParameterList(), newTypeParameters, TypeParameterList.INSTANCE, toRemoveParms); + final PsiTypeParameterList parameterList = myClass.getTypeParameterList(); + ChangeSignatureUtil.synchronizeList(parameterList, newTypeParameters, TypeParameterList.INSTANCE, toRemoveParms); + JavaCodeStyleManager.getInstance(myProject).shortenClassReferences(parameterList); } private boolean[] detectRemovedParameters(final PsiTypeParameter[] original) { diff --git a/java/java-impl/src/com/intellij/refactoring/changeClassSignature/TypeParameterInfo.java b/java/java-impl/src/com/intellij/refactoring/changeClassSignature/TypeParameterInfo.java index 003bc5ce6264..d714fc08ba9b 100644 --- a/java/java-impl/src/com/intellij/refactoring/changeClassSignature/TypeParameterInfo.java +++ b/java/java-impl/src/com/intellij/refactoring/changeClassSignature/TypeParameterInfo.java @@ -22,6 +22,7 @@ import com.intellij.util.IncorrectOperationException; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; /** * @author dsl @@ -42,6 +43,7 @@ public interface TypeParameterInfo { myBoundValue = boundValue != null ? CanonicalTypes.createTypeWrapper(boundValue) : null; } + @TestOnly public New(@NotNull PsiClass aClass, @NotNull @NonNls String name, @NotNull @NonNls String defaultValue, diff --git a/java/java-psi-api/src/com/intellij/psi/JavaCodeFragmentFactory.java b/java/java-psi-api/src/com/intellij/psi/JavaCodeFragmentFactory.java index e6c436207651..250a92248b33 100644 --- a/java/java-psi-api/src/com/intellij/psi/JavaCodeFragmentFactory.java +++ b/java/java-psi-api/src/com/intellij/psi/JavaCodeFragmentFactory.java @@ -67,6 +67,10 @@ public abstract class JavaCodeFragmentFactory { * Flag for {@linkplain #createTypeCodeFragment(String, PsiElement, boolean, int)} - allows disjunctive type. */ public static final int ALLOW_DISJUNCTION = 0x04; + /** + * Flag for {@linkplain #createTypeCodeFragment(String, PsiElement, boolean, int)} - allows conjunctive type. + */ + public static final int ALLOW_INTERSECTION = 0x08; /** * Creates a Java type code fragment from the text of the name of a Java type (the name @@ -98,7 +102,7 @@ public abstract class JavaCodeFragmentFactory { public abstract PsiTypeCodeFragment createTypeCodeFragment(@NotNull String text, @Nullable PsiElement context, boolean isPhysical, - @MagicConstant(flags = {ALLOW_VOID, ALLOW_ELLIPSIS, ALLOW_DISJUNCTION}) int flags); + @MagicConstant(flags = {ALLOW_VOID, ALLOW_ELLIPSIS, ALLOW_DISJUNCTION, ALLOW_INTERSECTION}) int flags); /** * Creates a Java reference code fragment from the text of a Java reference to a diff --git a/java/java-psi-api/src/com/intellij/psi/PsiIntersectionType.java b/java/java-psi-api/src/com/intellij/psi/PsiIntersectionType.java index f46e68012ed9..826ec1efc24f 100644 --- a/java/java-psi-api/src/com/intellij/psi/PsiIntersectionType.java +++ b/java/java-psi-api/src/com/intellij/psi/PsiIntersectionType.java @@ -127,8 +127,13 @@ public class PsiIntersectionType extends PsiType.Stub { @NotNull @Override - public String getCanonicalText(boolean annotated) { - return myConjuncts[0].getCanonicalText(annotated); + public String getCanonicalText(final boolean annotated) { + return StringUtil.join(myConjuncts, new Function() { + @Override + public String fun(PsiType psiType) { + return psiType.getCanonicalText(annotated); + } + }, " & "); } @NotNull diff --git a/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/JavaElementType.java b/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/JavaElementType.java index 8f21b751455a..db39b310f242 100644 --- a/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/JavaElementType.java +++ b/java/java-psi-impl/src/com/intellij/psi/impl/source/tree/JavaElementType.java @@ -240,12 +240,24 @@ public interface JavaElementType { } }; - IElementType TYPE_TEXT = new ICodeFragmentElementType("TYPE_TEXT", JavaLanguage.INSTANCE) { + IElementType TYPE_WITH_DISJUNCTIONS_TEXT = new TypeTextElementType("TYPE_WITH_DISJUNCTIONS_TEXT", ReferenceParser.DISJUNCTIONS); + IElementType TYPE_WITH_CONJUNCTIONS_TEXT = new TypeTextElementType("TYPE_WITH_CONJUNCTIONS_TEXT", ReferenceParser.CONJUNCTIONS); + + class TypeTextElementType extends ICodeFragmentElementType { + private final int myFlags; + + public TypeTextElementType(@NonNls String debugName, int flags) { + super(debugName, JavaLanguage.INSTANCE); + myFlags = flags; + } + private final JavaParserUtil.ParserWrapper myParser = new JavaParserUtil.ParserWrapper() { @Override public void parse(final PsiBuilder builder) { - JavaParser.INSTANCE.getReferenceParser().parseType(builder, ReferenceParser.EAT_LAST_DOT | ReferenceParser.ELLIPSIS | - ReferenceParser.WILDCARD | ReferenceParser.DISJUNCTIONS); + JavaParser.INSTANCE.getReferenceParser().parseType(builder, ReferenceParser.EAT_LAST_DOT | + ReferenceParser.ELLIPSIS | + ReferenceParser.WILDCARD | + myFlags); } }; @@ -254,7 +266,7 @@ public interface JavaElementType { public ASTNode parseContents(final ASTNode chameleon) { return JavaParserUtil.parseFragment(chameleon, myParser); } - }; + } class JavaDummyElementType extends ILazyParseableElementType implements ICompositeElementType { private JavaDummyElementType() { diff --git a/java/java-psi-impl/src/com/intellij/refactoring/util/CanonicalTypes.java b/java/java-psi-impl/src/com/intellij/refactoring/util/CanonicalTypes.java index bb303ccdea64..ccb12db66200 100644 --- a/java/java-psi-impl/src/com/intellij/refactoring/util/CanonicalTypes.java +++ b/java/java-psi-impl/src/com/intellij/refactoring/util/CanonicalTypes.java @@ -246,11 +246,13 @@ public class CanonicalTypes { } } - private static class DisjunctionType extends Type { + private static class LogicalOperationType extends Type { private final List myTypes; + private final boolean myDisjunction; - private DisjunctionType(List types) { + private LogicalOperationType(List types, boolean disjunction) { myTypes = types; + myDisjunction = disjunction; } @NotNull @@ -262,7 +264,7 @@ public class CanonicalTypes { return type.getType(context, manager); } }); - return new PsiDisjunctionType(types, manager); + return myDisjunction ? new PsiDisjunctionType(types, manager) : PsiIntersectionType.createIntersection(types); } @Override @@ -272,7 +274,7 @@ public class CanonicalTypes { public String fun(Type type) { return type.getTypeText(); } - }, "|"); + }, myDisjunction ? "|" : "&"); } @Override @@ -337,7 +339,19 @@ public class CanonicalTypes { return type.accept(Creator.this); } }); - return new DisjunctionType(types); + return new LogicalOperationType(types, true); + } + + @Nullable + @Override + public Type visitIntersectionType(PsiIntersectionType type) { + List types = ContainerUtil.map(type.getConjuncts(), new Function() { + @Override + public Type fun(PsiType type) { + return type.accept(Creator.this); + } + }); + return new LogicalOperationType(types, false); } } diff --git a/java/java-tests/testData/refactoring/changeClassSignature/AddBoundWithIntersection.java b/java/java-tests/testData/refactoring/changeClassSignature/AddBoundWithIntersection.java new file mode 100644 index 000000000000..e86a9a7683f2 --- /dev/null +++ b/java/java-tests/testData/refactoring/changeClassSignature/AddBoundWithIntersection.java @@ -0,0 +1,9 @@ +import java.io.Serializable; + +class C {} + +class Some implements Runnable, Serializable { + void m() { + C c = new C(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/changeClassSignature/AddBoundWithIntersection.java.after b/java/java-tests/testData/refactoring/changeClassSignature/AddBoundWithIntersection.java.after new file mode 100644 index 000000000000..82ffca05cccc --- /dev/null +++ b/java/java-tests/testData/refactoring/changeClassSignature/AddBoundWithIntersection.java.after @@ -0,0 +1,9 @@ +import java.io.Serializable; + +class C {} + +class Some implements Runnable, Serializable { + void m() { + C c = new C(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/changeClassSignature/AddWithBound.java.after b/java/java-tests/testData/refactoring/changeClassSignature/AddWithBound.java.after index 858463d55cb1..ee4f31b28bd4 100644 --- a/java/java-tests/testData/refactoring/changeClassSignature/AddWithBound.java.after +++ b/java/java-tests/testData/refactoring/changeClassSignature/AddWithBound.java.after @@ -1,8 +1,9 @@ +import java.util.Collection; import java.util.List; public class Main { - class B {} + class B {} public void someMethod() { B b = new B(); diff --git a/java/java-tests/testData/refactoring/changeClassSignature/ModifyWithBound.java.after b/java/java-tests/testData/refactoring/changeClassSignature/ModifyWithBound.java.after index f563e3b5cf6d..38e9ddc654a9 100644 --- a/java/java-tests/testData/refactoring/changeClassSignature/ModifyWithBound.java.after +++ b/java/java-tests/testData/refactoring/changeClassSignature/ModifyWithBound.java.after @@ -1,8 +1,9 @@ +import java.util.Collection; import java.util.List; public class Main { - class B {} + class B {} public void someMethod() { B b = new B<>(); diff --git a/java/java-tests/testSrc/com/intellij/refactoring/changeClassSignature/ChangeClassSignatureTest.java b/java/java-tests/testSrc/com/intellij/refactoring/changeClassSignature/ChangeClassSignatureTest.java index 9d72007b82f1..c5b4eb60c974 100644 --- a/java/java-tests/testSrc/com/intellij/refactoring/changeClassSignature/ChangeClassSignatureTest.java +++ b/java/java-tests/testSrc/com/intellij/refactoring/changeClassSignature/ChangeClassSignatureTest.java @@ -2,8 +2,7 @@ package com.intellij.refactoring.changeClassSignature; import com.intellij.JavaTestUtil; import com.intellij.codeInsight.TargetElementUtil; -import com.intellij.psi.PsiClass; -import com.intellij.psi.PsiElement; +import com.intellij.psi.*; import com.intellij.refactoring.LightRefactoringTestCase; import com.intellij.util.IncorrectOperationException; import org.jetbrains.annotations.NonNls; @@ -100,6 +99,19 @@ public class ChangeClassSignatureTest extends LightRefactoringTestCase { }); } + public void testAddBoundWithIntersection() throws Exception { + doTest(aClass -> { + final PsiElementFactory factory = JavaPsiFacade.getElementFactory(aClass.getProject()); + final PsiFile context = aClass.getContainingFile(); + return new TypeParameterInfo[]{ + new TypeParameterInfo.New("T", + factory.createTypeFromText("Some", context), + PsiIntersectionType.createIntersection(factory.createTypeFromText("java.lang.Runnable", context), + factory.createTypeFromText("java.io.Serializable", context))) + }; + }); + } + private void doTest(Function gen) throws Exception { @NonNls final String filePathBefore = getTestName(false) + ".java"; @NonNls final String filePathAfter = getTestName(false) + ".java.after";