From b938da3020ce07c59327b324916c220d057f67c8 Mon Sep 17 00:00:00 2001 From: "Roman.Ivanov" Date: Mon, 26 Mar 2018 18:21:23 +0700 Subject: [PATCH] AddExceptionToExistingCatch: more concrete message, scroll and highlight --- .../src/messages/QuickFixBundle.properties | 4 + .../AddExceptionToExistingCatchFix.java | 191 +++++++++++++----- .../com/intellij/psi/PsiDisjunctionType.java | 18 +- .../afterMultipleExceptions.java | 16 ++ .../afterMultipleExceptionsReplace.java | 16 ++ .../beforeMultipleExceptions.java | 16 ++ .../beforeMultipleExceptionsReplace.java | 16 ++ .../com/intellij/psi/util/PsiTreeUtil.java | 13 ++ 8 files changed, 240 insertions(+), 50 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/addExceptionToExistingCatch/afterMultipleExceptions.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/addExceptionToExistingCatch/afterMultipleExceptionsReplace.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/addExceptionToExistingCatch/beforeMultipleExceptions.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/addExceptionToExistingCatch/beforeMultipleExceptionsReplace.java diff --git a/java/java-analysis-impl/src/messages/QuickFixBundle.properties b/java/java-analysis-impl/src/messages/QuickFixBundle.properties index 46dc74384e11..b5bffc3a6486 100644 --- a/java/java-analysis-impl/src/messages/QuickFixBundle.properties +++ b/java/java-analysis-impl/src/messages/QuickFixBundle.properties @@ -14,6 +14,10 @@ method.is.inherited.warning.title=Method Is Inherited add.exception.to.throws.text=Add {0, choice, 0#exception|2#exceptions} to method signature add.exception.to.throws.family=Add exception to method signature add.exception.to.existing.catch.family=Add exception to existing catch clause +add.exception.to.existing.catch.generic=Add exception to existing catch clause +add.exception.to.existing.catch.replacement=Replace ''{0}'' with more generic ''{1}'' +add.exception.to.existing.catch.no.replacement=Replace ''{0}'' with ''{1}'' + add.method.body.text=Add method body add.method.family=Add Method add.method.text=Add Method ''{0}'' to Class ''{1}'' diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/AddExceptionToExistingCatchFix.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/AddExceptionToExistingCatchFix.java index 41998758b764..01f5d90047d3 100644 --- a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/AddExceptionToExistingCatchFix.java +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/AddExceptionToExistingCatchFix.java @@ -8,21 +8,31 @@ import com.intellij.openapi.application.Application; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.command.WriteCommandAction; import com.intellij.openapi.editor.Editor; +import com.intellij.openapi.editor.LogicalPosition; +import com.intellij.openapi.editor.ScrollType; +import com.intellij.openapi.editor.colors.EditorColors; +import com.intellij.openapi.editor.colors.EditorColorsManager; +import com.intellij.openapi.editor.markup.*; import com.intellij.openapi.project.Project; +import com.intellij.openapi.ui.popup.JBPopupAdapter; import com.intellij.openapi.ui.popup.JBPopupFactory; +import com.intellij.openapi.ui.popup.LightweightWindowEvent; +import com.intellij.openapi.util.TextRange; import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.PsiUtil; import com.intellij.ui.components.JBList; import com.intellij.util.IncorrectOperationException; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Objects; +import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; -import java.util.stream.Stream; public class AddExceptionToExistingCatchFix extends PsiElementBaseIntentionAction { private final PsiElement myErrorElement; @@ -31,25 +41,48 @@ public class AddExceptionToExistingCatchFix extends PsiElementBaseIntentionActio @Override public void invoke(@NotNull Project project, Editor editor, @NotNull PsiElement element) throws IncorrectOperationException { - PsiTryStatement tryStatement = PsiTreeUtil.getParentOfType(element, PsiTryStatement.class); - if (tryStatement == null) return; - PsiCatchSection[] catchSections = tryStatement.getCatchSections(); - if (catchSections.length == 0) return; - List unhandledExceptions = new ArrayList<>(ExceptionUtil.getOwnUnhandledExceptions(myErrorElement)); - if (unhandledExceptions.size() != 1) return; - List catchTexts = getAvailableCatchSections(catchSections) - .map(s -> s.getCatchType()) - .filter(Objects::nonNull) - .map(type -> type.getPresentableText()) - .collect(Collectors.toList()); + Context context = Context.from(myErrorElement); + if (context == null) return; + + List catchSections = context.myCatches; + List unhandledExceptions = context.myExceptions; + List catchTexts = catchSections.stream() + .map(s -> s.getCatchType()).filter(Objects::nonNull) + .map(type -> type.getPresentableText()) + .collect(Collectors.toList()); + + setText(context.getMessage()); Application application = ApplicationManager.getApplication(); - if (catchSections.length == 1 || application.isUnitTestMode()) { - PsiCatchSection selectedSection = catchSections[0]; - addTypeToCatch(unhandledExceptions.get(0), selectedSection); + + if (catchSections.size() == 1 || application.isUnitTestMode()) { + PsiCatchSection selectedSection = catchSections.get(0); + addTypeToCatch(unhandledExceptions, selectedSection); } else { JBList list = new JBList<>(catchTexts); + AtomicReference rangeHighlighter = new AtomicReference<>(); // to change in lambda + MarkupModel markupModel = editor.getMarkupModel(); + + list.addListSelectionListener(e -> { + dropHighlight(rangeHighlighter); + int selectedIndex = list.getSelectedIndex(); + if (selectedIndex < 0) return; + PsiParameter elementToHighlight = catchSections.get(selectedIndex).getParameter(); + assert elementToHighlight != null; + TextRange range = elementToHighlight.getTextRange(); + + final LogicalPosition logicalPosition = editor.offsetToLogicalPosition(range.getStartOffset()); + editor.getScrollingModel().scrollTo(logicalPosition, ScrollType.MAKE_VISIBLE); + + TextAttributes attributes = + EditorColorsManager.getInstance().getGlobalScheme().getAttributes(EditorColors.SEARCH_RESULT_ATTRIBUTES); + RangeHighlighter highlighter = markupModel.addRangeHighlighter(range.getStartOffset(), range.getEndOffset(), + HighlighterLayer.SELECTION - 1, attributes, + HighlighterTargetArea.EXACT_RANGE); + rangeHighlighter.set(highlighter); + }); + JBPopupFactory.getInstance().createListPopupBuilder(list) .setTitle("Select catch block") .setMovable(false) @@ -57,60 +90,61 @@ public class AddExceptionToExistingCatchFix extends PsiElementBaseIntentionActio .setRequestFocus(true) .setItemChoosenCallback(() -> { int selectedIndex = list.getSelectedIndex(); - PsiCatchSection selectedSection = catchSections[selectedIndex]; - addTypeToCatch(unhandledExceptions.get(0), selectedSection); + PsiCatchSection selectedSection = catchSections.get(selectedIndex); + addTypeToCatch(unhandledExceptions, selectedSection); + }) + .addListener(new JBPopupAdapter() { + @Override + public void onClosed(LightweightWindowEvent event) { + dropHighlight(rangeHighlighter); + } }) .createPopup() .showInBestPositionFor(editor); } } - @NotNull - private static Stream getAvailableCatchSections(PsiCatchSection[] catchSections) { - return Arrays.stream(catchSections) - .filter(catchSection -> { - PsiParameter parameter = catchSection.getParameter(); - if (parameter == null) return false; - return parameter.getTypeElement() != null; - }); + private static void dropHighlight(AtomicReference rangeHighlighter) { + RangeHighlighter old = rangeHighlighter.get(); + if (old != null) { + old.dispose(); + } } - private static void addTypeToCatch(@NotNull PsiClassType exceptionToAdd, @NotNull PsiCatchSection catchSection) { + + private static void addTypeToCatch(@NotNull List exceptionsToAdd, @NotNull PsiCatchSection catchSection) { WriteCommandAction.runWriteCommandAction(catchSection.getProject(), () -> { - if (!catchSection.isValid() || !exceptionToAdd.isValid()) return; + if (!catchSection.isValid() || !exceptionsToAdd.stream().allMatch(type -> type.isValid())) return; PsiParameter parameter = catchSection.getParameter(); if (parameter == null) return; PsiTypeElement typeElement = parameter.getTypeElement(); if (typeElement == null) return; PsiType parameterType = parameter.getType(); - boolean needReplace = exceptionToAdd.isAssignableFrom(parameterType); PsiElementFactory factory = JavaPsiFacade.getElementFactory(catchSection.getProject()); - String typeText = needReplace ? exceptionToAdd.getCanonicalText() - : parameterType.getCanonicalText() + " | " + exceptionToAdd.getCanonicalText(); - typeElement.replace(factory.createTypeElementFromText(typeText, parameter)); + String flattenText = getTypeText(exceptionsToAdd, parameter, parameterType, factory); + typeElement.replace(factory.createTypeElementFromText(flattenText, parameter)); }); } + private static String getTypeText(@NotNull List exceptionsToAdd, + PsiParameter parameter, + PsiType parameterType, + PsiElementFactory factory) { + String typeText = parameterType.getCanonicalText() + " | " + exceptionsToAdd.stream() + .map(type -> type.getCanonicalText()) + .collect(Collectors.joining(" | ")); + PsiTypeElement element = factory.createTypeElementFromText(typeText, parameter); + List flatten = PsiDisjunctionType.flattenAndRemoveDuplicates(((PsiDisjunctionType)element.getType()).getDisjunctions()); + return flatten.stream() + .map(type -> type.getCanonicalText()) + .collect(Collectors.joining(" | ")); + } + @Override public boolean isAvailable(@NotNull Project project, Editor editor, @NotNull PsiElement element) { - PsiTryStatement tryStatement = PsiTreeUtil.getParentOfType(element, PsiTryStatement.class); - if (tryStatement == null) return false; - PsiCatchSection[] catchSections = tryStatement.getCatchSections(); - if (catchSections.length == 0) return false; - if (notFinishedCatches(catchSections)) return false; - PsiElement parent = PsiTreeUtil.getParentOfType(element, PsiCallExpression.class, PsiThrowStatement.class); - if (parent == null) return false; - List unhandledExceptions = new ArrayList<>(ExceptionUtil.getOwnUnhandledExceptions(myErrorElement)); - return unhandledExceptions.size() == 1; + return Context.from(myErrorElement) != null; } - private static boolean notFinishedCatches(PsiCatchSection[] catchSections) { - return getAvailableCatchSections(catchSections) - .map(catchSection -> catchSection.getParameter()) - .noneMatch(parameter -> parameter != null && parameter.getTypeElement() != null); - } - - @Nls @NotNull @Override @@ -123,4 +157,67 @@ public class AddExceptionToExistingCatchFix extends PsiElementBaseIntentionActio public String getText() { return getFamilyName(); } + + private static class Context { + private final List myCatches; + private final List myExceptions; + + + private Context(List catches, List exceptions) { + myCatches = catches; + myExceptions = exceptions; + } + + @Nullable + static Context from(@NotNull PsiElement element) { + if (!PsiUtil.isLanguageLevel7OrHigher(element)) { + return null; + } + List unhandledExceptions = new ArrayList<>(ExceptionUtil.getOwnUnhandledExceptions(element)); + if (unhandledExceptions.isEmpty()) return null; + List tryStatements = + PsiTreeUtil.collectParentsOfType(element, PsiTryStatement.class, PsiLambdaExpression.class, PsiClass.class); + List sections = + tryStatements.stream() + .flatMap(stmt -> Arrays.stream(stmt.getCatchSections())) + .filter(catchSection -> { + PsiParameter parameter = catchSection.getParameter(); + if (parameter == null) return false; + return parameter.getTypeElement() != null; + }) + .collect(Collectors.toList()); + if (sections.isEmpty()) return null; + return new Context(sections, unhandledExceptions); + } + + private String getMessage() { + if (myCatches.size() == 1 && myExceptions.size() == 1) { + PsiClassType exceptionType = myExceptions.get(0); + PsiCatchSection catchSection = myCatches.get(0); + PsiParameter parameter = catchSection.getParameter(); + assert parameter != null; + PsiType catchType = parameter.getType(); + if (replacementNeeded(exceptionType, catchType)) { + return QuickFixBundle.message("add.exception.to.existing.catch.replacement", catchType.getPresentableText(), exceptionType.getPresentableText()); + } + else { + return QuickFixBundle.message("add.exception.to.existing.catch.no.replacement", catchType.getPresentableText(), exceptionType.getPresentableText()); + } + } + return QuickFixBundle.message("add.exception.to.existing.catch.generic"); + } + } + + private static boolean replacementNeeded(@NotNull PsiClassType newException, @NotNull PsiType catchType) { + if (catchType instanceof PsiDisjunctionType) { + PsiDisjunctionType disjunction = (PsiDisjunctionType)catchType; + for (PsiType type : disjunction.getDisjunctions()) { + if (type.isAssignableFrom(newException)) { + return true; + } + } + return false; + } + return catchType.isAssignableFrom(newException); + } } diff --git a/java/java-psi-api/src/com/intellij/psi/PsiDisjunctionType.java b/java/java-psi-api/src/com/intellij/psi/PsiDisjunctionType.java index 922e88dbe497..2f08dd924d1d 100644 --- a/java/java-psi-api/src/com/intellij/psi/PsiDisjunctionType.java +++ b/java/java-psi-api/src/com/intellij/psi/PsiDisjunctionType.java @@ -22,12 +22,10 @@ import com.intellij.psi.util.CachedValue; import com.intellij.psi.util.CachedValueProvider; import com.intellij.psi.util.CachedValuesManager; import com.intellij.psi.util.PsiModificationTracker; -import com.intellij.util.Function; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; -import java.util.Collections; -import java.util.List; +import java.util.*; /** * Composite type resulting from Project Coin's multi-catch statements, i.e. {@code FileNotFoundException | EOFException}. @@ -150,4 +148,18 @@ public class PsiDisjunctionType extends PsiType.Stub { return true; } + + public static List flattenAndRemoveDuplicates(@NotNull List types) { + List disjunctions = new ArrayList<>(types); + for (Iterator iterator = disjunctions.iterator(); iterator.hasNext(); ) { + PsiType d1 = iterator.next(); + for (PsiType d2 : disjunctions) { + if (d1 != d2 && d2.isAssignableFrom(d1)) { + iterator.remove(); + break; + } + } + } + return disjunctions; + } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/addExceptionToExistingCatch/afterMultipleExceptions.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/addExceptionToExistingCatch/afterMultipleExceptions.java new file mode 100644 index 000000000000..f4a5f301b2f7 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/addExceptionToExistingCatch/afterMultipleExceptions.java @@ -0,0 +1,16 @@ +// "Add exception to existing catch clause" "true" +import java.io.IOException; + +class A extends Exception {} +class B extends Exception {} +class C extends Exception {} + +class Test { + + static void foo() throws A, B {} + public static void main(String[] args) { + try { + foo(); + } catch (C | A | B e) {} + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/addExceptionToExistingCatch/afterMultipleExceptionsReplace.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/addExceptionToExistingCatch/afterMultipleExceptionsReplace.java new file mode 100644 index 000000000000..51e34bb95a8e --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/addExceptionToExistingCatch/afterMultipleExceptionsReplace.java @@ -0,0 +1,16 @@ +// "Add exception to existing catch clause" "true" +import java.io.IOException; + +class A extends Exception {} +class B extends Exception {} +class C extends A {} +class D extends A {} + +class Test { + static void foo() throws A {} + public static void main(String[] args) { + try { + foo(); + } catch (B | A e) {} + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/addExceptionToExistingCatch/beforeMultipleExceptions.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/addExceptionToExistingCatch/beforeMultipleExceptions.java new file mode 100644 index 000000000000..c124019a610f --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/addExceptionToExistingCatch/beforeMultipleExceptions.java @@ -0,0 +1,16 @@ +// "Add exception to existing catch clause" "true" +import java.io.IOException; + +class A extends Exception {} +class B extends Exception {} +class C extends Exception {} + +class Test { + + static void foo() throws A, B {} + public static void main(String[] args) { + try { + foo(); + } catch (C e) {} + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/addExceptionToExistingCatch/beforeMultipleExceptionsReplace.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/addExceptionToExistingCatch/beforeMultipleExceptionsReplace.java new file mode 100644 index 000000000000..e367b2df5e9a --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/addExceptionToExistingCatch/beforeMultipleExceptionsReplace.java @@ -0,0 +1,16 @@ +// "Add exception to existing catch clause" "true" +import java.io.IOException; + +class A extends Exception {} +class B extends Exception {} +class C extends A {} +class D extends A {} + +class Test { + static void foo() throws A {} + public static void main(String[] args) { + try { + foo(); + } catch (C | D | B e) {} + } +} \ No newline at end of file diff --git a/platform/core-api/src/com/intellij/psi/util/PsiTreeUtil.java b/platform/core-api/src/com/intellij/psi/util/PsiTreeUtil.java index e27e8943dfd2..7385787975ee 100644 --- a/platform/core-api/src/com/intellij/psi/util/PsiTreeUtil.java +++ b/platform/core-api/src/com/intellij/psi/util/PsiTreeUtil.java @@ -697,6 +697,19 @@ public class PsiTreeUtil { return aClass.cast(element); } + public static List collectParentsOfType(PsiElement element, Class parent, Class... stopClasses) { + element = element.getParent(); + List parents = new SmartList<>(); + while (element != null) { + if (instanceOf(element, stopClasses)) break; + if (parent.isInstance(element)) { + parents.add(parent.cast(element)); + } + element = element.getParent(); + } + return parents; + } + @Nullable public static PsiElement findSiblingForward(@NotNull final PsiElement element, @NotNull final IElementType elementType,