From 68b7ce46db80a6d18b51de0f3250b3572a7b3773 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Mon, 23 Dec 2013 20:36:12 +0100 Subject: [PATCH] fix and clarify "Cyclic class dependency" inspection warning --- .../util/RefEntityAlphabeticalComparator.java | 0 .../siyeh/InspectionGadgetsBundle.properties | 2 + .../CyclicClassDependencyInspection.java | 53 +++++++++++++------ .../cyclic_class_dependency/expected.xml | 44 +++++++++++++++ .../cyclic_class_dependency/src/Cyclic.java | 29 ++++++++++ .../CyclicClassDependencyInspectionTest.java | 29 ++++++++++ 6 files changed, 141 insertions(+), 16 deletions(-) rename platform/{lang-impl => analysis-api}/src/com/intellij/codeInspection/util/RefEntityAlphabeticalComparator.java (100%) create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/abstraction/cyclic_class_dependency/expected.xml create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/abstraction/cyclic_class_dependency/src/Cyclic.java create mode 100644 plugins/InspectionGadgets/testsrc/com/siyeh/ig/abstraction/CyclicClassDependencyInspectionTest.java diff --git a/platform/lang-impl/src/com/intellij/codeInspection/util/RefEntityAlphabeticalComparator.java b/platform/analysis-api/src/com/intellij/codeInspection/util/RefEntityAlphabeticalComparator.java similarity index 100% rename from platform/lang-impl/src/com/intellij/codeInspection/util/RefEntityAlphabeticalComparator.java rename to platform/analysis-api/src/com/intellij/codeInspection/util/RefEntityAlphabeticalComparator.java diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties index d5450a957c9d..0dacd8023fa7 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties @@ -1510,6 +1510,8 @@ class.with.too.many.transitive.dependencies.max.option=Maximum number of transit class.with.too.many.transitive.dependents.max.option=Maximum number of transitive dependents cyclic.class.dependency.display.name=Cyclic class dependency cyclic.class.dependency.problem.descriptor=Class ''{0}'' is cyclically dependent on {1} other classes +cyclic.class.dependency.1.problem.descriptor=Class ''{0}'' is cyclically dependent on class ''{1}'' +cyclic.class.dependency.2.problem.descriptor=Class ''{0}'' is cyclically dependent on classes ''{1}'' and ''{2}'' cyclic.package.dependency.display.name=Cyclic package dependency cyclic.package.dependency.problem.descriptor=Package ''{0}'' is cyclically dependent on {1} other packages class.unconnected.to.package.display.name=Class independent of its package diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/dependency/CyclicClassDependencyInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/dependency/CyclicClassDependencyInspection.java index 355d5933d511..cb512f97ecbd 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/dependency/CyclicClassDependencyInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/dependency/CyclicClassDependencyInspection.java @@ -16,17 +16,19 @@ package com.siyeh.ig.dependency; import com.intellij.analysis.AnalysisScope; -import com.intellij.codeInspection.CommonProblemDescriptor; -import com.intellij.codeInspection.GlobalInspectionContext; -import com.intellij.codeInspection.InspectionManager; +import com.intellij.codeInspection.*; import com.intellij.codeInspection.reference.RefClass; import com.intellij.codeInspection.reference.RefEntity; +import com.intellij.codeInspection.util.RefEntityAlphabeticalComparator; +import com.intellij.psi.PsiAnonymousClass; import com.intellij.psi.PsiClass; +import com.intellij.psi.PsiElement; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseGlobalInspection; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.Arrays; import java.util.HashSet; import java.util.Set; @@ -35,8 +37,7 @@ public class CyclicClassDependencyInspection extends BaseGlobalInspection { @NotNull @Override public String getDisplayName() { - return InspectionGadgetsBundle.message( - "cyclic.class.dependency.display.name"); + return InspectionGadgetsBundle.message("cyclic.class.dependency.display.name"); } @Override @@ -54,22 +55,42 @@ public class CyclicClassDependencyInspection extends BaseGlobalInspection { if (aClass == null || aClass.getContainingClass() != null) { return null; } - final Set dependencies = - DependencyUtils.calculateTransitiveDependenciesForClass(refClass); - final Set dependents = - DependencyUtils.calculateTransitiveDependentsForClass(refClass); - final Set mutualDependents = - new HashSet(dependencies); + final Set dependencies = DependencyUtils.calculateTransitiveDependenciesForClass(refClass); + final Set dependents = DependencyUtils.calculateTransitiveDependentsForClass(refClass); + final Set mutualDependents = new HashSet(dependencies); mutualDependents.retainAll(dependents); final int numMutualDependents = mutualDependents.size(); - if (numMutualDependents <= 1) { + if (numMutualDependents == 0) { return null; } - final String errorString = InspectionGadgetsBundle.message( - "cyclic.class.dependency.problem.descriptor", - refEntity.getName(), Integer.valueOf(numMutualDependents - 1)); + final String errorString; + if (numMutualDependents == 1) { + final RefClass[] classes = mutualDependents.toArray(new RefClass[1]); + errorString = InspectionGadgetsBundle.message("cyclic.class.dependency.1.problem.descriptor", + refEntity.getName(), classes[0].getExternalName()); + } + else if (numMutualDependents == 2) { + final RefClass[] classes = mutualDependents.toArray(new RefClass[2]); + Arrays.sort(classes, RefEntityAlphabeticalComparator.getInstance()); + errorString = InspectionGadgetsBundle.message("cyclic.class.dependency.2.problem.descriptor", + refEntity.getName(), classes[0].getExternalName(), classes[1].getExternalName()); + } + else { + errorString = InspectionGadgetsBundle.message("cyclic.class.dependency.problem.descriptor", + refEntity.getName(), Integer.valueOf(numMutualDependents)); + } + final PsiElement anchor; + if (aClass instanceof PsiAnonymousClass) { + final PsiAnonymousClass anonymousClass = (PsiAnonymousClass)aClass; + anchor = anonymousClass.getBaseClassReference(); + } + else { + anchor = aClass.getNameIdentifier(); + if (anchor == null) return null; + } return new CommonProblemDescriptor[]{ - inspectionManager.createProblemDescriptor(errorString) + inspectionManager.createProblemDescriptor(anchor, errorString, (LocalQuickFix)null, + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, false) }; } } diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/abstraction/cyclic_class_dependency/expected.xml b/plugins/InspectionGadgets/test/com/siyeh/igtest/abstraction/cyclic_class_dependency/expected.xml new file mode 100644 index 000000000000..9a82ac1597c4 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/abstraction/cyclic_class_dependency/expected.xml @@ -0,0 +1,44 @@ + + + + Cyclic.java + 9 + Cyclic class dependency + Class 'anonymous (java.lang.Object)' is cyclically dependent on 3 other classes + + + + Cyclic.java + 17 + Cyclic class dependency + Class 'Base' is cyclically dependent on classes 'com.siyeh.igtest.abstraction.cyclic_class_dependency.src.Cyclic' and 'com.siyeh.igtest.abstraction.cyclic_class_dependency.src.Top' + + + + Cyclic.java + 29 + Cyclic class dependency + Class 'Coffee' is cyclically dependent on class 'com.siyeh.igtest.abstraction.cyclic_class_dependency.src.FiveOClock' + + + + Cyclic.java + 25 + Cyclic class dependency + Class 'FiveOClock' is cyclically dependent on class 'com.siyeh.igtest.abstraction.cyclic_class_dependency.src.Coffee' + + + + Cyclic.java + 22 + Cyclic class dependency + Class 'Top' is cyclically dependent on classes 'com.siyeh.igtest.abstraction.cyclic_class_dependency.src.Base' and 'com.siyeh.igtest.abstraction.cyclic_class_dependency.src.Cyclic' + + + + Cyclic.java + 6 + Cyclic class dependency + Class 'Cyclic' is cyclically dependent on classes 'com.siyeh.igtest.abstraction.cyclic_class_dependency.src.Base' and 'com.siyeh.igtest.abstraction.cyclic_class_dependency.src.Top' + + \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/abstraction/cyclic_class_dependency/src/Cyclic.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/abstraction/cyclic_class_dependency/src/Cyclic.java new file mode 100644 index 000000000000..adf929840c41 --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/abstraction/cyclic_class_dependency/src/Cyclic.java @@ -0,0 +1,29 @@ +package com.siyeh.igtest.abstraction.cyclic_class_dependency.src; + +/** + * @author Bas Leijdekkers + */ +public class Cyclic extends Base { + + Cyclic() { + new Object() {{ + foo(); + }}; + } + + void foo() {} + +} +class Base { + void a() { + Top.m(); + } +} +class Top extends Cyclic { + public static void m() {} +} +interface FiveOClock { + void m(Coffee c); + +} +interface Coffee extends FiveOClock {} diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/abstraction/CyclicClassDependencyInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/abstraction/CyclicClassDependencyInspectionTest.java new file mode 100644 index 000000000000..08e2faa28d11 --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/abstraction/CyclicClassDependencyInspectionTest.java @@ -0,0 +1,29 @@ +/* + * Copyright 2000-2013 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.siyeh.ig.abstraction; + +import com.siyeh.ig.IGInspectionTestCase; +import com.siyeh.ig.dependency.CyclicClassDependencyInspection; + +/** + * @author Bas Leijdekkers + */ +public class CyclicClassDependencyInspectionTest extends IGInspectionTestCase { + + public void test() { + doTest("com/siyeh/igtest/abstraction/cyclic_class_dependency", new CyclicClassDependencyInspection()); + } +}