From d3e2f47a2d65c27eab7b0d197723827925fae410 Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Tue, 24 Jan 2017 12:00:42 +0300 Subject: [PATCH] introduce constant: ensure chosen visibility is acceptable (IDEA-166950) --- .../InplaceIntroduceConstantPopup.java | 18 ++++-- .../IntroduceConstantDialog.java | 64 +++++++++++-------- .../EnsureVisibility.java | 7 ++ .../EnsureVisibility_after.java | 8 +++ .../InplaceIntroduceConstantTest.java | 15 +++++ 5 files changed, 78 insertions(+), 34 deletions(-) create mode 100644 java/java-tests/testData/refactoring/inplaceIntroduceConstant/EnsureVisibility.java create mode 100644 java/java-tests/testData/refactoring/inplaceIntroduceConstant/EnsureVisibility_after.java diff --git a/java/java-impl/src/com/intellij/refactoring/introduceField/InplaceIntroduceConstantPopup.java b/java/java-impl/src/com/intellij/refactoring/introduceField/InplaceIntroduceConstantPopup.java index fc8ba728ceab..43a6fdf40e5e 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceField/InplaceIntroduceConstantPopup.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceField/InplaceIntroduceConstantPopup.java @@ -51,6 +51,7 @@ public class InplaceIntroduceConstantPopup extends AbstractInplaceIntroduceField private JCheckBox myReplaceAllCb; private JCheckBox myMoveToAnotherClassCb; + private String myVisibility; public InplaceIntroduceConstantPopup(Project project, Editor editor, @@ -117,6 +118,7 @@ public class InplaceIntroduceConstantPopup extends AbstractInplaceIntroduceField } + @NotNull private String getSelectedVisibility() { if (myParentClass != null && myParentClass.isInterface()) { return PsiModifier.PUBLIC; @@ -125,6 +127,12 @@ public class InplaceIntroduceConstantPopup extends AbstractInplaceIntroduceField if (initialVisibility == null) { initialVisibility = PsiModifier.PUBLIC; } + else { + String effectiveVisibility = IntroduceConstantDialog.getEffectiveVisibility(initialVisibility, myOccurrences, myParentClass); + if (effectiveVisibility != null) { + return effectiveVisibility; + } + } return initialVisibility; } @@ -141,10 +149,8 @@ public class InplaceIntroduceConstantPopup extends AbstractInplaceIntroduceField myParentClass); PsiUtil.setModifierProperty(field, PsiModifier.FINAL, true); PsiUtil.setModifierProperty(field, PsiModifier.STATIC, true); - final String visibility = getSelectedVisibility(); - if (visibility != null) { - PsiUtil.setModifierProperty(field, visibility, true); - } + myVisibility = getSelectedVisibility(); + PsiUtil.setModifierProperty(field, myVisibility, true); final PsiElement anchorElementIfAll = getAnchorElementIfAll(); PsiElement finalAnchorElement; for (finalAnchorElement = anchorElementIfAll; @@ -185,7 +191,7 @@ public class InplaceIntroduceConstantPopup extends AbstractInplaceIntroduceField @Override protected void saveSettings(@NotNull PsiVariable psiVariable) { super.saveSettings(psiVariable); - JavaRefactoringSettings.getInstance().INTRODUCE_CONSTANT_VISIBILITY = getSelectedVisibility(); + JavaRefactoringSettings.getInstance().INTRODUCE_CONSTANT_VISIBILITY = myVisibility; } @Override @@ -241,7 +247,7 @@ public class InplaceIntroduceConstantPopup extends AbstractInplaceIntroduceField isReplaceAllOccurrences(), true, true, BaseExpressionToFieldHandler.InitializationPlace.IN_FIELD_DECLARATION, - getSelectedVisibility(), (PsiLocalVariable)getLocalVariable(), + myVisibility, (PsiLocalVariable)getLocalVariable(), getType(), true, myParentClass, false, false); diff --git a/java/java-impl/src/com/intellij/refactoring/introduceField/IntroduceConstantDialog.java b/java/java-impl/src/com/intellij/refactoring/introduceField/IntroduceConstantDialog.java index c0e1de734b3f..1e968e4f826e 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceField/IntroduceConstantDialog.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceField/IntroduceConstantDialog.java @@ -53,7 +53,6 @@ import com.intellij.usageView.UsageViewUtil; import com.intellij.util.ArrayUtil; import com.intellij.util.IncorrectOperationException; import com.intellij.util.ui.UIUtil; -import gnu.trove.THashSet; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; @@ -63,6 +62,7 @@ import java.awt.event.ActionEvent; import java.awt.event.ActionListener; import java.awt.event.ItemEvent; import java.awt.event.ItemListener; +import java.util.ArrayList; import java.util.Iterator; import java.util.LinkedHashSet; import java.util.Set; @@ -387,37 +387,45 @@ class IntroduceConstantDialog extends DialogWrapper { else { UIUtil.setEnabled(myVisibilityPanel, true, true); // exclude all modifiers not visible from all occurrences - final Set visible = new THashSet<>(); - visible.add(PsiModifier.PRIVATE); - visible.add(PsiModifier.PROTECTED); - visible.add(PsiModifier.PACKAGE_LOCAL); - visible.add(PsiModifier.PUBLIC); - for (PsiExpression occurrence : myOccurrences) { - final PsiManager psiManager = PsiManager.getInstance(myProject); - for (Iterator iterator = visible.iterator(); iterator.hasNext();) { - String modifier = iterator.next(); - - try { - final String modifierText = PsiModifier.PACKAGE_LOCAL.equals(modifier) ? "" : modifier + " "; - final PsiField field = JavaPsiFacade.getInstance(psiManager.getProject()).getElementFactory().createFieldFromText(modifierText + "int xxx;", myTargetClass); - if (!JavaResolveUtil.isAccessible(field, myTargetClass, field.getModifierList(), occurrence, myTargetClass, null)) { - iterator.remove(); - } - } - catch (IncorrectOperationException e) { - LOG.error(e); - } - } - } - if (!visible.contains(getFieldVisibility())) { - if (visible.contains(PsiModifier.PUBLIC)) myVPanel.setVisibility(PsiModifier.PUBLIC); - if (visible.contains(PsiModifier.PACKAGE_LOCAL)) myVPanel.setVisibility(PsiModifier.PACKAGE_LOCAL); - if (visible.contains(PsiModifier.PROTECTED)) myVPanel.setVisibility(PsiModifier.PROTECTED); - if (visible.contains(PsiModifier.PRIVATE)) myVPanel.setVisibility(PsiModifier.PRIVATE); + String effectiveVisibility = getEffectiveVisibility(getFieldVisibility(), myOccurrences, myTargetClass); + if (effectiveVisibility != null) { + myVPanel.setVisibility(effectiveVisibility); } } } + public static String getEffectiveVisibility(String initialVisibility, + PsiExpression[] occurrences, + PsiClass targetClass) { + final ArrayList visible = new ArrayList<>(); + visible.add(PsiModifier.PRIVATE); + visible.add(PsiModifier.PROTECTED); + visible.add(PsiModifier.PACKAGE_LOCAL); + visible.add(PsiModifier.PUBLIC); + for (PsiExpression occurrence : occurrences) { + final PsiManager psiManager = targetClass.getManager(); + for (Iterator iterator = visible.iterator(); iterator.hasNext();) { + String modifier = iterator.next(); + + try { + final String modifierText = PsiModifier.PACKAGE_LOCAL.equals(modifier) ? "" : modifier + " "; + final PsiField field = JavaPsiFacade + .getInstance(psiManager.getProject()).getElementFactory().createFieldFromText(modifierText + "int xxx;", targetClass); + if (!JavaResolveUtil.isAccessible(field, targetClass, field.getModifierList(), occurrence, targetClass, null)) { + iterator.remove(); + } + } + catch (IncorrectOperationException e) { + LOG.error(e); + } + } + } + if (!visible.contains(initialVisibility) && !visible.isEmpty()) { + return visible.get(0); + } + return null; + } + protected void doOKAction() { final String targetClassName = getTargetClassName(); PsiClass newClass = myParentClass; diff --git a/java/java-tests/testData/refactoring/inplaceIntroduceConstant/EnsureVisibility.java b/java/java-tests/testData/refactoring/inplaceIntroduceConstant/EnsureVisibility.java new file mode 100644 index 000000000000..76338b6be385 --- /dev/null +++ b/java/java-tests/testData/refactoring/inplaceIntroduceConstant/EnsureVisibility.java @@ -0,0 +1,7 @@ +@interface Ann { + String value(); +} + +@Ann("bar") +class Foo { +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/inplaceIntroduceConstant/EnsureVisibility_after.java b/java/java-tests/testData/refactoring/inplaceIntroduceConstant/EnsureVisibility_after.java new file mode 100644 index 000000000000..2f5e660d4ff2 --- /dev/null +++ b/java/java-tests/testData/refactoring/inplaceIntroduceConstant/EnsureVisibility_after.java @@ -0,0 +1,8 @@ +@interface Ann { + String value(); +} + +@Ann(Foo.BAR) +class Foo { + protected static final String BAR = "bar"; +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/refactoring/InplaceIntroduceConstantTest.java b/java/java-tests/testSrc/com/intellij/refactoring/InplaceIntroduceConstantTest.java index 5bf89a205d42..3acc7bf3be24 100644 --- a/java/java-tests/testSrc/com/intellij/refactoring/InplaceIntroduceConstantTest.java +++ b/java/java-tests/testSrc/com/intellij/refactoring/InplaceIntroduceConstantTest.java @@ -21,6 +21,7 @@ import com.intellij.openapi.util.Pass; import com.intellij.psi.PsiExpression; import com.intellij.psi.PsiLiteralExpression; import com.intellij.psi.PsiLocalVariable; +import com.intellij.psi.PsiModifier; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.refactoring.introduce.inplace.AbstractInplaceIntroducer; import com.intellij.refactoring.introduceField.IntroduceConstantHandler; @@ -129,6 +130,20 @@ public class InplaceIntroduceConstantTest extends AbstractJavaInplaceIntroduceTe }); } + public void testEnsureVisibility() throws Exception { + JavaRefactoringSettings.getInstance().INTRODUCE_CONSTANT_VISIBILITY = PsiModifier.PRIVATE; + try { + doTest(new Pass() { + @Override + public void pass(AbstractInplaceIntroducer inplaceIntroduceFieldPopup) { + } + }); + } + finally { + JavaRefactoringSettings.getInstance().INTRODUCE_CONSTANT_VISIBILITY = null; + } + } + public void testCorrectFinalPosition() throws Exception { doTest(new Pass() {