show conflict if move as enum would change the semantics (IDEA-163248)
authorAnna.Kozlova <anna.kozlova@jetbrains.com>
Tue, 15 Nov 2016 10:34:43 +0000 (11:34 +0100)
committerAnna.Kozlova <anna.kozlova@jetbrains.com>
Tue, 15 Nov 2016 17:11:31 +0000 (18:11 +0100)
java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveJavaMemberHandler.java
java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMemberHandler.java
java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMembersOptions.java
java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMembersProcessor.java
java/java-tests/testData/refactoring/moveMembers/enumConstant/after/B.java
java/java-tests/testData/refactoring/moveMembers/enumConstant/before/B.java
java/java-tests/testSrc/com/intellij/refactoring/MoveMembersTest.java

index 4557dd9b7c15a4446351e82460fe19e5324514ab..2232d6b5f03ffc4a05f417d908c2a8d45ce5788d 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2009 JetBrains s.r.o.
+ * Copyright 2000-2016 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.
@@ -16,6 +16,8 @@
 package com.intellij.refactoring.move.moveMembers;
 
 import com.intellij.codeInsight.ChangeContextUtil;
+import com.intellij.codeInsight.ExpectedTypeInfo;
+import com.intellij.codeInsight.ExpectedTypesProvider;
 import com.intellij.codeInsight.highlighting.ReadWriteAccessDetector;
 import com.intellij.openapi.project.Project;
 import com.intellij.psi.*;
