extract enum: do not include constants when user don't ask to; check that variable was already migrated and getter is already not needed (IDEA-103509)

2026-09-27 10:03:11 +07:00 · 2013-04-12 18:31:06 +02:00
parent 2447ab7c9d
commit 33dee73821
10 changed files with 104 additions and 35 deletions
@@ -378,7 +378,7 @@ class ExtractClassDialog extends RefactoringDialog implements MemberInfoChangeLi
      final PsiMember member = info.getMember();
      if (isConstantField(member)) {
        if (enumConstants.isEmpty() || ((PsiField)enumConstants.get(0).getMember()).getType().equals(((PsiField)member).getType())) {
-          enumConstants.add(info);
+          if (!enumConstants.contains(info)) enumConstants.add(info);
          info.setToAbstract(true);
        }
      }
@@ -407,7 +407,9 @@ class ExtractClassDialog extends RefactoringDialog implements MemberInfoChangeLi
    if (extractAsEnum.isVisible()) {
      for (Object info : memberInfoChange.getChangedMembers()) {
        if (((MemberInfo)info).isToAbstract()) {
-          enumConstants.add((MemberInfo)info);
+          if (!enumConstants.contains(info)) {
+            enumConstants.add((MemberInfo)info);
+          }
        }
        else {
          enumConstants.remove((MemberInfo)info);
@@ -109,7 +109,9 @@ public class ExtractClassProcessor extends FixableUsagesRefactoringProcessor {
    myGenerateAccessors = generateAccessors;
    this.enumConstants = new ArrayList<PsiField>();
    for (MemberInfo constant : enumConstants) {
-      this.enumConstants.add((PsiField)constant.getMember());
+      if (constant.isChecked()) {
+        this.enumConstants.add((PsiField)constant.getMember());
+      }
    }
    this.fields = new ArrayList<PsiField>(fields);
    this.methods = new ArrayList<PsiMethod>(methods);
@@ -139,7 +141,7 @@ public class ExtractClassProcessor extends FixableUsagesRefactoringProcessor {
        result.setResult(buildClass());
      }
    }.execute().getResultObject();
-    myExtractEnumProcessor = new ExtractEnumProcessor(myProject, this.enumConstants, fields, myClass);
+    myExtractEnumProcessor = new ExtractEnumProcessor(myProject, this.enumConstants, myClass);
  }

  public PsiClass getCreatedClass() {
@@ -149,7 +151,7 @@ public class ExtractClassProcessor extends FixableUsagesRefactoringProcessor {
  @Override
  protected boolean preprocessUsages(final Ref<UsageInfo[]> refUsages) {
    final MultiMap<PsiElement, String> conflicts = new MultiMap<PsiElement, String>();
-    myExtractEnumProcessor.findEnumConstantConflicts(refUsages, conflicts);
+    myExtractEnumProcessor.findEnumConstantConflicts(refUsages);
    if (!DestinationFolderComboBox.isAccessible(myProject, sourceClass.getContainingFile().getVirtualFile(),
                                                myClass.getContainingFile().getContainingDirectory().getVirtualFile())) {
      conflicts.putValue(sourceClass, "Extracted class won't be accessible in " + RefactoringUIUtil.getDescription(sourceClass, true));
@@ -27,7 +27,7 @@ import com.intellij.psi.*;
 import com.intellij.psi.search.GlobalSearchScope;
 import com.intellij.psi.util.PropertyUtil;
 import com.intellij.psi.util.PsiTreeUtil;
-import com.intellij.psi.util.PsiUtilBase;
+import com.intellij.psi.util.PsiUtilCore;
 import com.intellij.refactoring.extractclass.usageInfo.ReplaceStaticVariableAccess;
 import com.intellij.refactoring.psi.MutationUtils;
 import com.intellij.refactoring.typeMigration.TypeMigrationProcessor;
@@ -37,7 +37,6 @@ import com.intellij.refactoring.util.FixableUsageInfo;
 import com.intellij.refactoring.util.RefactoringUIUtil;
 import com.intellij.usageView.UsageInfo;
 import com.intellij.util.IncorrectOperationException;
-import com.intellij.util.containers.MultiMap;

 import java.util.*;

@@ -47,17 +46,15 @@ public class ExtractEnumProcessor {
  private final PsiClass myClass;

  private TypeMigrationProcessor myTypeMigrationProcessor;
-  private List<PsiField> myFields;

-  public ExtractEnumProcessor(Project project, List<PsiField> enumConstants, List<PsiField> fields, PsiClass aClass) {
+  public ExtractEnumProcessor(Project project, List<PsiField> enumConstants, PsiClass aClass) {
    myProject = project;
    myEnumConstants = enumConstants;
-    myFields = fields;
    myClass = aClass;
  }


-  public void findEnumConstantConflicts(final Ref<UsageInfo[]> refUsages, final MultiMap<PsiElement, String> conflicts) {
+  public void findEnumConstantConflicts(final Ref<UsageInfo[]> refUsages) {
    if (hasUsages2Migrate()) {
      final List<UsageInfo> resolvableConflicts = new ArrayList<UsageInfo>();
      for (UsageInfo failedUsage : myTypeMigrationProcessor.getLabeler().getFailedUsages()) {
@@ -152,7 +149,7 @@ public class ExtractEnumProcessor {
      rules.setMigrationRootType(
        JavaPsiFacade.getElementFactory(myProject).createType(myClass));
      rules.setBoundScope(GlobalSearchScope.projectScope(myProject));
-      myTypeMigrationProcessor = new TypeMigrationProcessor(myProject, PsiUtilBase.toPsiElementArray(myEnumConstants), rules);
+      myTypeMigrationProcessor = new TypeMigrationProcessor(myProject, PsiUtilCore.toPsiElementArray(myEnumConstants), rules);
      for (UsageInfo usageInfo : myTypeMigrationProcessor.findUsages()) {
        final PsiElement migrateElement = usageInfo.getElement();
        if (migrateElement instanceof PsiField) {
@@ -21,7 +21,7 @@ import com.intellij.psi.util.PropertyUtil;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.refactoring.psi.MutationUtils;
 import com.intellij.refactoring.util.FixableUsageInfo;
-import com.intellij.util.ArrayUtil;
+import com.intellij.util.ArrayUtilRt;
 import com.intellij.util.IncorrectOperationException;

 public class ReplaceStaticVariableAccess extends FixableUsageInfo {
@@ -45,33 +45,53 @@ public class ReplaceStaticVariableAccess extends FixableUsageInfo {
        return;
      }
    }
-    boolean replaceWithGetEnumValue = myEnumConstant;
-    if (replaceWithGetEnumValue) {
-      final PsiMethodCallExpression callExpression = PsiTreeUtil.getParentOfType(expression, PsiMethodCallExpression.class);
-      if (callExpression != null) {
-        final PsiElement resolved = callExpression.getMethodExpression().resolve();
-        if (resolved instanceof PsiMethod) {
-          final PsiParameter[] parameters = ((PsiMethod)resolved).getParameterList().getParameters();
-          final PsiExpression[] args = callExpression.getArgumentList().getExpressions();
-          final int idx = ArrayUtil.find(args, expression);
-          if (idx != -1 && parameters[idx].getType().getCanonicalText().equals(delegateClass)) {
-            replaceWithGetEnumValue = false;
-          }
+    final boolean replaceWithGetEnumValue = myEnumConstant && !alreadyMigratedToEnum();
+    final String link = replaceWithGetEnumValue ? "." + PropertyUtil.suggestGetterName("value", expression.getType()) + "()" : "";
+    MutationUtils.replaceExpression(delegateClass + '.' + expression.getReferenceName() + link, expression);
+  }
+
+  private boolean alreadyMigratedToEnum() {
+    final PsiMethodCallExpression callExpression = PsiTreeUtil.getParentOfType(expression, PsiMethodCallExpression.class);
+    if (callExpression != null) {
+      final PsiElement resolved = callExpression.getMethodExpression().resolve();
+      if (resolved instanceof PsiMethod) {
+        final PsiParameter[] parameters = ((PsiMethod)resolved).getParameterList().getParameters();
+        final PsiExpression[] args = callExpression.getArgumentList().getExpressions();
+        final int idx = ArrayUtilRt.find(args, expression);
+        if (idx != -1 && parameters[idx].getType().equalsToText(delegateClass)) {
+          return true;
        }
      }
-      else {
-        final PsiReturnStatement returnStatement = PsiTreeUtil.getParentOfType(expression, PsiReturnStatement.class);
-        if (returnStatement != null) {
-          final PsiMethod psiMethod = PsiTreeUtil.getParentOfType(expression, PsiMethod.class);
-          LOGGER.assertTrue(psiMethod != null);
-          final PsiType returnType = psiMethod.getReturnType();
-          if (returnType != null && returnType.getCanonicalText().equals(delegateClass)) {
-            replaceWithGetEnumValue = false;
+    }
+    else {
+      final PsiReturnStatement returnStatement = PsiTreeUtil.getParentOfType(expression, PsiReturnStatement.class);
+      if (returnStatement != null) {
+        final PsiMethod psiMethod = PsiTreeUtil.getParentOfType(expression, PsiMethod.class);
+        LOGGER.assertTrue(psiMethod != null);
+        final PsiType returnType = psiMethod.getReturnType();
+        if (returnType != null && returnType.getCanonicalText().equals(delegateClass)) {
+          return true;
+        }
+      } else {
+        final PsiVariable psiVariable = PsiTreeUtil.getParentOfType(expression, PsiVariable.class);
+        if (psiVariable != null) {
+          if (psiVariable.getType().equalsToText(delegateClass)) {
+            return true;
+          }
+        } else {
+          final PsiAssignmentExpression assignmentExpression = PsiTreeUtil.getParentOfType(expression, PsiAssignmentExpression.class);
+          if (assignmentExpression != null && assignmentExpression.getRExpression() == expression) {
+            final PsiExpression lExpression = assignmentExpression.getLExpression();
+            if (lExpression instanceof PsiReferenceExpression) {
+              final PsiElement resolve = ((PsiReferenceExpression)lExpression).resolve();
+              if (resolve instanceof PsiVariable && ((PsiVariable)resolve).getType().equalsToText(delegateClass)) {
+                return true;
+              }
+            }
          }
        }
      }
    }
-    final String link = replaceWithGetEnumValue ? "." + PropertyUtil.suggestGetterName("value", expression.getType()) + "()" : "";
-    MutationUtils.replaceExpression(delegateClass + '.' + expression.getReferenceName() + link, expression);
+    return false;
  }
 }
@@ -0,0 +1,12 @@
+public enum EEnum {
+    FOO(0);
+    private int value;
+
+    public int getValue() {
+        return value;
+    }
+
+    EEnum(int value) {
+        this.value = value;
+    }
+}
@@ -0,0 +1,6 @@
+class Test {
+    public static final int BAR = 2;
+  void foo() {
+    System.out.println(EEnum.FOO.getValue());
+  }
+}
@@ -0,0 +1,9 @@
+class Usage {
+  void foo() {
+    EEnum i = EEnum.FOO;
+  }
+  
+  void bar(int i ) {
+    i = B;
+  }
+}
@@ -0,0 +1,7 @@
+class Test {
+  public static final int FOO = 0;
+  public static final int BAR = 2;
+  void foo() {
+    System.out.println(FOO);
+  }
+}
@@ -0,0 +1,9 @@
+class Usage {
+  void foo() {
+    int i = Test.FOO;
+  }
+  
+  void bar(int i ) {
+    i = B;
+  }
+}
@@ -59,6 +59,10 @@ public class ExtractEnumTest extends MultiFileTestCase {
    doTest(new RefactoringTestUtil.MemberDescriptor("FOO", PsiField.class, true));
  }

+  public void testUsageInVariableInitializer() throws Exception {
+    doTest(new RefactoringTestUtil.MemberDescriptor("FOO", PsiField.class, true));
+  }
+
  public void testForwardReferenceConflict() throws Exception {
    doTest("Unable to migrate statement to enum constant.", false,
           new RefactoringTestUtil.MemberDescriptor("FOO", PsiField.class, false),
@@ -149,6 +153,7 @@ public class ExtractEnumTest extends MultiFileTestCase {
            if (member.hasModifierProperty(PsiModifier.STATIC) && member.hasModifierProperty(PsiModifier.FINAL) && ((PsiField)member).hasInitializer()) {
              if (memberInfo.isToAbstract()) {
                enumConstants.add(memberInfo);
+                memberInfo.setChecked(true);
              }
            }
          }