Fixes for Annotation serialized/deserialize (#390)

* Fixes for Annotation serialized/deserialize

Made serializer agree with deserializer on text representation
Re-ordered serializations of Named subclasses to be C or C.m or C.m.c where C=circuit, m=module, c=component
Note: component may contain dots
Added serialize deserialize tests to AnnotationSpec
Did some style cleanup on AnnotationSpec
Added explicit return tupe on SimpleTransformSpec#execute

* Make explicit Util.error
remove commented code

* Make Annotation#serialize a nicer format
fix import there and remove new on case class

* In firrtl Driver.execute use annotations passed in through optionsManager#firrtlOptions if nonEmpty
 otherwise read the annotations in from an annotations file
 Add new option to override this behavior, --force-append-anno-file will append annotations in file
 to any that are passed in
A few other style fixes to Driver: remove new with case classes.  don't use match when if(boolean) will do

* Added tests of malformed component and circuit names

2 files changed, 78 insertions, 30 deletions

diff --git a/src/test/scala/firrtlTests/AnnotationTests.scala b/src/test/scala/firrtlTests/AnnotationTests.scala
index 9ffbbd85..7e1a35dc 100644
--- a/src/test/scala/firrtlTests/AnnotationTests.scala
+++ b/src/test/scala/firrtlTests/AnnotationTests.scala
@@ -2,32 +2,15 @@
 
 package firrtlTests
 
-import java.io.{Writer, StringWriter}
+import java.io.{File, FileWriter, StringWriter, Writer}
 
-import org.scalatest.FlatSpec
+import firrtl.annotations.AnnotationYamlProtocol._
+import firrtl.annotations._
+import firrtl._
+import firrtl.passes.InlineAnnotation
+import firrtl.passes.memlib.PinAnnotation
+import net.jcazevedo.moultingyaml._
 import org.scalatest.Matchers
-import org.scalatest.junit.JUnitRunner
-
-import firrtl.ir.Circuit
-import firrtl.{Parser, AnnotationMap}
-import firrtl.{
-   CircuitState,
-   ResolveAndCheck,
-   RenameMap,
-   Compiler,
-   ChirrtlForm,
-   LowForm,
-   VerilogCompiler,
-   Transform
-}
-import firrtl.annotations.{
-   Named,
-   CircuitName,
-   ModuleName,
-   ComponentName,
-   AnnotationException,
-   Annotation
-}
 
 /**
  * An example methodology for testing Firrtl annotations.
@@ -37,14 +20,14 @@ trait AnnotationSpec extends LowTransformSpec {
   def transform = new CustomResolveAndCheck(LowForm)
 
   // Check if Annotation Exception is thrown
-  override def failingexecute(writer: Writer, annotations: AnnotationMap, input: String) = {
+  override def failingexecute(writer: Writer, annotations: AnnotationMap, input: String): Exception = {
     intercept[AnnotationException] {
       compile(CircuitState(parse(input), ChirrtlForm, Some(annotations)), writer)
     }
   }
-  def execute(writer: Writer, annotations: AnnotationMap, input: String, check: Annotation) = {
+  def execute(writer: Writer, annotations: AnnotationMap, input: String, check: Annotation): Unit = {
     val cr = compile(CircuitState(parse(input), ChirrtlForm, Some(annotations)), writer)
-    (cr.annotations.get.annotations) should be (Seq(check))
+    cr.annotations.get.annotations should be (Seq(check))
   }
 }
 
@@ -57,8 +40,8 @@ trait AnnotationSpec extends LowTransformSpec {
  * Unstable, Fickle, and Insistent can be tested.
  */
 class AnnotationTests extends AnnotationSpec with Matchers {
-  def getAMap (a: Annotation): AnnotationMap = new AnnotationMap(Seq(a))
-  val input =
+  def getAMap (a: Annotation): AnnotationMap = AnnotationMap(Seq(a))
+  val input: String =
     """circuit Top :
        |  module Top :
        |    input a : UInt<1>[2]
@@ -74,4 +57,69 @@ class AnnotationTests extends AnnotationSpec with Matchers {
     val ta = Annotation(cName, classOf[Transform], "")
     execute(w, getAMap(ta), input, ta)
   }
+
+  "Annotations" should "be readable from file" in {
+    val annotationFile = new File("src/test/resources/annotations/SampleAnnotations.anno")
+    val annotationsYaml = io.Source.fromFile(annotationFile).getLines().mkString("\n").parseYaml
+    val annotationArray = annotationsYaml.convertTo[Array[Annotation]]
+    annotationArray.length should be (9)
+    annotationArray(0).targetString should be ("ModC")
+    annotationArray(7).transformClass should be ("firrtl.passes.InlineInstances")
+    val expectedValue = "TopOfDiamond\nWith\nSome new lines"
+    annotationArray(7).value should be (expectedValue)
+  }
+
+  "Badly formatted serializations" should "return reasonable error messages" in {
+    var badYaml =
+      """
+        |- transformClass: firrtl.passes.InlineInstances
+        |  targetString: circuit.module..
+        |  value: ModC.this params 16 32
+      """.stripMargin.parseYaml
+
+    var thrown = intercept[Exception] {
+      badYaml.convertTo[Array[Annotation]]
+    }
+    thrown.getMessage should include ("Illegal component name")
+
+    badYaml =
+      """
+        |- transformClass: firrtl.passes.InlineInstances
+        |  targetString: .circuit.module.component
+        |  value: ModC.this params 16 32
+      """.stripMargin.parseYaml
+
+    thrown = intercept[Exception] {
+      badYaml.convertTo[Array[Annotation]]
+    }
+    thrown.getMessage should include ("Illegal circuit name")
+  }
+
+  "Round tripping annotations through text file" should "preserve annotations" in {
+    val annos: Array[Annotation] = Seq(
+      InlineAnnotation(CircuitName("fox")),
+      InlineAnnotation(ModuleName("dog", CircuitName("bear"))),
+      InlineAnnotation(ComponentName("chocolate", ModuleName("like", CircuitName("i")))),
+      PinAnnotation(CircuitName("Pinniped"), Seq("sea-lion", "monk-seal"))
+    ).toArray
+
+    val annoFile = new File("temp-anno")
+    val writer = new FileWriter(annoFile)
+    writer.write(annos.toYaml.prettyPrint)
+    writer.close()
+
+    val yaml = io.Source.fromFile(annoFile).getLines().mkString("\n").parseYaml
+    annoFile.delete()
+
+    val readAnnos = yaml.convertTo[Array[Annotation]]
+
+    annos.zip(readAnnos).foreach { case (beforeAnno, afterAnno) =>
+      beforeAnno.targetString should be (afterAnno.targetString)
+      beforeAnno.target should be (afterAnno.target)
+      beforeAnno.transformClass should be (afterAnno.transformClass)
+      beforeAnno.transform should be (afterAnno.transform)
+
+      beforeAnno should be (afterAnno)
+    }
+  }
 }
diff --git a/src/test/scala/firrtlTests/PassTests.scala b/src/test/scala/firrtlTests/PassTests.scala
index 44bd0382..c417c4fb 100644
--- a/src/test/scala/firrtlTests/PassTests.scala
+++ b/src/test/scala/firrtlTests/PassTests.scala
@@ -41,7 +41,7 @@ abstract class SimpleTransformSpec extends FlatSpec with Matchers with Compiler
    def squash(c: Circuit): Circuit = RemoveEmpty.run(c)
 
    // Executes the test. Call in tests.
-   def execute(writer: Writer, annotations: AnnotationMap, input: String, check: String) = {
+   def execute(writer: Writer, annotations: AnnotationMap, input: String, check: String): Unit = {
       compile(CircuitState(parse(input), ChirrtlForm, Some(annotations)), writer)
       val actual = RemoveEmpty.run(parse(writer.toString)).serialize
       val expected = parse(check).serialize