Add support for multiple annotation files

Change loadAnnotations to return annotations instead of mutating firrtlOptions
Deprecate implicit annotation file (top.anno) and annotation file override

3 files changed, 90 insertions, 36 deletions

diff --git a/src/main/scala/firrtl/Driver.scala b/src/main/scala/firrtl/Driver.scala
index b5eb1531..bc5102d2 100644
--- a/src/main/scala/firrtl/Driver.scala
+++ b/src/main/scala/firrtl/Driver.scala
@@ -89,34 +89,58 @@ object Driver {
     println("-"*78 + Console.RESET)
   }
 
-  /**
-    * Load annotation file based on options
-    * @param optionsManager use optionsManager config to load annotation file if it exists
-    *                       update the firrtlOptions with new annotations if it does
+  /** Load annotations from specified files and options
+    *
+    * @param optionsManager use optionsManager config to load annotation files
+    * @return Annotations read from files
     */
-  def loadAnnotations(optionsManager: ExecutionOptionsManager with HasFirrtlOptions): Unit = {
+  //scalastyle:off cyclomatic.complexity method.length
+  def loadAnnotations(
+      optionsManager: ExecutionOptionsManager with HasFirrtlOptions
+  ): Seq[Annotation] = {
+    val firrtlConfig = optionsManager.firrtlOptions
+
+    //noinspection ScalaDeprecation
+    val oldAnnoFileName = firrtlConfig.getAnnotationFileName(optionsManager)
+    val oldAnnoFile = new File(oldAnnoFileName).getCanonicalFile
+
+    val (annoFiles, usingImplicitAnnoFile) = {
+      val afs = firrtlConfig.annotationFileNames.map { x =>
+        new File(x).getCanonicalFile
+      }
+      // Implicit anno file could be included explicitly, only include it and
+      // warn if it's not also explicit
+      val use = oldAnnoFile.exists && !afs.contains(oldAnnoFile)
+      if (use) (oldAnnoFile +: afs, true) else (afs, false)
+    }
 
-    def firrtlConfig = optionsManager.firrtlOptions
+    // Warnings to get people to change to drop old API
+    if (firrtlConfig.annotationFileNameOverride.nonEmpty) {
+      val msg = "annotationFileNameOverride is deprecated! " +
+                "Use annotationFileNames"
+      Driver.dramaticWarning(msg)
+    } else if (usingImplicitAnnoFile) {
+      val msg = "Implicit .anno file from top-name is deprecated!\n" +
+             (" "*9) + "Use explicit -faf option or annotationFileNames"
+      Driver.dramaticWarning(msg)
+    }
+
+    val loadedAnnos = annoFiles.flatMap { file =>
+      if (!file.exists) {
+        throw new FileNotFoundException(s"Annotation file $file not found!")
+      }
+      val yaml = io.Source.fromFile(file).getLines().mkString("\n").parseYaml
+      yaml.convertTo[List[Annotation]]
 
-    val annotationFileName = firrtlConfig.getAnnotationFileName(optionsManager)
-    val annotationFile = new File(annotationFileName)
-    if (annotationFile.exists) {
-      val annotationsYaml = io.Source.fromFile(annotationFile).getLines().mkString("\n").parseYaml
-      val annotationArray = annotationsYaml.convertTo[Array[Annotation]]
-      optionsManager.firrtlOptions = firrtlConfig.copy(annotations = firrtlConfig.annotations ++ annotationArray)
     }
 
-    if(firrtlConfig.annotations.nonEmpty) {
-      val targetDirAnno = List(Annotation(
+    val targetDirAnno =
+      List(Annotation(
         CircuitName("All"),
         classOf[BlackBoxSourceHelper],
         BlackBoxTargetDir(optionsManager.targetDirName).serialize
       ))
 
-      optionsManager.firrtlOptions = optionsManager.firrtlOptions.copy(
-        annotations = firrtlConfig.annotations ++ targetDirAnno)
-    }
-
     // Output Annotations
     val outputAnnos = firrtlConfig.getEmitterAnnos(optionsManager)
 
@@ -124,9 +148,7 @@ object Driver {
       (if (firrtlConfig.dontCheckCombLoops) Seq(DontCheckCombLoopsAnnotation()) else Seq()) ++
       (if (firrtlConfig.noDCE) Seq(NoDCEAnnotation()) else Seq())
 
-    optionsManager.firrtlOptions = optionsManager.firrtlOptions.copy(
-      annotations = firrtlConfig.annotations ++ outputAnnos ++ globalAnnos)
-
+    targetDirAnno ++ outputAnnos ++ globalAnnos ++ firrtlConfig.annotations ++ loadedAnnos
   }
 
   /**
@@ -169,7 +191,7 @@ object Driver {
           }
       }
 
-      loadAnnotations(optionsManager)
+      val annos = loadAnnotations(optionsManager)
 
       val parsedInput = Parser.parse(firrtlSource, firrtlConfig.infoMode)
 
@@ -179,7 +201,7 @@ object Driver {
       val finalState = firrtlConfig.compiler.compile(
         CircuitState(parsedInput,
                      ChirrtlForm,
-                     Some(AnnotationMap(firrtlConfig.annotations))),
+                     Some(AnnotationMap(annos))),
         firrtlConfig.customTransforms
       )
 
diff --git a/src/main/scala/firrtl/ExecutionOptionsManager.scala b/src/main/scala/firrtl/ExecutionOptionsManager.scala
index 8bbb8dcd..e5bd95ef 100644
--- a/src/main/scala/firrtl/ExecutionOptionsManager.scala
+++ b/src/main/scala/firrtl/ExecutionOptionsManager.scala
@@ -177,6 +177,7 @@ case class FirrtlExecutionOptions(
     firrtlSource:           Option[String] = None,
     customTransforms:       Seq[Transform] = List.empty,
     annotations:            List[Annotation] = List.empty,
+    annotationFileNames:    List[String] = List.empty,
     annotationFileNameOverride: String = "",
     outputAnnotationFileName: String = "",
     emitOneFilePerModule:   Boolean = false,
@@ -273,6 +274,7 @@ case class FirrtlExecutionOptions(
     * @param optionsManager this is needed to access build function and its common options
     * @return
     */
+  @deprecated("Use FirrtlOptions.annotationFileNames instead", "1.1")
   def getAnnotationFileName(optionsManager: ExecutionOptionsManager): String = {
     optionsManager.getBuildFileName("anno", annotationFileNameOverride)
   }
@@ -309,12 +311,12 @@ trait HasFirrtlOptions {
 
   parser.opt[String]("annotation-file")
     .abbr("faf")
-    .valueName ("<input-anno-file>")
+    .unbounded()
+    .valueName("<input-anno-file>")
     .foreach { x =>
-      firrtlOptions = firrtlOptions.copy(annotationFileNameOverride = x)
-    }.text {
-    "use this to override the default annotation file name, default is empty"
-  }
+      val annoFiles = x +: firrtlOptions.annotationFileNames
+      firrtlOptions = firrtlOptions.copy(annotationFileNames = annoFiles)
+    }.text("Used to specify annotation files (can appear multiple times)")
 
   parser.opt[Unit]("force-append-anno-file")
     .abbr("ffaaf")
diff --git a/src/test/scala/firrtlTests/DriverSpec.scala b/src/test/scala/firrtlTests/DriverSpec.scala
index b589f69a..3e3b7570 100644
--- a/src/test/scala/firrtlTests/DriverSpec.scala
+++ b/src/test/scala/firrtlTests/DriverSpec.scala
@@ -2,7 +2,7 @@
 
 package firrtlTests
 
-import java.io.{File, FileNotFoundException, FileOutputStream}
+import java.io.File
 import org.scalatest.{FreeSpec, Matchers}
 
 import firrtl.passes.InlineInstances
@@ -150,7 +150,23 @@ class DriverSpec extends FreeSpec with Matchers with BackendCompilationUtilities
     }
   }
 
-  "Annotations can be read from a file" in {
+  // Deprecated
+  "Annotations can be read implicitly from the name of the circuit" in {
+    val top = "foo"
+    val optionsManager = new ExecutionOptionsManager("test") with HasFirrtlOptions {
+      commonOptions = commonOptions.copy(topName = top)
+    }
+    val annoFile =  new File(optionsManager.commonOptions.targetDirName, top + ".anno")
+    copyResourceToFile("/annotations/SampleAnnotations.anno", annoFile)
+    optionsManager.firrtlOptions.annotations.length should be (0)
+    val annos = Driver.loadAnnotations(optionsManager)
+    annos.length should be (12) // 9 from circuit plus 3 general purpose
+    annos.count(_.transformClass == "firrtl.passes.InlineInstances") should be (9)
+    annoFile.delete()
+  }
+
+  // Deprecated
+  "Annotations can be read using annotationFileNameOverride" in {
     val optionsManager = new ExecutionOptionsManager("test") with HasFirrtlOptions {
       commonOptions = commonOptions.copy(topName = "a.fir")
       firrtlOptions = firrtlOptions.copy(
@@ -160,10 +176,26 @@ class DriverSpec extends FreeSpec with Matchers with BackendCompilationUtilities
     val annotationsTestFile =  new File(optionsManager.commonOptions.targetDirName, optionsManager.firrtlOptions.annotationFileNameOverride + ".anno")
     copyResourceToFile("/annotations/SampleAnnotations.anno", annotationsTestFile)
     optionsManager.firrtlOptions.annotations.length should be (0)
-    Driver.loadAnnotations(optionsManager)
-    optionsManager.firrtlOptions.annotations.length should be (12) // 9 from circuit plus 3 general purpose
+    val annos = Driver.loadAnnotations(optionsManager)
+    annos.length should be (12) // 9 from circuit plus 3 general purpose
+    annos.count(_.transformClass == "firrtl.passes.InlineInstances") should be (9)
+    annotationsTestFile.delete()
+  }
 
-    optionsManager.firrtlOptions.annotations.head.transformClass should be ("firrtl.passes.InlineInstances")
+  "Annotations can be read from multiple files" in {
+    val filename = "SampleAnnotations.anno"
+    val optionsManager = new ExecutionOptionsManager("test") with HasFirrtlOptions {
+      commonOptions = commonOptions.copy(topName = "a.fir")
+      firrtlOptions = firrtlOptions.copy(
+        annotationFileNames = List.fill(2)(filename) // just read the safe file twice
+      )
+    }
+    val annotationsTestFile = new File(optionsManager.commonOptions.targetDirName, filename)
+    copyResourceToFile(s"/annotations/$filename", annotationsTestFile)
+    optionsManager.firrtlOptions.annotations.length should be (0)
+    val annos = Driver.loadAnnotations(optionsManager)
+    annos.length should be (21) // 18 from files plus 3 general purpose
+    annos.count(_.transformClass == "firrtl.passes.InlineInstances") should be (18)
     annotationsTestFile.delete()
   }
 
@@ -181,9 +213,7 @@ class DriverSpec extends FreeSpec with Matchers with BackendCompilationUtilities
     val firrtlOptions = optionsManager.firrtlOptions
     firrtlOptions.annotations.length should be (1) // infer-rw
 
-    Driver.loadAnnotations(optionsManager)
-
-    val anns = optionsManager.firrtlOptions.annotations.groupBy(_.transform)
+    val anns = Driver.loadAnnotations(optionsManager).groupBy(_.transform)
     anns(classOf[BlackBoxSourceHelper]).length should be (1) // built in to loadAnnotations
     anns(classOf[InferReadWrite]).length should be (1) // --infer-rw
     anns(classOf[InlineInstances]).length should be (9) // annotations file