Better bad annotation file error reporting (#771)

* Propagate exceptions from JsonProtocol deserialization

* Add AnnotationFileNotFoundException for better error reporting

* Add AnnotationClassNotFoundException for better error reporting

* Better propagate JSON parsing errors

Also report the file if there is a error deserializing a JSON file

* Make exception for non-array JSON file more explicit

4 files changed, 121 insertions, 32 deletions

diff --git a/src/main/scala/firrtl/Driver.scala b/src/main/scala/firrtl/Driver.scala
index 3ff465e2..f9ba6141 100644
--- a/src/main/scala/firrtl/Driver.scala
+++ b/src/main/scala/firrtl/Driver.scala
@@ -116,23 +116,22 @@ object Driver {
 
     val loadedAnnos = annoFiles.flatMap { file =>
       if (!file.exists) {
-        throw new FileNotFoundException(s"Annotation file $file not found!")
+        throw new AnnotationFileNotFoundException(file)
       }
       // Try new protocol first
-      JsonProtocol.deserializeTry(file).getOrElse {
-        val annos = Try {
+      JsonProtocol.deserializeTry(file).recoverWith { case jsonException =>
+        // Try old protocol if new one fails
+        Try {
           val yaml = io.Source.fromFile(file).getLines().mkString("\n").parseYaml
-          yaml.convertTo[List[LegacyAnnotation]]
+          val result = yaml.convertTo[List[LegacyAnnotation]]
+          val msg = s"$file is a YAML file!\n" +
+                    (" "*9) + "YAML Annotation files are deprecated! Use JSON"
+          Driver.dramaticWarning(msg)
+          result
+        }.orElse { // Propagate original JsonProtocol exception if YAML also fails
+          Failure(jsonException)
         }
-        annos match {
-          case Success(result) =>
-            val msg = s"$file is a YAML file!\n" +
-                      (" "*9) + "YAML Annotation files are deprecated! Use JSON"
-            Driver.dramaticWarning(msg)
-            result
-          case Failure(_) => throw new InvalidAnnotationFileException(file.toString)
-        }
-      }
+      }.get
     }
 
     val targetDirAnno = List(BlackBoxTargetDirAnno(optionsManager.targetDirName))
diff --git a/src/main/scala/firrtl/annotations/AnnotationUtils.scala b/src/main/scala/firrtl/annotations/AnnotationUtils.scala
index 6b7f9a60..517cea26 100644
--- a/src/main/scala/firrtl/annotations/AnnotationUtils.scala
+++ b/src/main/scala/firrtl/annotations/AnnotationUtils.scala
@@ -3,6 +3,8 @@
 package firrtl
 package annotations
 
+import java.io.File
+
 import org.json4s._
 import org.json4s.native.JsonMethods._
 import org.json4s.native.Serialization
@@ -14,7 +16,15 @@ import firrtl.annotations.AnnotationYamlProtocol._
 import firrtl.ir._
 import firrtl.Utils.error
 
-class InvalidAnnotationFileException(msg: String) extends FIRRTLException(msg)
+case class InvalidAnnotationFileException(file: File, cause: Throwable = null)
+  extends FIRRTLException(s"$file, see cause below", cause)
+case class InvalidAnnotationJSONException(msg: String) extends FIRRTLException(msg)
+case class AnnotationFileNotFoundException(file: File) extends FIRRTLException(
+  s"Annotation file $file not found!"
+)
+case class AnnotationClassNotFoundException(className: String) extends FIRRTLException(
+  s"Annotation class $className not found! Please check spelling and classpath"
+)
 
 object AnnotationUtils {
   def toYaml(a: LegacyAnnotation): String = a.toYaml.prettyPrint
diff --git a/src/main/scala/firrtl/annotations/JsonProtocol.scala b/src/main/scala/firrtl/annotations/JsonProtocol.scala
index 5005ccb0..7b2617f5 100644
--- a/src/main/scala/firrtl/annotations/JsonProtocol.scala
+++ b/src/main/scala/firrtl/annotations/JsonProtocol.scala
@@ -2,7 +2,7 @@
 package firrtl
 package annotations
 
-import scala.util.Try
+import scala.util.{Try, Failure}
 
 import org.json4s._
 import org.json4s.native.JsonMethods._
@@ -13,18 +13,6 @@ import firrtl.ir._
 import firrtl.Utils.error
 
 object JsonProtocol {
-
-  // Helper for error messages
-  private def prettifyJsonInput(in: JsonInput): String = {
-    def defaultToString(base: String, obj: Any): String = s"$base@${obj.hashCode.toHexString}"
-    in match {
-      case FileInput(file) => file.toString
-      case StringInput(o) => defaultToString("String", o)
-      case ReaderInput(o) => defaultToString("Reader", o)
-      case StreamInput(o) => defaultToString("Stream", o)
-    }
-  }
-
   class TransformClassSerializer extends CustomSerializer[Class[_ <: Transform]](format => (
     { case JString(s) => Class.forName(s).asInstanceOf[Class[_ <: Transform]] },
     { case x: Class[_] => JString(x.getName) }
@@ -66,19 +54,31 @@ object JsonProtocol {
   def deserialize(in: JsonInput): Seq[Annotation] = deserializeTry(in).get
 
   def deserializeTry(in: JsonInput): Try[Seq[Annotation]] = Try({
-    def throwError() = throw new InvalidAnnotationFileException(prettifyJsonInput(in))
     val parsed = parse(in)
     val annos = parsed match {
       case JArray(objs) => objs
-      case _ => throwError()
+      case x => throw new InvalidAnnotationJSONException(
+        s"Annotations must be serialized as a JArray, got ${x.getClass.getSimpleName} instead!")
     }
     // Gather classes so we can deserialize arbitrary Annotations
     val classes = annos.map({
       case JObject(("class", JString(c)) :: tail) => c
-      case _ => throwError()
+      case obj => throw new InvalidAnnotationJSONException(s"Expected field 'class' not found! $obj")
     }).distinct
     val loaded = classes.map(Class.forName(_).asInstanceOf[Class[_ <: Annotation]])
     implicit val formats = jsonFormat(loaded)
     read[List[Annotation]](in)
-  })
+  }).recoverWith {
+    // Translate some generic errors to specific ones
+    case e: java.lang.ClassNotFoundException =>
+      Failure(new AnnotationClassNotFoundException(e.getMessage))
+    case e: org.json4s.ParserUtil.ParseException =>
+      Failure(new InvalidAnnotationJSONException(e.getMessage))
+  }.recoverWith { // If the input is a file, wrap in InvalidAnnotationFileException
+    case e => in match {
+      case FileInput(file) =>
+        Failure(new InvalidAnnotationFileException(file, e))
+      case _ => Failure(e)
+    }
+  }
 }
diff --git a/src/test/scala/firrtlTests/AnnotationTests.scala b/src/test/scala/firrtlTests/AnnotationTests.scala
index f0d4cb25..c1e36b67 100644
--- a/src/test/scala/firrtlTests/AnnotationTests.scala
+++ b/src/test/scala/firrtlTests/AnnotationTests.scala
@@ -10,6 +10,7 @@ import firrtl._
 import firrtl.transforms.OptimizableExtModuleAnnotation
 import firrtl.passes.InlineAnnotation
 import firrtl.passes.memlib.PinAnnotation
+import firrtl.util.BackendCompilationUtilities
 import firrtl.transforms.DontTouchAnnotation
 import net.jcazevedo.moultingyaml._
 import org.scalatest.Matchers
@@ -476,7 +477,7 @@ class LegacyAnnotationTests extends AnnotationTests {
   }
 }
 
-class JsonAnnotationTests extends AnnotationTests {
+class JsonAnnotationTests extends AnnotationTests with BackendCompilationUtilities {
   // Helper annotations
   case class SimpleAnno(target: ComponentName, value: String) extends
       SingleTargetAnnotation[ComponentName] {
@@ -511,4 +512,83 @@ class JsonAnnotationTests extends AnnotationTests {
 
     annos should be (readAnnos)
   }
+
+  private def setupManager(annoFileText: Option[String]) = {
+    val source = """
+      |circuit test :
+      |  module test :
+      |    input x : UInt<1>
+      |    output z : UInt<1>
+      |    z <= x
+      |    node y = x""".stripMargin
+    val testDir = createTestDirectory(this.getClass.getSimpleName)
+    val annoFile = new File(testDir, "anno.json")
+
+    annoFileText.foreach { text =>
+      val w = new FileWriter(annoFile)
+      w.write(text)
+      w.close()
+    }
+
+    new ExecutionOptionsManager("annos") with HasFirrtlOptions {
+      commonOptions = CommonOptions(targetDirName = testDir.getPath)
+      firrtlOptions = FirrtlExecutionOptions(
+        firrtlSource = Some(source),
+        annotationFileNames = List(annoFile.getPath)
+      )
+    }
+  }
+
+  "Annotation file not found" should "give a reasonable error message" in {
+    val manager = setupManager(None)
+
+    an [AnnotationFileNotFoundException] shouldBe thrownBy {
+      Driver.execute(manager)
+    }
+  }
+
+  "Annotation class not found" should "give a reasonable error message" in {
+    val anno = """
+      |[
+      |  {
+      |    "class":"ThisClassDoesNotExist",
+      |    "target":"test.test.y"
+      |  }
+      |] """.stripMargin
+    val manager = setupManager(Some(anno))
+
+    the [Exception] thrownBy Driver.execute(manager) should matchPattern {
+      case InvalidAnnotationFileException(_, _: AnnotationClassNotFoundException) =>
+    }
+  }
+
+  "Malformed annotation file" should "give a reasonable error message" in {
+    val anno = """
+      |[
+      |  {
+      |    "class":
+      |    "target":"test.test.y"
+      |  }
+      |] """.stripMargin
+    val manager = setupManager(Some(anno))
+
+    the [Exception] thrownBy Driver.execute(manager) should matchPattern {
+      case InvalidAnnotationFileException(_, _: InvalidAnnotationJSONException) =>
+    }
+  }
+
+  "Non-array annotation file" should "give a reasonable error message" in {
+    val anno = """
+      |{
+      |  "class":"firrtl.transforms.DontTouchAnnotation",
+      |  "target":"test.test.y"
+      |}
+      |""".stripMargin
+    val manager = setupManager(Some(anno))
+
+    the [Exception] thrownBy Driver.execute(manager) should matchPattern {
+      case InvalidAnnotationFileException(_, InvalidAnnotationJSONException(msg))
+        if msg.contains("JObject") =>
+    }
+  }
 }