type migration: multiple variable declaration in single statement migration IDEA-154937

This commit is contained in:
Dmitry Batkovich
2016-07-15 14:22:44 +03:00
parent 3189889f97
commit 7eaa505a9e
11 changed files with 162 additions and 26 deletions
@@ -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<UsageInfo, Object> myRemainConversions;
private final MultiMap<PsiTypeElement, TypeMigrationUsageInfo> myVariableMigration = new MultiMap<PsiTypeElement, TypeMigrationUsageInfo>() {
@NotNull
@Override
protected Map<PsiTypeElement, Collection<TypeMigrationUsageInfo>> createMap() {
return new THashMap<>();
}
};
private MigrationProducer(Map<UsageInfo, Object> conversions) {
myRemainConversions = conversions;
}
public void change(@NotNull final TypeMigrationUsageInfo usageInfo,
@NotNull Consumer<PsiNewExpression> consumer,
@NotNull TypeMigrationLabeler labeler) {
@NotNull Consumer<PsiNewExpression> 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<PsiTypeElement, Collection<TypeMigrationUsageInfo>> entry : myVariableMigration.entrySet()) {
final PsiTypeElement typeElement = entry.getKey();
if (!typeElement.isValid()) continue;
final Collection<TypeMigrationUsageInfo> migrations = entry.getValue();
if (migrations.size() != 1) {
MultiMap<PsiType, PsiVariable> 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<PsiType, Collection<PsiVariable>> 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) {
@@ -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() {
@@ -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];
@@ -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<java.lang.String>", 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();
@@ -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.<PsiElement, PsiType>constant(migrationType), rules);
public TestTypeMigrationProcessor(final Project project, final PsiElement[] roots, final PsiType migrationType, final TypeMigrationRules rules) {
super(project, roots, Functions.<PsiElement, PsiType>constant(migrationType), rules);
}
}
}
@@ -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:
@@ -0,0 +1,7 @@
public class Test {
Object a = "a", b = "";
void mm() {
Object c = a, d = b;
}
}
@@ -0,0 +1,7 @@
public class Test {
String a = "a", b = "";
void mm() {
String c = a, d = b;
}
}
@@ -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:
@@ -0,0 +1,8 @@
public class Test {
Object a = "a", b = "";
void mm() {
Object c = a;Object d = b;
String ololo = "intellij-idea";
}
}
@@ -0,0 +1,7 @@
public class Test {
String a = "a", b = "";
void mm() {
String c = a, d = b, ololo = "intellij-idea";
}
}