From e394a262f87201ea9c646495e6c06f793447caf9 Mon Sep 17 00:00:00 2001 From: Evgeny Mandrikov Date: Thu, 5 Jul 2012 16:40:24 +0600 Subject: [PATCH] SONAR-2333 Add new rule to report about cycles between packages --- .../org/sonar/l10n/squidjava.properties | 2 + .../rules/squid/CycleBetweenPackages.html | 7 +++ .../check/CycleBetweenPackagesCheck.java | 48 +++++++++++++++++++ .../plugins/squid/SquidRuleRepository.java | 1 + .../plugins/squid/bridges/DesignBridge.java | 27 +++++++++++ 5 files changed, 85 insertions(+) create mode 100644 plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/squidjava/rules/squid/CycleBetweenPackages.html create mode 100644 plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/squid/check/CycleBetweenPackagesCheck.java diff --git a/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/squidjava.properties b/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/squidjava.properties index c178b1b078c..d74bc3c0bf5 100644 --- a/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/squidjava.properties +++ b/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/squidjava.properties @@ -29,3 +29,5 @@ rule.squid.UnusedPrivateMethod.name=Unused private method rule.squid.UnusedProtectedMethod.name=Unused protected method rule.squid.CommentedOutCodeLine.name=Avoid commented-out lines of code + +rule.squid.CycleBetweenPackages.name=Avoid cycle between java packages diff --git a/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/squidjava/rules/squid/CycleBetweenPackages.html b/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/squidjava/rules/squid/CycleBetweenPackages.html new file mode 100644 index 00000000000..6bab29f493b --- /dev/null +++ b/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/squidjava/rules/squid/CycleBetweenPackages.html @@ -0,0 +1,7 @@ +

+When several packages are involved in a cycle (package A > package B > package C > package A where ">" means "depends upon"), +that means that those packages are highly coupled and that there is no way to reuse/extract one of those packages without importing all the other packages. +Such cycle could quickly increase the effort required to maintain an application and to embrace business change. +Sonar not only detect cycles between packages but also determines what is the minimum effort to break those cycles. +This rule log a violation on each source file having an outgoing dependency to be but in order to break a cycle. +

diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/squid/check/CycleBetweenPackagesCheck.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/squid/check/CycleBetweenPackagesCheck.java new file mode 100644 index 00000000000..dfa13c23536 --- /dev/null +++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/squid/check/CycleBetweenPackagesCheck.java @@ -0,0 +1,48 @@ +/* + * Sonar, open source software quality management tool. + * Copyright (C) 2008-2012 SonarSource + * mailto:contact AT sonarsource DOT com + * + * Sonar is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * Sonar is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with Sonar; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package org.sonar.java.squid.check; + +import org.sonar.api.checks.CheckFactory; +import org.sonar.api.rules.ActiveRule; +import org.sonar.check.Priority; +import org.sonar.check.Rule; + +import javax.annotation.CheckForNull; + +/** + * Companion of {@link org.sonar.plugins.squid.bridges.DesignBridge} which actually does the job on finding cycles and creation of violations. + */ +@Rule(key = "CycleBetweenPackages", priority = Priority.MAJOR) +public class CycleBetweenPackagesCheck extends SquidCheck { + + /** + * @return null, if this check is inactive + */ + @CheckForNull + public static ActiveRule getActiveRule(CheckFactory checkFactory) { + for (Object check : checkFactory.getChecks()) { + if (check.getClass().getName() == CycleBetweenPackagesCheck.class.getName()) { + return checkFactory.getActiveRule(check); + } + } + return null; + } + +} diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/plugins/squid/SquidRuleRepository.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/plugins/squid/SquidRuleRepository.java index 4e410c4fba9..12a9dbaaa45 100644 --- a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/plugins/squid/SquidRuleRepository.java +++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/plugins/squid/SquidRuleRepository.java @@ -60,6 +60,7 @@ public final class SquidRuleRepository extends RuleRepository { UndocumentedApiCheck.class, ContinueCheck.class, BreakCheck.class, // Squid checks DITCheck.class, ClassComplexityCheck.class, MethodComplexityCheck.class, NoSonarCheck.class, EmptyFileCheck.class, + CycleBetweenPackagesCheck.class, CommentedOutCodeLineCheck.class); } } diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/plugins/squid/bridges/DesignBridge.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/plugins/squid/bridges/DesignBridge.java index cfffeb395b9..6e279b03220 100644 --- a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/plugins/squid/bridges/DesignBridge.java +++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/plugins/squid/bridges/DesignBridge.java @@ -29,8 +29,11 @@ import org.sonar.api.measures.Metric; import org.sonar.api.measures.PersistenceMode; import org.sonar.api.resources.Project; import org.sonar.api.resources.Resource; +import org.sonar.api.rules.ActiveRule; +import org.sonar.api.rules.Violation; import org.sonar.api.utils.TimeProfiler; import org.sonar.graph.*; +import org.sonar.java.squid.check.CycleBetweenPackagesCheck; import org.sonar.squid.Squid; import org.sonar.squid.api.SourceCode; import org.sonar.squid.api.SourceCodeEdge; @@ -70,6 +73,7 @@ public class DesignBridge extends Bridge { LOG.debug("{} feedback edges", feedbackEdges.size()); int tangles = cyclesAndFESSolver.getWeightOfFeedbackEdgeSet(); + saveViolations(feedbackEdges); savePositiveMeasure(sonarProject, CoreMetrics.PACKAGE_CYCLES, cyclesAndFESSolver.getCycles().size()); savePositiveMeasure(sonarProject, CoreMetrics.PACKAGE_FEEDBACK_EDGES, feedbackEdges.size()); savePositiveMeasure(sonarProject, CoreMetrics.PACKAGE_TANGLES, tangles); @@ -145,6 +149,29 @@ public class DesignBridge extends Bridge { } } + private void saveViolations(Set feedbackEdges) { + ActiveRule rule = CycleBetweenPackagesCheck.getActiveRule(checkFactory); + if (rule == null) { + // Rule inactive + return; + } + for (Edge feedbackEdge : feedbackEdges) { + SourceCode fromPackage = (SourcePackage) feedbackEdge.getFrom(); + SourceCode toPackage = (SourcePackage) feedbackEdge.getTo(); + SourceCodeEdge edge = squid.getEdge(fromPackage, toPackage); + for (SourceCodeEdge subEdge : edge.getRootEdges()) { + Resource fromFile = resourceIndex.get(subEdge.getFrom()); + Resource toFile = resourceIndex.get(subEdge.getTo()); + // If resource cannot be obtained, then silently ignore, because anyway warning will be printed by method saveEdge + if ((fromFile != null) && (toFile != null)) { + Violation violation = Violation.create(rule, fromFile) + .setMessage("Remove the dependency on the source file \"" + toFile.getLongName() + "\" to break a package cycle."); + context.saveViolation(violation); + } + } + } + } + /** * Save file dependencies */ -- 2.39.5