From 38116b22c1aaeb7ccbec6aed03dded9cc2f174a1 Mon Sep 17 00:00:00 2001 From: "Anna.Kozlova" Date: Tue, 15 Nov 2016 11:34:43 +0100 Subject: [PATCH] show conflict if move as enum would change the semantics (IDEA-163248) --- .../moveMembers/MoveJavaMemberHandler.java | 39 ++++++++++++++++--- .../move/moveMembers/MoveMemberHandler.java | 23 +++++++---- .../move/moveMembers/MoveMembersOptions.java | 15 ++++++- .../moveMembers/MoveMembersProcessor.java | 14 +++---- .../moveMembers/enumConstant/after/B.java | 3 ++ .../moveMembers/enumConstant/before/B.java | 5 ++- .../intellij/refactoring/MoveMembersTest.java | 10 ++++- 7 files changed, 83 insertions(+), 26 deletions(-) diff --git a/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveJavaMemberHandler.java b/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveJavaMemberHandler.java index 4557dd9b7c15..2232d6b5f03f 100644 --- a/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveJavaMemberHandler.java +++ b/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveJavaMemberHandler.java @@ -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 membersToMove, + @NotNull MoveMembersOptions moveMembersOptions, @NotNull MultiMap 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); diff --git a/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMemberHandler.java b/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMemberHandler.java index 9846d3690f82..f7abbadab2f8 100644 --- a/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMemberHandler.java +++ b/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMemberHandler.java @@ -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 membersToMove, @NotNull PsiClass targetClass); - void checkConflictsOnUsage(@NotNull MoveMembersProcessor.MoveMembersUsageInfo usageInfo, - @Nullable String newVisibility, - @Nullable PsiModifierList modifierListCopy, - @NotNull PsiClass targetClass, - @NotNull Set membersToMove, - @NotNull MultiMap conflicts); + default void checkConflictsOnUsage(@NotNull MoveMembersProcessor.MoveMembersUsageInfo usageInfo, + @Nullable PsiModifierList modifierListCopy, + @NotNull PsiClass targetClass, + @NotNull Set membersToMove, + MoveMembersOptions moveMembersOptions, + @NotNull MultiMap 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 membersToMove, + @NotNull MultiMap conflicts) {} void checkConflictsOnMember(@NotNull PsiMember member, @Nullable String newVisibility, diff --git a/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMembersOptions.java b/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMembersOptions.java index 99595c07d1dd..a28bd00e09c5 100644 --- a/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMembersOptions.java +++ b/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMembersOptions.java @@ -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(); } diff --git a/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMembersProcessor.java b/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMembersProcessor.java index b7687e100049..893efa29e4a5 100644 --- a/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMembersProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/move/moveMembers/MoveMembersProcessor.java @@ -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 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 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 membersToMove, - String newVisibility, @NotNull PsiClass targetClass, Map modifierListCopies, + MoveMembersOptions options, MultiMap 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); } } } diff --git a/java/java-tests/testData/refactoring/moveMembers/enumConstant/after/B.java b/java/java-tests/testData/refactoring/moveMembers/enumConstant/after/B.java index ebbe4dc08cb2..0efd250fa599 100644 --- a/java/java-tests/testData/refactoring/moveMembers/enumConstant/after/B.java +++ b/java/java-tests/testData/refactoring/moveMembers/enumConstant/after/B.java @@ -1,2 +1,5 @@ public class B { + { + Object o = A.ONE; + } } \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/moveMembers/enumConstant/before/B.java b/java/java-tests/testData/refactoring/moveMembers/enumConstant/before/B.java index acf3478236a6..2de88dcc3357 100644 --- a/java/java-tests/testData/refactoring/moveMembers/enumConstant/before/B.java +++ b/java/java-tests/testData/refactoring/moveMembers/enumConstant/before/B.java @@ -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 diff --git a/java/java-tests/testSrc/com/intellij/refactoring/MoveMembersTest.java b/java/java-tests/testSrc/com/intellij/refactoring/MoveMembersTest.java index 7221ece56edc..490ca58a1741 100644 --- a/java/java-tests/testSrc/com/intellij/refactoring/MoveMembersTest.java +++ b/java/java-tests/testSrc/com/intellij/refactoring/MoveMembersTest.java @@ -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 {