Drop usage of deprecated SecurityManager (#13)

* drop usage of deprecated SecurityManager

* small fix

* patch call to main vs mainInner

src/main/scala/firrtl/options/Shell.scala

@@ -16,7 +16,10 @@ import java.util.ServiceLoader
 class Shell(val applicationName: String) {
 
   /** Command line argument parser (OptionParser) with modifications */
-  protected val parser = new OptionParser[AnnotationSeq](applicationName) with DuplicateHandling with ExceptOnError
+  protected val parser = new OptionParser[AnnotationSeq](applicationName)
+    with DuplicateHandling
+    with ExceptOnError
+    with DoNotTerminateOnExit
 
   /** Contains all discovered [[RegisteredLibrary]] */
   final lazy val registeredLibraries: Seq[RegisteredLibrary] = {

src/main/scala/firrtl/options/Stage.scala

@@ -54,9 +54,9 @@ abstract class Stage extends Phase {
     * @return output annotations
     * @throws firrtl.options.OptionsException if command line or annotation validation fails
     */
-  final def execute(args: Array[String], annotations: AnnotationSeq): AnnotationSeq =
+  final def execute(args: Array[String], annotations: AnnotationSeq): AnnotationSeq = {
     transform(shell.parse(args, annotations))
-
+  }
 }
 
 /** Provides a main method for a [[Stage]]
@@ -67,14 +67,19 @@ class StageMain(val stage: Stage) {
   /** The main function that serves as this stage's command line interface.
     * @param args command line arguments
     */
-  final def main(args: Array[String]): Unit = try {
-    stage.execute(args, Seq.empty)
-  } catch {
-    case a: StageError =>
-      System.exit(a.code.number)
-    case a: OptionsException =>
-      StageUtils.dramaticUsageError(a.message)
-      System.exit(1)
-  }
+  final def main(args: Array[String]): Unit =
+    System.exit(mainInner(args))
 
+  final def mainInner(args: Array[String]): Int = {
+    try {
+      stage.execute(args, Seq.empty)
+      0
+    } catch {
+      case a: StageError =>
+        a.code.number
+      case a: OptionsException =>
+        StageUtils.dramaticUsageError(a.message)
+        1
+    }
+  }
 }

src/main/scala/firrtl/stage/FirrtlCli.scala

@@ -21,9 +21,7 @@ trait FirrtlCli { this: Shell =>
     firrtl.EmitCircuitAnnotation,
     firrtl.EmitAllModulesAnnotation,
     NoCircuitDedupAnnotation,
-    WarnNoScalaVersionDeprecation,
     PrettyNoExprInlining,
-    DisableFold,
     OptimizeForFPGA,
     CurrentFirrtlStateAnnotation,
     CommonSubexpressionElimination,

src/test/scala/firrtl/testutils/FirrtlSpec.scala

@@ -3,7 +3,6 @@
 package firrtl.testutils
 
 import java.io._
-import java.security.Permission
 import scala.sys.process._
 
 import logger.{LazyLogging, LogLevel, LogLevelAnnotation}
@@ -458,75 +457,6 @@ trait Utils {
     val ret = scala.Console.withOut(stdout) { scala.Console.withErr(stderr) { thunk } }
     (stdout.toString, stderr.toString, ret)
   }
-
-  /** Encodes a System.exit exit code
-    * @param status the exit code
-    */
-  private case class ExitException(status: Int) extends SecurityException(s"Found a sys.exit with code $status")
-
-  /** A security manager that converts calls to System.exit into [[ExitException]]s by explicitly disabling the ability of
-    * a thread to actually exit. For more information, see:
-    *   - https://docs.oracle.com/javase/tutorial/essential/environment/security.html
-    */
-  private class ExceptOnExit extends SecurityManager {
-    override def checkPermission(perm: Permission): Unit = {}
-    override def checkPermission(perm: Permission, context: Object): Unit = {}
-    override def checkExit(status: Int): Unit = {
-      super.checkExit(status)
-      throw ExitException(status)
-    }
-  }
-
-  /** Encodes a file that some code tries to write to
-    * @param the file name
-    */
-  private case class WriteException(file: String) extends SecurityException(s"Tried to write to file $file")
-
-  /** A security manager that converts writes to any file into [[WriteException]]s.
-    */
-  private class ExceptOnWrite extends SecurityManager {
-    override def checkPermission(perm: Permission): Unit = {}
-    override def checkPermission(perm: Permission, context: Object): Unit = {}
-    override def checkWrite(file: String): Unit = {
-      super.checkWrite(file)
-      throw WriteException(file)
-    }
-  }
-
-  /** Run some Scala code (a thunk) in an environment where all System.exit are caught and returned. This avoids a
-    * situation where a test results in something actually exiting and killing the entire test. This is necessary if you
-    * want to test a command line program, e.g., the `main` method of [[firrtl.options.Stage Stage]].
-    *
-    * NOTE: THIS WILL NOT WORK IN SITUATIONS WHERE THE THUNK IS CATCHING ALL [[Exception]]s OR [[Throwable]]s, E.G.,
-    * SCOPT. IF THIS IS HAPPENING THIS WILL NOT WORK. REPEAT THIS WILL NOT WORK.
-    * @param thunk some Scala code
-    * @return either the output of the thunk (`Right[T]`) or an exit code (`Left[Int]`)
-    */
-  def catchStatus[T](thunk: => T): Either[Int, T] = {
-    try {
-      System.setSecurityManager(new ExceptOnExit())
-      Right(thunk)
-    } catch {
-      case ExitException(a) => Left(a)
-    } finally {
-      System.setSecurityManager(null)
-    }
-  }
-
-  /** Run some Scala code (a thunk) in an environment where file writes are caught and the file that a program tries to
-    * write to is returned. This is useful if you want to test that some thunk either tries to write to a specific file
-    * or doesn't try to write at all.
-    */
-  def catchWrites[T](thunk: => T): Either[String, T] = {
-    try {
-      System.setSecurityManager(new ExceptOnWrite())
-      Right(thunk)
-    } catch {
-      case WriteException(a) => Left(a)
-    } finally {
-      System.setSecurityManager(null)
-    }
-  }
 }
 
 /** Super class for equivalence driven Firrtl tests */

src/test/scala/firrtlTests/annotationTests/UnrecognizedAnnotationSpec.scala

@@ -66,7 +66,7 @@ class UnrecognizedAnnotationSpec extends FirrtlFlatSpec {
     val fileNames = setupFiles(addAllowUnrecognizedFlag = false, addAllowUnrecognizedAnno = false)
     val args = makeCommandLineArgs(fileNames)
     val e = intercept[InvalidAnnotationFileException] {
-      FirrtlMain.main(args)
+      FirrtlMain.mainInner(args)
     }
 
     e.getMessage should include(fileNames.inputAnnotations)
@@ -91,7 +91,7 @@ class UnrecognizedAnnotationSpec extends FirrtlFlatSpec {
 
   def shouldSucceed(fileNames: TestFileNames): Unit = {
     val args = makeCommandLineArgs(fileNames)
-    FirrtlMain.main(args)
+    FirrtlMain.mainInner(args)
 
     val outputAnnotationText = FileUtils.getText(fileNames.outputAnnotationsFull)
     outputAnnotationText should include("freechips.rocketchip.util.RegFieldDescMappingAnnotation")

src/test/scala/firrtlTests/options/OptionParserSpec.scala

@@ -19,28 +19,41 @@ class OptionParserSpec extends AnyFlatSpec with Matchers with firrtl.testutils.U
 
   /* An option parser that prepends to a Seq[Int] */
   class IntParser extends OptionParser[AnnotationSeq]("Int Parser") {
-    opt[Int]("integer").abbr("n").unbounded.action((x, c) => IntAnnotation(x) +: c)
+    opt[Int]("integer").abbr("n").unbounded().action((x, c) => IntAnnotation(x) +: c)
     help("help")
   }
 
   trait DuplicateShortOption { this: OptionParser[AnnotationSeq] =>
-    opt[Int]("not-an-integer").abbr("n").unbounded.action((x, c) => IntAnnotation(x) +: c)
+    opt[Int]("not-an-integer").abbr("n").unbounded().action((x, c) => IntAnnotation(x) +: c)
   }
 
   trait DuplicateLongOption { this: OptionParser[AnnotationSeq] =>
-    opt[Int]("integer").abbr("m").unbounded.action((x, c) => IntAnnotation(x) +: c)
+    opt[Int]("integer").abbr("m").unbounded().action((x, c) => IntAnnotation(x) +: c)
   }
 
-  trait WithIntParser { val parser = new IntParser }
+  var terminateStatus: Either[String, Unit] = null
+  trait TerminateToVariable[T] extends OptionParser[T] {
+    override def terminate(exitState: Either[String, Unit]): Unit = terminateStatus = exitState
+  }
+
+  trait WithIntParser {
+    val parser = new IntParser
+  }
+
+  trait WithIntTerminateCaptureParser {
+    val parser = new IntParser with TerminateToVariable[AnnotationSeq]
+  }
 
   behavior.of("A default OptionsParser")
 
-  it should "call sys.exit if terminate is called" in new WithIntParser {
+  it should "call sys.exit if terminate is called" in new WithIntTerminateCaptureParser {
     info("exit status of 1 for failure")
-    catchStatus { parser.terminate(Left("some message")) } should be(Left(1))
+    parser.terminate(Left("some message"))
+    terminateStatus should be(Left("some message"))
 
     info("exit status of 0 for success")
-    catchStatus { parser.terminate(Right(())) } should be(Left(0))
+    parser.terminate(Right(()))
+    terminateStatus should be(Right(()))
   }
 
   it should "print to stderr on an invalid option" in new WithIntParser {
@@ -52,11 +65,12 @@ class OptionParserSpec extends AnyFlatSpec with Matchers with firrtl.testutils.U
   it should "disable sys.exit for terminate method" in {
     val parser = new IntParser with DoNotTerminateOnExit
 
+    // parser.terminate() should never call sys.exit(), if it does, then the JVM will terminate
     info("no exit for failure")
-    catchStatus { parser.terminate(Left("some message")) } should be(Right(()))
+    parser.terminate(Left("some message"))
 
     info("no exit for success")
-    catchStatus { parser.terminate(Right(())) } should be(Right(()))
+    parser.terminate(Right(()))
   }
 
   behavior.of("An OptionParser with DuplicateHandling mixed in")

src/test/scala/firrtlTests/options/phases/WriteOutputAnnotationsSpec.scala

@@ -106,25 +106,6 @@ class WriteOutputAnnotationsSpec extends AnyFlatSpec with Matchers with firrtl.t
     fileContainsAnnotations(file, expected)
   }
 
-  it should "do nothing if no output annotation file is specified" in new Fixture {
-    val annotations = Seq(
-      WriteOutputAnnotationsSpec.FooAnnotation,
-      WriteOutputAnnotationsSpec.BarAnnotation(0),
-      WriteOutputAnnotationsSpec.BarAnnotation(1)
-    )
-
-    val out = catchWrites { phase.transform(annotations) } match {
-      case Right(a) =>
-        info("no file writes occurred")
-        a
-      case Left(a) =>
-        fail(s"No file writes expected, but a write to '$a' ocurred!")
-    }
-
-    info("annotations are unmodified")
-    out.toSeq should be(annotations)
-  }
-
   it should "write CustomFileEmission annotations" in new Fixture {
     val file = new File("write-CustomFileEmission-annotations.anno.json")
     val annotations = Seq(

src/test/scala/firrtlTests/stage/FirrtlMainSpec.scala

@@ -72,7 +72,7 @@ class FirrtlMainSpec extends AnyFeatureSpec with GivenWhenThen with Matchers wit
 
       When(s"""the user tries to compile with '${p.argsString}'""")
       val (stdout, stderr, result) =
-        grabStdOutErr { catchStatus { f.stage.main(inputFile ++ Array("-td", td.buildDir.toString) ++ p.args) } }
+        grabStdOutErr { f.stage.mainInner(inputFile ++ Array("-td", td.buildDir.toString) ++ p.args) }
 
       p.stdout match {
         case Some(a) =>
@@ -95,10 +95,10 @@ class FirrtlMainSpec extends AnyFeatureSpec with GivenWhenThen with Matchers wit
       p.result match {
         case 0 =>
           And(s"the exit code should be 0")
-          result shouldBe a[Right[_, _]]
+          result shouldBe 0
         case a =>
           And(s"the exit code should be $a")
-          result shouldBe (Left(a))
+          result shouldBe a
       }
 
       p.files.foreach { f =>
@@ -163,12 +163,7 @@ class FirrtlMainSpec extends AnyFeatureSpec with GivenWhenThen with Matchers wit
       val f = new FirrtlMainFixture
 
       When("the user passes '--help'")
-      /* Note: THIS CANNOT CATCH THE STATUS BECAUSE SCOPT CATCHES ALL THROWABLE!!! The catchStatus is only used to prevent
-       * sys.exit from killing the test. However, this should additionally return an exit code of 0 and not print an
-       * error. The nature of running this through catchStatus causes scopt to intercept the custom SecurityException
-       * and then use that as evidence to exit with a code of 1.
-       */
-      val (out, _, result) = grabStdOutErr { catchStatus { f.stage.main(Array("--help")) } }
+      val (out, _, result) = grabStdOutErr { f.stage.mainInner(Array("--help")) }
 
       Then("the usage text should be shown")
       out should include("Usage: firrtl")
@@ -278,20 +273,17 @@ class FirrtlMainSpec extends AnyFeatureSpec with GivenWhenThen with Matchers wit
       val outName = "FirrtlMainSpecNoTargetDirectory"
       val out = new File(s"$outName.hi.fir")
       out.delete()
-      val result = catchStatus {
-        f.stage.main(
-          Array("-i", "src/test/resources/integration/GCDTester.fir", "-o", outName, "-X", "high", "-E", "high")
-        )
-      }
+      val result = f.stage.mainInner(
+        Array("-i", "src/test/resources/integration/GCDTester.fir", "-o", outName, "-X", "high", "-E", "high")
+      )
 
       Then("outputs should be written to current directory")
       out should (exist)
       out.delete()
 
       And("the exit code should be 0")
-      result shouldBe a[Right[_, _]]
+      result shouldBe 0
     }
-
   }
 
   info("As a FIRRTL command line user")
@@ -301,7 +293,7 @@ class FirrtlMainSpec extends AnyFeatureSpec with GivenWhenThen with Matchers wit
       val f = new FirrtlMainFixture
 
       When("the user passes no arguments")
-      val (out, err, result) = grabStdOutErr { catchStatus { f.stage.main(Array.empty) } }
+      val (out, err, result) = grabStdOutErr { f.stage.mainInner(Array.empty) }
 
       Then("an error should be printed on stdout")
       out should include(s"Error: Unable to determine FIRRTL source to read")
@@ -313,7 +305,7 @@ class FirrtlMainSpec extends AnyFeatureSpec with GivenWhenThen with Matchers wit
       err should be(empty)
 
       And("the exit code should be 1")
-      result should be(Left(1))
+      result should be(1)
     }
   }
 
@@ -362,7 +354,7 @@ class FirrtlMainSpec extends AnyFeatureSpec with GivenWhenThen with Matchers wit
       val f = new FirrtlMainFixture
 
       When("the user passes '--show-registrations'")
-      val (out, _, result) = grabStdOutErr { catchStatus { f.stage.main(Array("--show-registrations")) } }
+      val (out, _, result) = grabStdOutErr { f.stage.mainInner(Array("--show-registrations")) }
 
       Then("stdout should show registered transforms")
       out should include("firrtl.passes.InlineInstances")
@@ -371,7 +363,7 @@ class FirrtlMainSpec extends AnyFeatureSpec with GivenWhenThen with Matchers wit
       out should include("firrtl.passes.memlib.MemLibOptions")
 
       And("the exit code should be 1")
-      result should be(Left(1))
+      result should be(1)
     }
   }