SONAR-2333 Add new rule to report about cycles between packages

5 files changed, 85 insertions, 0 deletions

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 @@
+<p>
+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.
+</p>
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<Edge> 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
    */