Improve tooling and sbt-plugin error handling

sbt-scala-native-bindgen/src/main/scala/org/scalanative/bindgen/sbt/ScalaNativeBindgenPlugin.scala

@@ -129,20 +129,29 @@ object ScalaNativeBindgenPlugin extends AutoPlugin {
           val bindgenPath     = nativeBindgenPath.value
           val bindings        = nativeBindings.value
           val outputDirectory = (target in nativeBindgen).value
+          val logger          = streams.value.log
 
           bindings.map {
             binding =>
               val output = outputDirectory / s"${binding.name}.scala"
-
-              Bindgen()
+              val result = Bindgen()
                 .bindgenExecutable(bindgenPath)
                 .header(binding.header)
                 .name(binding.name)
                 .maybe(binding.link, _.link)
                 .maybe(binding.packageName, _.packageName)
                 .maybe(binding.excludePrefix, _.excludePrefix)
                 .generate()
-                .writeToFile(output)
+
+              result match {
+                case Right(bindings) =>
+                  bindings.writeToFile(output)
+                  bindings.errors.foreach(error => logger.error(error))
+                case Left(errors) =>
+                  errors.foreach(error => logger.error(error))
+                  sys.error(
+                    "scala-native-bindgen failed with non-zero exit code")
+              }
 
               output
           }

tests/src/test/scala/org/scalanative/bindgen/BindgenReportingSpec.scala

@@ -19,12 +19,12 @@ class BindgenReportingSpec extends FunSpec {
       }
     }
 
