From 7f0f1aaffa616c606ab5154cb1eac15a6fb7b4e1 Mon Sep 17 00:00:00 2001 From: anna Date: Mon, 22 Jul 2013 13:43:08 +0200 Subject: [PATCH] safe delete: delete @Override annotation each time super method is deleted; rise a conflict when a usage is in overriding method which won't be deleted (IDEA-110840; IDEA-110841) --- .../safeDelete/JavaSafeDeleteProcessor.java | 25 ++++++----- .../SafeDeleteOverrideAnnotation.java | 43 +++++++++++++++++++ .../usageInfo/SafeDeletePrivatizeMethod.java | 13 +----- .../overrideAnnotation/after/Super.java | 13 ++++++ .../overrideAnnotation/before/Super.java | 15 +++++++ .../safeDelete/superCall/after/Super.java | 13 ++++++ .../safeDelete/superCall/before/Super.java | 14 ++++++ .../intellij/refactoring/SafeDeleteTest.java | 18 ++++++++ 8 files changed, 130 insertions(+), 24 deletions(-) create mode 100644 java/java-impl/src/com/intellij/refactoring/safeDelete/usageInfo/SafeDeleteOverrideAnnotation.java create mode 100644 java/java-tests/testData/refactoring/safeDelete/overrideAnnotation/after/Super.java create mode 100644 java/java-tests/testData/refactoring/safeDelete/overrideAnnotation/before/Super.java create mode 100644 java/java-tests/testData/refactoring/safeDelete/superCall/after/Super.java create mode 100644 java/java-tests/testData/refactoring/safeDelete/superCall/before/Super.java 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 ac7a6d9ece69..46b604e8d0a7 100644 --- a/java/java-impl/src/com/intellij/refactoring/safeDelete/JavaSafeDeleteProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/safeDelete/JavaSafeDeleteProcessor.java @@ -457,13 +457,6 @@ public class JavaSafeDeleteProcessor extends SafeDeleteProcessorDelegateBase { removeDeletedMethods(OverridingMethodsSearch.search(psiMethod, true).toArray(PsiMethod.EMPTY_ARRAY), allElementsToDelete); - for (PsiReference reference : references) { - final PsiElement element = reference.getElement(); - if (!isInside(element, allElementsToDelete) && !isInside(element, overridingMethods)) { - usages.add(new SafeDeleteReferenceJavaDeleteUsageInfo(element, psiMethod, PsiTreeUtil.getParentOfType(element, PsiImportStaticStatement.class) != null)); - } - } - final HashMap> methodToReferences = new HashMap>(); for (PsiMethod overridingMethod : overridingMethods) { final Collection overridingReferences = ReferencesSearch.search(overridingMethod).findAll(); @@ -472,6 +465,12 @@ public class JavaSafeDeleteProcessor extends SafeDeleteProcessorDelegateBase { final Set validOverriding = validateOverridingMethods(psiMethod, references, Arrays.asList(overridingMethods), methodToReferences, usages, allElementsToDelete); + for (PsiReference reference : references) { + final PsiElement element = reference.getElement(); + if (!isInside(element, allElementsToDelete) && !isInside(element, validOverriding)) { + usages.add(new SafeDeleteReferenceJavaDeleteUsageInfo(element, psiMethod, PsiTreeUtil.getParentOfType(element, PsiImportStaticStatement.class) != null)); + } + } return new Condition() { public boolean value(PsiElement usage) { if(usage instanceof PsiFile) return false; @@ -594,12 +593,12 @@ public class JavaSafeDeleteProcessor extends SafeDeleteProcessorDelegateBase { } for (PsiMethod method : overridingMethods) { - if (!validOverriding.contains(method) && !multipleInterfaceImplementations.contains(method)) { - final boolean methodCanBePrivate = - canBePrivate(method, methodToReferences.get(method), validOverriding, allElementsToDelete); - if (methodCanBePrivate) { - usages.add(new SafeDeletePrivatizeMethod(method, originalMethod)); - } + if (!validOverriding.contains(method) && + !multipleInterfaceImplementations.contains(method) && + canBePrivate(method, methodToReferences.get(method), validOverriding, allElementsToDelete)) { + usages.add(new SafeDeletePrivatizeMethod(method, originalMethod)); + } else { + usages.add(new SafeDeleteOverrideAnnotation(method, originalMethod)); } } return validOverriding; diff --git a/java/java-impl/src/com/intellij/refactoring/safeDelete/usageInfo/SafeDeleteOverrideAnnotation.java b/java/java-impl/src/com/intellij/refactoring/safeDelete/usageInfo/SafeDeleteOverrideAnnotation.java new file mode 100644 index 000000000000..9a3159564ae6 --- /dev/null +++ b/java/java-impl/src/com/intellij/refactoring/safeDelete/usageInfo/SafeDeleteOverrideAnnotation.java @@ -0,0 +1,43 @@ +/* + * Copyright 2000-2013 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.refactoring.safeDelete.usageInfo; + +import com.intellij.codeInsight.AnnotationUtil; +import com.intellij.psi.PsiAnnotation; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiMethod; +import com.intellij.util.IncorrectOperationException; + +/** + * User: anna + * Date: 7/22/13 + */ +public class SafeDeleteOverrideAnnotation extends SafeDeleteUsageInfo implements SafeDeleteCustomUsageInfo { + public SafeDeleteOverrideAnnotation(PsiElement element, PsiElement referencedElement) { + super(element, referencedElement); + } + + public PsiMethod getMethod() { + return (PsiMethod)getElement(); + } + + public void performRefactoring() throws IncorrectOperationException { + final PsiAnnotation annotation = AnnotationUtil.findAnnotation(getMethod(), true, Override.class.getName()); + if (annotation != null) { + annotation.delete(); + } + } +} diff --git a/java/java-impl/src/com/intellij/refactoring/safeDelete/usageInfo/SafeDeletePrivatizeMethod.java b/java/java-impl/src/com/intellij/refactoring/safeDelete/usageInfo/SafeDeletePrivatizeMethod.java index 0e0e70fc8837..70a6b44ce033 100644 --- a/java/java-impl/src/com/intellij/refactoring/safeDelete/usageInfo/SafeDeletePrivatizeMethod.java +++ b/java/java-impl/src/com/intellij/refactoring/safeDelete/usageInfo/SafeDeletePrivatizeMethod.java @@ -15,8 +15,6 @@ */ package com.intellij.refactoring.safeDelete.usageInfo; -import com.intellij.codeInsight.AnnotationUtil; -import com.intellij.psi.PsiAnnotation; import com.intellij.psi.PsiMethod; import com.intellij.psi.PsiModifier; import com.intellij.psi.util.PsiUtil; @@ -25,20 +23,13 @@ import com.intellij.util.IncorrectOperationException; /** * @author dsl */ -public class SafeDeletePrivatizeMethod extends SafeDeleteUsageInfo implements SafeDeleteCustomUsageInfo { +public class SafeDeletePrivatizeMethod extends SafeDeleteOverrideAnnotation { public SafeDeletePrivatizeMethod(PsiMethod method, PsiMethod overridenMethod) { super(method, overridenMethod); } - public PsiMethod getMethod() { - return (PsiMethod) getElement(); - } - public void performRefactoring() throws IncorrectOperationException { PsiUtil.setModifierProperty(getMethod(), PsiModifier.PRIVATE, true); - final PsiAnnotation annotation = AnnotationUtil.findAnnotation(getMethod(), true, Override.class.getName()); - if (annotation != null) { - annotation.delete(); - } + super.performRefactoring(); } } diff --git a/java/java-tests/testData/refactoring/safeDelete/overrideAnnotation/after/Super.java b/java/java-tests/testData/refactoring/safeDelete/overrideAnnotation/after/Super.java new file mode 100644 index 000000000000..10bb0c89efb6 --- /dev/null +++ b/java/java-tests/testData/refactoring/safeDelete/overrideAnnotation/after/Super.java @@ -0,0 +1,13 @@ +class Super { +} + +class Child extends Super { + void foo() { + } +} + +class Usage { + void bar(Child c) { + c.foo(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/safeDelete/overrideAnnotation/before/Super.java b/java/java-tests/testData/refactoring/safeDelete/overrideAnnotation/before/Super.java new file mode 100644 index 000000000000..eb492f6b6c1d --- /dev/null +++ b/java/java-tests/testData/refactoring/safeDelete/overrideAnnotation/before/Super.java @@ -0,0 +1,15 @@ +class Super { + void foo() {} +} + +class Child extends Super { + @Override + void foo() { + } +} + +class Usage { + void bar(Child c) { + c.foo(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/safeDelete/superCall/after/Super.java b/java/java-tests/testData/refactoring/safeDelete/superCall/after/Super.java new file mode 100644 index 000000000000..10bb0c89efb6 --- /dev/null +++ b/java/java-tests/testData/refactoring/safeDelete/superCall/after/Super.java @@ -0,0 +1,13 @@ +class Super { +} + +class Child extends Super { + void foo() { + } +} + +class Usage { + void bar(Child c) { + c.foo(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/safeDelete/superCall/before/Super.java b/java/java-tests/testData/refactoring/safeDelete/superCall/before/Super.java new file mode 100644 index 000000000000..83463f9200d0 --- /dev/null +++ b/java/java-tests/testData/refactoring/safeDelete/superCall/before/Super.java @@ -0,0 +1,14 @@ +class Super { + void foo() {} +} + +class Child extends Super { + { + foo(); + } + + @Override + void foo() { + super.foo(); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/refactoring/SafeDeleteTest.java b/java/java-tests/testSrc/com/intellij/refactoring/SafeDeleteTest.java index 83dbbeff5ffd..2e3da90b2733 100644 --- a/java/java-tests/testSrc/com/intellij/refactoring/SafeDeleteTest.java +++ b/java/java-tests/testSrc/com/intellij/refactoring/SafeDeleteTest.java @@ -122,6 +122,24 @@ public class SafeDeleteTest extends MultiFileTestCase { doTest("Super"); } + public void testOverrideAnnotation() throws Exception { + myDoCompare = false; + doTest("Super"); + } + + public void testSuperCall() throws Exception { + myDoCompare = false; + try { + doTest("Super"); + fail("Conflict was not detected"); + } + catch (BaseRefactoringProcessor.ConflictsInTestsException e) { + String message = e.getMessage(); + assertTrue(message, message.equals("method Super.foo() has 1 usage that is not safe to delete.\n" + + "Of those 0 usages are in strings, comments, or non-code files.")); + } + } + public void testMethodDeepHierarchy() throws Exception { myDoCompare = false; doTest("Super");