Merge pull request #3767 from Gedochao/maintenance/repl-with-java-24

Prevent the REPL from warning about restricted `java.lang.System` API on JDK 24

Author: Gedochao

modules/build/src/main/scala/scala/build/ReplArtifacts.scala

@@ -5,6 +5,8 @@ import coursier.core.Repository
 import coursier.util.Task
 import dependency.*
 
+import java.io.File
+
 import scala.build.EitherCps.{either, value}
 import scala.build.errors.BuildException
 import scala.build.internal.CsLoggerUtil.*
@@ -89,40 +91,69 @@ object ReplArtifacts {
     logger: Logger,
     cache: FileCache[Task],
     repositories: Seq[Repository],
-    addScalapy: Option[String]
+    addScalapy: Option[String],
+    javaVersion: Int
   ): Either[BuildException, ReplArtifacts] = either {
     val isScala2 = scalaParams.scalaVersion.startsWith("2.")
     val replDep  =
-      if (isScala2) dep"org.scala-lang:scala-compiler:${scalaParams.scalaVersion}"
+      if isScala2 then dep"org.scala-lang:scala-compiler:${scalaParams.scalaVersion}"
       else dep"org.scala-lang::scala3-compiler:${scalaParams.scalaVersion}"
     val scalapyDeps =
       addScalapy.map(ver => dep"${Artifacts.scalaPyOrganization(ver)}::scalapy-core::$ver").toSeq
-    val externalDeps  = dependencies ++ scalapyDeps
-    val replArtifacts =
+    val externalDeps                          = dependencies ++ scalapyDeps
+    val replArtifacts: Seq[(String, os.Path)] = value {
       Artifacts.artifacts(
         Seq(replDep).map(Positioned.none),
         repositories,
         Some(scalaParams),
         logger,
         cache.withMessage(s"Downloading Scala compiler ${scalaParams.scalaVersion}")
       )
-    val depArtifacts = Artifacts.artifacts(
-      externalDeps.map(Positioned.none),
-      repositories,
-      Some(scalaParams),
-      logger,
-      cache.withMessage(s"Downloading REPL dependencies")
-    )
+    }
+    val depArtifacts: Seq[(String, os.Path)] = value {
+      Artifacts.artifacts(
+        externalDeps.map(Positioned.none),
+        repositories,
+        Some(scalaParams),
+        logger,
+        cache.withMessage(s"Downloading REPL dependencies")
+      )
+    }
     val mainClass =
-      if (isScala2) "scala.tools.nsc.MainGenericRunner"
+      if isScala2 then "scala.tools.nsc.MainGenericRunner"
       else "dotty.tools.repl.Main"
+    val defaultReplJavaOpts = Seq("-Dscala.usejavacp=true")
+    val jlineArtifacts      =
+      replArtifacts
+        .map(_._2.toString)
+        .filter(_.contains("jline"))
+    val jlineJavaOpts: Seq[String] =
+      if javaVersion >= 24 && jlineArtifacts.nonEmpty then {
+        val modulePath    = Seq("--module-path", jlineArtifacts.mkString(File.pathSeparator))
+        val remainingOpts =
+          if isScala2 then
+            Seq(
+              "--add-modules",
+              "org.jline",
+              "--enable-native-access=org.jline"
+            )
+          else
+            Seq(
+              "--add-modules",
+              "org.jline.terminal",
+              "--enable-native-access=org.jline.nativ"
+            )
+        modulePath ++ remainingOpts
+      }
+      else Seq.empty
+    val replJavaOpts = defaultReplJavaOpts ++ jlineJavaOpts
     ReplArtifacts(
-      replArtifacts = value(replArtifacts),
-      depArtifacts = value(depArtifacts),
+      replArtifacts = replArtifacts,
+      depArtifacts = depArtifacts,
       extraClassPath = extraClassPath,
       extraSourceJars = Nil,
       replMainClass = mainClass,
-      replJavaOpts = Seq("-Dscala.usejavacp=true"),
+      replJavaOpts = replJavaOpts,
       addSourceJars = false
     )
   }

modules/cli/src/main/scala/scala/cli/commands/ScalaCommand.scala

@@ -244,16 +244,18 @@ abstract class ScalaCommand[T <: HasGlobalOptions](implicit myParser: Parser[T],
           }
           else if (shared.helpGroups.helpRepl) {
             val initialBuildOptions = buildOptionsOrExit(options)
-            val artifacts     = initialBuildOptions.artifacts(logger, Scope.Main).orExit(logger)
-            val replArtifacts = value {
+            val artifacts        = initialBuildOptions.artifacts(logger, Scope.Main).orExit(logger)
+            val javaVersion: Int = initialBuildOptions.javaHome().value.version
+            val replArtifacts    = value {
               ReplArtifacts.default(
-                scalaParams,
-                artifacts.userDependencies,
-                Nil,
-                logger,
-                buildOptions.finalCache,
-                Nil,
-                None
+                scalaParams = scalaParams,
+                dependencies = artifacts.userDependencies,
+                extraClassPath = Nil,
+                logger = logger,
+                cache = buildOptions.finalCache,
+                repositories = Nil,
+                addScalapy = None,
+                javaVersion = javaVersion
               )
             }
             replArtifacts.replClassPath -> replArtifacts.replMainClass

modules/cli/src/main/scala/scala/cli/commands/repl/Repl.scala

@@ -452,9 +452,10 @@ object Repl extends ScalaCommand[ReplOptions] with BuildCommandHelpers {
           cache,
           value(options.finalRepositories),
           addScalapy =
-            if (setupPython)
+            if setupPython then
               Some(options.notForBloopOptions.scalaPyVersion.getOrElse(Constants.scalaPyVersion))
-            else None
+            else None,
+          javaVersion = options.javaHome().value.version
         )
       }
     }

