From abfb0af852ec48e1ef1ccf530ed052ebbf3209f4 Mon Sep 17 00:00:00 2001 From: anna Date: Fri, 21 May 2010 13:53:13 +0400 Subject: [PATCH] safe delete: enable on local variables; treat side effects as conflicts ( IDEA-17366 ) --- .../java/JavaRefactoringSupportProvider.java | 2 +- .../safeDelete/JavaSafeDeleteProcessor.java | 27 +++++++++++++------ .../safeDelete/localVariable/after/Super.java | 5 ++++ .../localVariable/before/Super.java | 8 ++++++ .../localVariableSideEffect/after/Super.java | 5 ++++ .../localVariableSideEffect/before/Super.java | 9 +++++++ .../intellij/refactoring/SafeDeleteTest.java | 20 ++++++++++++-- 7 files changed, 65 insertions(+), 11 deletions(-) create mode 100644 java/java-tests/testData/refactoring/safeDelete/localVariable/after/Super.java create mode 100644 java/java-tests/testData/refactoring/safeDelete/localVariable/before/Super.java create mode 100644 java/java-tests/testData/refactoring/safeDelete/localVariableSideEffect/after/Super.java create mode 100644 java/java-tests/testData/refactoring/safeDelete/localVariableSideEffect/before/Super.java diff --git a/java/java-impl/src/com/intellij/lang/java/JavaRefactoringSupportProvider.java b/java/java-impl/src/com/intellij/lang/java/JavaRefactoringSupportProvider.java index 81728e746576..edbc348f41fc 100644 --- a/java/java-impl/src/com/intellij/lang/java/JavaRefactoringSupportProvider.java +++ b/java/java-impl/src/com/intellij/lang/java/JavaRefactoringSupportProvider.java @@ -40,7 +40,7 @@ public class JavaRefactoringSupportProvider extends DefaultRefactoringSupportPro public boolean isSafeDeleteAvailable(PsiElement element) { return element instanceof PsiClass || element instanceof PsiMethod || element instanceof PsiField || (element instanceof PsiParameter && ((PsiParameter)element).getDeclarationScope() instanceof PsiMethod) || - element instanceof PsiPackage; + element instanceof PsiPackage || element instanceof PsiLocalVariable; } public RefactoringActionHandler getIntroduceConstantHandler() { diff --git a/java/java-impl/src/com/intellij/refactoring/safeDelete/JavaSafeDeleteProcessor.java b/java/java-impl/src/com/intellij/refactoring/safeDelete/JavaSafeDeleteProcessor.java index 2173e1901a91..ff55de638015 100644 --- a/java/java-impl/src/com/intellij/refactoring/safeDelete/JavaSafeDeleteProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/safeDelete/JavaSafeDeleteProcessor.java @@ -15,6 +15,7 @@ */ package com.intellij.refactoring.safeDelete; +import com.intellij.codeInsight.daemon.impl.quickfix.RemoveUnusedVariableFix; import com.intellij.ide.util.SuperMethodWarningUtil; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.diagnostic.Logger; @@ -30,10 +31,7 @@ import com.intellij.psi.javadoc.PsiDocTag; import com.intellij.psi.search.searches.OverridingMethodsSearch; import com.intellij.psi.search.searches.ReferencesSearch; import com.intellij.psi.search.searches.SuperMethodsSearch; -import com.intellij.psi.util.MethodSignatureBackedByPsiMethod; -import com.intellij.psi.util.MethodSignatureUtil; -import com.intellij.psi.util.PropertyUtil; -import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.*; import com.intellij.refactoring.RefactoringBundle; import com.intellij.refactoring.safeDelete.usageInfo.*; import com.intellij.refactoring.util.RefactoringMessageUtil; @@ -53,7 +51,7 @@ public class JavaSafeDeleteProcessor implements SafeDeleteProcessorDelegate { public boolean handlesElement(final PsiElement element) { return element instanceof PsiClass || element instanceof PsiMethod || - element instanceof PsiField || element instanceof PsiParameter; + element instanceof PsiField || element instanceof PsiParameter || element instanceof PsiLocalVariable; } @Nullable @@ -75,6 +73,20 @@ public class JavaSafeDeleteProcessor implements SafeDeleteProcessorDelegate { LOG.assertTrue(((PsiParameter) element).getDeclarationScope() instanceof PsiMethod); findParameterUsages((PsiParameter)element, usages); } + else if (element instanceof PsiLocalVariable) { + for (PsiReference reference : ReferencesSearch.search(element)) { + PsiReferenceExpression referencedElement = (PsiReferenceExpression)reference.getElement(); + final PsiStatement statement = PsiTreeUtil.getParentOfType(referencedElement, PsiStatement.class); + + boolean isSafeToDelete = PsiUtil.isAccessedForWriting(referencedElement); + boolean hasSideEffects = false; + if (PsiUtil.isOnAssignmentLeftHand(referencedElement)) { + hasSideEffects = + RemoveUnusedVariableFix.checkSideEffects(((PsiAssignmentExpression)referencedElement.getParent()).getRExpression(), ((PsiLocalVariable)element), new ArrayList()); + } + usages.add(new SafeDeleteReferenceJavaDeleteUsageInfo(statement, element, isSafeToDelete && !hasSideEffects)); + } + } return new NonCodeUsageSearchInfo(insideDeletedCondition, element); } @@ -468,7 +480,6 @@ public class JavaSafeDeleteProcessor implements SafeDeleteProcessorDelegate { @Nullable private static PsiMethod getOverridingConstructorOfSuperCall(final PsiElement element) { - PsiMethod overridingConstructor = null; if(element instanceof PsiReferenceExpression && "super".equals(element.getText())) { PsiElement parent = element.getParent(); if(parent instanceof PsiMethodCallExpression) { @@ -478,13 +489,13 @@ public class JavaSafeDeleteProcessor implements SafeDeleteProcessorDelegate { if(parent instanceof PsiCodeBlock) { parent = parent.getParent(); if(parent instanceof PsiMethod && ((PsiMethod) parent).isConstructor()) { - overridingConstructor = (PsiMethod) parent; + return (PsiMethod) parent; } } } } } - return overridingConstructor; + return null; } private static boolean canBePrivate(PsiMethod method, Collection references, Collection deleted, diff --git a/java/java-tests/testData/refactoring/safeDelete/localVariable/after/Super.java b/java/java-tests/testData/refactoring/safeDelete/localVariable/after/Super.java new file mode 100644 index 000000000000..91041682ab61 --- /dev/null +++ b/java/java-tests/testData/refactoring/safeDelete/localVariable/after/Super.java @@ -0,0 +1,5 @@ +class Super { + void foo() { + } + +} diff --git a/java/java-tests/testData/refactoring/safeDelete/localVariable/before/Super.java b/java/java-tests/testData/refactoring/safeDelete/localVariable/before/Super.java new file mode 100644 index 000000000000..64296e259814 --- /dev/null +++ b/java/java-tests/testData/refactoring/safeDelete/localVariable/before/Super.java @@ -0,0 +1,8 @@ +class Super { + void foo() { + int varName = 0; + varName++; + varName = 10; + } + +} diff --git a/java/java-tests/testData/refactoring/safeDelete/localVariableSideEffect/after/Super.java b/java/java-tests/testData/refactoring/safeDelete/localVariableSideEffect/after/Super.java new file mode 100644 index 000000000000..91041682ab61 --- /dev/null +++ b/java/java-tests/testData/refactoring/safeDelete/localVariableSideEffect/after/Super.java @@ -0,0 +1,5 @@ +class Super { + void foo() { + } + +} diff --git a/java/java-tests/testData/refactoring/safeDelete/localVariableSideEffect/before/Super.java b/java/java-tests/testData/refactoring/safeDelete/localVariableSideEffect/before/Super.java new file mode 100644 index 000000000000..f47b1446360b --- /dev/null +++ b/java/java-tests/testData/refactoring/safeDelete/localVariableSideEffect/before/Super.java @@ -0,0 +1,9 @@ +class Super { + void foo() { + int varName = 0; + varName++; + varName = bar(); + } + int bar() {return 0;} + +} diff --git a/java/java-tests/testSrc/com/intellij/refactoring/SafeDeleteTest.java b/java/java-tests/testSrc/com/intellij/refactoring/SafeDeleteTest.java index dcb36ec5b878..87d91cc1a793 100644 --- a/java/java-tests/testSrc/com/intellij/refactoring/SafeDeleteTest.java +++ b/java/java-tests/testSrc/com/intellij/refactoring/SafeDeleteTest.java @@ -2,7 +2,6 @@ package com.intellij.refactoring; import com.intellij.JavaTestUtil; import com.intellij.codeInsight.TargetElementUtilBase; -import com.intellij.idea.Bombed; import com.intellij.openapi.roots.ProjectRootManager; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.PsiClass; @@ -13,7 +12,6 @@ import com.intellij.testFramework.IdeaTestUtil; import org.jetbrains.annotations.NonNls; import java.io.File; -import java.util.Calendar; public class SafeDeleteTest extends MultiFileTestCase { private VirtualFile myRootBefore; @@ -79,6 +77,24 @@ public class SafeDeleteTest extends MultiFileTestCase { } } + public void testLocalVariable() throws Exception { + myDoCompare = false; + doTest("Super"); + } + + public void testLocalVariableSideEffect() throws Exception { + myDoCompare = false; + try { + doTest("Super"); + fail("Side effect was ignored"); + } + catch (BaseRefactoringProcessor.ConflictsInTestsException e) { + String message = e.getMessage(); + assertTrue(message, message.startsWith("local variable varName has 1 usage that is not safe to delete.\n" + + "Of those 0 usages are in strings, comments, or non-Java files.")); + } + } + private void doTest(@NonNls final String qClassName) throws Exception { doTest(new PerformAction() { public void performAction(VirtualFile rootDir, VirtualFile rootAfter) throws Exception {