show conflict if move as enum would change the semantics (IDEA-163248)

This commit is contained in:
Anna.Kozlova
2016-11-15 18:11:31 +01:00
parent 6298068d8b
commit 38116b22c1
7 changed files with 83 additions and 26 deletions
@@ -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);
@@ -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,
@@ -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();
}
@@ -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);
}
}
}
@@ -1,2 +1,5 @@
public class B {
{
Object o = A.ONE;
}
}
@@ -1,3 +1,6 @@
public class B {
public static final String ONE = "";
public static final String ONE = "";
{
Object o = ONE;
}
}
@@ -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 {