modules/integration/src/test/scala/scala/cli/integration/RunJdkTestDefinitions.scala

@@ -2,7 +2,8 @@ package scala.cli.integration
 
 import com.eed3si9n.expecty.Expecty.expect
 
-import scala.util.Properties
+import scala.cli.integration.TestUtil.ProcOps
+import scala.util.{Properties, Try}
 
 trait RunJdkTestDefinitions { _: RunTestDefinitions =>
   def javaIndex(javaVersion: Int): String =
@@ -129,5 +130,26 @@ trait RunJdkTestDefinitions { _: RunTestDefinitions =>
             }
         }
       }
+
+    // the warnings were introduced in JDK 24, so we only test this for JDKs >= 24
+    // the issue never affected Scala 2.12, so we skip it for that version
+    if (
+      !actualScalaVersion.startsWith("2.12") &&
+      !useScalaInstallationWrapper &&
+      Try(index.toInt).map(_ >= 24).getOrElse(false)
+    )
+      // TODO: test with Scala installation wrapper when the fix gets propagated there
+      test(s"REPL does not warn about restricted java.lang.System API called on JDK $index") {
+        TestInputs.empty.fromRoot { root =>
+          TestUtil.withProcessWatching(
+            proc = os.proc(TestUtil.cli, "repl", extraOptions, "--jvm", index)
+              .spawn(cwd = root, stderr = os.Pipe)
+          ) { (proc, _, ec) =>
+            proc.printStderrUntilJlineRevertsToDumbTerminal(proc) { s =>
+              expect(!s.contains("A restricted method in java.lang.System has been called"))
+            }(ec)
+          }
+        }
+      }
   }
 }

modules/integration/src/test/scala/scala/cli/integration/RunWithWatchTestDefinitions.scala

@@ -1,24 +1,12 @@
 package scala.cli.integration
 
 import com.eed3si9n.expecty.Expecty.expect
-import os.SubProcess
 
-import scala.concurrent.ExecutionContext
-import scala.concurrent.duration.{Duration, DurationInt}
+import scala.cli.integration.TestUtil.ProcOps
+import scala.concurrent.duration.DurationInt
 import scala.util.{Properties, Try}
 
 trait RunWithWatchTestDefinitions { _: RunTestDefinitions =>
-  implicit class ProcOps(proc: SubProcess) {
-    def printStderrUntilRerun(timeout: Duration)(implicit ec: ExecutionContext): Unit = {
-      def rerunWasTriggered(): Boolean = {
-        val stderrOutput = TestUtil.readLine(proc.stderr, ec, timeout)
-        println(stderrOutput)
-        stderrOutput.contains("re-run")
-      }
-      while (!rerunWasTriggered()) Thread.sleep(100L)
-    }
-  }
-
   // TODO make this pass reliably on Mac CI
   if (!Properties.isMac || !TestUtil.isCI) {
     val expectedMessage1 = "Hello"

modules/integration/src/test/scala/scala/cli/integration/TestUtil.scala

@@ -368,12 +368,38 @@ object TestUtil {
     finally if (proc.isAlive()) {
         proc.destroy()
         Thread.sleep(200L)
-        if (proc.isAlive()) proc.destroyForcibly()
+        if (proc.isAlive()) proc.destroy(shutdownGracePeriod = 0)
       }
 
   implicit class StringOps(a: String) {
     def countOccurrences(b: String): Int =
       if (b.isEmpty) 0 // Avoid infinite splitting
       else a.sliding(b.length).count(_ == b)
   }
+
+  def printStderrUntilCondition(
+    proc: os.SubProcess,
+    timeout: Duration = 90.seconds
+  )(condition: String => Boolean)(
+    f: String => Unit = _ => ()
+  )(implicit ec: ExecutionContext): Unit = {
+    def revertTriggered(): Boolean = {
+      val stderrOutput = TestUtil.readLine(proc.stderr, ec, timeout)
+      println(stderrOutput)
+      f(stderrOutput)
+      condition(stderrOutput)
+    }
+
+    while (!revertTriggered()) Thread.sleep(100L)
+  }
+
+  implicit class ProcOps(proc: os.SubProcess) {
+    def printStderrUntilJlineRevertsToDumbTerminal(proc: os.SubProcess)(
+      f: String => Unit
+    )(implicit ec: ExecutionContext): Unit =
+      TestUtil.printStderrUntilCondition(proc)(_.contains("creating a dumb terminal"))(f)
+
+    def printStderrUntilRerun(timeout: Duration)(implicit ec: ExecutionContext): Unit =
+      TestUtil.printStderrUntilCondition(proc, timeout)(_.contains("re-run"))()
+  }
 }

project/deps/package.mill.scala

@@ -83,7 +83,7 @@ object Java {
   def minimumBloopJava: Int      = 17
   def minimumInternalJava: Int   = 16
   def defaultJava: Int           = minimumBloopJava
-  def mainJavaVersions: Seq[Int] = Seq(8, 11, 17, 21, 23)
+  def mainJavaVersions: Seq[Int] = Seq(8, 11, 17, 21, 23, 24)
   def allJavaVersions: Seq[Int]  =
     (mainJavaVersions ++ Seq(minimumBloopJava, minimumInternalJava, defaultJava)).distinct
 }