-    def bindgen(input: String): Bindings = {
+    def assertBindgenError(input: String, errors: Seq[String]): Unit = {
       val tempFile = File.createTempFile("scala-native-bindgen-tests", ".h")
       try {
         writeToFile(tempFile, input)
 
-        Bindgen()
+        val result = Bindgen()
           .bindgenExecutable(new File(bindgenPath))
           .header(tempFile)
           .name("BindgenTests")
@@ -33,51 +33,59 @@ class BindgenReportingSpec extends FunSpec {
           .excludePrefix("__")
           .generate()
 
+        result match {
+          case Right(binding) =>
+            assert(binding.errors == errors)
+          case Left(errors) =>
+            fail(s"Non-zero exit code:\n${errors.mkString("\n")}")
+        }
       } finally {
         tempFile.delete()
       }
     }
 
     it("Skips functions that pass struct or union by value") {
-      val bindings =
-        bindgen(input = """struct s { int a; };
-                          |void useStruct(struct s);
-                          |typedef struct s s;
-                          |s returnStruct();
-                          |
-                          |union u { int a; };
-                          |void useUnion(union u);
-                          |typedef union u u;
-                          |u returnUnion();
-                          |""".stripMargin)
-      assert(
-        bindings.errs ==
-          """Warning: Function useStruct is skipped because Scala Native does not support passing structs and arrays by value.
-            |Warning: Function returnStruct is skipped because Scala Native does not support passing structs and arrays by value.
-            |Warning: Function useUnion is skipped because Scala Native does not support passing structs and arrays by value.
-            |Warning: Function returnUnion is skipped because Scala Native does not support passing structs and arrays by value.""".stripMargin
+      assertBindgenError(
+        """struct s { int a; };
+          |void useStruct(struct s);
+          |typedef struct s s;
+          |s returnStruct();
+          |
+          |union u { int a; };
+          |void useUnion(union u);
+          |typedef union u u;
+          |u returnUnion();
+          |""".stripMargin,
+        Seq(
+          "Warning: Function useStruct is skipped because Scala Native does not support passing structs and arrays by value.",
+          "Warning: Function returnStruct is skipped because Scala Native does not support passing structs and arrays by value.",
+          "Warning: Function useUnion is skipped because Scala Native does not support passing structs and arrays by value.",
+          "Warning: Function returnUnion is skipped because Scala Native does not support passing structs and arrays by value."
+        )
       )
     }
 
     it("Skips variable with opaque type") {
-      val bindings =
-        bindgen(input = """struct undefinedStruct;
-                          |extern struct undefinedStruct removedExtern;
-                          |#define removedExternAlias removedExtern
-                          |""".stripMargin)
-      assert(
-        bindings.errs == """Error: Variable removedExtern is skipped because it has incomplete type.
-                           |Error: Variable alias removedExternAlias is skipped because it has incomplete type.""".stripMargin)
-
+      assertBindgenError(
+        """struct undefinedStruct;
+          |extern struct undefinedStruct removedExtern;
+          |#define removedExternAlias removedExtern
+          |""".stripMargin,
+        Seq(
+          "Error: Variable removedExtern is skipped because it has incomplete type.",
+          "Error: Variable alias removedExternAlias is skipped because it has incomplete type."
+        )
+      )
     }
 
     it("Skips unused alias for opaque type") {
-      val bindings =
-        bindgen(input = """union undefinedUnion;
-                          |typedef union undefinedUnion aliasForUndefinedUnion;
-                          |""".stripMargin)
-      assert(
-        bindings.errs == "Warning: type alias aliasForUndefinedUnion is skipped because it is an unused alias for incomplete type.")
+      assertBindgenError(
+        """union undefinedUnion;
+          |typedef union undefinedUnion aliasForUndefinedUnion;
+          |""".stripMargin,
+        Seq(
+          "Warning: type alias aliasForUndefinedUnion is skipped because it is an unused alias for incomplete type.")
+      )
     }
   }
 }

tests/src/test/scala/org/scalanative/bindgen/BindgenSpec.scala

@@ -18,15 +18,21 @@ class BindgenSpec extends FunSpec {
     }
 
     def bindgen(inputFile: File, name: String, outputFile: File): Unit = {
-      Bindgen()
+      val result = Bindgen()
         .bindgenExecutable(new File(bindgenPath))
         .header(inputFile)
         .name(name)
         .link("bindgentests")
         .packageName("org.scalanative.bindgen.samples")
         .excludePrefix("__")
         .generate()
-        .writeToFile(outputFile)
+
+      result match {
+        case Right(binding) =>
+          binding.writeToFile(outputFile)
+        case Left(errors) =>
+          fail("scala-native-bindgen failed: " + errors.mkString("\n"))
+      }
     }
 
     def contentOf(file: File) =

tools/src/main/scala/org/scalanative/bindgen/Bindgen.scala

@@ -3,6 +3,7 @@ package org.scalanative.bindgen
 import java.io.File
 
 import scala.collection.immutable.Seq
+import scala.collection.mutable.ListBuffer
 import scala.sys.process.{Process, ProcessLogger}
 
 sealed trait Bindgen {
@@ -51,8 +52,9 @@ sealed trait Bindgen {
 
   /**
    * Run binding generator
+   * @return errors if exit code was not 0, otherwise return bindings
    */
-  def generate(): Bindings
+  def generate(): Either[Seq[String], Bindings]
 }
 
 object Bindgen {
@@ -69,46 +71,47 @@ object Bindgen {
       extends Bindgen {
 
     def bindgenExecutable(executable: File): Bindgen = {
-      require(executable.exists())
+      require(executable.exists(), s"Executable does not exist: $executable")
       copy(executable = Option(executable))
     }
 
     def header(header: File): Bindgen = {
-      require(header.exists())
+      require(header.exists(), s"Header file does not exist: $header")
       copy(header = Option(header))
     }
 
     def link(library: String): Bindgen = {
-      require(!library.isEmpty)
+      require(library.nonEmpty, "Library must be non-empty")
       copy(library = Option(library))
     }
 
     def name(name: String): Bindgen = {
-      require(!name.isEmpty)
+      require(name.nonEmpty, "Name must be non-empty")
       copy(name = Option(name))
     }
 
     def packageName(packageName: String): Bindgen = {
-      require(!packageName.isEmpty)
+      require(packageName.nonEmpty, "Package name must be non-empty")
       copy(packageName = Option(packageName))
     }
 
     def excludePrefix(prefix: String): Bindgen = {
-      require(!prefix.isEmpty)
+      require(prefix.nonEmpty, "Exclude prefix must be non-empty")
       copy(excludePrefix = Option(prefix))
     }
 
     def extraArg(args: String*): Bindgen = {
-      require(args.forall(_.nonEmpty))
+      require(args.forall(_.nonEmpty), "All extra-args must be non-empty")
       copy(extraArg = extraArg ++ args)
     }
 
     def extraArgBefore(args: String*): Bindgen = {
-      require(args.forall(_.nonEmpty))
+      require(args.forall(_.nonEmpty),
+              "All extra-args-before must be non-empty")
       copy(extraArgBefore = extraArgBefore ++ args)
     }
 
-    def generate(): Bindings = {
+    def generate(): Either[Seq[String], Bindings] = {
       require(executable.isDefined, "The executable must be specified")
       require(header.isDefined, "Header file must be specified")
 
@@ -129,13 +132,15 @@ object Bindgen {
         withArgs("--extra-arg-before", extraArgBefore) ++
         Seq(header.get.getAbsolutePath, "--")
 
-      var errs = Seq[String]()
+      val stdout = ListBuffer[String]()
+      val stderr = ListBuffer[String]()
+      val nl     = System.lineSeparator()
+      val logger = ProcessLogger(stdout.+=, stderr.+=)
 
-      val output = Process(cmd).!!(ProcessLogger { err: String =>
-        errs :+= err
-      })
-
-      new Bindings(output, errs.mkString("\n"))
+      Process(cmd).!(logger) match {
+        case 0 => Right(new Bindings(stdout.mkString(nl), Seq(stderr: _*)))
+        case _ => Left(Seq(stderr: _*))
+      }
     }
   }
 }

tools/src/main/scala/org/scalanative/bindgen/Bindings.scala

@@ -1,8 +1,9 @@
 package org.scalanative.bindgen
 
 import java.io.{File, PrintWriter}
+import scala.collection.immutable.Seq
 
-class Bindings(private val bindings: String, val errs: String) {
+class Bindings(private val bindings: String, val errors: Seq[String]) {
   def writeToFile(file: File): Unit = {
     file.getParentFile.mkdirs()
     new PrintWriter(file) {