diff --git a/java/java-impl/src/com/intellij/refactoring/typeMigration/TypeMigrationLabeler.java b/java/java-impl/src/com/intellij/refactoring/typeMigration/TypeMigrationLabeler.java index ec9dd94b0e43..e286c2bdce19 100644 --- a/java/java-impl/src/com/intellij/refactoring/typeMigration/TypeMigrationLabeler.java +++ b/java/java-impl/src/com/intellij/refactoring/typeMigration/TypeMigrationLabeler.java @@ -45,8 +45,10 @@ import com.intellij.usageView.UsageInfo; import com.intellij.util.*; import com.intellij.util.concurrency.Semaphore; import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.containers.MultiMap; import com.intellij.util.graph.DFSTBuilder; import com.intellij.util.graph.GraphGenerator; +import gnu.trove.THashMap; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; @@ -278,14 +280,20 @@ public class TypeMigrationLabeler { class MigrationProducer { private final Map myRemainConversions; + private final MultiMap myVariableMigration = new MultiMap() { + @NotNull + @Override + protected Map> createMap() { + return new THashMap<>(); + } + }; private MigrationProducer(Map conversions) { myRemainConversions = conversions; } public void change(@NotNull final TypeMigrationUsageInfo usageInfo, - @NotNull Consumer consumer, - @NotNull TypeMigrationLabeler labeler) { + @NotNull Consumer consumer) { final PsiElement element = usageInfo.getElement(); if (element == null) return; final Project project = element.getProject(); @@ -321,6 +329,11 @@ public class TypeMigrationLabeler { } } } + else if ((element instanceof PsiField || element instanceof PsiLocalVariable) && + isMultiVariableDeclaration((PsiVariable)element)) { + final PsiTypeElement typeElement = ((PsiVariable)element).getTypeElement(); + myVariableMigration.putValue(typeElement, usageInfo); + } else { TypeMigrationReplacementUtil.migrateMemberOrVariableType(element, project, getTypeEvaluator().getType(usageInfo)); if (usageInfo instanceof OverridenUsageInfo) { @@ -332,9 +345,55 @@ public class TypeMigrationLabeler { } } + public void flush() { + for (Map.Entry> entry : myVariableMigration.entrySet()) { + final PsiTypeElement typeElement = entry.getKey(); + if (!typeElement.isValid()) continue; + final Collection migrations = entry.getValue(); + if (migrations.size() != 1) { + MultiMap variablesByMigrationType = new MultiMap<>(); + for (TypeMigrationUsageInfo migration : migrations) { + final PsiElement var = migration.getElement(); + if (var == null || !(var instanceof PsiLocalVariable || var instanceof PsiField)) { + continue; + } + final PsiType type = getTypeEvaluator().getType(migration); + variablesByMigrationType.putValue(type, (PsiVariable)var); + } + if (variablesByMigrationType.size() == 1) { + final Map.Entry> migrationTypeAndVariables = + ContainerUtil.getFirstItem(variablesByMigrationType.entrySet()); + LOG.assertTrue(migrationTypeAndVariables != null); + final PsiVariable[] variables = PsiTreeUtil.getChildrenOfType(typeElement.getParent().getParent(), PsiVariable.class); + if (variables != null && variables.length == migrationTypeAndVariables.getValue().size()) { + typeElement + .replace(JavaPsiFacade.getElementFactory(variables[0].getProject()).createTypeElement(migrationTypeAndVariables.getKey())); + continue; + } + } + } + for (TypeMigrationUsageInfo info : entry.getValue()) migrateMultiDeclarationVariable(info); + } + } + + private void migrateMultiDeclarationVariable(TypeMigrationUsageInfo varUsageInfo) { + final PsiElement var = varUsageInfo.getElement(); + if (var == null || !(var instanceof PsiLocalVariable || var instanceof PsiField)) return; + ((PsiVariable) var).normalizeDeclaration(); + TypeMigrationReplacementUtil.migrateMemberOrVariableType(var, var.getProject(), getTypeEvaluator().getType(varUsageInfo)); + } + Object getConversion(UsageInfo info) { return myRemainConversions.remove(info); } + + private boolean isMultiVariableDeclaration(PsiVariable variable) { + final PsiElement parent = variable.getParent(); + LOG.assertTrue(parent != null); + final PsiVariable[] variables = PsiTreeUtil.getChildrenOfType(parent, PsiVariable.class); + LOG.assertTrue(variables != null); + return variables.length != 1; + } } void postProcessNewExpression(@NotNull PsiNewExpression expression) { diff --git a/java/java-impl/src/com/intellij/refactoring/typeMigration/TypeMigrationProcessor.java b/java/java-impl/src/com/intellij/refactoring/typeMigration/TypeMigrationProcessor.java index 3e4ae14f1a70..42129652c07b 100644 --- a/java/java-impl/src/com/intellij/refactoring/typeMigration/TypeMigrationProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/typeMigration/TypeMigrationProcessor.java @@ -228,11 +228,6 @@ public class TypeMigrationProcessor extends BaseRefactoringProcessor { @Override public void performRefactoring(@NotNull UsageInfo[] usages) { - for (PsiElement element : myRoot) { - if (element instanceof PsiVariable && ((PsiVariable)element).getTypeElement() != null) { - ((PsiVariable)element).normalizeDeclaration(); - } - } change(usages, myLabeler, myProject); } @@ -250,8 +245,8 @@ public class TypeMigrationProcessor extends BaseRefactoringProcessor { element instanceof PsiExpression || element instanceof PsiReferenceParameterList) { producer.change((TypeMigrationUsageInfo)usage, - expression -> newExpressionsToCheckDiamonds.add(smartPointerManager.createSmartPsiElementPointer(expression)), - labeler); + expression -> newExpressionsToCheckDiamonds.add(smartPointerManager.createSmartPsiElementPointer(expression)) + ); } else { nonCodeUsages.add(usage); @@ -280,6 +275,8 @@ public class TypeMigrationProcessor extends BaseRefactoringProcessor { } } } + + producer.flush(); } public TypeMigrationLabeler getLabeler() { diff --git a/java/typeMigration/test/com/intellij/refactoring/MigrateTypeSignatureTest.java b/java/typeMigration/test/com/intellij/refactoring/MigrateTypeSignatureTest.java index 464d76bf41cb..a7dfbcbd464b 100644 --- a/java/typeMigration/test/com/intellij/refactoring/MigrateTypeSignatureTest.java +++ b/java/typeMigration/test/com/intellij/refactoring/MigrateTypeSignatureTest.java @@ -527,7 +527,7 @@ public class MigrateTypeSignatureTest extends TypeMigrationTestBase { } @Override - public PsiElement victims(PsiClass aClass) { + public PsiElement victim(PsiClass aClass) { final PsiAnonymousClass anonymousClass = PsiTreeUtil.findChildOfType(aClass, PsiAnonymousClass.class); assertNotNull(anonymousClass); return anonymousClass.findMethodsByName(methodName, false)[0]; diff --git a/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTest.java b/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTest.java index e91b68c1ff1d..cb1c31913433 100644 --- a/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTest.java +++ b/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTest.java @@ -714,7 +714,7 @@ public class TypeMigrationTest extends TypeMigrationTestBase { } @Override - public PsiElement victims(final PsiClass aClass) { + public PsiElement victim(final PsiClass aClass) { final PsiCatchSection catchSection = PsiTreeUtil.findChildOfType(aClass, PsiCatchSection.class); assert catchSection != null : aClass.getText(); final PsiParameter parameter = catchSection.getParameter(); @@ -838,6 +838,14 @@ public class TypeMigrationTest extends TypeMigrationTestBase { doTestFieldType("b", myJavaFacade.getElementFactory().createTypeFromText("Test.Base", null)); } + public void testMultiVarDeclaration1() { + doTestFieldsType("Test", myFactory.createTypeFromText(CommonClassNames.JAVA_LANG_OBJECT, null), "a", "b"); + } + + public void testMultiVarDeclaration2() { + doTestFieldsType("Test", myFactory.createTypeFromText(CommonClassNames.JAVA_LANG_OBJECT, null), "a", "b"); + } + private void doTestReturnType(final String methodName, final String migrationType) { start(new RulesProvider() { @Override @@ -846,7 +854,7 @@ public class TypeMigrationTest extends TypeMigrationTestBase { } @Override - public PsiElement victims(PsiClass aClass) { + public PsiElement victim(PsiClass aClass) { for (PsiMethod method : PsiTreeUtil.findChildrenOfType(aClass, PsiMethod.class)) { if (methodName.equals(method.getName())) { return method; @@ -865,7 +873,7 @@ public class TypeMigrationTest extends TypeMigrationTestBase { } @Override - public PsiElement victims(final PsiClass aClass) { + public PsiElement victim(final PsiClass aClass) { final PsiForeachStatement foreachStatement = PsiTreeUtil.findChildOfType(aClass, PsiForeachStatement.class); assert foreachStatement != null : aClass.getText(); return foreachStatement.getIterationParameter(); diff --git a/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTestBase.java b/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTestBase.java index 229e78c1e34c..b0f5819feb09 100644 --- a/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTestBase.java +++ b/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTestBase.java @@ -30,12 +30,14 @@ import com.intellij.refactoring.typeMigration.TypeMigrationRules; import com.intellij.testFramework.PlatformTestUtil; import com.intellij.usageView.UsageInfo; import com.intellij.util.Functions; +import junit.framework.TestCase; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import java.io.File; import java.io.FileOutputStream; import java.io.PrintWriter; +import java.util.Arrays; /** * @author anna @@ -62,7 +64,7 @@ public abstract class TypeMigrationTestBase extends MultiFileTestCase { } @Override - public PsiElement victims(PsiClass aClass) { + public PsiElement victim(PsiClass aClass) { for (PsiLocalVariable variable : PsiTreeUtil.findChildrenOfType(aClass, PsiLocalVariable.class)) { if (assignmentVariableName.equals(variable.getName())) { final PsiAnonymousClass anonymousClass = PsiTreeUtil.findChildOfType(variable.getInitializer(), PsiAnonymousClass.class); @@ -82,6 +84,10 @@ public abstract class TypeMigrationTestBase extends MultiFileTestCase { } protected void doTestFieldType(@NonNls final String fieldName, String className, final PsiType migrationType) { + doTestFieldsType(className, migrationType, fieldName); + } + + protected void doTestFieldsType(@NotNull String className, @NotNull final PsiType migrationType, final String... fieldNames) { final RulesProvider provider = new RulesProvider() { @Override public PsiType migrationType(PsiElement context) throws Exception { @@ -89,10 +95,12 @@ public abstract class TypeMigrationTestBase extends MultiFileTestCase { } @Override - public PsiElement victims(PsiClass aClass) { - final PsiField field = aClass.findFieldByName(fieldName, false); - assert field != null : fieldName + " not found in " + aClass; - return field; + public PsiElement[] victims(PsiClass aClass) { + return Arrays + .stream(fieldNames) + .map(n -> aClass.findFieldByName(n, false)) + .peek(TestCase::assertNotNull) + .toArray(PsiElement[]::new); } }; @@ -113,7 +121,7 @@ public abstract class TypeMigrationTestBase extends MultiFileTestCase { } @Override - public PsiElement victims(PsiClass aClass) { + public PsiElement victim(PsiClass aClass) { return aClass.findMethodsByName(methodName, false)[0]; } }; @@ -133,7 +141,7 @@ public abstract class TypeMigrationTestBase extends MultiFileTestCase { } @Override - public PsiElement victims(PsiClass aClass) { + public PsiElement victim(PsiClass aClass) { return aClass.findMethodsByName(methodName, false)[0].getParameterList().getParameters()[0]; } }; @@ -159,11 +167,11 @@ public abstract class TypeMigrationTestBase extends MultiFileTestCase { assertNotNull("Class " + className + " not found", aClass); - final PsiElement migrationElement = provider.victims(aClass); - final PsiType migrationType = provider.migrationType(migrationElement); + final PsiElement[] migrationElements = provider.victims(aClass); + final PsiType migrationType = provider.migrationType(migrationElements[0]); final TypeMigrationRules rules = new TypeMigrationRules(); rules.setBoundScope(new LocalSearchScope(aClass.getContainingFile())); - final TestTypeMigrationProcessor pr = new TestTypeMigrationProcessor(getProject(), migrationElement, migrationType, rules); + final TestTypeMigrationProcessor pr = new TestTypeMigrationProcessor(getProject(), migrationElements, migrationType, rules); final UsageInfo[] usages = pr.findUsages(); final String report = pr.getLabeler().getMigrationReport(); @@ -209,12 +217,19 @@ public abstract class TypeMigrationTestBase extends MultiFileTestCase { interface RulesProvider { PsiType migrationType(PsiElement context) throws Exception; - PsiElement victims(PsiClass aClass); + default PsiElement victim(PsiClass aClass) { + fail("You need to override one of victim(PsiClass) or victims(PsiClass) methods"); + return null; + } + + default PsiElement[] victims(PsiClass aClass) { + return new PsiElement[] {victim(aClass)}; + } } private static class TestTypeMigrationProcessor extends TypeMigrationProcessor { - public TestTypeMigrationProcessor(final Project project, final PsiElement root, final PsiType migrationType, final TypeMigrationRules rules) { - super(project, new PsiElement[] {root}, Functions.constant(migrationType), rules); + public TestTypeMigrationProcessor(final Project project, final PsiElement[] roots, final PsiType migrationType, final TypeMigrationRules rules) { + super(project, roots, Functions.constant(migrationType), rules); } } } \ No newline at end of file diff --git a/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration1/after/Test.items b/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration1/after/Test.items new file mode 100644 index 000000000000..96907b1fc305 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration1/after/Test.items @@ -0,0 +1,14 @@ +Types: +PsiField:a : java.lang.Object +PsiField:b : java.lang.Object +PsiLocalVariable:c : java.lang.Object +PsiLocalVariable:d : java.lang.Object +PsiReferenceExpression:a : java.lang.Object +PsiReferenceExpression:b : java.lang.Object + +Conversions: +"" -> $ +"a" -> $ + +New expression type changes: +Fails: diff --git a/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration1/after/test.java b/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration1/after/test.java new file mode 100644 index 000000000000..374f07ea0897 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration1/after/test.java @@ -0,0 +1,7 @@ +public class Test { + Object a = "a", b = ""; + + void mm() { + Object c = a, d = b; + } +} \ No newline at end of file diff --git a/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration1/before/test.java b/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration1/before/test.java new file mode 100644 index 000000000000..396aa1efa669 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration1/before/test.java @@ -0,0 +1,7 @@ +public class Test { + String a = "a", b = ""; + + void mm() { + String c = a, d = b; + } +} \ No newline at end of file diff --git a/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration2/after/Test.items b/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration2/after/Test.items new file mode 100644 index 000000000000..96907b1fc305 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration2/after/Test.items @@ -0,0 +1,14 @@ +Types: +PsiField:a : java.lang.Object +PsiField:b : java.lang.Object +PsiLocalVariable:c : java.lang.Object +PsiLocalVariable:d : java.lang.Object +PsiReferenceExpression:a : java.lang.Object +PsiReferenceExpression:b : java.lang.Object + +Conversions: +"" -> $ +"a" -> $ + +New expression type changes: +Fails: diff --git a/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration2/after/test.java b/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration2/after/test.java new file mode 100644 index 000000000000..480d7191ce82 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration2/after/test.java @@ -0,0 +1,8 @@ +public class Test { + Object a = "a", b = ""; + + void mm() { + Object c = a;Object d = b; + String ololo = "intellij-idea"; + } +} \ No newline at end of file diff --git a/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration2/before/test.java b/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration2/before/test.java new file mode 100644 index 000000000000..5a78d752aaab --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/multiVarDeclaration2/before/test.java @@ -0,0 +1,7 @@ +public class Test { + String a = "a", b = ""; + + void mm() { + String c = a, d = b, ololo = "intellij-idea"; + } +} \ No newline at end of file