diff --git a/java/java-impl/src/com/intellij/codeInspection/magicConstant/MagicConstantInspection.java b/java/java-impl/src/com/intellij/codeInspection/magicConstant/MagicConstantInspection.java index f35db162d255..57407af9e25c 100644 --- a/java/java-impl/src/com/intellij/codeInspection/magicConstant/MagicConstantInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/magicConstant/MagicConstantInspection.java @@ -7,11 +7,18 @@ import com.intellij.codeInsight.ExternalAnnotationsManager; import com.intellij.codeInsight.daemon.GroupNames; import com.intellij.codeInspection.*; import com.intellij.ide.util.treeView.AbstractTreeNode; +import com.intellij.openapi.command.WriteCommandAction; +import com.intellij.openapi.fileEditor.FileEditor; +import com.intellij.openapi.fileEditor.FileEditorManager; +import com.intellij.openapi.fileEditor.TextEditor; import com.intellij.openapi.project.Project; import com.intellij.openapi.projectRoots.Sdk; import com.intellij.openapi.projectRoots.SdkModificator; import com.intellij.openapi.projectRoots.impl.JavaSdkImpl; import com.intellij.openapi.roots.JdkUtils; +import com.intellij.openapi.ui.popup.JBPopupFactory; +import com.intellij.openapi.ui.popup.PopupStep; +import com.intellij.openapi.ui.popup.util.BaseListPopupStep; import com.intellij.openapi.util.Comparing; import com.intellij.openapi.util.Key; import com.intellij.openapi.util.io.FileUtil; @@ -252,6 +259,20 @@ public class MagicConstantInspection extends AbstractBaseJavaLocalInspectionTool } } + static String formatMember(PsiAnnotationMemberValue value) { + if (value instanceof PsiReferenceExpression) { + PsiReferenceExpression ref = (PsiReferenceExpression)value; + PsiExpression qualifier = ref.getQualifierExpression(); + if (qualifier instanceof PsiJavaCodeReferenceElement) { + PsiClass psiClass = tryCast(((PsiJavaCodeReferenceElement)qualifier).resolve(), PsiClass.class); + if (psiClass != null) { + return psiClass.getName() + "." + ref.getReferenceName(); + } + } + } + return value.getText(); + } + static class AllowedValues { @NotNull final PsiAnnotationMemberValue[] values; final boolean canBeOred; @@ -553,36 +574,32 @@ public class MagicConstantInspection extends AbstractBaseJavaLocalInspectionTool } private static void registerProblem(@NotNull PsiExpression argument, @NotNull AllowedValues allowedValues, @NotNull ProblemsHolder holder) { - Function formatter = value -> { - if (value instanceof PsiReferenceExpression) { - PsiElement resolved = ((PsiReferenceExpression)value).resolve(); - if (resolved instanceof PsiVariable) { - return PsiFormatUtil.formatVariable((PsiVariable)resolved, - PsiFormatUtilBase.SHOW_NAME | PsiFormatUtilBase.SHOW_CONTAINING_CLASS, PsiSubstitutor.EMPTY); - } - } - return value.getText(); - }; - String values = StreamEx.of(allowedValues.values).map(formatter).collect(Joining.with(", ").cutAfterDelimiter().maxCodePoints(100)); + String values = StreamEx.of(allowedValues.values).map(MagicConstantInspection::formatMember).collect( + Joining.with(", ").cutAfterDelimiter().maxCodePoints(100)); String message = "Should be one of: " + values + (allowedValues.canBeOred ? " or their combination" : ""); - holder.registerProblem(argument, message, suggestMagicConstant(argument, allowedValues)); + holder.registerProblem(argument, message, suggestMagicConstant(argument, allowedValues, holder.isOnTheFly())); } @Nullable // null means no quickfix available private static LocalQuickFix suggestMagicConstant(@NotNull PsiExpression argument, - @NotNull AllowedValues allowedValues) { + @NotNull AllowedValues allowedValues, boolean onTheFly) { Object argumentValue = JavaConstantExpressionEvaluator.computeConstantExpression(argument, null, false); - if (argumentValue == null) return null; + if (argumentValue == null) { + return onTheFly && !allowedValues.canBeOred ? new ReplaceWithMagicConstantFix(argument, true, allowedValues.values) : null; + } if (!allowedValues.canBeOred) { for (PsiAnnotationMemberValue value : allowedValues.values) { if (value instanceof PsiExpression) { Object constantValue = JavaConstantExpressionEvaluator.computeConstantExpression((PsiExpression)value, null, false); if (argumentValue.equals(constantValue)) { - return new ReplaceWithMagicConstantFix(argument, value); + return new ReplaceWithMagicConstantFix(argument, false, value); } } } + if (onTheFly) { + return new ReplaceWithMagicConstantFix(argument, true, allowedValues.values); + } } else { Long longArgument = evaluateLongConstant(argument); @@ -616,7 +633,7 @@ public class MagicConstantInspection extends AbstractBaseJavaLocalInspectionTool } } if (!flags.isEmpty()) { - return new ReplaceWithMagicConstantFix(argument, flags.toArray(PsiAnnotationMemberValue.EMPTY_ARRAY)); + return new ReplaceWithMagicConstantFix(argument, false, flags.toArray(PsiAnnotationMemberValue.EMPTY_ARRAY)); } } } @@ -764,9 +781,11 @@ public class MagicConstantInspection extends AbstractBaseJavaLocalInspectionTool private static class ReplaceWithMagicConstantFix extends LocalQuickFixOnPsiElement { private final List> myMemberValuePointers; + private final boolean mySelect; - ReplaceWithMagicConstantFix(@NotNull PsiExpression argument, @NotNull PsiAnnotationMemberValue... values) { + ReplaceWithMagicConstantFix(@NotNull PsiExpression argument, boolean select, @NotNull PsiAnnotationMemberValue... values) { super(argument); + mySelect = select; myMemberValuePointers = Arrays.stream(values).map( value -> SmartPointerManager.getInstance(argument.getProject()).createSmartPsiElementPointer(value)).collect(Collectors.toList()); } @@ -781,15 +800,27 @@ public class MagicConstantInspection extends AbstractBaseJavaLocalInspectionTool @NotNull @Override public String getText() { + if (mySelect) return getFamilyName(); List names = myMemberValuePointers.stream().map(SmartPsiElementPointer::getElement).filter(Objects::nonNull) .map(PsiElement::getText).collect(Collectors.toList()); String expression = StringUtil.join(names, " | "); return "Replace with '" + expression + "'"; } - @Override public void invoke(@NotNull Project project, @NotNull PsiFile file, @NotNull PsiElement startElement, @NotNull PsiElement endElement) { List values = myMemberValuePointers.stream().map(SmartPsiElementPointer::getElement).collect(Collectors.toList()); + if (values.isEmpty() || values.contains(null)) return; + if (mySelect) { + selectFrom(project, startElement, values); + } + else { + WriteCommandAction.runWriteCommandAction(project, getFamilyName(), null, () -> replace(project, startElement, values)); + } + } + + private static void replace(@NotNull Project project, + @NotNull PsiElement startElement, + @NotNull List values) { String text = StringUtil.join(Collections.nCopies(values.size(), "0"), " | "); PsiExpression concatExp = PsiElementFactory.SERVICE.getInstance(project).createExpressionFromText(text, startElement); @@ -824,6 +855,35 @@ public class MagicConstantInspection extends AbstractBaseJavaLocalInspectionTool }); } + @Override + public boolean startInWriteAction() { + return false; + } + + private void selectFrom(Project project, PsiElement element, List values) { + FileEditor editor = FileEditorManager.getInstance(project).getSelectedEditor(element.getContainingFile().getVirtualFile()); + if (!(editor instanceof TextEditor)) return; + JBPopupFactory.getInstance().createListPopup(new BaseListPopupStep("Choose constant", values) { + @Override + public PopupStep onChosen(PsiAnnotationMemberValue selectedValue, boolean finalChoice) { + WriteCommandAction.runWriteCommandAction(project, getFamilyName(), null, + () -> replace(project, element, Collections.singletonList(selectedValue))); + return FINAL_CHOICE; + } + + @Override + public boolean isSpeedSearchEnabled() { + return true; + } + + @NotNull + @Override + public String getTextFor(PsiAnnotationMemberValue value) { + return formatMember(value); + } + }).showInBestPositionFor(((TextEditor)editor).getEditor()); + } + @Override public boolean isAvailable(@NotNull Project project, @NotNull PsiFile file,