Repository: RBusarow/ModuleCheck (kotlin)

Issue:
graph reports with custom output directory are a hot mess
If graphs output is set to a custom directory, like `$rootDir/graphs`, then the files for each sub-module are supposed to be put into relative paths from that root directory, like:
```
./graphs/lib-1/main.dot
./graphs/lib-1/test.dot
./graphs/lib-2/api/main.dot
./graphs/lib-2/api/test.dot
./graphs/lib-2/impl/main.dot
./graphs/lib-2/impl/test.dot
```

Instead, we get this thing:
```
./graphs/build/reports/modulecheck/graphs/main.dot
./graphs/build/reports/modulecheck/graphs/test.dot
./graphs/build/reports/modulecheck/graphs/testFixtures.dot
```

There are two problems:
1. Every module has the same relative output path (`build/reports/modulecheck/graphs/<source set>.dot`), so they all just overwrite each other.
2. The parent directories `build/reports/modulecheck/graphs` should not be there.  This is especially bad because the inclusion of `/build/` means that the report will be automatically ignored by many `.gitignore` files.

Pull request:
fixes #575

Patch:
diff --git a/build.gradle.kts b/build.gradle.kts
index 20d5ec5422..e419691599 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -49,6 +49,12 @@ moduleCheck {
   checks {
     sortDependencies = true
   }
+  reports {
+    graphs {
+      enabled = true
+      outputDir = File(buildDir, "depths").path
+    }
+  }
 }
 
 tasks.named("ktlintFormat") {
diff --git a/modulecheck-core/build.gradle.kts b/modulecheck-core/build.gradle.kts
index a66e1bfc34..bfbf7b08cc 100644
--- a/modulecheck-core/build.gradle.kts
+++ b/modulecheck-core/build.gradle.kts
@@ -57,7 +57,6 @@ dependencies {
   testImplementation(project(path = ":modulecheck-config:fake"))
   testImplementation(project(path = ":modulecheck-internal-testing"))
   testImplementation(project(path = ":modulecheck-parsing:source:api"))
-  testImplementation(project(path = ":modulecheck-project:api"))
   testImplementation(project(path = ":modulecheck-project:testing"))
   testImplementation(project(path = ":modulecheck-runtime:testing"))
   testImplementation(project(path = ":modulecheck-utils"))
diff --git a/modulecheck-reporting/graphviz/build.gradle.kts b/modulecheck-reporting/graphviz/build.gradle.kts
index 953f357d0c..36979b6344 100644
--- a/modulecheck-reporting/graphviz/build.gradle.kts
+++ b/modulecheck-reporting/graphviz/build.gradle.kts
@@ -34,4 +34,11 @@ dependencies {
   testImplementation(libs.bundles.hermit)
   testImplementation(libs.bundles.jUnit)
   testImplementation(libs.bundles.kotest)
+  testImplementation(libs.kotlin.reflect)
+  testImplementation(libs.rickBusarow.dispatch.test.core)
+
+  testImplementation(project(path = ":modulecheck-core"))
+  testImplementation(project(path = ":modulecheck-project:api"))
+  testImplementation(project(path = ":modulecheck-project:testing"))
+  testImplementation(project(path = ":modulecheck-runtime:testing"))
 }
diff --git a/modulecheck-reporting/graphviz/src/main/kotlin/modulecheck/reporting/graphviz/GraphvizFactory.kt b/modulecheck-reporting/graphviz/src/main/kotlin/modulecheck/reporting/graphviz/GraphvizFactory.kt
index e412faa6ff..410e2bce51 100644
--- a/modulecheck-reporting/graphviz/src/main/kotlin/modulecheck/reporting/graphviz/GraphvizFactory.kt
+++ b/modulecheck-reporting/graphviz/src/main/kotlin/modulecheck/reporting/graphviz/GraphvizFactory.kt
@@ -49,8 +49,11 @@ class GraphvizFactory @Inject constructor(
 
       appendLine(
         """
-        |ratio=0.5;
-        |node [style="rounded,filled" shape=box];
+        |ratio = 0.5625;
+        |node [style = "rounded,filled" shape = box];
+        |
+        |labelloc = "t"
+        |label = "${root.dependentPath.value} -- ${root.sourceSetName.value}";
         |
         """.trimMargin()
       )
@@ -139,7 +142,7 @@ class GraphvizFactory @Inject constructor(
           .sortedBy { it.dependentPath }
           .joinToString("; ", postfix = ";") { it.dependentProject.pathString() }
 
-        appendLine("{rank = same; $paths}")
+        appendLine("{ rank = same; $paths }")
       }
   }
 
diff --git a/modulecheck-reporting/graphviz/src/main/kotlin/modulecheck/reporting/graphviz/GraphvizFileWriter.kt b/modulecheck-reporting/graphviz/src/main/kotlin/modulecheck/reporting/graphviz/GraphvizFileWriter.kt
index caa342a38b..3c73b1ea50 100644
--- a/modulecheck-reporting/graphviz/src/main/kotlin/modulecheck/reporting/graphviz/GraphvizFileWriter.kt
+++ b/modulecheck-reporting/graphviz/src/main/kotlin/modulecheck/reporting/graphviz/GraphvizFileWriter.kt
@@ -34,14 +34,6 @@ class GraphvizFileWriter @Inject constructor(
       val rootOrNull = settings.reports.graphs.outputDir?.let { File(it) }
 
       depths
-        .filter {
-          // Don't generate a graph if the SourceSet doesn't exist at all.
-          // For example, if it's an Android project there will be an `androidTest` SourceSet,
-          // but if there are no `androidTestImplementation` dependencies and no files, then skip it.
-          it.depth != 0 || it.dependentProject
-            .sourceSets[it.sourceSetName]
-            ?.hasExistingSourceFiles == true
-        }
         // Generate the low-depth graphs first, because their data is memoized and used to create the
         // graphs for high-depth projects.
         .sorted()
@@ -49,15 +41,17 @@ class GraphvizFileWriter @Inject constructor(
 
           launchIO {
 
-            val root = rootOrNull ?: depth.dependentProject.projectDir
-
-            val graphFile = root.child(
-              "build",
-              "reports",
-              "modulecheck",
-              "graphs",
+            val graphFile = rootOrNull?.child(
+              depth.dependentPath.value.replace(":", File.separator),
               "${depth.sourceSetName.value}.dot"
             )
+              ?: depth.dependentProject.projectDir.child(
+                "build",
+                "reports",
+                "modulecheck",
+                "graphs",
+                "${depth.sourceSetName.value}.dot"
+              )
 
             val depthReport = graphvizFactory.create(depth)
 


Test patch:
diff --git a/modulecheck-gradle/plugin/src/test/kotlin/modulecheck/gradle/GraphReportTaskTest.kt b/modulecheck-gradle/plugin/src/test/kotlin/modulecheck/gradle/GraphReportTaskTest.kt
index 4fb776ea89..e26afdd938 100644
--- a/modulecheck-gradle/plugin/src/test/kotlin/modulecheck/gradle/GraphReportTaskTest.kt
+++ b/modulecheck-gradle/plugin/src/test/kotlin/modulecheck/gradle/GraphReportTaskTest.kt
@@ -109,8 +109,11 @@ internal class GraphReportTaskTest : BasePluginTest() {
       "app", "build", "reports", "modulecheck", "graphs", "main.dot"
     ) shouldHaveText """
       strict digraph DependencyGraph {
-        ratio=0.5;
-        node [style="rounded,filled" shape=box];
+        ratio = 0.5625;
+        node [style = "rounded,filled" shape = box];
+
+        labelloc = "t"
+        label = ":app -- main";
 
         ":app" [fillcolor = "#F89820"];
         ":lib1" [fillcolor = "#F89820"];
@@ -121,9 +124,9 @@ internal class GraphReportTaskTest : BasePluginTest() {
 
         ":lib2" -> ":lib1" [style = bold; color = "#007744"];
 
-        {rank = same; ":lib1";}
-        {rank = same; ":lib2";}
-        {rank = same; ":app";}
+        { rank = same; ":lib1"; }
+        { rank = same; ":lib2"; }
+        { rank = same; ":app"; }
       }
     """
   }
diff --git a/modulecheck-core/src/test/kotlin/modulecheck/core/GraphVizReportTest.kt b/modulecheck-reporting/graphviz/src/test/kotlin/modulecheck/reporting/graphviz/GraphVizReportTest.kt
similarity index 61%
rename from modulecheck-core/src/test/kotlin/modulecheck/core/GraphVizReportTest.kt
rename to modulecheck-reporting/graphviz/src/test/kotlin/modulecheck/reporting/graphviz/GraphVizReportTest.kt
index 7ca10eea4b..31be7a80a9 100644
--- a/modulecheck-core/src/test/kotlin/modulecheck/core/GraphVizReportTest.kt
+++ b/modulecheck-reporting/graphviz/src/test/kotlin/modulecheck/reporting/graphviz/GraphVizReportTest.kt
@@ -13,8 +13,9 @@
  * limitations under the License.
  */
 
-package modulecheck.core
+package modulecheck.reporting.graphviz
 
+import io.kotest.inspectors.forAll
 import modulecheck.core.rule.DepthRule
 import modulecheck.core.rule.SingleRuleFindingFactory
 import modulecheck.parsing.gradle.model.ConfigurationName
@@ -81,8 +82,104 @@ internal class GraphVizReportTest : RunnerTest() {
 
     app.graphFile() shouldHaveText """
       strict digraph DependencyGraph {
-        ratio=0.5;
-        node [style="rounded,filled" shape=box];
+        ratio = 0.5625;
+        node [style = "rounded,filled" shape = box];
+
+        labelloc = "t"
+        label = ":app -- main";
+
+        ":app" [fillcolor = "#F89820"];
+        ":lib1" [fillcolor = "#F89820"];
+        ":lib2" [fillcolor = "#F89820"];
+
+        ":app" -> ":lib1" [style = bold; color = "#007744"];
+        ":app" -> ":lib2" [style = bold; color = "#007744"];
+
+        ":lib2" -> ":lib1" [style = bold; color = "#007744"];
+
+        { rank = same; ":lib1"; }
+        { rank = same; ":lib2"; }
+        { rank = same; ":app"; }
+      }
+    """
+  }
+
+  // https://github.com/RBusarow/ModuleCheck/issues/575
+  @Test
+  fun `custom report dir should put all graphs in relative directories`() {
+
+    val graphsDir = testProjectDir.child("graphs")
+    fun graph(project: McProject, sourceSetName: SourceSetName): File {
+      return graphsDir.child(
+        project.path.value.removePrefix(":"),
+        "${sourceSetName.value}.dot"
+      )
+    }
+
+    settings.reports.graphs.enabled = true
+    settings.reports.graphs.outputDir = graphsDir.path
+
+    val lib1 = kotlinProject(":lib1")
+
+    val lib2 = kotlinProject(":lib2") {
+      addDependency(ConfigurationName.implementation, lib1)
+    }
+
+    val app = kotlinProject(":app") {
+      addDependency(ConfigurationName.implementation, lib1)
+      addDependency(ConfigurationName.implementation, lib2)
+    }
+
+    run(autoCorrect = false).isSuccess shouldBe true
+
+    // These are the simple, no-dependency graphs
+    listOf(
+      lib1 to SourceSetName.MAIN,
+      lib1 to SourceSetName.TEST,
+      lib2 to SourceSetName.TEST,
+      app to SourceSetName.TEST
+    ).forAll { (project, sourceSet) ->
+
+      graph(project, sourceSet) shouldHaveText """
+        strict digraph DependencyGraph {
+          ratio = 0.5625;
+          node [style = "rounded,filled" shape = box];
+
+          labelloc = "t"
+          label = "${project.path.value} -- ${sourceSet.value}";
+
+          "${project.path.value}" [fillcolor = "#F89820"];
+
+          { rank = same; "${project.path.value}"; }
+        }
+        """
+    }
+
+    graph(lib2, SourceSetName.MAIN) shouldHaveText """
+      strict digraph DependencyGraph {
+        ratio = 0.5625;
+        node [style = "rounded,filled" shape = box];
+
+        labelloc = "t"
+        label = ":lib2 -- main";
+
+        ":lib1" [fillcolor = "#F89820"];
+        ":lib2" [fillcolor = "#F89820"];
+
+        ":lib2" -> ":lib1" [style = bold; color = "#007744"];
+
+        { rank = same; ":lib1"; }
+        { rank = same; ":lib2"; }
+      }
+    """
+
+    graph(app, SourceSetName.MAIN) shouldHaveText """
+      strict digraph DependencyGraph {
+        ratio = 0.5625;
+        node [style = "rounded,filled" shape = box];
+
+        labelloc = "t"
+        label = ":app -- main";
 
         ":app" [fillcolor = "#F89820"];
         ":lib1" [fillcolor = "#F89820"];
@@ -93,9 +190,9 @@ internal class GraphVizReportTest : RunnerTest() {
 
         ":lib2" -> ":lib1" [style = bold; color = "#007744"];
 
-        {rank = same; ":lib1";}
-        {rank = same; ":lib2";}
-        {rank = same; ":app";}
+        { rank = same; ":lib1"; }
+        { rank = same; ":lib2"; }
+        { rank = same; ":app"; }
       }
     """
   }
@@ -113,26 +210,43 @@ internal class GraphVizReportTest : RunnerTest() {
 
     lib1.graphFile() shouldHaveText """
       strict digraph DependencyGraph {
-        ratio=0.5;
-        node [style="rounded,filled" shape=box];
+        ratio = 0.5625;
+        node [style = "rounded,filled" shape = box];
+
+        labelloc = "t"
+        label = ":lib1 -- main";
 
         ":lib1" [fillcolor = "#F89820"];
 
-        {rank = same; ":lib1";}
+        { rank = same; ":lib1"; }
       }
     """
   }
 
   @Test
-  fun `graph should not be created for an existing source set if it has no files`() {
+  fun `graph should be created for an existing source set with no files`() {
 
     settings.reports.graphs.enabled = true
 
-    val lib1 = kotlinProject(":lib1")
+    val lib1 = kotlinProject(":lib1") {
+      addKotlinSource("", directory = "com/test", fileName = "MyFile.kt")
+    }
 
     run(autoCorrect = false).isSuccess shouldBe true
 
-    lib1.graphFile().exists() shouldBe false
+    lib1.graphFile() shouldHaveText """
+      strict digraph DependencyGraph {
+        ratio = 0.5625;
+        node [style = "rounded,filled" shape = box];
+
+        labelloc = "t"
+        label = ":lib1 -- main";
+
+        ":lib1" [fillcolor = "#F89820"];
+
+        { rank = same; ":lib1"; }
+      }
+    """
   }
 
   @Test
@@ -165,8 +279,11 @@ internal class GraphVizReportTest : RunnerTest() {
 
     app.graphFile() shouldHaveText """
       strict digraph DependencyGraph {
-        ratio=0.5;
-        node [style="rounded,filled" shape=box];
+        ratio = 0.5625;
+        node [style = "rounded,filled" shape = box];
+
+        labelloc = "t"
+        label = ":app -- main";
 
         ":app" [fillcolor = "#F89820"];
         ":lib1" [fillcolor = "#F89820"];
@@ -177,16 +294,19 @@ internal class GraphVizReportTest : RunnerTest() {
 
         ":lib2" -> ":lib1" [style = bold; color = "#007744"];
 
-        {rank = same; ":lib1";}
-        {rank = same; ":lib2";}
-        {rank = same; ":app";}
+        { rank = same; ":lib1"; }
+        { rank = same; ":lib2"; }
+        { rank = same; ":app"; }
       }
     """
 
     app.graphFile("test") shouldHaveText """
       strict digraph DependencyGraph {
-        ratio=0.5;
-        node [style="rounded,filled" shape=box];
+        ratio = 0.5625;
+        node [style = "rounded,filled" shape = box];
+
+        labelloc = "t"
+        label = ":app -- test";
 
         ":app" [fillcolor = "#F89820"];
         ":lib1" [fillcolor = "#F89820"];
@@ -200,17 +320,20 @@ internal class GraphVizReportTest : RunnerTest() {
         ":test1" -> ":lib1" [style = bold; color = "#007744"];
         ":test1" -> ":lib2" [style = bold; color = "#007744"];
 
-        {rank = same; ":lib1";}
-        {rank = same; ":lib2";}
-        {rank = same; ":test1";}
-        {rank = same; ":app";}
+        { rank = same; ":lib1"; }
+        { rank = same; ":lib2"; }
+        { rank = same; ":test1"; }
+        { rank = same; ":app"; }
       }
     """
 
     lib1.graphFile("test") shouldHaveText """
       strict digraph DependencyGraph {
-        ratio=0.5;
-        node [style="rounded,filled" shape=box];
+        ratio = 0.5625;
+        node [style = "rounded,filled" shape = box];
+
+        labelloc = "t"
+        label = ":lib1 -- test";
 
         ":lib1" [fillcolor = "#F89820"];
         ":lib2" [fillcolor = "#F89820"];
@@ -219,8 +342,8 @@ internal class GraphVizReportTest : RunnerTest() {
 
         ":lib2" -> ":lib1" [style = bold; color = "#007744"];
 
-        {rank = same; ":lib2";}
-        {rank = same; ":lib1";}
+        { rank = same; ":lib2"; }
+        { rank = same; ":lib1"; }
       }
     """
   }
@@ -254,8 +377,11 @@ internal class GraphVizReportTest : RunnerTest() {
 
     app.graphFile() shouldHaveText """
       strict digraph DependencyGraph {
-        ratio=0.5;
-        node [style="rounded,filled" shape=box];
+        ratio = 0.5625;
+        node [style = "rounded,filled" shape = box];
+
+        labelloc = "t"
+        label = ":app -- main";
 
         ":app" [fillcolor = "#F89820"];
         ":lib1" [fillcolor = "#F89820"];
@@ -266,16 +392,19 @@ internal class GraphVizReportTest : RunnerTest() {
 
         ":lib2" -> ":lib1" [style = bold; color = "#007744"];
 
-        {rank = same; ":lib1";}
-        {rank = same; ":lib2";}
-        {rank = same; ":app";}
+        { rank = same; ":lib1"; }
+        { rank = same; ":lib2"; }
+        { rank = same; ":app"; }
       }
     """
 
     app.graphFile("debug") shouldHaveText """
       strict digraph DependencyGraph {
-        ratio=0.5;
-        node [style="rounded,filled" shape=box];
+        ratio = 0.5625;
+        node [style = "rounded,filled" shape = box];
+
+        labelloc = "t"
+        label = ":app -- debug";
 
         ":app" [fillcolor = "#F89820"];
         ":debug1" [fillcolor = "#F89820"];
@@ -293,10 +422,10 @@ internal class GraphVizReportTest : RunnerTest() {
 
         ":lib2" -> ":lib1" [style = bold; color = "#007744"];
 
-        {rank = same; ":lib1";}
-        {rank = same; ":debug1"; ":lib2";}
-        {rank = same; ":debug2";}
-        {rank = same; ":app";}
+        { rank = same; ":lib1"; }
+        { rank = same; ":debug1"; ":lib2"; }
+        { rank = same; ":debug2"; }
+        { rank = same; ":app"; }
       }
     """
   }