Check fqn prefix for dependencies, not direct match.

dub.sdl

@@ -11,7 +11,7 @@ configuration "debug" {
     targetPath "build"
 }
 configuration "unittest" {
-    dependency "dshould" version="~>0"
+    dependency "dshould" version="~>1"
     dependency "unit-threaded" version="*"
     mainSourceFile "build/ut.d"
     excludedSourceFiles "src/main.d"

src/deps.d

@@ -1,5 +1,6 @@
 module deps;
 
+import std.algorithm;
 import std.range;
 import std.regex;
 import std.stdio;
@@ -85,3 +86,81 @@ unittest
 
     reader(only(line)).should.equal(only(tuple(client, supplier)));
 }
+
+class DependencyChecker
+{
+    private Dependency[] targetDependencies;
+
+    // Packages in this list, when used in a dependency, implicitly include subpackages.
+    // Set to false if there's a dependency from a package inside <package> to a package outside <package>.
+    private bool[const string] transitivePackages;
+
+    this(Dependency[] targetDependencies)
+    {
+        import util : crossedPackageBoundaries;
+
+        this.targetDependencies = targetDependencies;
+        this.transitivePackages = targetDependencies.map!(
+            dependency => crossedPackageBoundaries(dependency.client, dependency.supplier))
+            .joiner.assocArray(false.repeat); // crossed package boundaries are not transitive packages
+    }
+
+    bool canDepend(const string client, const string supplier)
+    {
+        import util : fqnStartsWith;
+
+        bool dependencyMatches(const Dependency dependency)
+        {
+            // A -> A.X never allows subpackages of A!
+            // because A -> A.X does not break A's transitivity, there would otherwise
+            // be no way to refer to "submodules of A".
+            const dependencyIsInternal = dependency.supplier.fqnStartsWith(dependency.client);
+
+            bool moduleMatches(const string module_, const string dependencyModule)
+            {
+                const packageIsTransitive = this.transitivePackages.get(dependencyModule, true);
+
+                if (packageIsTransitive && !dependencyIsInternal)
+                {
+                    return module_.fqnStartsWith(dependencyModule);
+                }
+                return module_ == dependencyModule;
+            }
+            return moduleMatches(client, dependency.client) && moduleMatches(supplier, dependency.supplier);
+        }
+
+        return this.targetDependencies.canFind!dependencyMatches;
+    }
+}
+
+@("dependency inside transitive package")
+unittest
+{
+    with (new DependencyChecker([Dependency("X", "Y")]))
+    {
+        canDepend("X.A", "Y.B").should.be(true);
+    }
+}
+
+@("dependency inside non-transitive package")
+unittest
+{
+    // X.A -> Y breaks the transitivity of X because it crosses the X boundary
+    with (new DependencyChecker([
+        Dependency("X", "Y"),
+        Dependency("X.Q", "Y")]))
+    {
+        canDepend("X.A", "Y.B").should.be(false);
+    }
+}
+
+@("cross-dependency inside package where client is transitive and has interior dependency")
+unittest
+{
+    with (new DependencyChecker([
+        Dependency("X", "X.A"),
+        Dependency("X", "X.B")]))
+    {
+        canDepend("X.A", "X.B").should.be(false);
+    }
+}

src/main.d

@@ -6,13 +6,14 @@
 import core.stdc.stdlib;
 import deps;
 import graph;
-import settings : packages, readSettings = read;
+import settings : readSettings = read;
 import std.algorithm;
 import std.array;
 import std.regex;
 import std.stdio;
 import std.typecons;
 import uml;
+import util : crossedPackageBoundaries, fqnStartsWith, packages;
 
 void main(string[] args)
 {
@@ -74,38 +75,43 @@ void main(string[] args)
                 .map!(depsFile => readDependencies(File(depsFile)))
                 .join;
         actualDependencies = actualDependencies.sort.uniq.array;
+
         if (!targetFiles.empty)
         {
+            import std.range : repeat;
             import uml : read;
 
-            uint count = 0;
+            bool errored = false;
             Dependency[] targetDependencies = null;
 
             foreach (targetFile; targetFiles)
                 targetDependencies ~= read(File(targetFile).byLine);
 
             if (!transitive)
                 targetDependencies.transitiveClosure;
+
+            auto checker = new DependencyChecker(targetDependencies);
+
             foreach (dependency; actualDependencies)
             {
-                const client = dependency.client;
-                const supplier = dependency.supplier;
-
-                if (detail)
-                    dependency = Dependency(client, supplier);
-                else
-                {
-                    dependency = Dependency(client.packages, supplier.packages);
-                    if (dependency.client.empty || dependency.supplier.empty || dependency.client == dependency.supplier)
-                        continue;
-                }
-                if (!targetDependencies.canFind(dependency))
+                with (dependency)
                 {
-                    stderr.writefln("error: unintended dependency %s -> %s", client, supplier);
-                    ++count;
+                    if (!detail)
+                    {
+                        dependency = Dependency(client.packages, supplier.packages);
+                        if (client.empty ||
+                            supplier.empty ||
+                            client == supplier)
+                            continue;
+                    }
+                    if (!checker.canDepend(client, supplier))
+                    {
+                        stderr.writefln("error: unintended dependency %s -> %s", client, supplier);
+                        errored = true;
+                    }
                 }
             }
-            if (count > 0)
+            if (errored)
                 exit(EXIT_FAILURE);
         }
         if (dot || targetFiles.empty)

src/settings.d

@@ -87,19 +87,3 @@ unittest
         detail.should.be(true);
     }
 }