@@ -32,7 +34,10 @@ import com.intellij.util.containers.MultiMap;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
 
 /**
  * @author Maxim.Medvedev
@@ -77,10 +82,10 @@ public class MoveJavaMemberHandler implements MoveMemberHandler {
 
   @Override
   public void checkConflictsOnUsage(@NotNull MoveMembersProcessor.MoveMembersUsageInfo usageInfo,
-                                    @Nullable String newVisibility,
                                     @Nullable PsiModifierList modifierListCopy,
                                     @NotNull PsiClass targetClass,
                                     @NotNull Set<PsiMember> membersToMove,
+                                    @NotNull MoveMembersOptions moveMembersOptions,
                                     @NotNull MultiMap<PsiElement, String> conflicts) {
     final PsiElement element = usageInfo.getElement();
     if (element == null) return;
@@ -94,6 +99,7 @@ public class MoveJavaMemberHandler implements MoveMemberHandler {
       }
 
       if (!JavaResolveUtil.isAccessible(member, targetClass, modifierListCopy, element, accessObjectClass, null)) {
+        String newVisibility = moveMembersOptions.getExplicitMemberVisibility();
         String visibility = newVisibility != null ? newVisibility : VisibilityUtil.getVisibilityStringToDisplay(member);
         String message = RefactoringBundle.message("0.with.1.visibility.is.not.accessible.from.2",
                                                    RefactoringUIUtil.getDescription(member, false),
@@ -120,12 +126,27 @@ public class MoveJavaMemberHandler implements MoveMemberHandler {
       conflicts.putValue(usageInfo.member, "final variable initializer won't be available after move.");
     }
 
+    if (toBeConvertedToEnum(moveMembersOptions, member, targetClass) && !isEnumAcceptable(element, targetClass)) {
+      conflicts.putValue(element, "Enum type won't be applicable in the current context");
+    }
+
     final PsiReference reference = usageInfo.getReference();
     if (reference != null) {
       RefactoringConflictsUtil.checkAccessibilityConflicts(reference, member, modifierListCopy, targetClass, membersToMove, conflicts);
     }
   }
 
+  private static boolean isEnumAcceptable(PsiElement element, PsiClass targetClass) {
+    if (element instanceof PsiExpression) {
+      ExpectedTypeInfo[] types = ExpectedTypesProvider.getExpectedTypes((PsiExpression)element, false);
+      if (types.length == 1) {
+        PsiType type = types[0].getType();
+        return type.isAssignableFrom(JavaPsiFacade.getElementFactory(element.getProject()).createType(targetClass));
+      }
+    }
+    return false;
+  }
+
   @Override
   public void checkConflictsOnMember(@NotNull PsiMember member,
                                      @Nullable String newVisibility,
@@ -216,9 +237,7 @@ public class MoveJavaMemberHandler implements MoveMemberHandler {
     ChangeContextUtil.encodeContextInfo(member, true);
 
     final PsiMember memberCopy;
-    if (options.makeEnumConstant() &&
-        member instanceof PsiVariable &&
-        EnumConstantsUtil.isSuitableForEnumConstant(((PsiVariable)member).getType(), targetClass)) {
+    if (toBeConvertedToEnum(options, member, targetClass)) {
       memberCopy = EnumConstantsUtil.createEnumConstant(targetClass, member.getName(), ((PsiVariable)member).getInitializer());
     }
     else {
@@ -237,6 +256,14 @@ public class MoveJavaMemberHandler implements MoveMemberHandler {
     return anchor != null ? (PsiMember)targetClass.addAfter(memberCopy, anchor) : (PsiMember)targetClass.add(memberCopy);
   }
 
+  private static boolean toBeConvertedToEnum(@NotNull MoveMembersOptions options,
+                                             @NotNull PsiMember member,
+                                             @NotNull PsiClass targetClass) {
+    return options.makeEnumConstant() &&
+           member instanceof PsiVariable &&
+           EnumConstantsUtil.isSuitableForEnumConstant(((PsiVariable)member).getType(), targetClass);
+  }
+
   @Override
   public void decodeContextInfo(@NotNull PsiElement scope) {
     ChangeContextUtil.decodeContextInfo(scope, null, null);
index 9846d3690f82c1e9218c483296b2384d417e8407..f7abbadab2f820d04f2fc0bca100538d0e1ce856 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2009 JetBrains s.r.o.
+ * Copyright 2000-2016 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.
@@ -35,12 +35,21 @@ public interface MoveMemberHandler {
                                                      @NotNull Set<PsiMember> membersToMove,
                                                      @NotNull PsiClass targetClass);
 
-  void checkConflictsOnUsage(@NotNull MoveMembersProcessor.MoveMembersUsageInfo usageInfo,
-                             @Nullable String newVisibility,
-                             @Nullable PsiModifierList modifierListCopy,
-                             @NotNull PsiClass targetClass,
-                             @NotNull Set<PsiMember> membersToMove,
-                             @NotNull MultiMap<PsiElement, String> conflicts);
+  default void checkConflictsOnUsage(@NotNull MoveMembersProcessor.MoveMembersUsageInfo usageInfo,
+                                     @Nullable PsiModifierList modifierListCopy,
+                                     @NotNull PsiClass targetClass,
+                                     @NotNull Set<PsiMember> membersToMove,
+                                     MoveMembersOptions moveMembersOptions,
+                                     @NotNull MultiMap<PsiElement, String> conflicts) {
+    checkConflictsOnUsage(usageInfo, moveMembersOptions.getExplicitMemberVisibility(), modifierListCopy, targetClass, membersToMove, conflicts);
+  }
+
+  default void checkConflictsOnUsage(@NotNull MoveMembersProcessor.MoveMembersUsageInfo usageInfo,
+                                     @Nullable String newVisibility,
+                                     @Nullable PsiModifierList modifierListCopy,
+                                     @NotNull PsiClass targetClass,
+                                     @NotNull Set<PsiMember> membersToMove,
+                                     @NotNull MultiMap<PsiElement, String> conflicts) {}
 
   void checkConflictsOnMember(@NotNull PsiMember member,
                               @Nullable String newVisibility,
index 99595c07d1dd6d5f849743fba941d0250a2499d3..a28bd00e09c5c0dc39c4e8116f81cd30c6f24866 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2009 JetBrains s.r.o.
+ * Copyright 2000-2016 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.
@@ -16,6 +16,8 @@
 package com.intellij.refactoring.move.moveMembers;
 
 import com.intellij.psi.PsiMember;
+import com.intellij.psi.PsiModifier;
+import com.intellij.util.VisibilityUtil;
 import org.jetbrains.annotations.Nullable;
 
 /**
@@ -26,8 +28,19 @@ public interface MoveMembersOptions {
 
   String getTargetClassName();
 
+  @PsiModifier.ModifierConstant
   @Nullable
   String getMemberVisibility();
 
+  @PsiModifier.ModifierConstant
+  @Nullable
+  default String getExplicitMemberVisibility() {
+    String visibility = getMemberVisibility();
+    if (VisibilityUtil.ESCALATE_VISIBILITY.equals(visibility)) {
+      return PsiModifier.PUBLIC;
+    }
+    return visibility;
+  }
+
   boolean makeEnumConstant();
 }
index b7687e100049ac5c36da183d4cf7f58c7a66bbcc..893efa29e4a5d2d945818c84eafad3e92da67aad 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2015 JetBrains s.r.o.
+ * Copyright 2000-2016 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.
@@ -282,11 +282,7 @@ public class MoveMembersProcessor extends BaseRefactoringProcessor {
     final MultiMap<PsiElement, String> conflicts = new MultiMap<>();
     final UsageInfo[] usages = refUsages.get();
 
-    String newVisibility = myNewVisibility;
-    if (VisibilityUtil.ESCALATE_VISIBILITY.equals(newVisibility)) { // still need to check for access object
-      newVisibility = PsiModifier.PUBLIC;
-    }
-
+    String newVisibility = myOptions.getExplicitMemberVisibility(); // still need to check for access object
     final Map<PsiMember, PsiModifierList> modifierListCopies = new HashMap<>();
     for (PsiMember member : myMembersToMove) {
       PsiModifierList modifierListCopy = member.getModifierList();
@@ -304,7 +300,7 @@ public class MoveMembersProcessor extends BaseRefactoringProcessor {
       modifierListCopies.put(member, modifierListCopy);
     }
 
-    analyzeConflictsOnUsages(usages, myMembersToMove, newVisibility, myTargetClass, modifierListCopies, conflicts);
+    analyzeConflictsOnUsages(usages, myMembersToMove, myTargetClass, modifierListCopies, myOptions, conflicts);
     analyzeConflictsOnMembers(myMembersToMove, newVisibility, myTargetClass, modifierListCopies, conflicts);
 
     RefactoringConflictsUtil.analyzeModuleConflicts(myProject, myMembersToMove, usages, myTargetClass, conflicts);
@@ -314,9 +310,9 @@ public class MoveMembersProcessor extends BaseRefactoringProcessor {
 
   private static void analyzeConflictsOnUsages(UsageInfo[] usages,
                                                Set<PsiMember> membersToMove,
-                                               String newVisibility,
                                                @NotNull PsiClass targetClass,
                                                Map<PsiMember, PsiModifierList> modifierListCopies,
+                                               MoveMembersOptions options,
                                                MultiMap<PsiElement, String> conflicts) {
     for (UsageInfo usage : usages) {
       if (!(usage instanceof MoveMembersUsageInfo)) continue;
@@ -324,7 +320,7 @@ public class MoveMembersProcessor extends BaseRefactoringProcessor {
       final PsiMember member = usageInfo.member;
       final MoveMemberHandler handler = MoveMemberHandler.EP_NAME.forLanguage(member.getLanguage());
       if (handler != null) {
-        handler.checkConflictsOnUsage(usageInfo, newVisibility, modifierListCopies.get(member), targetClass, membersToMove, conflicts);
+        handler.checkConflictsOnUsage(usageInfo, modifierListCopies.get(member), targetClass, membersToMove, options, conflicts);
       }
     }
   }
index ebbe4dc08cb26d3252c912992cfad4f6b1383162..0efd250fa59905058fecb7ef9b3aba9a55ce7388 100644 (file)
@@ -1,2 +1,5 @@
 public class B {
+    {
+        Object o = A.ONE;
+    }
 }
\ No newline at end of file
index acf3478236a6a092c6a58668b6620f67be51a371..2de88dcc3357c14bf62f5954c9dc951d2af572bd 100644 (file)
@@ -1,3 +1,6 @@
 public class B {
-    public static final String ONE = ""; 
+    public static final String ONE = "";
+    {
+        Object o = ONE;
+    }
 }
\ No newline at end of file
index 7221ece56edc82262f9c445adb320e3f6d7b31d8..490ca58a17413ce0e90aa760fe0dc69b4fc2ebb9 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2014 JetBrains s.r.o.
+ * Copyright 2000-2016 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.
@@ -119,7 +119,13 @@ public class MoveMembersTest extends MultiFileTestCase {
   }
 
   public void testEnumConstantFromCaseStatement() throws Exception {
-    doTest("B", "A", 0);
+    try {
+      doTest("B", "A", 0);
+      fail("Conflict expected");
+    }
+    catch (BaseRefactoringProcessor.ConflictsInTestsException e) {
+      assertEquals("Enum type won't be applicable in the current context", e.getMessage());
+    }
   }
 
   public void testStringConstantFromCaseStatement() throws Exception {