From 8e3b9cea219c5899f7fea1de09f5f7f169aa59fb Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Fri, 29 Mar 2024 16:24:44 +0100 Subject: [PATCH] improvement: handle java home setting (#6204) * improvement: add `-release` option to pc when needed * use metals JDK for running bloop * update build server jdk on `javaHome` setting change * switch java home for sbt * delete old scaladoc * use correct `java home` for `-release` flag * simplify logic of setting bloop java home * add warn if Metals version too low * add tests + handle mill * try fixing Ci * review fixes --- .../scala/meta/internal/bsp/BspServers.scala | 5 +- .../meta/internal/builds/ShellRunner.scala | 3 +- .../meta/internal/metals/BloopServers.scala | 205 +++--------------- .../meta/internal/metals/Compilers.scala | 55 ++++- .../meta/internal/metals/JavaBinary.scala | 2 +- .../scala/meta/internal/metals/Messages.scala | 61 +++--- .../internal/metals/MetalsLspService.scala | 81 ++++++- .../meta/internal/metals/JdkSources.scala | 27 +++ .../scala/tests/mill/MillServerSuite.scala | 16 +- .../scala/tests/sbt/SbtBloopLspSuite.scala | 21 +- .../test/scala/tests/sbt/SbtServerSuite.scala | 9 +- .../main/scala/tests/JavaHomeChangeTest.scala | 65 ++++++ .../src/main/scala/tests/TestingClient.scala | 9 + .../scala/tests/BloopJavaHomeLspSuite.scala | 13 +- .../src/test/scala/tests/Java8Suite.scala | 61 ++++++ 15 files changed, 387 insertions(+), 246 deletions(-) create mode 100644 tests/unit/src/main/scala/tests/JavaHomeChangeTest.scala create mode 100644 tests/unit/src/test/scala/tests/Java8Suite.scala diff --git a/metals/src/main/scala/scala/meta/internal/bsp/BspServers.scala b/metals/src/main/scala/scala/meta/internal/bsp/BspServers.scala index e2a18d0e236..54a14c2958f 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BspServers.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BspServers.scala @@ -100,10 +100,7 @@ final class BspServers( args, projectDirectory, redirectErrorOutput = false, - JdkSources - .defaultJavaHome(userConfig().javaHome) - .map("JAVA_HOME" -> _.toString()) - .toMap, + JdkSources.envVariables(userConfig().javaHome), processOut = None, processErr = Some(l => scribe.info("BSP server: " + l)), discardInput = false, diff --git a/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala b/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala index ea6868a288f..bdc55461807 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/ShellRunner.scala @@ -12,6 +12,7 @@ import scala.util.Properties import scala.meta.internal.metals.Cancelable import scala.meta.internal.metals.JavaBinary +import scala.meta.internal.metals.JdkSources import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.MutableCancelable import scala.meta.internal.metals.StatusBar @@ -118,7 +119,7 @@ class ShellRunner( logInfo: Boolean = true, ): Future[Int] = { val elapsed = new Timer(time) - val env = additionalEnv ++ javaHome.map("JAVA_HOME" -> _).toMap + val env = additionalEnv ++ JdkSources.envVariables(javaHome) val ps = SystemProcess.run( args, directory, diff --git a/metals/src/main/scala/scala/meta/internal/metals/BloopServers.scala b/metals/src/main/scala/scala/meta/internal/metals/BloopServers.scala index dc9a0c53d54..d3964850148 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/BloopServers.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/BloopServers.scala @@ -17,7 +17,6 @@ import scala.util.Try import scala.meta.internal.bsp.BuildChange import scala.meta.internal.bsp.ConnectionBspStatus -import scala.meta.internal.metals.BloopJsonUpdateCause.BloopJsonUpdateCause import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.clients.language.MetalsLanguageClient import scala.meta.io.AbsolutePath @@ -25,9 +24,7 @@ import scala.meta.io.AbsolutePath import bloop.bloopgun.BloopgunCli import bloop.bloopgun.core.Shell import bloop.launcher.LauncherMain -import org.eclipse.lsp4j.MessageActionItem import org.eclipse.lsp4j.MessageType -import org.eclipse.lsp4j.Position /** * Establishes a connection with a bloop server using Bloop Launcher. @@ -52,12 +49,13 @@ final class BloopServers( private val bloopJsonPath: Option[AbsolutePath] = getBloopFilePath(fileName = "bloop.json") + + // historically used file created by Metals + // now we just delete it if existed to cleanup private val bloopLockFile: Option[AbsolutePath] = getBloopFilePath(fileName = "created_by_metals.lock") - private def bloopLastModifiedTime: Long = bloopJsonPath - .flatMap(path => Try(path.toNIO.toFile().lastModified()).toOption) - .getOrElse(0L) + private def metalsJavaHome = sys.props.get("java.home") def shutdownServer(): Boolean = { val dummyIn = new ByteArrayInputStream(new Array(0)) @@ -145,10 +143,9 @@ final class BloopServers( } private def writeJVMPropertiesToBloopGlobalJsonFile( - maybeBloopJvmProperties: List[String], - maybeJavaHome: Option[String], + maybeBloopJvmProperties: List[String] ): Try[Unit] = Try { - if (maybeJavaHome.isDefined || maybeBloopJvmProperties.nonEmpty) { + if (metalsJavaHome.isDefined || maybeBloopJvmProperties.nonEmpty) { val javaOptionsField = if (maybeBloopJvmProperties.nonEmpty) Some( @@ -159,74 +156,31 @@ final class BloopServers( else None val fields: List[(String, ujson.Value)] = List( - maybeJavaHome.map(v => "javaHome" -> ujson.Str(v.trim())), + metalsJavaHome.map(v => "javaHome" -> ujson.Str(v.trim())), javaOptionsField, ).flatten val obj = ujson.Obj.from(fields) val jvmPropertiesString = ujson.write(obj) bloopJsonPath.foreach(_.writeText(jvmPropertiesString)) - bloopLockFile.foreach(_.writeText(bloopLastModifiedTime.toString())) - } - } - - private def processUserPreferenceForBloopJvmProperties( - messageActionItem: MessageActionItem, - maybeBloopJvmProperties: List[String], - maybeJavaHome: Option[String], - reconnect: () => Future[BuildChange], - ): Future[Unit] = { - (messageActionItem, bloopJsonPath) match { - case (item, _) - if item == Messages.BloopGlobalJsonFilePremodified.applyAndRestart => - writeJVMPropertiesToBloopGlobalJsonFile( - maybeBloopJvmProperties, - maybeJavaHome, - ) match { - case Failure(exception) => Future.failed(exception) - case Success(_) => - shutdownServer() - reconnect().ignoreValue - } - - case (item, Some(bloopPath)) - if item == Messages.BloopGlobalJsonFilePremodified.openGlobalJsonFile => - val position = new Position(0, 0) - val range = new org.eclipse.lsp4j.Range(position, position) - val command = ClientCommands.GotoLocation.toExecuteCommandParams( - ClientCommands.WindowLocation( - bloopPath.toURI.toString, - range, - ) - ) - Future.successful(languageClient.metalsExecuteClientCommand(command)) - - case (item, _) - if item == Messages.BloopGlobalJsonFilePremodified.useGlobalFile => - tables.dismissedNotifications.UpdateBloopJson.dismissForever() - Future.unit - case _ => Future.unit - + bloopLockFile.foreach(_.deleteIfExists()) } } private def updateBloopGlobalJsonFileThenRestart( maybeBloopJvmProperties: List[String], - maybeJavaHome: Option[String], reconnect: () => Future[BuildChange], - bloopJsonUpdateCause: BloopJsonUpdateCause, ): Future[Unit] = { languageClient .showMessageRequest( - Messages.BloopJvmPropertiesChange.params(bloopJsonUpdateCause) + Messages.BloopJvmPropertiesChange.params() ) .asScala .flatMap { case messageActionItem if messageActionItem == Messages.BloopJvmPropertiesChange.reconnect => writeJVMPropertiesToBloopGlobalJsonFile( - maybeBloopJvmProperties, - maybeJavaHome, + maybeBloopJvmProperties ) match { case Failure(exception) => Future.failed(exception) case Success(_) => @@ -260,32 +214,10 @@ final class BloopServers( (maybeJavaHome, maybeJavaOptions.getOrElse(Nil)) } - /** - * Determine whether or not we need to update the javaHome setting in the bloop.json file. - * - * @param metalsJavaHome javaHome being passed in from the user - * @param bloopJavaHome bloop javaHome that is in the global config - * @return whether or not the javaHome needs to be updated - */ - private def needsJavaHomeUpdate( - metalsJavaHome: Option[String], - bloopJavaHome: Option[String], - ) = { - (metalsJavaHome, bloopJavaHome) match { - // Metals is set but Bloop isn't - case (Some(_), None) => true - // Metals and Bloop are set, but they aren't the same - case (Some(m), Some(b)) if m != b => true - case _ => false - } - } - /** * First we check if the user requested to update the Bloop JVM * properties through the extension. - *

If so, we also check if the Bloop's Global Json file exists - * and if it was pre-modified by the user. - *

Then, through consultation with the user through appropriate + *

Through consultation with the user through appropriate * dialogues we decide if we should *

* - * @param maybeRequestedBloopJvmProperties Bloop JVM Properties requested - * through the Metals Extension settings - * @param reconnect function to connect back to the - * build server. + * @param requestedBloopJvmProperties Bloop JVM Properties requested + * through the Metals Extension settings + * @param reconnect function to connect back to the + * build server. * @return `Future.successful` if the purpose is achieved or `Future.failure` - * if a problem occured such as lacking enough permissions to open or + * if a problem occurred such as lacking enough permissions to open or * write to files */ def ensureDesiredJvmSettings( - maybeRequestedBloopJvmProperties: Option[List[String]], - maybeRequestedMetalsJavaHome: Option[String], + requestedBloopJvmProperties: List[String], reconnect: () => Future[BuildChange], ): Future[Unit] = { val result = @@ -313,80 +244,23 @@ final class BloopServers( if bloopPath.canWrite (maybeBloopGlobalJsonJavaHome, maybeBloopGlobalJsonJvmProperties) = maybeLoadBloopGlobalJsonFile(bloopPath) - bloopJsonUpdateCause <- - if ( - maybeRequestedBloopJvmProperties - .exists(requested => - requested != maybeBloopGlobalJsonJvmProperties - ) - ) Some(BloopJsonUpdateCause.JVM_OPTS) - else if ( - needsJavaHomeUpdate( - maybeRequestedMetalsJavaHome, - maybeBloopGlobalJsonJavaHome, - ) - ) - Some(BloopJsonUpdateCause.JAVA_HOME) - else None - maybeBloopJvmProperties = maybeRequestedBloopJvmProperties.getOrElse( - maybeBloopGlobalJsonJvmProperties - ) + if (requestedBloopJvmProperties != maybeBloopGlobalJsonJvmProperties) } yield updateBloopJvmProperties( - maybeBloopJvmProperties, - maybeRequestedMetalsJavaHome, + requestedBloopJvmProperties, reconnect, - bloopJsonUpdateCause, ) - - result.getOrElse { - Future.unit - } + result.getOrElse(Future.unit) } private def updateBloopJvmProperties( maybeBloopJvmProperties: List[String], - maybeJavaHome: Option[String], reconnect: () => Future[BuildChange], - bloopJsonUpdateCause: BloopJsonUpdateCause, ): Future[Unit] = { - val lockFileTime = bloopLockFile - .flatMap(file => Try(file.readText.toLong).toOption) - .getOrElse(0L) if (tables.dismissedNotifications.UpdateBloopJson.isDismissed) Future.unit - else if ( - bloopJsonPath.exists(_.exists) && bloopLastModifiedTime > lockFileTime - ) { - // the global json file was previously modified by the user through other means; - // therefore overwriting it requires user input - languageClient - .showMessageRequest( - Messages.BloopGlobalJsonFilePremodified.params(bloopJsonUpdateCause) - ) - .asScala - .flatMap { - processUserPreferenceForBloopJvmProperties( - _, - maybeBloopJvmProperties, - maybeJavaHome, - reconnect, - ) andThen { - case Failure(exception) => - languageClient.showMessage( - MessageType.Error, - exception.getMessage, - ) - case Success(_) => Future.unit - } - } - } else { - // bloop global json file did not exist; or it was last modified by metals; - // hence it can get created or overwritten by Metals with no worries - // about overriding the user preferred settings + else { updateBloopGlobalJsonFileThenRestart( maybeBloopJvmProperties, - maybeJavaHome, reconnect, - bloopJsonUpdateCause, ) andThen { case Failure(exception) => languageClient.showMessage( @@ -401,10 +275,6 @@ final class BloopServers( private def updateBloopJavaHomeBeforeLaunch( userConfiguration: UserConfiguration ) = { - def metalsJavaHome = - userConfiguration.javaHome - .orElse(sys.env.get("JAVA_HOME")) - .orElse(sys.props.get("java.home")) // we should set up Java before running Bloop in order to not restart it bloopJsonPath match { case Some(bloopPath) if !bloopPath.exists => @@ -413,34 +283,17 @@ final class BloopServers( } // we want to use the same java version as Metals, so it's ok to use java.home writeJVMPropertiesToBloopGlobalJsonFile( - userConfiguration.bloopJvmProperties.getOrElse(Nil), - metalsJavaHome, + userConfiguration.bloopJvmProperties.getOrElse(Nil) ) case Some(bloopPath) if bloopPath.exists => maybeLoadBloopGlobalJsonFile(bloopPath) match { case (Some(javaHome), opts) => - Try { - val homePath = AbsolutePath(Paths.get(javaHome)) - // fix java home in case it changed - if (!homePath.exists) { + metalsJavaHome.foreach { newHome => + if (newHome != javaHome) { scribe.info( - s"Detected non existing java path in $bloopPath file" - ) - writeJVMPropertiesToBloopGlobalJsonFile( - opts, - metalsJavaHome, + s"Replacing bloop java home $javaHome with java home at $newHome." ) - metalsJavaHome.foreach { newHome => - scribe.info( - s"Replacing it with java home at $newHome" - ) - } - } else { - scribe.info(s"Bloop uses $javaHome defined at $bloopPath") - if (opts.nonEmpty) - scribe.info( - s"Bloop currently uses settings: ${opts.mkString(",")}" - ) + writeJVMPropertiesToBloopGlobalJsonFile(opts) } } case _ => @@ -528,12 +381,6 @@ final class BloopServers( } } -object BloopJsonUpdateCause extends Enumeration { - type BloopJsonUpdateCause = Value - val JAVA_HOME: Value = Value("Metals Java Home") - val JVM_OPTS: Value = Value("Bloop JVM Properties") -} - object BloopServers { val name = "Bloop" diff --git a/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala b/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala index 8ed669053dc..3ae03934867 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala @@ -103,6 +103,59 @@ class Compilers( private val worksheetsDigests = new TrieMap[AbsolutePath, String]() + private def enrichWithReleaseOption(scalaTarget: ScalaTarget) = { + val scalacOptions = scalaTarget.scalac.getOptions().asScala.toSeq + def existsReleaseSetting = scalacOptions.exists(opt => + opt.startsWith("-release") || + opt.startsWith("--release") || + opt.startsWith("-java-output-version") + ) + if (existsReleaseSetting) scalacOptions + else { + def optBuildTargetJvmVersion = + scalaTarget.jvmVersion + .flatMap(version => JdkVersion.parse(version)) + .orElse { + val javaHome = + scalaTarget.jvmHome + .flatMap(_.toAbsolutePathSafe) + .orElse { + for { + javaHomeString <- userConfig().javaHome.map(_.trim()) + if (javaHomeString.nonEmpty) + javaHome <- Try(AbsolutePath(javaHomeString)).toOption + } yield javaHome + } + JdkVersion.maybeJdkVersionFromJavaHome(javaHome) + } + + val releaseVersion = + for { + jvmVersion <- optBuildTargetJvmVersion + metalsJavaVersion <- Option(sys.props("java.version")) + .flatMap(JdkVersion.parse) + _ <- + if (jvmVersion.major < metalsJavaVersion.major) Some(()) + else if (metalsJavaVersion.major > jvmVersion.major) { + scribe.warn( + s"""|Your project uses JDK version ${jvmVersion.major} and + |Metals server is running on JDK version ${metalsJavaVersion.major}. + |This might cause incorrect completions, since + |Metals JDK version should be greater or equal the project's JDK version. + |""".stripMargin + ) + None + } else None + } yield jvmVersion.major + + releaseVersion match { + case Some(version) => + scalacOptions ++ List("-release", version.toString()) + case _ => scalacOptions + } + } + } + private val cache = jcache.asScala private def buildTargetPCFromCache( id: BuildTargetIdentifier @@ -1244,7 +1297,7 @@ class Compilers( ): PresentationCompiler = { newCompiler( mtags, - target.scalac.getOptions().asScala.toSeq, + enrichWithReleaseOption(target), classpath, search, target.scalac.getTarget.getUri, diff --git a/metals/src/main/scala/scala/meta/internal/metals/JavaBinary.scala b/metals/src/main/scala/scala/meta/internal/metals/JavaBinary.scala index 4a4c2d6a099..6fbea711b46 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/JavaBinary.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/JavaBinary.scala @@ -37,7 +37,7 @@ object JavaBinary { private def fromAbsolutePath(javaPath: Iterable[AbsolutePath]) = { javaPath - .flatMap(home => List(home.resolve("bin"), home.resolve("bin/jre"))) + .flatMap(home => List(home.resolve("bin"), home.resolve("jre/bin"))) .flatMap(bin => List("java", "java.exe").map(bin.resolve)) .withFilter(_.exists) .flatMap(binary => List(binary.dealias, binary)) diff --git a/metals/src/main/scala/scala/meta/internal/metals/Messages.scala b/metals/src/main/scala/scala/meta/internal/metals/Messages.scala index 2673798475e..370c42204d6 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Messages.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Messages.scala @@ -7,7 +7,6 @@ import scala.collection.mutable import scala.meta.internal.builds.BuildTool import scala.meta.internal.builds.VersionRecommendation import scala.meta.internal.jdk.CollectionConverters._ -import scala.meta.internal.metals.BloopJsonUpdateCause.BloopJsonUpdateCause import scala.meta.internal.metals.clients.language.MetalsInputBoxParams import scala.meta.internal.metals.clients.language.MetalsSlowTaskParams import scala.meta.internal.metals.clients.language.MetalsStatusParams @@ -388,36 +387,6 @@ object Messages { } } - object BloopGlobalJsonFilePremodified { - def applyAndRestart: MessageActionItem = - new MessageActionItem("Apply and Restart Bloop") - - def useGlobalFile: MessageActionItem = - new MessageActionItem("Use the Global File's JVM Properties") - - def openGlobalJsonFile: MessageActionItem = - new MessageActionItem("Open the Global File") - - def params( - bloopJsonUpdateCause: BloopJsonUpdateCause - ): ShowMessageRequestParams = { - val params = new ShowMessageRequestParams() - params.setMessage( - s"""|Setting $bloopJsonUpdateCause will result in updating Bloop's global Json file by Metals, which has been previously modified manually! - |Do you want to replace them with the new properties and restart the running Bloop server?""".stripMargin - ) - params.setType(MessageType.Warning) - params.setActions( - List( - applyAndRestart, - useGlobalFile, - openGlobalJsonFile, - ).asJava - ) - params - } - } - object BloopJvmPropertiesChange { def reconnect: MessageActionItem = new MessageActionItem("Apply and restart Bloop") @@ -425,12 +394,10 @@ object Messages { def notNow: MessageActionItem = new MessageActionItem("Not now") - def params( - bloopJsonUpdateCause: BloopJsonUpdateCause - ): ShowMessageRequestParams = { + def params(): ShowMessageRequestParams = { val params = new ShowMessageRequestParams() params.setMessage( - s"""|Setting $bloopJsonUpdateCause will result in updating Bloop's global Json file, by Metals. + s"""|Setting Bloop JVM Properties will result in updating Bloop's global Json file, by Metals. |Bloop will need to be restarted in order for these changes to take effect.""".stripMargin ) params.setType(MessageType.Warning) @@ -1019,6 +986,30 @@ object Messages { } } + object ProjectJavaHomeUpdate { + val restart: MessageActionItem = + new MessageActionItem("Restart/Reconnect to the build server") + + val notNow: MessageActionItem = + new MessageActionItem("Not now") + + def params(isRestart: Boolean): ShowMessageRequestParams = { + val params = new ShowMessageRequestParams() + params.setMessage( + s"Java home has been updated, do you want to ${if (isRestart) "restart" + else "reconnect"} to the BSP server?" + ) + params.setType(MessageType.Info) + params.setActions( + List( + restart, + notNow, + ).asJava + ) + params + } + } + object RequestTimeout { val cancel = new MessageActionItem("Cancel") diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala index b6ce1a5bfb5..1095dac3e6b 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -35,6 +35,7 @@ import scala.meta.internal.builds.BuildTool import scala.meta.internal.builds.BuildToolSelector import scala.meta.internal.builds.BuildTools import scala.meta.internal.builds.Digest +import scala.meta.internal.builds.MillBuildTool import scala.meta.internal.builds.SbtBuildTool import scala.meta.internal.builds.ScalaCliBuildTool import scala.meta.internal.builds.ShellRunner @@ -963,7 +964,10 @@ class MetalsLspService( } } - if (userConfig.symbolPrefixes != old.symbolPrefixes) { + if ( + userConfig.symbolPrefixes != old.symbolPrefixes || + userConfig.javaHome != old.javaHome + ) { compilers.restartAll() } @@ -995,11 +999,19 @@ class MetalsLspService( () => autoConnectToBuildServer, ) .flatMap { _ => - bloopServers.ensureDesiredJvmSettings( - userConfig.bloopJvmProperties, - userConfig.javaHome, - () => autoConnectToBuildServer(), - ) + userConfig.bloopJvmProperties + .map( + bloopServers.ensureDesiredJvmSettings( + _, + () => autoConnectToBuildServer(), + ) + ) + .getOrElse(Future.unit) + } + .flatMap { _ => + if (userConfig.javaHome != old.javaHome) { + updateBspJavaHome(session) + } else Future.unit } } else if ( userConfig.ammoniteJvmProperties != old.ammoniteJvmProperties && buildTargets.allBuildTargetIds @@ -1013,13 +1025,13 @@ class MetalsLspService( if item == Messages.AmmoniteJvmParametersChange.restart => ammonite.reload() case _ => - Future.successful(()) + Future.unit } - } else { - Future.successful(()) - } + } else if (userConfig.javaHome != old.javaHome) { + updateBspJavaHome(session) + } else Future.unit } - .getOrElse(Future.successful(())) + .getOrElse(Future.unit) for { _ <- slowConnect @@ -1027,6 +1039,53 @@ class MetalsLspService( } yield () } + private def updateBspJavaHome(session: BspSession) = { + if (session.main.isBazel) { + languageClient.showMessage( + MessageType.Warning, + "Java home setting is not available for Bazel bsp, please use env var instead.", + ) + Future.successful(()) + } else { + languageClient + .showMessageRequest( + Messages.ProjectJavaHomeUpdate + .params(isRestart = !session.main.isBloop) + ) + .asScala + .flatMap { + case Messages.ProjectJavaHomeUpdate.restart => + buildTool match { + case Some(sbt: SbtBuildTool) if session.main.isSbt => + for { + _ <- disconnectOldBuildServer() + _ <- sbt.shutdownBspServer(shellRunner) + _ <- sbt.generateBspConfig( + folder, + bspConfigGenerator.runUnconditionally(sbt, _), + statusBar, + ) + _ <- autoConnectToBuildServer() + } yield () + case Some(mill: MillBuildTool) if session.main.isMill => + for { + _ <- mill.generateBspConfig( + folder, + bspConfigGenerator.runUnconditionally(mill, _), + statusBar, + ) + _ <- autoConnectToBuildServer() + } yield () + case _ if session.main.isBloop => + slowConnectToBuildServer(forceImport = true) + case _ => Future.successful(()) + } + case Messages.ProjectJavaHomeUpdate.notNow => + Future.successful(()) + } + } + } + override def didOpen( params: DidOpenTextDocumentParams ): CompletableFuture[Unit] = { diff --git a/mtags/src/main/scala/scala/meta/internal/metals/JdkSources.scala b/mtags/src/main/scala/scala/meta/internal/metals/JdkSources.scala index a72caef7a9f..31da36c53ab 100644 --- a/mtags/src/main/scala/scala/meta/internal/metals/JdkSources.scala +++ b/mtags/src/main/scala/scala/meta/internal/metals/JdkSources.scala @@ -4,6 +4,7 @@ import java.nio.file.Paths import java.util.logging.Logger import scala.util.Failure +import scala.util.Properties import scala.util.Success import scala.util.Try @@ -51,6 +52,32 @@ object JdkSources { fromString(System.getProperty("java.home")).toList } + def envVariables(userJavaHome: Option[String]): Map[String, String] = { + JdkSources + .defaultJavaHome(userJavaHome) + .flatMap(path => + List( + (path, path.resolve("bin")), + (path, path.resolve("jre/bin")) + ) + ) + .collectFirst { + case (javaHome, javaBin) if javaHome.exists && javaBin.exists => + val oldPath = System.getenv().getOrDefault("PATH", "") + val newPath = + if (oldPath.isEmpty()) javaBin.toString() + else { + val sep = if (Properties.isWin) ";" else ":" + s"$javaBin$sep$oldPath" + } + Map( + "JAVA_HOME" -> javaHome.toString(), + "PATH" -> newPath + ) + } + .getOrElse(Map.empty) + } + private def candidates(userJavaHome: Option[String]): List[AbsolutePath] = { def isJdkCandidate(path: AbsolutePath): Boolean = { def containsJre = path.resolve("jre").exists diff --git a/tests/slow/src/test/scala/tests/mill/MillServerSuite.scala b/tests/slow/src/test/scala/tests/mill/MillServerSuite.scala index 71d18cad900..9f37859f598 100644 --- a/tests/slow/src/test/scala/tests/mill/MillServerSuite.scala +++ b/tests/slow/src/test/scala/tests/mill/MillServerSuite.scala @@ -13,6 +13,7 @@ import scala.meta.internal.metals.{BuildInfo => V} import scala.meta.io.AbsolutePath import tests.BaseImportSuite +import tests.JavaHomeChangeTest import tests.MillBuildLayout import tests.MillServerInitializer @@ -20,7 +21,8 @@ import tests.MillServerInitializer * Basic suite to ensure that a connection to a Mill server can be made. */ class MillServerSuite - extends BaseImportSuite("mill-server", MillServerInitializer) { + extends BaseImportSuite("mill-server", MillServerInitializer) + with JavaHomeChangeTest { val preBspVersion = "0.9.10" val supportedBspVersion = V.millVersion @@ -172,4 +174,16 @@ class MillServerSuite } yield {} } + + checkJavaHomeUpdate( + "mill-java-home-update", + fileContent => s"""|/build.sc + |import mill._, scalalib._ + |object a extends ScalaModule { + | def scalaVersion = "${V.scala213}" + |} + |$fileContent + |""".stripMargin, + ) + } diff --git a/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala b/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala index 462e5744938..5742b23a45e 100644 --- a/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala +++ b/tests/slow/src/test/scala/tests/sbt/SbtBloopLspSuite.scala @@ -20,12 +20,14 @@ import scala.meta.io.AbsolutePath import com.google.gson.JsonObject import com.google.gson.JsonPrimitive import tests.BaseImportSuite +import tests.JavaHomeChangeTest import tests.ScriptsAssertions import tests.TestSemanticTokens class SbtBloopLspSuite extends BaseImportSuite("sbt-bloop-import") - with ScriptsAssertions { + with ScriptsAssertions + with JavaHomeChangeTest { val sbtVersion = V.sbtVersion val scalaVersion = V.scala213 @@ -881,4 +883,21 @@ class SbtBloopLspSuite } yield () } + checkJavaHomeUpdate( + "bloop-java-home-update", + fileContent => s"""|/build.sbt + |scalaVersion := "$scalaVersion" + |val a = project.in(file("a")) + |$fileContent + |""".stripMargin, + errorMessage = + """|a/src/main/scala/a/A.scala:2:8: error: object random is not a member of package java.util + |import java.util.random.RandomGenerator + | ^^^^^^^^^^^^^^^^ + |a/src/main/scala/a/A.scala:4:13: error: not found: value RandomGenerator + | val gen = RandomGenerator.of("name") + | ^^^^^^^^^^^^^^^ + |""".stripMargin, + ) + } diff --git a/tests/slow/src/test/scala/tests/sbt/SbtServerSuite.scala b/tests/slow/src/test/scala/tests/sbt/SbtServerSuite.scala index 2264ba95a00..3fe43a9d6c2 100644 --- a/tests/slow/src/test/scala/tests/sbt/SbtServerSuite.scala +++ b/tests/slow/src/test/scala/tests/sbt/SbtServerSuite.scala @@ -21,6 +21,7 @@ import scribe.output.LogOutput import scribe.output.format.OutputFormat import scribe.writer.Writer import tests.BaseImportSuite +import tests.JavaHomeChangeTest import tests.SbtBuildLayout import tests.SbtServerInitializer import tests.ScriptsAssertions @@ -31,7 +32,8 @@ import tests.TestSemanticTokens */ class SbtServerSuite extends BaseImportSuite("sbt-server", SbtServerInitializer) - with ScriptsAssertions { + with ScriptsAssertions + with JavaHomeChangeTest { val preBspVersion = "1.3.13" val supportedMetaBuildVersion = "1.7.0" @@ -524,4 +526,9 @@ class SbtServerSuite } yield () } + checkJavaHomeUpdate( + "sbt-java-home-update", + fileContent => SbtBuildLayout(fileContent, V.scala213), + ) + } diff --git a/tests/unit/src/main/scala/tests/JavaHomeChangeTest.scala b/tests/unit/src/main/scala/tests/JavaHomeChangeTest.scala new file mode 100644 index 00000000000..53974295554 --- /dev/null +++ b/tests/unit/src/main/scala/tests/JavaHomeChangeTest.scala @@ -0,0 +1,65 @@ +package tests + +import scala.meta.internal.metals.Messages + +import coursierapi.JvmManager +import munit.Location +import munit.TestOptions + +trait JavaHomeChangeTest { self: BaseLspSuite => + val pathToJava11: String = JvmManager.create().get("11").toString() + + def checkJavaHomeUpdate( + name: TestOptions, + makeLayout: String => String, + errorMessage: String = + """|a/src/main/scala/a/A.scala:2:8: error: object random is not a member of package java.util + |did you mean Random? + |import java.util.random.RandomGenerator + | ^^^^^^^^^^^^^^^^ + |a/src/main/scala/a/A.scala:4:13: error: not found: value RandomGenerator + | val gen = RandomGenerator.of("name") + | ^^^^^^^^^^^^^^^ + |""".stripMargin, + )(implicit loc: Location): Unit = { + test(name) { + val java11Code = + """|package a + |object O + |""".stripMargin + val java17Code = + """|package a + |import java.util.random.RandomGenerator + |object O { + | val gen = RandomGenerator.of("name") + |} + |""".stripMargin + cleanWorkspace() + client.shouldReloadAfterJavaHomeUpdate = + Messages.ProjectJavaHomeUpdate.restart + for { + _ <- initialize( + makeLayout( + s"""|/a/src/main/scala/a/A.scala + |$java17Code + |""".stripMargin + ) + ) + _ <- server.didOpen("a/src/main/scala/a/A.scala") + _ = assertNoDiagnostics() + _ <- server.didChange("a/src/main/scala/a/A.scala")(_ => java11Code) + _ <- server.didSave("a/src/main/scala/a/A.scala")(identity) + _ = assertNoDiagnostics() + _ <- server.server.onUserConfigUpdate( + userConfig.copy(javaHome = Some(pathToJava11)) + ) + _ <- server.didChange("a/src/main/scala/a/A.scala")(_ => java17Code) + _ <- server.didSave("a/src/main/scala/a/A.scala")(identity) + _ = assertNoDiff( + client.workspaceDiagnostics, + errorMessage, + ) + } yield () + } + } +} diff --git a/tests/unit/src/main/scala/tests/TestingClient.scala b/tests/unit/src/main/scala/tests/TestingClient.scala index 41c8e182175..8d4f3c53ed1 100644 --- a/tests/unit/src/main/scala/tests/TestingClient.scala +++ b/tests/unit/src/main/scala/tests/TestingClient.scala @@ -93,6 +93,7 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) var importScalaCliScript = new MessageActionItem(ImportScalaScript.dismiss) var resetWorkspace = new MessageActionItem(ResetWorkspace.cancel) var regenerateAndRestartScalaCliBuildSever = FileOutOfScalaCliBspScope.ignore + var shouldReloadAfterJavaHomeUpdate = ProjectJavaHomeUpdate.notNow val resources = new ResourceOperations(buffers) val diagnostics: TrieMap[AbsolutePath, Seq[Diagnostic]] = @@ -385,6 +386,14 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) params.getMessage().startsWith("For which folder would you like to") ) { chooseWorkspaceFolder(params.getActions().asScala.toSeq) + } else if ( + List(true, false) + .map(isRestart => + ProjectJavaHomeUpdate.params(isRestart).getMessage() + ) + .contains(params.getMessage()) + ) { + shouldReloadAfterJavaHomeUpdate } else { throw new IllegalArgumentException(params.toString) } diff --git a/tests/unit/src/test/scala/tests/BloopJavaHomeLspSuite.scala b/tests/unit/src/test/scala/tests/BloopJavaHomeLspSuite.scala index 7832a3d9654..50d9cb18240 100644 --- a/tests/unit/src/test/scala/tests/BloopJavaHomeLspSuite.scala +++ b/tests/unit/src/test/scala/tests/BloopJavaHomeLspSuite.scala @@ -9,16 +9,12 @@ class BloopJavaHomeLspSuite extends BaseLspSuite("java-home") { val jsonFilePath: Option[AbsolutePath] = BloopServers.getBloopFilePath("bloop.json") val jsonFile: Option[AbsolutePath] = jsonFilePath.filter(_.exists) - val lockFile: Option[AbsolutePath] = - BloopServers.getBloopFilePath("created_by_metals.lock").filter(_.exists) val contents: Option[String] = jsonFile.map(_.readText) - val javaHome: String = - sys.env.get("JAVA_HOME").orElse(sys.props.get("java.home")).getOrElse("") + val javaHome: String = sys.props.get("java.home").getOrElse("") def stashBloopJson(): Unit = { jsonFile.foreach(_.delete()) - lockFile.foreach(_.delete()) } def unStashBloopJson(): Unit = { @@ -27,11 +23,6 @@ class BloopJavaHomeLspSuite extends BaseLspSuite("java-home") { file.touch() file.writeText(content) } - lockFile.zip(jsonFile).foreach { case (lockfile, jsonFile) => - lockfile.delete() - lockfile.touch() - lockfile.writeText(jsonFile.toNIO.toFile().lastModified().toString()) - } } override def afterAll(): Unit = { @@ -44,7 +35,7 @@ class BloopJavaHomeLspSuite extends BaseLspSuite("java-home") { super.beforeEach(context) } - test("broken-home") { + test("incorrect-home") { cleanWorkspace() jsonFilePath.foreach( _.writeText( diff --git a/tests/unit/src/test/scala/tests/Java8Suite.scala b/tests/unit/src/test/scala/tests/Java8Suite.scala new file mode 100644 index 00000000000..28b34db1df7 --- /dev/null +++ b/tests/unit/src/test/scala/tests/Java8Suite.scala @@ -0,0 +1,61 @@ +package tests + +import scala.meta.internal.metals.UserConfiguration + +import com.google.gson.JsonPrimitive +import coursierapi.JvmManager + +class Java8Suite extends BaseCompletionLspSuite("completion-java-8") { + val pathToJava8: String = JvmManager.create().get("8").toString() + override def userConfig: UserConfiguration = + UserConfiguration(javaHome = Some(pathToJava8)) + + def escapedPathToJava8: String = + new JsonPrimitive(pathToJava8).toString() + + test("java-8-completions") { + cleanWorkspace() + for { + _ <- initialize(s"""|/metals.json + |{ + | "a": { "platformJavaHome": $escapedPathToJava8 } + |} + |/.metals/bsp.trace.json + | + |/a/src/main/scala/a/A.scala + |import java.nio.file.Path + |object A { + | // @@ + |} + |""".stripMargin) + _ <- server.didOpen("a/src/main/scala/a/A.scala") + _ = assertNoDiagnostics() + _ <- assertCompletion( + "val p = Path.o@@", + """|""".stripMargin, + ) + } yield () + } + + test("java-8-workspace-completions") { + cleanWorkspace() + for { + _ <- initialize(s"""|/metals.json + |{ + | "a": { "platformJavaHome": $escapedPathToJava8 } + |} + |/a/src/main/scala/a/A.scala + |object A { + | // @@ + |} + |""".stripMargin) + _ <- server.didOpen("a/src/main/scala/a/A.scala") + _ = assertNoDiagnostics() + _ <- assertCompletion( + "HttpClient@@", + "", + ) + } yield () + } + +}