-
-string packages(string fullyQualifiedName)
-{
-    import std.range : dropBackOne;
-
-    return fullyQualifiedName.split('.')
-        .dropBackOne
-        .join('.');
-}
-
-@("split packages from a fully-qualified module name")
-unittest
-{
-    packages("bar.baz.foo").should.equal("bar.baz");
-    packages("foo").should.be.empty;
-}

src/util.d

@@ -0,0 +1,84 @@
+module util;
+
+version(unittest) import dshould;
+import std.algorithm;
+import std.range;
+import std.string;
+
+string packages(string fullyQualifiedName)
+{
+    return fullyQualifiedName.split('.')
+        .dropBackOne
+        .join('.');
+}
+
+@("split packages from a fully-qualified module name")
+unittest
+{
+    packages("bar.baz.foo").should.equal("bar.baz");
+    packages("foo").should.be.empty;
+}
+
+bool fqnStartsWith(string haystack, string needle)
+{
+    return haystack.splitter(".").startsWith(needle.splitter("."));
+}
+
+@("fully-qualified module name starts with other name")
+unittest
+{
+    "foo".fqnStartsWith("foo").should.be(true);
+    "foo.bar.baz".fqnStartsWith("foo.bar").should.be(true);
+    "foo".fqnStartsWith("fo").should.be(false);
+    "foo.bar.baz".fqnStartsWith("foo.ba").should.be(false);
+}
+
+/**
+ * This function takes two modules and returns the crossed package boundaries.
+ *
+ * Imagine a diagram of modules, where each module is recursively contained in
+ * successive packages. Now imagine an arrow drawn from the first to the second
+ * module. There may be some package boundaries that this arrow will necessarily have
+ * to cross to get to its destination. This function returns those boundaries.
+ */
+const(string)[] crossedPackageBoundaries(string from, string to)
+{
+    const fromPackages = from.successiveSplitPrefixes(".");
+    const toPackages = to.successiveSplitPrefixes(".");
+    const sharedPackages = fromPackages.commonPrefix(toPackages);
+    const numSharedPackages = count(sharedPackages);
+    const lastCommonPackage = sharedPackages.back; // at least ""
+    const fromUniquePackages = fromPackages.drop(numSharedPackages);
+    const toUniquePackages = toPackages.drop(numSharedPackages);
+    const traversalPath = chain(fromUniquePackages.retro, lastCommonPackage.only, toUniquePackages)
+        .dropOne.dropBackOne;
+
+    return traversalPath.filter!(a => !a.empty).array;
+}
+
+@("import crosses package boundaries")
+unittest
+{
+    crossedPackageBoundaries("a", "b").should.be.empty;
+    crossedPackageBoundaries("a", "b.y").should.be(["b"]);
+    crossedPackageBoundaries("a.x", "b").should.be(["a"]);
+    crossedPackageBoundaries("a.x", "b.y").should.be(["a", "b"]);
+    crossedPackageBoundaries("a.x", "a").should.be.empty;
+    crossedPackageBoundaries("a.x.y", "a").should.be(["a.x"]);
+}
+
+private string[] successiveSplitPrefixes(string haystack, string needle)
+{
+    auto parts = haystack.split(needle);
+
+    return (parts.length + 1).iota.map!(i => parts.take(i).join(needle)).array;
+}
+
+@("generate successive prefixes of a string split at a marker")
+unittest
+{
+    "".successiveSplitPrefixes(".").should.be([""]);
+    "a".successiveSplitPrefixes(".").should.be(["", "a"]);
+    "a.b".successiveSplitPrefixes(".").should.be(["", "a", "a.b"]);
+    "a.b.c".successiveSplitPrefixes(".").should.be(["", "a", "a.b", "a.b.c"]);
+}