From 977c3bf40949cdde03f73380305889800216b5d1 Mon Sep 17 00:00:00 2001
From: Tim Nieradzik <tim@sparse.tech>
Date: Tue, 17 Sep 2019 16:37:00 +0200
Subject: [PATCH] BuildConfig: Fix validation for module platform compatibility

The validation triggered a false positive for modules with custom
build targets.

The compatibility must be checked for all platform-specific modules
separately since these may depend on additional modules which are
not included on the base module.
---
 src/main/scala/seed/config/BuildConfig.scala  | 36 ++++++++-----
 .../scala/seed/config/BuildConfigSpec.scala   | 53 ++++++++++++++++++-
 2 files changed, 75 insertions(+), 14 deletions(-)

diff --git a/src/main/scala/seed/config/BuildConfig.scala b/src/main/scala/seed/config/BuildConfig.scala
index d58b797..03a178c 100644
--- a/src/main/scala/seed/config/BuildConfig.scala
+++ b/src/main/scala/seed/config/BuildConfig.scala
@@ -287,13 +287,27 @@ object BuildConfig {
       hasCycle(name, Set())
     }
 
-    val incompatibleModuleDepPlatform =
-      module.moduleDeps
-        .flatMap { name =>
-          build.get(name).map(m => name -> m.module)
-        }
-        .map { case (n, m) => (n, module.targets.diff(m.targets)) }
-        .find(_._2.nonEmpty)
+    // Check whether platforms are missing on the given module's dependencies
+    val missingModuleDepPlatform: Option[(String, Platform)] = {
+
+      /** @return None if module has custom build targets */
+      def resolveModule(name: String) =
+        build.get(name).map(m => name -> m.module).filter(_._2.target.isEmpty)
+
+      def platform(p: Platform) =
+        platformModule(module, p).flatMap(
+          _.moduleDeps.flatMap(resolveModule).collectFirst {
+            case (name, module) if !module.targets.contains(p) => (name, p)
+          }
+        )
+
+      // Compatibility must be checked for all platform-specific modules
+      // separately since these may depend on additional modules which are not
+      // included on the base module
+      platform(JVM)
+        .orElse(platform(JavaScript))
+        .orElse(platform(Native))
+    }
 
     val moduleName = Ansi.italic(name)
 
@@ -389,12 +403,10 @@ object BuildConfig {
       error(s"Module $moduleName cannot depend on itself")
     else if (cyclicModuleDep2)
       error(s"Cycle detected in dependencies of module $moduleName")
-    else if (incompatibleModuleDepPlatform.isDefined)
+    else if (missingModuleDepPlatform.isDefined)
       error(
-        s"Module ${Ansi.italic(incompatibleModuleDepPlatform.get._1)} has missing target platform(s) (${incompatibleModuleDepPlatform.get._2
-          .map(_.id)
-          .map(Ansi.italic)
-          .mkString(", ")}) required by $moduleName"
+        s"Module ${Ansi.italic(missingModuleDepPlatform.get._1)} has missing target platform ${Ansi
+          .italic(missingModuleDepPlatform.get._2.id)} required by $moduleName"
       )
     else true
   }
diff --git a/src/test/scala/seed/config/BuildConfigSpec.scala b/src/test/scala/seed/config/BuildConfigSpec.scala
index 3c5f2bb..39e487b 100644
--- a/src/test/scala/seed/config/BuildConfigSpec.scala
+++ b/src/test/scala/seed/config/BuildConfigSpec.scala
@@ -675,13 +675,62 @@ object BuildConfigSpec extends SimpleTestSuite {
     assert(
       messages.exists(
         _.contains(
-          s"Module ${Ansi.italic("foo")} has missing target platform(s) (${Ansi
-            .italic("native")}) required by ${Ansi.italic("bar")}"
+          s"Module ${Ansi.italic("foo")} has missing target platform ${Ansi
+            .italic("native")} required by ${Ansi.italic("bar")}"
         )
       )
     )
   }
 
+  test("Platform compatibility when inheriting (2)") {
+    val buildToml = """
+      |[project]
+      |scalaJsVersion     = "0.6.26"
+      |scalaNativeVersion = "0.3.7"
+      |
+      |[module.foo.js]
+      |scalaVersion = "2.11.11"
+      |sources      = ["foo/"]
+      |
+      |[module.bar.native]
+      |scalaVersion = "2.11.11"
+      |moduleDeps   = ["foo"]
+      |sources      = ["bar/"]
+    """.stripMargin
+
+    val messages = ListBuffer[String]()
+    val log      = new Log(messages += _, identity, LogLevel.Error, false)
+    parseBuild(buildToml, log, fail = true)(_ => "")
+    assert(
+      messages.exists(
+        _.contains(
+          s"Module ${Ansi.italic("foo")} has missing target platform ${Ansi
+            .italic("native")} required by ${Ansi.italic("bar")}"
+        )
+      )
+    )
+  }
+
+  test("Custom build targets do not need to set any platforms") {
+    val buildToml = """
+      |[project]
+      |scalaVersion = "2.11.11"
+      |
+      |[module.template.target.scss]
+      |root    = "scss"
+      |command = "yarn install && yarn run gulp"
+      |
+      |[module.app.jvm]
+      |moduleDeps = ["template"]
+      |sources    = ["src/"]
+    """.stripMargin
+
+    val messages = ListBuffer[String]()
+    val log      = new Log(messages += _, identity, LogLevel.Error, false)
+    val build    = parseBuild(buildToml, log)(_ => "")
+    assertEquals(build("app").module.targets, List(JVM))
+  }
+
   test("Scala version compatibility") {
     val buildToml = """
       |[module.foo.jvm]