From 1712207c499ac1682690e4866509b13ab2f67263 Mon Sep 17 00:00:00 2001 From: peter Date: Tue, 9 Jan 2018 23:22:33 +0100 Subject: [PATCH] IDEA-126898 Auto-complete for places where exceptions are expected should favor subclasses of Throwable and should also analyze method body for runtime exceptions thrown. --- .../JavaCompletionStatistician.java | 5 +- .../JavaDocCompletionContributor.java | 11 ++- .../completion/PreferByKindWeigher.java | 81 ++++++++++--------- .../PreferExceptionsInJavadocThrows.java | 8 ++ .../PreferExceptionsInThrowsList.java | 5 ++ .../NormalCompletionOrderingTest.groovy | 17 ++++ 6 files changed, 86 insertions(+), 41 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/completion/normalSorting/PreferExceptionsInJavadocThrows.java create mode 100644 java/java-tests/testData/codeInsight/completion/normalSorting/PreferExceptionsInThrowsList.java diff --git a/java/java-impl/src/com/intellij/codeInsight/completion/JavaCompletionStatistician.java b/java/java-impl/src/com/intellij/codeInsight/completion/JavaCompletionStatistician.java index e7d1797d9606..5e7256feb2b3 100644 --- a/java/java-impl/src/com/intellij/codeInsight/completion/JavaCompletionStatistician.java +++ b/java/java-impl/src/com/intellij/codeInsight/completion/JavaCompletionStatistician.java @@ -84,7 +84,10 @@ public class JavaCompletionStatistician extends CompletionStatistician{ } PsiType expectedType = firstInfo != null ? firstInfo.getDefaultType() : null; - String context = JavaClassNameCompletionContributor.AFTER_NEW.accepts(position) ? JavaStatisticsManager.getAfterNewKey(expectedType) : ""; + String context = + JavaClassNameCompletionContributor.AFTER_NEW.accepts(position) ? JavaStatisticsManager.getAfterNewKey(expectedType) : + PreferByKindWeigher.isExceptionPosition(position) ? "exception" : + ""; return new StatisticsInfo(context, JavaStatisticsManager.getMemberUseKey2(psiClass)); } diff --git a/java/java-impl/src/com/intellij/codeInsight/completion/JavaDocCompletionContributor.java b/java/java-impl/src/com/intellij/codeInsight/completion/JavaDocCompletionContributor.java index f27f6a6d5203..6fae6ea513c5 100644 --- a/java/java-impl/src/com/intellij/codeInsight/completion/JavaDocCompletionContributor.java +++ b/java/java-impl/src/com/intellij/codeInsight/completion/JavaDocCompletionContributor.java @@ -32,6 +32,7 @@ import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Comparing; import com.intellij.openapi.util.Conditions; import com.intellij.openapi.util.text.StringUtil; +import com.intellij.patterns.PsiElementPattern; import com.intellij.patterns.PsiJavaPatterns; import com.intellij.profile.codeInspection.InspectionProjectProfileManager; import com.intellij.psi.*; @@ -76,13 +77,16 @@ public class JavaDocCompletionContributor extends CompletionContributor { } } }; + static final PsiElementPattern.Capture THROWS_TAG_EXCEPTION = psiElement().inside( + psiElement(PsiDocTag.class).withName( + string().oneOf(PsiKeyword.THROWS, "exception"))); public JavaDocCompletionContributor() { extend(CompletionType.BASIC, PsiJavaPatterns.psiElement(JavaDocTokenType.DOC_TAG_NAME), new TagChooser()); extend(CompletionType.BASIC, PsiJavaPatterns.psiElement().inside(PsiDocComment.class), new CompletionProvider() { @Override - protected void addCompletions(@NotNull final CompletionParameters parameters, final ProcessingContext context, @NotNull final CompletionResultSet result) { + protected void addCompletions(@NotNull final CompletionParameters parameters, final ProcessingContext context, @NotNull CompletionResultSet result) { final PsiElement position = parameters.getPosition(); boolean isArg = PsiJavaPatterns.psiElement().afterLeaf("(").accepts(position); PsiDocTag tag = PsiTreeUtil.getParentOfType(position, PsiDocTag.class); @@ -90,6 +94,7 @@ public class JavaDocCompletionContributor extends CompletionContributor { final PsiReference ref = position.getContainingFile().findReferenceAt(parameters.getOffset()); if (ref instanceof PsiJavaReference) { + result = JavaCompletionSorting.addJavaSorting(parameters, result); result.stopHere(); for (LookupElement item : completeJavadocReference(position, (PsiJavaReference)ref)) { @@ -116,9 +121,7 @@ public class JavaDocCompletionContributor extends CompletionContributor { } }); - extend(CompletionType.SMART, psiElement().inside( - psiElement(PsiDocTag.class).withName( - string().oneOf(PsiKeyword.THROWS, "exception"))), new CompletionProvider() { + extend(CompletionType.SMART, THROWS_TAG_EXCEPTION, new CompletionProvider() { @Override public void addCompletions(@NotNull final CompletionParameters parameters, final ProcessingContext context, @NotNull final CompletionResultSet result) { final PsiElement element = parameters.getPosition(); diff --git a/java/java-impl/src/com/intellij/codeInsight/completion/PreferByKindWeigher.java b/java/java-impl/src/com/intellij/codeInsight/completion/PreferByKindWeigher.java index 9e549fcdb786..fd635c94aa68 100644 --- a/java/java-impl/src/com/intellij/codeInsight/completion/PreferByKindWeigher.java +++ b/java/java-impl/src/com/intellij/codeInsight/completion/PreferByKindWeigher.java @@ -21,14 +21,13 @@ import com.intellij.codeInsight.ExpectedTypeInfo; import com.intellij.codeInsight.lookup.LookupElement; import com.intellij.codeInsight.lookup.LookupElementWeigher; import com.intellij.codeInsight.lookup.TypedLookupItem; -import com.intellij.openapi.util.Condition; -import com.intellij.openapi.util.Conditions; import com.intellij.openapi.util.Key; import com.intellij.openapi.util.text.StringUtil; import com.intellij.patterns.ElementPattern; import com.intellij.psi.*; import com.intellij.psi.filters.getters.MembersGetter; import com.intellij.psi.impl.source.tree.JavaElementType; +import com.intellij.psi.javadoc.PsiDocComment; import com.intellij.psi.util.*; import com.intellij.psi.util.proximity.KnownElementWeigher; import com.intellij.util.ThreeState; @@ -38,6 +37,7 @@ import org.jetbrains.annotations.NotNull; import java.util.Arrays; import java.util.List; import java.util.Set; +import java.util.function.Function; import static com.intellij.patterns.PsiJavaPatterns.elementType; import static com.intellij.patterns.PsiJavaPatterns.psiElement; @@ -49,19 +49,19 @@ import static com.intellij.patterns.StandardPatterns.or; public class PreferByKindWeigher extends LookupElementWeigher { public static final Key INTRODUCED_VARIABLE = Key.create("INTRODUCED_VARIABLE"); - static final ElementPattern IN_CATCH_TYPE = + private static final ElementPattern IN_CATCH_TYPE = psiElement().withParent(psiElement(PsiJavaCodeReferenceElement.class). withParent(psiElement(PsiTypeElement.class). withParent(or(psiElement(PsiCatchSection.class), psiElement(PsiVariable.class).withParent(PsiCatchSection.class))))); - static final ElementPattern IN_MULTI_CATCH_TYPE = + private static final ElementPattern IN_MULTI_CATCH_TYPE = or(psiElement().afterLeaf(psiElement().withText("|"). withParent(PsiTypeElement.class).withSuperParent(2, PsiCatchSection.class)), psiElement().afterLeaf(psiElement().withText("|"). withParent(PsiTypeElement.class).withSuperParent(2, PsiParameter.class).withSuperParent(3, PsiCatchSection.class))); - static final ElementPattern INSIDE_METHOD_THROWS_CLAUSE = + private static final ElementPattern INSIDE_METHOD_THROWS_CLAUSE = psiElement().afterLeaf(PsiKeyword.THROWS, ",").inside(psiElement(JavaElementType.THROWS_LIST)); static final ElementPattern IN_RESOURCE = @@ -69,11 +69,13 @@ public class PreferByKindWeigher extends LookupElementWeigher { psiElement(PsiJavaCodeReferenceElement.class).withParent(PsiTypeElement.class). withSuperParent(2, or(psiElement(PsiResourceVariable.class), psiElement(PsiResourceList.class))), psiElement(PsiReferenceExpression.class).withParent(PsiResourceExpression.class))); + private static final Function + PREFER_THROWABLE = psiClass -> preferClassIf(InheritanceUtil.isInheritor(psiClass, CommonClassNames.JAVA_LANG_THROWABLE)); private final CompletionType myCompletionType; private final PsiElement myPosition; private final Set myNonInitializedFields; - private final Condition myRequiredSuper; + private final Function myClassSuitability; private final ExpectedTypeInfo[] myExpectedTypes; public PreferByKindWeigher(CompletionType completionType, final PsiElement position, ExpectedTypeInfo[] expectedTypes) { @@ -81,50 +83,57 @@ public class PreferByKindWeigher extends LookupElementWeigher { myCompletionType = completionType; myPosition = position; myNonInitializedFields = CheckInitialized.getNonInitializedFields(position); - myRequiredSuper = createSuitabilityCondition(position); + myClassSuitability = createSuitabilityCondition(position); myExpectedTypes = expectedTypes; } @NotNull - private static Condition createSuitabilityCondition(final PsiElement position) { - if (IN_CATCH_TYPE.accepts(position) || IN_MULTI_CATCH_TYPE.accepts(position)) { - PsiTryStatement tryStatement = PsiTreeUtil.getParentOfType(position, PsiTryStatement.class); - final List thrownExceptions = ContainerUtil.newArrayList(); - if (tryStatement != null && tryStatement.getTryBlock() != null) { - for (PsiClassType type : ExceptionUtil.getThrownExceptions(tryStatement.getTryBlock())) { - ContainerUtil.addIfNotNull(thrownExceptions, type.resolve()); - } - } - if (thrownExceptions.isEmpty()) { - ContainerUtil.addIfNotNull(thrownExceptions, - JavaPsiFacade.getInstance(position.getProject()).findClass( - CommonClassNames.JAVA_LANG_THROWABLE, position.getResolveScope())); - } - return psiClass -> { - for (PsiClass exception : thrownExceptions) { - if (InheritanceUtil.isInheritorOrSelf(psiClass, exception, true)) { - return true; + private static Function createSuitabilityCondition(final PsiElement position) { + if (isExceptionPosition(position)) { + PsiElement container = PsiTreeUtil.getParentOfType(position, PsiTryStatement.class, PsiMethod.class); + List thrownExceptions = ContainerUtil.newArrayList(); + if (container != null) { + PsiElement block = container instanceof PsiTryStatement ? ((PsiTryStatement)container).getTryBlock() : container; + if (block != null) { + for (PsiClassType type : ExceptionUtil.getThrownExceptions(block)) { + ContainerUtil.addIfNotNull(thrownExceptions, type.resolve()); } } - return false; + } + return psiClass -> { + if (ContainerUtil.exists(thrownExceptions, t -> InheritanceUtil.isInheritorOrSelf(psiClass, t, true))) { + return MyResult.verySuitableClass; + } + return PREFER_THROWABLE.apply(psiClass); }; } - else if (JavaSmartCompletionContributor.AFTER_THROW_NEW.accepts(position) || INSIDE_METHOD_THROWS_CLAUSE.accepts(position)) { - return psiClass -> InheritanceUtil.isInheritor(psiClass, CommonClassNames.JAVA_LANG_THROWABLE); + else if (JavaSmartCompletionContributor.AFTER_THROW_NEW.accepts(position)) { + return PREFER_THROWABLE; } if (IN_RESOURCE.accepts(position)) { - return psiClass -> InheritanceUtil.isInheritor(psiClass, CommonClassNames.JAVA_LANG_AUTO_CLOSEABLE); + return psiClass -> preferClassIf(InheritanceUtil.isInheritor(psiClass, CommonClassNames.JAVA_LANG_AUTO_CLOSEABLE)); } if (psiElement().withParents(PsiJavaCodeReferenceElement.class, PsiAnnotation.class).accepts(position)) { final PsiAnnotation annotation = PsiTreeUtil.getParentOfType(position, PsiAnnotation.class); assert annotation != null; final PsiAnnotation.TargetType[] targets = AnnotationTargetUtil.getTargetsForLocation(annotation.getOwner()); - return psiClass -> psiClass.isAnnotationType() && AnnotationTargetUtil.findAnnotationTarget(psiClass, targets) != null; + return psiClass -> preferClassIf(psiClass.isAnnotationType() && AnnotationTargetUtil.findAnnotationTarget(psiClass, targets) != null); } - return Conditions.alwaysFalse(); + return aClass -> MyResult.classNameOrGlobalStatic; + } + + static boolean isExceptionPosition(PsiElement position) { + return IN_CATCH_TYPE.accepts(position) || IN_MULTI_CATCH_TYPE.accepts(position) || + INSIDE_METHOD_THROWS_CLAUSE.accepts(position) || + JavaDocCompletionContributor.THROWS_TAG_EXCEPTION.accepts(position); + } + + @NotNull + private static MyResult preferClassIf(boolean condition) { + return condition ? MyResult.suitableClass : MyResult.classNameOrGlobalStatic; } enum MyResult { @@ -145,6 +154,7 @@ public class PreferByKindWeigher extends LookupElementWeigher { normal, collectionFactory, expectedTypeMethod, + verySuitableClass, suitableClass, nonInitialized, classNameOrGlobalStatic, @@ -171,7 +181,9 @@ public class PreferByKindWeigher extends LookupElementWeigher { if (object instanceof PsiLocalVariable || object instanceof PsiParameter || object instanceof PsiThisExpression || object instanceof PsiField && !((PsiField)object).hasModifierProperty(PsiModifier.STATIC)) { - return isExpectedTypeItem(item) ? MyResult.expectedTypeVariable : MyResult.variable; + if (PsiTreeUtil.getParentOfType(myPosition, PsiDocComment.class) == null) { + return isExpectedTypeItem(item) ? MyResult.expectedTypeVariable : MyResult.variable; + } } if (object instanceof String && item.getUserData(JavaCompletionUtil.SUPER_METHOD_PARAMETERS) == Boolean.TRUE) { @@ -238,10 +250,7 @@ public class PreferByKindWeigher extends LookupElementWeigher { } if (object instanceof PsiClass) { - if (myRequiredSuper.value((PsiClass)object)) { - return MyResult.suitableClass; - } - return MyResult.classNameOrGlobalStatic; + return myClassSuitability.apply((PsiClass)object); } if (object instanceof PsiField && myNonInitializedFields.contains(object)) { diff --git a/java/java-tests/testData/codeInsight/completion/normalSorting/PreferExceptionsInJavadocThrows.java b/java/java-tests/testData/codeInsight/completion/normalSorting/PreferExceptionsInJavadocThrows.java new file mode 100644 index 000000000000..33d026523a6e --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/normalSorting/PreferExceptionsInJavadocThrows.java @@ -0,0 +1,8 @@ +class IFoo { + /** + * @throws I + */ + void foo() { + throw new IllegalArgumentException(); + } +} diff --git a/java/java-tests/testData/codeInsight/completion/normalSorting/PreferExceptionsInThrowsList.java b/java/java-tests/testData/codeInsight/completion/normalSorting/PreferExceptionsInThrowsList.java new file mode 100644 index 000000000000..5b05eb081439 --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/normalSorting/PreferExceptionsInThrowsList.java @@ -0,0 +1,5 @@ +class Foo { + void foo() throws I { + throw new IllegalStateException(); + } +} diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/completion/NormalCompletionOrderingTest.groovy b/java/java-tests/testSrc/com/intellij/java/codeInsight/completion/NormalCompletionOrderingTest.groovy index af9e031817e7..c701a284ecf2 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/completion/NormalCompletionOrderingTest.groovy +++ b/java/java-tests/testSrc/com/intellij/java/codeInsight/completion/NormalCompletionOrderingTest.groovy @@ -828,4 +828,21 @@ class Foo { checkPreferredItems 0, 'String', 'public String getZoo', 'public String toString' } + void testPreferExceptionsInCatch() { + myFixture.configureByText 'a.java', 'class Foo { { Enu } }' + myFixture.completeBasic() + myFixture.type('m\n') // select 'Enum' + myFixture.type('; try {} catch(E') + myFixture.completeBasic() + myFixture.assertPreferredCompletionItems 0, 'Exception', 'Error' + } + + void testPreferExceptionsInThrowsList() { + checkPreferredItems 0, 'IllegalStateException', 'IllegalAccessException', 'IllegalArgumentException' + } + + void testPreferExceptionsInJavadocThrows() { + checkPreferredItems 0, 'IllegalArgumentException', 'IllegalAccessException', 'IllegalStateException' + } + }