From 82099584a0db2d832e608cb639e6625f23514f6d Mon Sep 17 00:00:00 2001 From: Nikita Eshkeev Date: Wed, 22 Jul 2020 01:15:02 +0300 Subject: [PATCH] [codeInsight] IDEA-201714 Missing "fix all" for "Redundant throws clause" This patch fixes the notes from the code review it includes: - splitting the code that detects related @throws in the local inspection from the code that is used in the global one - related @throws are no longer returned if one of the duplicates in the throws list is being removed - related @throw tags are not being removed if there is an element in the throws list that can be assigned with the class of the @throws tag Signed-off-by: Nikita Eshkeev GitOrigin-RevId: e68823093d88427dbbb7efa0cd1171988d1f5360 --- .../RedundantThrowsDeclarationInspection.java | 72 +++++++++++++++++-- ...ndantThrowsDeclarationLocalInspection.java | 67 +++++++++++------ .../methodThrows/afterRemoveDuplicate.java | 4 ++ .../afterRemoveIgnoreSpecific.java | 1 + .../methodThrows/afterRemoveSpecific.java | 1 + 5 files changed, 117 insertions(+), 28 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/unneededThrows/RedundantThrowsDeclarationInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/unneededThrows/RedundantThrowsDeclarationInspection.java index a15736415902..9828abe7466f 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/unneededThrows/RedundantThrowsDeclarationInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/unneededThrows/RedundantThrowsDeclarationInspection.java @@ -4,19 +4,21 @@ package com.intellij.codeInspection.unneededThrows; import com.intellij.analysis.AnalysisScope; import com.intellij.codeInsight.FileModificationService; import com.intellij.codeInsight.daemon.impl.analysis.JavaHighlightUtil; +import com.intellij.codeInsight.javadoc.JavaDocUtil; import com.intellij.codeInspection.*; import com.intellij.codeInspection.reference.*; import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; -import com.intellij.codeInspection.unneededThrows.RedundantThrowsDeclarationLocalInspection.RedundantThrowsVisitor.RedundantThrowsQuickFix; import com.intellij.codeInspection.unneededThrows.RedundantThrowsDeclarationLocalInspection.ThrowRefType; import com.intellij.java.analysis.JavaAnalysisBundle; import com.intellij.lang.jvm.JvmModifier; import com.intellij.openapi.application.WriteAction; import com.intellij.openapi.project.Project; import com.intellij.psi.*; +import com.intellij.psi.javadoc.PsiDocComment; import com.intellij.psi.javadoc.PsiDocTag; import com.intellij.psi.search.searches.OverridingMethodsSearch; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.PsiTypesUtil; import com.intellij.util.ObjectUtils; import com.intellij.util.Query; import com.intellij.util.containers.ContainerUtil; @@ -27,6 +29,7 @@ import org.jetbrains.annotations.Nullable; import javax.swing.*; import java.util.Arrays; +import java.util.List; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -212,7 +215,7 @@ public final class RedundantThrowsDeclarationInspection extends GlobalJavaBatchI final StreamEx elements = RedundantThrowsDeclarationLocalInspection.getRedundantThrowsCandidates(psiMethod, myIgnoreEntryPoints) .filter(throwRefType -> exceptionType.isAssignableFrom(throwRefType.getType())) .map(ThrowRefType::getReference) - .flatMap(ref -> appendRelatedJavadocThrows(psiMethod, ref)); + .flatMap(ref -> appendRelatedJavadocThrows(refMethod, psiMethod, ref)); final Stream tail; if (refMethod != null) { @@ -229,10 +232,69 @@ public final class RedundantThrowsDeclarationInspection extends GlobalJavaBatchI return elements.append(tail); } - private static StreamEx appendRelatedJavadocThrows(@NotNull final PsiMethod psiMethod, @NotNull final PsiJavaCodeReferenceElement ref) { - final Stream relatedJavadocThrows = RedundantThrowsQuickFix.getRelatedJavadocThrows(ref, psiMethod.getDocComment()); + /** + * The method constructs a {@link StreamEx} or {@link PsiElement} by concatenating + * a singleton {@link Stream} that contains the current throws list's element and the related javadoc. + * Related javadoc are the ones that can be assigned to any of the redundant throws list elements + * + * @param refMethod a node in the reference graph corresponding to the Java method. + * @param psiMethod an instance of {@link PsiMethod} the current throws list's element is related to + * @param ref the current throws list's element to append related @throws tags to + * @return a {@link StreamEx} that contains both the current throws list's element and its related javadoc @throws tags + */ + private StreamEx appendRelatedJavadocThrows(@Nullable final RefMethod refMethod, + @NotNull final PsiMethod psiMethod, + @NotNull final PsiJavaCodeReferenceElement ref) { + final StreamEx res = StreamEx.of(ref); + if (refMethod == null) return res; - return StreamEx.of((PsiElement)ref).append(relatedJavadocThrows); + final PsiDocComment comment = psiMethod.getDocComment(); + if (comment == null) return res; + + final PsiClass[] unThrown = refMethod.getUnThrownExceptions(); + if (unThrown == null) return res; + + final Set unThrownSet = ContainerUtil.set(unThrown); + + final List redundantThrows = RedundantThrowsDeclarationLocalInspection.getRedundantThrowsCandidates(psiMethod, myIgnoreEntryPoints) + .filter(throwRefType -> unThrownSet.contains(throwRefType.getType().resolve())) + .map(ThrowRefType::getType) + .toList(); + + final StreamEx javadocThrows = StreamEx.of(comment.getTags()) + .filterBy(PsiDocTag::getName, "throws"); + + // if there is only one element in the throws list and there is one redundant throws element, + // it must be the same element, so return all the @throws tags that are in the javadoc + final PsiJavaCodeReferenceElement[] throwsListElements = psiMethod.getThrowsList().getReferenceElements(); + if (throwsListElements.length == 1 && redundantThrows.size() == 1) { + return res.append(javadocThrows); + } + + final StreamEx relatedJavadocThrows = javadocThrows + .filter(tag -> isTagRelatedToRedundantThrow(tag, redundantThrows)); + + return res.append(relatedJavadocThrows); + } + + /** + * A @throws tag is considered related to an element of redundant throws declarations + * if it can be assigned to the element. + * @param tag the @throws tag in the javadoc + * @param redundantThrows the list of redundant throws declarations in the throws list of a method + * @return true if there is at least one element in the list of redundant throws that can be assigned with the class of the @throws tag, + * false otherwise + */ + private static boolean isTagRelatedToRedundantThrow(@NotNull final PsiDocTag tag, + @NotNull final List redundantThrows) { + assert "throws".equals(tag.getName()) : "the tag has to be of the @throws kind"; + + final PsiClass throwsClass = JavaDocUtil.resolveClassInTagValue(tag.getValueElement()); + if (throwsClass == null) return false; + final PsiClassType type = PsiTypesUtil.getClassType(throwsClass); + + return redundantThrows.stream() + .anyMatch(e -> e.isAssignableFrom(type)); } @Override diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/unneededThrows/RedundantThrowsDeclarationLocalInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/unneededThrows/RedundantThrowsDeclarationLocalInspection.java index 5e3ab0c28903..918f283b958b 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/unneededThrows/RedundantThrowsDeclarationLocalInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/unneededThrows/RedundantThrowsDeclarationLocalInspection.java @@ -1,4 +1,4 @@ -// Copyrioht 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +// Copyright 2000-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.intellij.codeInspection.unneededThrows; import com.intellij.codeInsight.ExceptionUtil; @@ -20,6 +20,7 @@ import com.intellij.psi.javadoc.PsiDocComment; import com.intellij.psi.javadoc.PsiDocTag; import com.intellij.psi.search.GlobalSearchScope; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.PsiTypesUtil; import com.intellij.util.ArrayUtil; import com.siyeh.ig.JavaOverridingMethodUtil; import com.siyeh.ig.psiutils.CommentTracker; @@ -27,6 +28,7 @@ import one.util.streamex.StreamEx; import org.jetbrains.annotations.*; import java.util.Arrays; +import java.util.List; import java.util.Set; import java.util.function.Predicate; import java.util.stream.Stream; @@ -156,17 +158,17 @@ public final class RedundantThrowsDeclarationLocalInspection extends AbstractBas } /** - * The method returns a {@link Stream} of @throws declarations from the javadoc that are related to the throw declaration. + * The method returns a {@link Stream} of @throws tags from the javadoc that are related to the throw declaration. * This method works like this: *
    *
  • If the javadoc is null, then return {@link Stream#empty()}
    • - *
  • All the @throws declarations that are subclasses of the current throw declaration are returned except if there is a more specific throw declaration in the throws list that matches exactly the @throws tag.
    • *
  • If there are no other throws declarations in the throws list but the current one then all the @throws declarations are returned.
    • - *
  • If there are duplicates in the throws list and one of the duplicates is being removed then all @throws tag that match the throws list's declaration are returned (either the same class or an inheritor of it).
    • + *
  • All the @throws tags that are subclasses of the current throws declaration are returned except if there are no other throws declarations in the throws list that can be parents of the same @throws tag.
    • + *
  • If there are duplicates in the throws list and one of them is being removed then no related @throws tags is returned.
    • *
- * @param currentThrowsRef current throws list element to get the {@link Stream} for + * @param currentThrowsRef current throws list's element to get the {@link Stream} for * @param comment current javadoc - * @return + * @return a {@link Stream} of {@link PsiDocTag} with related @throws tag from the javadoc */ @NotNull static Stream getRelatedJavadocThrows(@NotNull final PsiJavaCodeReferenceElement currentThrowsRef, @@ -180,27 +182,47 @@ public final class RedundantThrowsDeclarationLocalInspection extends AbstractBas // return all @throws declarations from javadoc if the last throws declaration in the throws list is getting eliminated. if (throwsList.getReferenceElements().length == 1) { - return Arrays.stream(comment.getTags()) - .filter(tag -> "throws".equals(tag.getName())); + return StreamEx.of(comment.getTags()) + .filterBy(PsiDocTag::getName, "throws"); } final PsiElement maybeClass = currentThrowsRef.resolve(); if (!(maybeClass instanceof PsiClass)) return Stream.empty(); final PsiClass reference = (PsiClass)maybeClass; - if (reference.getQualifiedName() == null) return Stream.empty(); - final Set throwsListWithoutCurrent = getThrowsListWithoutCurrent(throwsList, currentThrowsRef); + final List throwsListWithoutCurrent = getThrowsListWithoutCurrent(throwsList, currentThrowsRef); + + final PsiClassType referenceType = PsiTypesUtil.getClassType(reference); + if (throwsListWithoutCurrent.contains(referenceType)) return Stream.empty(); + + final PsiManager manager = reference.getManager(); + + final Predicate isTagRelatedToCurrentThrowsRef = tag -> { + final PsiClass throwsClass = JavaDocUtil.resolveClassInTagValue(tag.getValueElement()); + if (throwsClass == null) return false; + // either the tag's class is exactly the current throws reference + // or it's a inheritor of the throws declaration and there are no other parents in the throws list + return manager.areElementsEquivalent(throwsClass, reference) || + (throwsClass.isInheritor(reference, true) && !isParentInThrowsListPresent(throwsClass, throwsListWithoutCurrent)); + }; return StreamEx.of(comment.getTags()) - .filterBy(tag -> tag.getName(), "throws") - .filter(tag -> { - final PsiClass throwsClass = JavaDocUtil.resolveClassInTagValue(tag.getValueElement()); - if (throwsClass == null) return false; - if (throwsClass.getQualifiedName() == null) return false; - return throwsClass.getManager().areElementsEquivalent(throwsClass, reference) || - (!throwsListWithoutCurrent.contains(throwsClass.getQualifiedName()) && throwsClass.isInheritor(reference, true)); - }); + .filterBy(PsiDocTag::getName, "throws") + .filter(isTagRelatedToCurrentThrowsRef); + } + + /** + * Checks if there are classes in the throws list that can be parents of the class + * @param clazz a class to check if there are parents in the throws list for it + * @param throwsList a list of throws list + * @return true if there is at least one element in the list that can be a parent of the class, false otherwise + */ + private static boolean isParentInThrowsListPresent(@NotNull final PsiClass clazz, + @NotNull final List throwsList) { + final PsiClassType type = PsiTypesUtil.getClassType(clazz); + return throwsList.stream() + .anyMatch(e -> e.isAssignableFrom(type)); } /** @@ -210,13 +232,12 @@ public final class RedundantThrowsDeclarationLocalInspection extends AbstractBas * @param currentRef the currently eliminated throws declaration in the throws list * @return the set of throws declarations as strings from the throws list excluding the currently eliminated throws declaration */ - private static Set getThrowsListWithoutCurrent(@NotNull final PsiReferenceList throwsList, - @NotNull final PsiJavaCodeReferenceElement currentRef) { + private static List getThrowsListWithoutCurrent(@NotNull final PsiReferenceList throwsList, + @NotNull final PsiJavaCodeReferenceElement currentRef) { return StreamEx.zip(throwsList.getReferenceElements(), throwsList.getReferencedTypes(), Pair::create) .filter(pair -> pair.getFirst() != currentRef) .map(pair -> pair.getSecond()) - .map(PsiType::getCanonicalText) - .toSet(); + .toList(); } } } @@ -309,4 +330,4 @@ public final class RedundantThrowsDeclarationLocalInspection extends AbstractBas return myType; } } -} +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/methodThrows/afterRemoveDuplicate.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/methodThrows/afterRemoveDuplicate.java index a83e02d92fb1..e2b6e15d68f2 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/methodThrows/afterRemoveDuplicate.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/methodThrows/afterRemoveDuplicate.java @@ -7,6 +7,10 @@ class A { /** * @since 2020.3 * @author me + * @throws Exception first exception + * @throws Exception second exception + * @throws FileNotFoundException file not found + * @throws IOException IO exception */ void f() throws /* 1 */ Exception /* 2 */ /* 3 */ /* 4 */ {} } diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/methodThrows/afterRemoveIgnoreSpecific.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/methodThrows/afterRemoveIgnoreSpecific.java index d2b554786f10..d504ef49e612 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/methodThrows/afterRemoveIgnoreSpecific.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/methodThrows/afterRemoveIgnoreSpecific.java @@ -7,6 +7,7 @@ class A { /** * @since 2020.3 * @author me + * @throws FileNotFoundException file not found * @throws IOException IO exception */ void f() throws /* 1 */ /* 2 */ /* 3 */ IOException /* 4 */ {} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/methodThrows/afterRemoveSpecific.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/methodThrows/afterRemoveSpecific.java index 3f01cc983998..f813dd807c88 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/methodThrows/afterRemoveSpecific.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/methodThrows/afterRemoveSpecific.java @@ -9,6 +9,7 @@ class A { * @author me * @throws Exception first exception * @throws Exception second exception + * @throws FileNotFoundException file not found */ void f() throws /* 1 */ Exception /* 2 */ /* 3 */ /* 4 */ {} }