From 63e73c1fe8f1de681bbafe5725d1e0f78ddb4753 Mon Sep 17 00:00:00 2001 From: Dmitry Batkovich Date: Mon, 15 Aug 2016 18:55:16 +0300 Subject: [PATCH] type migration: to void: remove unused variables in simple cases IDEA-158196 --- .../impl/SplitDeclarationAction.java | 10 ++- .../TypeMigrationStatementProcessor.java | 78 +++++++++++++++++-- .../refactoring/TypeMigrationTest.java | 16 ++++ .../after/Test.items | 10 +++ .../voidMigrationInAssignment/after/test.java | 11 +++ .../before/test.java | 12 +++ .../after/Test.items | 10 +++ .../after/test.java | 12 +++ .../before/test.java | 13 ++++ .../voidMigrationInVarDecl/after/Test.items | 10 +++ .../voidMigrationInVarDecl/after/test.java | 12 +++ .../voidMigrationInVarDecl/before/test.java | 12 +++ .../after/Test.items | 10 +++ .../after/test.java | 10 +++ .../before/test.java | 11 +++ 15 files changed, 225 insertions(+), 12 deletions(-) create mode 100644 java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignment/after/Test.items create mode 100644 java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignment/after/test.java create mode 100644 java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignment/before/test.java create mode 100644 java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignmentFailed/after/Test.items create mode 100644 java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignmentFailed/after/test.java create mode 100644 java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignmentFailed/before/test.java create mode 100644 java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDecl/after/Test.items create mode 100644 java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDecl/after/test.java create mode 100644 java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDecl/before/test.java create mode 100644 java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDeclFailed/after/Test.items create mode 100644 java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDeclFailed/after/test.java create mode 100644 java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDeclFailed/before/test.java diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/SplitDeclarationAction.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/SplitDeclarationAction.java index 9ea64d81463b..b418e0a7c54b 100644 --- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/SplitDeclarationAction.java +++ b/java/java-impl/src/com/intellij/codeInsight/intention/impl/SplitDeclarationAction.java @@ -110,8 +110,8 @@ public class SplitDeclarationAction extends PsiElementBaseIntentionAction { } } - private static void invokeOnDeclarationStatement(PsiDeclarationStatement decl, PsiManager psiManager, - Project project) throws IncorrectOperationException { + public static PsiAssignmentExpression invokeOnDeclarationStatement(PsiDeclarationStatement decl, PsiManager psiManager, + Project project) throws IncorrectOperationException { if (decl.getDeclaredElements().length == 1) { PsiLocalVariable var = (PsiLocalVariable)decl.getDeclaredElements()[0]; var.normalizeDeclaration(); @@ -140,7 +140,7 @@ public class SplitDeclarationAction extends PsiElementBaseIntentionAction { } final PsiElement parent = block.getParent(); - decl.replace(statement); + final PsiAssignmentExpression replaced = (PsiAssignmentExpression)decl.replace(statement); if (!(parent instanceof PsiCodeBlock)) { final PsiBlockStatement blockStatement = (PsiBlockStatement)JavaPsiFacade.getElementFactory(project).createStatementFromText("{}", null); @@ -152,13 +152,15 @@ public class SplitDeclarationAction extends PsiElementBaseIntentionAction { else { parent.addBefore(varDeclStatement, block); } + return replaced; } else { - block.addAfter(statement, decl); + return (PsiAssignmentExpression)((PsiExpressionStatement)block.addAfter(statement, decl)).getExpression(); } } else { ((PsiLocalVariable)decl.getDeclaredElements()[0]).normalizeDeclaration(); } + return null; } } diff --git a/java/java-impl/src/com/intellij/refactoring/typeMigration/TypeMigrationStatementProcessor.java b/java/java-impl/src/com/intellij/refactoring/typeMigration/TypeMigrationStatementProcessor.java index 578799603aaf..36ac3bda1952 100644 --- a/java/java-impl/src/com/intellij/refactoring/typeMigration/TypeMigrationStatementProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/typeMigration/TypeMigrationStatementProcessor.java @@ -16,15 +16,20 @@ package com.intellij.refactoring.typeMigration; import com.intellij.codeInsight.generation.GetterSetterPrototypeProvider; +import com.intellij.codeInsight.intention.impl.SplitDeclarationAction; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Pair; import com.intellij.psi.*; +import com.intellij.psi.controlFlow.DefUseUtil; import com.intellij.psi.impl.PsiSubstitutorImpl; import com.intellij.psi.search.GlobalSearchScope; +import com.intellij.psi.search.searches.ReferencesSearch; import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.*; import com.intellij.refactoring.typeMigration.usageInfo.TypeMigrationUsageInfo; +import com.intellij.util.CommonProcessors; +import com.intellij.util.IncorrectOperationException; import com.intellij.util.containers.HashMap; import org.jetbrains.annotations.NotNull; @@ -85,7 +90,13 @@ class TypeMigrationStatementProcessor extends JavaRecursiveElementVisitor { break; case TypeInfection.RIGHT_INFECTED: - myLabeler.migrateExpressionType(lExpression, rtype, myStatement, TypeConversionUtil.isAssignable(ltype, rtype), false); + if (lExpression instanceof PsiReferenceExpression && + ((PsiReferenceExpression)lExpression).resolve() instanceof PsiLocalVariable && + !canBeVariableType(rtype)) { + tryToRemoveLocalVariableAssignment((PsiLocalVariable)((PsiReferenceExpression)lExpression).resolve(), rExpression, rtype); + } else { + myLabeler.migrateExpressionType(lExpression, rtype, myStatement, TypeConversionUtil.isAssignable(ltype, rtype), false); + } break; case TypeInfection.BOTH_INFECTED: @@ -515,14 +526,27 @@ class TypeMigrationStatementProcessor extends JavaRecursiveElementVisitor { case TypeInfection.RIGHT_INFECTED: PsiType psiType = migrationType != null ? migrationType : right.getType(); - if (psiType != null && declarationType != null && - !myLabeler.addMigrationRoot(variable, psiType, myStatement, TypeConversionUtil.isAssignable(declarationType, psiType), true) && - !TypeConversionUtil.isAssignable(left.getType(), psiType)) { - PsiType initialType = left.getType(); - if (initialType instanceof PsiEllipsisType) { - initialType = ((PsiEllipsisType)initialType).getComponentType(); + if (psiType != null) { + if (canBeVariableType(psiType)) { + if (declarationType != null && + !myLabeler.addMigrationRoot(variable, psiType, myStatement, TypeConversionUtil.isAssignable(declarationType, psiType), true) && + !TypeConversionUtil.isAssignable(left.getType(), psiType)) { + PsiType initialType = left.getType(); + if (initialType instanceof PsiEllipsisType) { + initialType = ((PsiEllipsisType)initialType).getComponentType(); + } + myLabeler.convertExpression(value, psiType, initialType, isCovariantPosition); + } + } + else { + if (variable instanceof PsiLocalVariable) { + final PsiDeclarationStatement decl = PsiTreeUtil.getParentOfType(variable, PsiDeclarationStatement.class); + if (decl != null && decl.getDeclaredElements().length == 1) { + tryToRemoveLocalVariableAssignment((PsiLocalVariable)variable, value, psiType); + } + break; + } } - myLabeler.convertExpression(value, psiType, initialType, isCovariantPosition); } break; @@ -535,6 +559,44 @@ class TypeMigrationStatementProcessor extends JavaRecursiveElementVisitor { } } + private void tryToRemoveLocalVariableAssignment(@NotNull PsiLocalVariable variable, @NotNull PsiExpression valueExpression, @NotNull PsiType migrationType) { + final PsiCodeBlock codeBlock = PsiTreeUtil.getParentOfType(variable, PsiCodeBlock.class); + final PsiElement[] refs = DefUseUtil.getRefs(codeBlock, variable, valueExpression); + if (refs.length == 0) { + myLabeler.setConversionMapping(valueExpression, new TypeConversionDescriptorBase() { + @Override + public PsiExpression replace(PsiExpression expression, @NotNull TypeEvaluator evaluator) throws IncorrectOperationException { + final PsiElement parent = expression.getParent(); + if (parent instanceof PsiLocalVariable) { + final PsiLocalVariable var = (PsiLocalVariable)parent; + final PsiDeclarationStatement decl = PsiTreeUtil.getParentOfType(var, PsiDeclarationStatement.class); + if (decl == null) return null; + final Project project = var.getProject(); + final PsiAssignmentExpression assignment = + SplitDeclarationAction.invokeOnDeclarationStatement(decl, PsiManager.getInstance(project), project); + final PsiExpression rExpression = assignment.getRExpression(); + if (rExpression == null) return null; + assignment.replace(rExpression); + if (ReferencesSearch.search(var, var.getUseScope()).forEach(new CommonProcessors.FindFirstProcessor<>())) { + var.delete(); + } + } + else if (parent instanceof PsiAssignmentExpression) { + final PsiExpression rExpression = ((PsiAssignmentExpression)parent).getRExpression(); + return rExpression == null ? null : (PsiExpression)parent.replace(rExpression); + } + return null; + } + }); + } else { + myLabeler.markFailedConversion(Pair.pair(null, migrationType), valueExpression); + } + } + + private static boolean canBeVariableType(@NotNull PsiType type) { + return !type.getDeepComponentType().equals(PsiType.VOID); + } + private static PsiType adjustMigrationTypeIfGenericArrayCreation(PsiType migrationType, PsiExpression expression) { if (expression instanceof PsiNewExpression) { if (migrationType instanceof PsiArrayType) { diff --git a/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTest.java b/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTest.java index cb1c31913433..ad7548e1e444 100644 --- a/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTest.java +++ b/java/typeMigration/test/com/intellij/refactoring/TypeMigrationTest.java @@ -846,6 +846,22 @@ public class TypeMigrationTest extends TypeMigrationTestBase { doTestFieldsType("Test", myFactory.createTypeFromText(CommonClassNames.JAVA_LANG_OBJECT, null), "a", "b"); } + public void testVoidMigrationInVarDecl() { + doTestMethodType("migrationMethod", PsiType.VOID); + } + + public void testVoidMigrationInVarDeclFailed() { + doTestMethodType("migrationMethod", PsiType.VOID); + } + + public void testVoidMigrationInAssignment() { + doTestMethodType("migrationMethod", PsiType.VOID); + } + + public void testVoidMigrationInAssignmentFailed() { + doTestMethodType("migrationMethod", PsiType.VOID); + } + private void doTestReturnType(final String methodName, final String migrationType) { start(new RulesProvider() { @Override diff --git a/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignment/after/Test.items b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignment/after/Test.items new file mode 100644 index 000000000000..b00e81eb2f02 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignment/after/Test.items @@ -0,0 +1,10 @@ +Types: +PsiMethod:migrationMethod : void +PsiMethodCallExpression:migrationMethod() : void + +Conversions: +"" -> $ +migrationMethod() -> $ + +New expression type changes: +Fails: diff --git a/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignment/after/test.java b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignment/after/test.java new file mode 100644 index 000000000000..0bc3bea2e708 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignment/after/test.java @@ -0,0 +1,11 @@ +class Test { + + static void migrationMethod() { + } + + void m() { + String sss; + migrationMethod(); + sss = ""; + } +} \ No newline at end of file diff --git a/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignment/before/test.java b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignment/before/test.java new file mode 100644 index 000000000000..daa00c099db9 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignment/before/test.java @@ -0,0 +1,12 @@ +class Test { + + static String migrationMethod() { + return ""; + } + + void m() { + String sss; + sss = migrationMethod(); + sss = ""; + } +} \ No newline at end of file diff --git a/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignmentFailed/after/Test.items b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignmentFailed/after/Test.items new file mode 100644 index 000000000000..aa24a2d30913 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignmentFailed/after/Test.items @@ -0,0 +1,10 @@ +Types: +PsiMethod:migrationMethod : void +PsiMethodCallExpression:migrationMethod() : void + +Conversions: +"" -> $ + +New expression type changes: +Fails: +migrationMethod()->void diff --git a/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignmentFailed/after/test.java b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignmentFailed/after/test.java new file mode 100644 index 000000000000..54100fb8253e --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignmentFailed/after/test.java @@ -0,0 +1,12 @@ +class Test { + + static void migrationMethod() { + } + + void m() { + String sss; + sss = migrationMethod(); + System.out.println(sss); + sss = ""; + } +} \ No newline at end of file diff --git a/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignmentFailed/before/test.java b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignmentFailed/before/test.java new file mode 100644 index 000000000000..c0c37988011c --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInAssignmentFailed/before/test.java @@ -0,0 +1,13 @@ +class Test { + + static String migrationMethod() { + return ""; + } + + void m() { + String sss; + sss = migrationMethod(); + System.out.println(sss); + sss = ""; + } +} \ No newline at end of file diff --git a/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDecl/after/Test.items b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDecl/after/Test.items new file mode 100644 index 000000000000..b00e81eb2f02 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDecl/after/Test.items @@ -0,0 +1,10 @@ +Types: +PsiMethod:migrationMethod : void +PsiMethodCallExpression:migrationMethod() : void + +Conversions: +"" -> $ +migrationMethod() -> $ + +New expression type changes: +Fails: diff --git a/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDecl/after/test.java b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDecl/after/test.java new file mode 100644 index 000000000000..d85fea5f7522 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDecl/after/test.java @@ -0,0 +1,12 @@ +class Test { + + static void migrationMethod() { + } + + void m() { + String sss; + migrationMethod(); + sss = ""; + System.out.println(sss); + } +} \ No newline at end of file diff --git a/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDecl/before/test.java b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDecl/before/test.java new file mode 100644 index 000000000000..8a92d5bb1d20 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDecl/before/test.java @@ -0,0 +1,12 @@ +class Test { + + static String migrationMethod() { + return ""; + } + + void m() { + String sss = migrationMethod(); + sss = ""; + System.out.println(sss); + } +} \ No newline at end of file diff --git a/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDeclFailed/after/Test.items b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDeclFailed/after/Test.items new file mode 100644 index 000000000000..aa24a2d30913 --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDeclFailed/after/Test.items @@ -0,0 +1,10 @@ +Types: +PsiMethod:migrationMethod : void +PsiMethodCallExpression:migrationMethod() : void + +Conversions: +"" -> $ + +New expression type changes: +Fails: +migrationMethod()->void diff --git a/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDeclFailed/after/test.java b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDeclFailed/after/test.java new file mode 100644 index 000000000000..749d889055ed --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDeclFailed/after/test.java @@ -0,0 +1,10 @@ +class Test { + + static void migrationMethod() { + } + + void m() { + String sss = migrationMethod(); + System.out.println(sss); + } +} \ No newline at end of file diff --git a/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDeclFailed/before/test.java b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDeclFailed/before/test.java new file mode 100644 index 000000000000..23965754cf5a --- /dev/null +++ b/java/typeMigration/testData/refactoring/typeMigration/voidMigrationInVarDeclFailed/before/test.java @@ -0,0 +1,11 @@ +class Test { + + static String migrationMethod() { + return ""; + } + + void m() { + String sss = migrationMethod(); + System.out.println(sss); + } +} \ No newline at end of file