From a744084a7281f9906c73e6022b7a2bee630a4feb Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Tue, 12 Dec 2023 10:45:44 +0100 Subject: [PATCH 01/21] feature: Add option to run verbose compilation Previously, there was no easy way to run verbose compilation and see what is going on underneath in Zinc. Now, it's possible, which might be super useful for debuggung incremental compilation issues. --- .../scala/meta/internal/metals/Compilations.scala | 6 ++++++ .../meta/internal/metals/MetalsLspService.scala | 1 + .../meta/internal/metals/UserConfiguration.scala | 13 +++++++++++++ 3 files changed, 20 insertions(+) diff --git a/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala b/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala index 0eabb90d8a5..281c764e5eb 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Compilations.scala @@ -25,6 +25,7 @@ final class Compilations( isCurrentlyFocused: b.BuildTargetIdentifier => Boolean, compileWorksheets: Seq[AbsolutePath] => Future[Unit], onStartCompilation: () => Unit, + userConfiguration: () => UserConfiguration, )(implicit ec: ExecutionContext) { // we are maintaining a separate queue for cascade compilation since those must happen ASAP @@ -228,6 +229,11 @@ final class Compilations( targets: Seq[b.BuildTargetIdentifier], ): CancelableFuture[b.CompileResult] = { val params = new b.CompileParams(targets.asJava) + if ( + userConfiguration().verboseCompilation && (connection.isBloop || connection.isScalaCLI) + ) { + params.setArguments(List("--verbose").asJava) + } targets.foreach(target => isCompiling(target) = true) val compilation = connection.compile(params) 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 1625d5ae708..75d8a447492 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -258,6 +258,7 @@ class MetalsLspService( buildTarget => focusedDocumentBuildTarget.get() == buildTarget, worksheets => onWorksheetChanged(worksheets), onStartCompilation, + () => userConfig, ) private val fileWatcher = register( new FileWatcher( diff --git a/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala b/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala index 4361ec15ea4..901723b3e25 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala @@ -54,6 +54,7 @@ case class UserConfiguration( javaFormatConfig: Option[JavaFormatConfig] = None, scalafixRulesDependencies: List[String] = Nil, customProjectRoot: Option[String] = None, + verboseCompilation: Boolean = false, scalaCliLauncher: Option[String] = None, ) { @@ -329,6 +330,15 @@ object UserConfiguration { """Optional relative path to your project's root. |If you want your project root to be the workspace/workspace root set it to "." .""".stripMargin, ), + UserConfigurationOption( + "verbose-compilation", + "false", + "true", + "Show all compilation debugging information", + """|If a build server supports it (for example Bloop or Scala CLI), setting it to true + |will make the logs contain all the possible debugging information including + |about incremental compilation in Zinc.""".stripMargin, + ), ) def fromJson( @@ -542,6 +552,8 @@ object UserConfiguration { getStringListKey("scalafix-rules-dependencies").getOrElse(Nil) val customProjectRoot = getStringKey("custom-project-root") + val verboseCompilation = + getBooleanKey("verbose-compilation").getOrElse(false) if (errors.isEmpty) { Right( @@ -574,6 +586,7 @@ object UserConfiguration { javaFormatConfig, scalafixRulesDependencies, customProjectRoot, + verboseCompilation, ) ) } else { From c64a463304a74faf0a4bc0df417d56abad6bda19 Mon Sep 17 00:00:00 2001 From: Chris Kipp Date: Fri, 15 Dec 2023 13:20:28 +0100 Subject: [PATCH 02/21] fix: use client commands inside of BSP status (#5944) These commands that are attached to status' are meant for the client to execute and send to the server. I noticed this because in nvim-metals I've gotten multiple reports of these commands not working when they arrive since normally the client commands are prefaced with `metals-` whereas the server ones aren't. I'm not sure how this was working in VS Code or if it was, but I believe this change is correct. --- .../scala/meta/internal/bsp/ConnectionBspStatus.scala | 6 +++--- .../scala/scala/meta/internal/metals/ClientCommands.scala | 7 +++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/bsp/ConnectionBspStatus.scala b/metals/src/main/scala/scala/meta/internal/bsp/ConnectionBspStatus.scala index d2c748449d4..7f8f2ecb315 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/ConnectionBspStatus.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/ConnectionBspStatus.scala @@ -4,10 +4,10 @@ import java.nio.file.Path import java.util.concurrent.atomic.AtomicReference import scala.meta.internal.metals.BspStatus +import scala.meta.internal.metals.ClientCommands import scala.meta.internal.metals.Icons import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.ReportContext -import scala.meta.internal.metals.ServerCommands import scala.meta.internal.metals.clients.language.MetalsStatusParams import scala.meta.internal.metals.clients.language.StatusType import scala.meta.io.AbsolutePath @@ -124,7 +124,7 @@ object ConnectionBspStatus { "error", show = true, tooltip = s"Build sever ($serverName) is not responding.", - command = ServerCommands.ConnectBuildServer.id, + command = ClientCommands.ConnectBuildServer.id, commandTooltip = "Reconnect.", ).withStatusType(StatusType.bsp) @@ -139,7 +139,7 @@ object ConnectionBspStatus { "warn", show = true, tooltip = message.trimTo(TOOLTIP_MAX_LENGTH), - command = ServerCommands.RunDoctor.id, + command = ClientCommands.RunDoctor.id, commandTooltip = "Open doctor.", ).withStatusType(StatusType.bsp) diff --git a/metals/src/main/scala/scala/meta/internal/metals/ClientCommands.scala b/metals/src/main/scala/scala/meta/internal/metals/ClientCommands.scala index 2a9ff56f846..79cf9ff3408 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ClientCommands.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ClientCommands.scala @@ -360,6 +360,12 @@ object ClientCommands { "[string], the markdown representation of the stacktrace", ) + val ConnectBuildServer = new Command( + "metals-build-connect", + "Connect to build server.", + ServerCommands.ConnectBuildServer.description, + ) + def all: List[BaseCommand] = List( OpenFolder, @@ -374,5 +380,6 @@ object ClientCommands { CopyWorksheetOutput, StartRunSession, StartDebugSession, + ConnectBuildServer, ) } From e41a83e834f7823357670395042532b70c3a2f2c Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Fri, 15 Dec 2023 13:25:58 +0100 Subject: [PATCH 03/21] fix: extract method with empty lines --- .../meta/internal/pc/ExtractMethodUtils.scala | 16 ++++--- .../scala/tests/pc/ExtractMethodSuite.scala | 46 +++++++++++++++++++ 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/mtags-shared/src/main/scala/scala/meta/internal/pc/ExtractMethodUtils.scala b/mtags-shared/src/main/scala/scala/meta/internal/pc/ExtractMethodUtils.scala index d88db6ba7c9..e738e33808d 100644 --- a/mtags-shared/src/main/scala/scala/meta/internal/pc/ExtractMethodUtils.scala +++ b/mtags-shared/src/main/scala/scala/meta/internal/pc/ExtractMethodUtils.scala @@ -5,14 +5,16 @@ trait ExtractMethodUtils { line: String, newIndent: String, oldIndentLen: Int - ): String = { - var i = 0 - val additional = if (newIndent.indexOf("\t") != -1) "\t" else " " - while ((line(i) == ' ' || line(i) == '\t') && i < oldIndentLen) { - i += 1 + ): String = + if (line.forall(c => c == '\t' || c == ' ' || c == '\r')) "" + else { + var i = 0 + val additional = if (newIndent.indexOf("\t") != -1) "\t" else " " + while ((line(i) == ' ' || line(i) == '\t') && i < oldIndentLen) { + i += 1 + } + newIndent + additional + line.drop(i) } - newIndent + additional + line.drop(i) - } def genName(usedNames: Set[String], prefix: String): String = { if (!usedNames(prefix)) prefix diff --git a/tests/cross/src/test/scala/tests/pc/ExtractMethodSuite.scala b/tests/cross/src/test/scala/tests/pc/ExtractMethodSuite.scala index 25ff86de641..25a88cb43a5 100644 --- a/tests/cross/src/test/scala/tests/pc/ExtractMethodSuite.scala +++ b/tests/cross/src/test/scala/tests/pc/ExtractMethodSuite.scala @@ -507,4 +507,50 @@ class ExtractMethodSuite extends BaseExtractMethodSuite { | } |}""".stripMargin ) + + checkEdit( + "empty-lines", + s"""|object Hello { + | val a: Int = 2 + | @@def m() = { + | <> + | + | a + 2 + | } + |} + |""".stripMargin, + s"""|object Hello { + | val a: Int = 2 + | def newMethod(): Unit = { + | print("1") + | + | print(a) + | } + | def m() = { + | newMethod() + | + | a + 2 + | } + |} + |""".stripMargin, + compat = Map( + "3" -> + s"""|object Hello { + | val a: Int = 2 + | def newMethod(): Unit = + | print("1") + | + | print(a) + | + | def m() = { + | newMethod() + | + | a + 2 + | } + |} + |""".stripMargin + ) + ) } From 644090c26cfd164f2106444d1557c2f38acd2f92 Mon Sep 17 00:00:00 2001 From: Scalameta Bot Date: Fri, 15 Dec 2023 00:39:06 +0000 Subject: [PATCH 04/21] build(deps): Update sbt-jmh from 0.4.6 to 0.4.7 --- project/plugins.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/plugins.sbt b/project/plugins.sbt index 68c89a97771..71a8488076f 100644 --- a/project/plugins.sbt +++ b/project/plugins.sbt @@ -1,4 +1,4 @@ -addSbtPlugin("pl.project13.scala" % "sbt-jmh" % "0.4.6") +addSbtPlugin("pl.project13.scala" % "sbt-jmh" % "0.4.7") addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.11.1") addSbtPlugin("com.github.sbt" % "sbt-ci-release" % "1.5.12") addSbtPlugin("org.scalameta" % "sbt-mdoc" % "2.5.1") From 615add640710e25dd0c4a1b01176ffca2e15b365 Mon Sep 17 00:00:00 2001 From: scalameta-bot <50175807+scalameta-bot@users.noreply.github.com> Date: Fri, 15 Dec 2023 19:16:43 +0100 Subject: [PATCH 05/21] build(deps): Update sbt, scripted-plugin from 1.9.7 to 1.9.8 (#5946) * build(deps): Update sbt, scripted-plugin from 1.9.7 to 1.9.8 * Make the sbt tests pass --------- Co-authored-by: Tomasz Godzik --- project/build.properties | 2 +- tests/slow/src/test/scala/tests/sbt/SbtStepDapSuite.scala | 8 +++++++- .../src/main/scala/tests/debug/BaseStepDapSuite.scala | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/project/build.properties b/project/build.properties index e8a1e246e8a..abbbce5da4a 100644 --- a/project/build.properties +++ b/project/build.properties @@ -1 +1 @@ -sbt.version=1.9.7 +sbt.version=1.9.8 diff --git a/tests/slow/src/test/scala/tests/sbt/SbtStepDapSuite.scala b/tests/slow/src/test/scala/tests/sbt/SbtStepDapSuite.scala index dbdc3beee47..fb1f8be4fd2 100644 --- a/tests/slow/src/test/scala/tests/sbt/SbtStepDapSuite.scala +++ b/tests/slow/src/test/scala/tests/sbt/SbtStepDapSuite.scala @@ -1,5 +1,7 @@ package tests.sbt +import scala.meta.internal.metals.BuildInfo + import tests.SbtBuildLayout import tests.SbtServerInitializer import tests.debug.BaseStepDapSuite @@ -9,4 +11,8 @@ class SbtStepDapSuite s"sbt-debug-step", SbtServerInitializer, SbtBuildLayout, - ) + ) { + + // otherwise we get both Scala 2.12 and 2.13 dependencies, whchich is more tricky for the tests + override def scalaVersion: String = BuildInfo.scala212 +} diff --git a/tests/unit/src/main/scala/tests/debug/BaseStepDapSuite.scala b/tests/unit/src/main/scala/tests/debug/BaseStepDapSuite.scala index a598c8ae062..44d897de5be 100644 --- a/tests/unit/src/main/scala/tests/debug/BaseStepDapSuite.scala +++ b/tests/unit/src/main/scala/tests/debug/BaseStepDapSuite.scala @@ -109,7 +109,7 @@ abstract class BaseStepDapSuite( .at("a/src/main/scala/Main.scala", line = 5)(StepIn) .atDependency( server.toPathFromSymbol("scala.Predef", "scala/Predef.scala"), - line = 427, + line = if (scalaVersion.startsWith("2.13")) 427 else 405, )(Continue) }, ) From 72692d2c899bd1546cc3f368a5e3d104fbc3e9dc Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Fri, 15 Dec 2023 15:34:09 +0100 Subject: [PATCH 06/21] improvement: log when invalid `textDocument` uri in `build/publishDiagnostics` --- .../meta/internal/metals/Diagnostics.scala | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/Diagnostics.scala b/metals/src/main/scala/scala/meta/internal/metals/Diagnostics.scala index 87453784391..b50050a4e7f 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Diagnostics.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Diagnostics.scala @@ -6,6 +6,7 @@ import java.{util => ju} import scala.collection.concurrent.TrieMap import scala.collection.mutable +import scala.util.Try import scala.meta.inputs.Input import scala.meta.internal.metals.JsonParser._ @@ -140,12 +141,25 @@ final class Diagnostics( def onBuildPublishDiagnostics( params: bsp4j.PublishDiagnosticsParams ): Unit = { - val path = params.getTextDocument.getUri.toAbsolutePath - onPublishDiagnostics( - path, - params.getDiagnostics().asScala.map(_.toLsp).toSeq, - params.getReset(), - ) + val diagnostics = params.getDiagnostics().asScala.map(_.toLsp).toSeq + val publish = + for { + path <- Try(params.getTextDocument.getUri.toAbsolutePath).toOption + if (path.isFile) + } yield onPublishDiagnostics( + path, + params.getDiagnostics().asScala.map(_.toLsp).toSeq, + params.getReset(), + ) + + publish.getOrElse { + scribe.warn( + s"Invalid text document uri received from build server: ${params.getTextDocument.getUri}" + ) + diagnostics.map(_.getMessage()).foreach { msg => + scribe.info(s"BSP server: $msg") + } + } } def onPublishDiagnostics( From 73706fa60a638bddb9d20357dfd0ad8e291c860c Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Mon, 18 Dec 2023 12:45:10 +0100 Subject: [PATCH 07/21] improvement: add custom bsp as possible build tool (#5791) --- .../meta/internal/bsp/BspConnector.scala | 49 ++- .../scala/meta/internal/bsp/BspServers.scala | 11 +- .../meta/internal/builds/BloopInstall.scala | 4 +- .../builds/BloopInstallProvider.scala | 6 +- .../scala/meta/internal/builds/BspOnly.scala | 25 ++ .../meta/internal/builds/BuildTool.scala | 39 +-- .../meta/internal/builds/BuildTools.scala | 45 ++- .../scala/meta/internal/builds/Digest.scala | 17 + .../internal/builds/GradleBuildTool.scala | 3 +- .../meta/internal/builds/MavenBuildTool.scala | 3 +- .../meta/internal/builds/MillBuildTool.scala | 3 +- .../meta/internal/builds/SbtBuildTool.scala | 3 +- .../internal/builds/ScalaCliBuildTool.scala | 22 +- .../builds/VersionRecommendation.scala | 7 + .../scala/meta/internal/metals/Configs.scala | 1 + .../scala/meta/internal/metals/Indexer.scala | 39 ++- .../scala/meta/internal/metals/Messages.scala | 9 +- .../internal/metals/MetalsLspService.scala | 301 ++++++++++-------- .../internal/metals/PopupChoiceReset.scala | 1 - .../internal/metals/UserConfiguration.scala | 12 + .../internal/metals/scalacli/ScalaCli.scala | 3 +- .../tests/MultipleBuildFilesLspSuite.scala | 21 ++ .../scala/tests/scalacli/ScalaCliSuite.scala | 13 + .../src/main/scala/tests/TestingClient.scala | 17 +- .../src/test/scala/tests/BillLspSuite.scala | 1 + 25 files changed, 417 insertions(+), 238 deletions(-) create mode 100644 metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala create mode 100644 metals/src/main/scala/scala/meta/internal/builds/VersionRecommendation.scala diff --git a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala index 163119842e2..a208dbb12e3 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala @@ -7,8 +7,10 @@ import scala.concurrent.Future import scala.meta.internal.bsp.BspConfigGenerationStatus._ import scala.meta.internal.builds.BuildServerProvider +import scala.meta.internal.builds.BuildTool import scala.meta.internal.builds.BuildTools import scala.meta.internal.builds.SbtBuildTool +import scala.meta.internal.builds.ScalaCliBuildTool import scala.meta.internal.builds.ShellRunner import scala.meta.internal.metals.BloopServers import scala.meta.internal.metals.BuildServerConnection @@ -18,6 +20,7 @@ import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.StatusBar import scala.meta.internal.metals.Tables import scala.meta.internal.metals.UserConfiguration +import scala.meta.internal.metals.scalacli.ScalaCli import scala.meta.internal.semver.SemVer import scala.meta.io.AbsolutePath @@ -43,13 +46,9 @@ class BspConnector( * Resolves the current build servers that either have a bsp entry or if the * workspace can support Bloop, it will also resolve Bloop. */ - def resolve(): BspResolvedResult = { + def resolve(buildTool: Option[BuildTool]): BspResolvedResult = { resolveExplicit().getOrElse { - if ( - buildTools - .loadSupported() - .exists(_.isBloopDefaultBsp) || buildTools.isBloop - ) + if (buildTool.exists(_.isBloopInstallProvider) || buildTools.isBloop) ResolvedBloop else bspServers.resolve() } @@ -61,7 +60,10 @@ class BspConnector( else bspServers .findAvailableServers() - .find(_.getName == sel) + .find(buildServer => + (ScalaCli.names(buildServer.getName()) && ScalaCli.names(sel)) || + buildServer.getName == sel + ) .map(ResolvedBspOne) } } @@ -74,11 +76,12 @@ class BspConnector( * of the bsp entry has already happened at this point. */ def connect( - projectRoot: AbsolutePath, + buildTool: Option[BuildTool], workspace: AbsolutePath, userConfiguration: UserConfiguration, shellRunner: ShellRunner, )(implicit ec: ExecutionContext): Future[Option[BspSession]] = { + val projectRoot = buildTool.map(_.projectRoot).getOrElse(workspace) def connect( projectRoot: AbsolutePath, bspTraceRoot: AbsolutePath, @@ -86,7 +89,7 @@ class BspConnector( ): Future[Option[BuildServerConnection]] = { def bspStatusOpt = Option.when(addLivenessMonitor)(bspStatus) scribe.info("Attempting to connect to the build server...") - resolve() match { + resolve(buildTool) match { case ResolvedNone => scribe.info("No build server found") Future.successful(None) @@ -131,6 +134,7 @@ class BspConnector( .map(Some(_)) case ResolvedBspOne(details) => tables.buildServers.chooseServer(details.getName()) + optSetBuildTool(details.getName()) bspServers .newServer(projectRoot, bspTraceRoot, details, bspStatusOpt) .map(Some(_)) @@ -165,6 +169,7 @@ class BspConnector( ) ) _ = tables.buildServers.chooseServer(item.getName()) + _ = optSetBuildTool(item.getName()) conn <- bspServers.newServer( projectRoot, bspTraceRoot, @@ -180,7 +185,10 @@ class BspConnector( possibleBuildServerConn match { case None => Future.successful(None) case Some(buildServerConn) - if buildServerConn.isBloop && buildTools.isSbt => + if buildServerConn.isBloop && buildTool.exists { + case _: SbtBuildTool => true + case _ => false + } => // NOTE: (ckipp01) we special case this here since sbt bsp server // doesn't yet support metabuilds. So in the future when that // changes, re-work this and move the creation of this out above @@ -196,6 +204,23 @@ class BspConnector( } } + /** + * Looks for a build tool matching the chosen build server, and sets it as the chosen build server. + * Only for `bloop` there will be no matching build tool and the previously chosen one remains. + */ + private def optSetBuildTool(buildServerName: String): Unit = + buildTools.loadSupported + .find { + case _: ScalaCliBuildTool if ScalaCli.names(buildServerName) => true + case buildTool: BuildServerProvider + if buildTool.buildServerName.contains(buildServerName) => + true + case buildTool => buildTool.executableName == buildServerName + } + .foreach(buildTool => + tables.buildTool.chooseBuildTool(buildTool.executableName) + ) + private def sbtMetaWorkspaces(root: AbsolutePath): List[AbsolutePath] = { def recursive( p: AbsolutePath, @@ -277,7 +302,9 @@ class BspConnector( BspConnectionDetails, ]] = { if ( - bloopPresent || buildTools.loadSupported().exists(_.isBloopDefaultBsp) + bloopPresent || buildTools + .loadSupported() + .exists(_.isBloopInstallProvider) ) new BspConnectionDetails( BloopServers.name, 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 f252c6abafb..838a6c185f8 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BspServers.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BspServers.scala @@ -47,6 +47,8 @@ final class BspServers( config: MetalsServerConfig, userConfig: () => UserConfiguration, )(implicit ec: ExecutionContextExecutorService) { + private def customProjectRoot = + userConfig().getCustomProjectRoot(mainWorkspace) def resolve(): BspResolvedResult = { findAvailableServers() match { @@ -153,7 +155,7 @@ final class BspServers( * may be a server in the current workspace */ def findAvailableServers(): List[BspConnectionDetails] = { - val jsonFiles = findJsonFiles(mainWorkspace) + val jsonFiles = findJsonFiles() val gson = new Gson() for { candidate <- jsonFiles @@ -172,9 +174,7 @@ final class BspServers( } } - private def findJsonFiles( - projectDirectory: AbsolutePath - ): List[AbsolutePath] = { + private def findJsonFiles(): List[AbsolutePath] = { val buf = List.newBuilder[AbsolutePath] def visit(dir: AbsolutePath): Unit = dir.list.foreach { p => @@ -182,7 +182,8 @@ final class BspServers( buf += p } } - visit(projectDirectory.resolve(".bsp")) + visit(mainWorkspace.resolve(".bsp")) + customProjectRoot.map(_.resolve(".bsp")).foreach(visit) bspGlobalInstallDirectories.foreach(visit) buf.result() } diff --git a/metals/src/main/scala/scala/meta/internal/builds/BloopInstall.scala b/metals/src/main/scala/scala/meta/internal/builds/BloopInstall.scala index d6593e8f1f8..a24bcb62466 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BloopInstall.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BloopInstall.scala @@ -37,7 +37,7 @@ final class BloopInstall( override def toString: String = s"BloopInstall($workspace)" def runUnconditionally( - buildTool: BuildTool, + buildTool: BloopInstallProvider, isImportInProcess: AtomicBoolean, ): Future[WorkspaceLoadedStatus] = { if (isImportInProcess.compareAndSet(false, true)) { @@ -121,7 +121,7 @@ final class BloopInstall( // notifications. This method is synchronized to prevent asking the user // twice whether to import the build. def runIfApproved( - buildTool: BuildTool, + buildTool: BloopInstallProvider, digest: String, isImportInProcess: AtomicBoolean, ): Future[WorkspaceLoadedStatus] = diff --git a/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala b/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala index ca18924a2a4..c18e118046c 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BloopInstallProvider.scala @@ -6,10 +6,10 @@ import scala.meta.io.AbsolutePath /** * Helper trait for build tools that have a Bloop plugin */ -trait BloopInstallProvider { this: BuildTool => +trait BloopInstallProvider extends BuildTool { /** - * Method used to generate the necesary .bloop files for the + * Method used to generate the necessary .bloop files for the * build tool. */ def bloopInstall( @@ -22,4 +22,6 @@ trait BloopInstallProvider { this: BuildTool => * Args necessary for build tool to generate the .bloop files. */ def bloopInstallArgs(workspace: AbsolutePath): List[String] + + override val isBloopInstallProvider = true } diff --git a/metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala b/metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala new file mode 100644 index 00000000000..7347a536600 --- /dev/null +++ b/metals/src/main/scala/scala/meta/internal/builds/BspOnly.scala @@ -0,0 +1,25 @@ +package scala.meta.internal.builds + +import java.security.MessageDigest + +import scala.meta.internal.builds.Digest +import scala.meta.internal.mtags.MD5 +import scala.meta.io.AbsolutePath + +/** + * Build tool for custom bsp detected in `.bsp/.json` or `bspGlobalDirectories` + */ +case class BspOnly( + override val executableName: String, + override val projectRoot: AbsolutePath, + pathToBspConfig: AbsolutePath, +) extends BuildTool { + override def digest(workspace: AbsolutePath): Option[String] = { + val digest = MessageDigest.getInstance("MD5") + val isSuccess = + Digest.digestJson(pathToBspConfig, digest) + if (isSuccess) Some(MD5.bytesToHex(digest.digest())) + else None + } + override val forcesBuildServer = true +} diff --git a/metals/src/main/scala/scala/meta/internal/builds/BuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/BuildTool.scala index 5724e745a19..9dff270b338 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BuildTool.scala @@ -4,32 +4,12 @@ import java.nio.file.Files import java.nio.file.Path import java.nio.file.StandardCopyOption -import scala.concurrent.Future - import scala.meta.io.AbsolutePath trait BuildTool { - /** - * Export the build to Bloop - * - * This operation should be roughly equivalent to running `sbt bloopInstall` - * and should work for both updating an existing Bloop build or creating a new - * Bloop build. - */ - def bloopInstall( - workspace: AbsolutePath, - systemProcess: List[String] => Future[WorkspaceLoadedStatus], - ): Future[WorkspaceLoadedStatus] - def digest(workspace: AbsolutePath): Option[String] - def version: String - - def minimumVersion: String - - def recommendedVersion: String - protected lazy val tempDir: Path = { val dir = Files.createTempDirectory("metals") dir.toFile.deleteOnExit() @@ -40,15 +20,16 @@ trait BuildTool { def executableName: String - def isBloopDefaultBsp = true - def projectRoot: AbsolutePath + val forcesBuildServer = false + + val isBloopInstallProvider = false + } object BuildTool { - case class Found(buildTool: BuildTool, digest: String) def copyFromResource( tempDir: Path, filePath: String, @@ -62,4 +43,16 @@ object BuildTool { outFile } + trait Verified + case class IncompatibleVersion(buildTool: VersionRecommendation) + extends Verified { + def message: String = s"Unsupported $buildTool version ${buildTool.version}" + } + case class NoChecksum(buildTool: BuildTool, root: AbsolutePath) + extends Verified { + def message: String = + s"Could not calculate checksum for ${buildTool.executableName} in $root" + } + case class Found(buildTool: BuildTool, digest: String) extends Verified + } diff --git a/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala b/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala index b9c44746772..bc36c2fdc34 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala @@ -9,6 +9,7 @@ import scala.meta.internal.io.PathIO import scala.meta.internal.metals.BloopServers import scala.meta.internal.metals.MetalsEnrichments._ import scala.meta.internal.metals.UserConfiguration +import scala.meta.internal.metals.scalacli.ScalaCli import scala.meta.io.AbsolutePath import ujson.ParsingFailedException @@ -54,6 +55,7 @@ final class BuildTools( def isBloop: Boolean = bloopProject.isDefined def isBsp: Boolean = { hasJsonFile(workspace.resolve(".bsp")) || + customProjectRoot.exists(root => hasJsonFile(root.resolve(".bsp"))) || bspGlobalDirectories.exists(hasJsonFile) } private def hasJsonFile(dir: AbsolutePath): Boolean = { @@ -121,16 +123,34 @@ final class BuildTools( ) def isBazel: Boolean = bazelProject.isDefined - private def customProjectRoot = - userConfig().customProjectRoot - .map(relativePath => workspace.resolve(relativePath.trim())) - .filter { projectRoot => - val exists = projectRoot.exists - if (!exists) { - scribe.error(s"custom project root $projectRoot does not exist") + private def customBsps: List[BspOnly] = { + val bspFolders = + (workspace :: customProjectRoot.toList).distinct + .map(_.resolve(".bsp")) ++ bspGlobalDirectories + val root = customProjectRoot.getOrElse(workspace) + for { + bspFolder <- bspFolders + if (bspFolder.exists && bspFolder.isDirectory) + buildTool <- bspFolder.toFile + .listFiles() + .collect { + case file + if file.isFile() && file.getName().endsWith(".json") && + !knownBsps(file.getName().stripSuffix(".json")) => + BspOnly( + file.getName().stripSuffix(".json"), + root, + AbsolutePath(file.toPath()), + ) } - exists - } + .toList + } yield buildTool + } + + private def knownBsps = + Set(SbtBuildTool.name, MillBuildTool.name) ++ ScalaCli.names + + private def customProjectRoot = userConfig().getCustomProjectRoot(workspace) private def searchForBuildTool( isProjectRoot: AbsolutePath => Boolean @@ -187,6 +207,7 @@ final class BuildTools( mavenProject.foreach(buf += MavenBuildTool(userConfig, _)) millProject.foreach(buf += MillBuildTool(userConfig, _)) scalaCliProject.foreach(buf += ScalaCliBuildTool(workspace, _, userConfig)) + buf.addAll(customBsps) buf.result() } @@ -208,6 +229,11 @@ final class BuildTools( Some(MavenBuildTool.name) else if (isMill && MillBuildTool.isMillRelatedPath(path)) Some(MillBuildTool.name) + else if ( + path.isFile && path.filename.endsWith(".json") && + path.parent.filename == ".bsp" + ) + Some(path.filename.stripSuffix(".json")) else None } @@ -221,6 +247,7 @@ final class BuildTools( val before = lastDetectedBuildTools.getAndUpdate(_ + buildTool) !before.contains(buildTool) } + } object BuildTools { diff --git a/metals/src/main/scala/scala/meta/internal/builds/Digest.scala b/metals/src/main/scala/scala/meta/internal/builds/Digest.scala index 719e47dc3d5..bb2fc11a5b7 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/Digest.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/Digest.scala @@ -4,6 +4,8 @@ import java.nio.charset.StandardCharsets import java.nio.file.Files import java.security.MessageDigest +import scala.util.Success +import scala.util.Try import scala.util.control.NonFatal import scala.xml.Comment import scala.xml.Node @@ -15,6 +17,8 @@ import scala.meta.internal.mtags.MD5 import scala.meta.internal.parsing.Trees import scala.meta.io.AbsolutePath +import ujson.BytesRenderer + case class Digest( md5: String, status: Status, @@ -174,6 +178,19 @@ object Digest { false } } + + def digestJson( + file: AbsolutePath, + digest: MessageDigest, + ): Boolean = { + Try(ujson.read(file.readText)) match { + case Success(json) => + val bytes = json.transform(BytesRenderer()).toByteArray() + digest.update(bytes) + true + case _ => false + } + } } trait Digestable { diff --git a/metals/src/main/scala/scala/meta/internal/builds/GradleBuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/GradleBuildTool.scala index 3b7f38deefb..077817c9260 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/GradleBuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/GradleBuildTool.scala @@ -20,7 +20,8 @@ case class GradleBuildTool( userConfig: () => UserConfiguration, projectRoot: AbsolutePath, ) extends BuildTool - with BloopInstallProvider { + with BloopInstallProvider + with VersionRecommendation { private val initScriptName = "init-script.gradle" private val gradleBloopVersion = BuildInfo.gradleBloopVersion diff --git a/metals/src/main/scala/scala/meta/internal/builds/MavenBuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/MavenBuildTool.scala index a2ab5d48ffe..ba655d9f601 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/MavenBuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/MavenBuildTool.scala @@ -10,7 +10,8 @@ case class MavenBuildTool( userConfig: () => UserConfiguration, projectRoot: AbsolutePath, ) extends BuildTool - with BloopInstallProvider { + with BloopInstallProvider + with VersionRecommendation { private lazy val embeddedMavenLauncher: AbsolutePath = { val out = BuildTool.copyFromResource(tempDir, "maven-wrapper.jar") diff --git a/metals/src/main/scala/scala/meta/internal/builds/MillBuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/MillBuildTool.scala index 0510de98a62..48c373e036d 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/MillBuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/MillBuildTool.scala @@ -14,7 +14,8 @@ case class MillBuildTool( projectRoot: AbsolutePath, ) extends BuildTool with BloopInstallProvider - with BuildServerProvider { + with BuildServerProvider + with VersionRecommendation { private def getMillVersion(workspace: AbsolutePath): String = { import scala.meta.internal.jdk.CollectionConverters._ diff --git a/metals/src/main/scala/scala/meta/internal/builds/SbtBuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/SbtBuildTool.scala index 14690d8a747..2850245e564 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/SbtBuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/SbtBuildTool.scala @@ -26,7 +26,8 @@ case class SbtBuildTool( userConfig: () => UserConfiguration, ) extends BuildTool with BloopInstallProvider - with BuildServerProvider { + with BuildServerProvider + with VersionRecommendation { import SbtBuildTool._ diff --git a/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala b/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala index 091f0b85e18..5fbd6013164 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/ScalaCliBuildTool.scala @@ -17,7 +17,8 @@ class ScalaCliBuildTool( val projectRoot: AbsolutePath, userConfig: () => UserConfiguration, ) extends BuildTool - with BuildServerProvider { + with BuildServerProvider + with VersionRecommendation { lazy val runScalaCliCommand: Option[Seq[String]] = ScalaCli.localScalaCli(userConfig()) @@ -47,12 +48,6 @@ class ScalaCliBuildTool( ) ) - override def bloopInstall( - workspace: AbsolutePath, - systemProcess: List[String] => Future[WorkspaceLoadedStatus], - ): Future[WorkspaceLoadedStatus] = - Future.successful(WorkspaceLoadedStatus.Dismissed) - override def digest(workspace: AbsolutePath): Option[String] = ScalaCliDigest.current(workspace) @@ -64,16 +59,19 @@ class ScalaCliBuildTool( override def executableName: String = ScalaCliBuildTool.name - override def isBloopDefaultBsp = false + override val forcesBuildServer = true + + def isBspGenerated(workspace: AbsolutePath): Boolean = + ScalaCliBuildTool.pathsToScalaCliBsp(workspace).exists(_.isFile) } object ScalaCliBuildTool { def name = "scala-cli" - def pathsToScalaCliBsp(root: AbsolutePath): List[AbsolutePath] = List( - root.resolve(".bsp").resolve("scala-cli.json"), - root.resolve(".bsp").resolve("scala.json"), - ) + def pathsToScalaCliBsp(root: AbsolutePath): List[AbsolutePath] = + ScalaCli.names.toList.map(name => + root.resolve(".bsp").resolve(s"$name.json") + ) def apply( workspace: AbsolutePath, diff --git a/metals/src/main/scala/scala/meta/internal/builds/VersionRecommendation.scala b/metals/src/main/scala/scala/meta/internal/builds/VersionRecommendation.scala new file mode 100644 index 00000000000..5d70db053b2 --- /dev/null +++ b/metals/src/main/scala/scala/meta/internal/builds/VersionRecommendation.scala @@ -0,0 +1,7 @@ +package scala.meta.internal.builds + +trait VersionRecommendation { self: BuildTool => + def minimumVersion: String + def recommendedVersion: String + def version: String +} diff --git a/metals/src/main/scala/scala/meta/internal/metals/Configs.scala b/metals/src/main/scala/scala/meta/internal/metals/Configs.scala index 4e9b040388c..874e9cacc33 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Configs.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Configs.scala @@ -40,6 +40,7 @@ object Configs { new FileSystemWatcher( Either.forLeft(s"$root/.metals/.reports/bloop/*/*") ), + new FileSystemWatcher(Either.forLeft(s"$root/**/.bsp/*.json")), ).asJava ) } diff --git a/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala b/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala index 57ddb7b2006..4ecf03eb904 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Indexer.scala @@ -19,6 +19,7 @@ import scala.meta.dialects._ import scala.meta.inputs.Input import scala.meta.internal.bsp.BspSession import scala.meta.internal.bsp.BuildChange +import scala.meta.internal.builds.BspOnly import scala.meta.internal.builds.BuildTool import scala.meta.internal.builds.BuildTools import scala.meta.internal.builds.Digest.Status @@ -88,25 +89,33 @@ final case class Indexer( buildTool: BuildTool, checksum: String, importBuild: BspSession => Future[Unit], + reconnectToBuildServer: () => Future[BuildChange], ): Future[BuildChange] = { def reloadAndIndex(session: BspSession): Future[BuildChange] = { workspaceReload().persistChecksumStatus(Status.Started, buildTool) - session - .workspaceReload() - .flatMap(_ => importBuild(session)) - .map { _ => - scribe.info("Correctly reloaded workspace") - profiledIndexWorkspace(doctorCheck) - workspaceReload().persistChecksumStatus(Status.Installed, buildTool) - BuildChange.Reloaded - } - .recoverWith { case NonFatal(e) => - scribe.error(s"Unable to reload workspace: ${e.getMessage()}") - workspaceReload().persistChecksumStatus(Status.Failed, buildTool) - languageClient.showMessage(Messages.ReloadProjectFailed) - Future.successful(BuildChange.Failed) - } + buildTool match { + case _: BspOnly => reconnectToBuildServer() + case _ => + session + .workspaceReload() + .flatMap(_ => importBuild(session)) + .map { _ => + scribe.info("Correctly reloaded workspace") + profiledIndexWorkspace(doctorCheck) + workspaceReload().persistChecksumStatus( + Status.Installed, + buildTool, + ) + BuildChange.Reloaded + } + .recoverWith { case NonFatal(e) => + scribe.error(s"Unable to reload workspace: ${e.getMessage()}") + workspaceReload().persistChecksumStatus(Status.Failed, buildTool) + languageClient.showMessage(Messages.ReloadProjectFailed) + Future.successful(BuildChange.Failed) + } + } } bspSession() match { 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 779636c0aa5..5722b47f6b5 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Messages.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Messages.scala @@ -5,6 +5,7 @@ import java.nio.file.Path 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 @@ -193,13 +194,13 @@ object Messages { } object ChooseBuildTool { + def message = + "Multiple build definitions found. Which would you like to use?" def params(builtTools: List[BuildTool]): ShowMessageRequestParams = { val messageActionItems = builtTools.map(bt => new MessageActionItem(bt.executableName)) val params = new ShowMessageRequestParams() - params.setMessage( - "Multiple build definitions found. Which would you like to use?" - ) + params.setMessage(message) params.setType(MessageType.Info) params.setActions(messageActionItems.asJava) params @@ -288,7 +289,7 @@ object Messages { def learnMoreUrl: String = Urls.docs("import-build") - def params(tool: BuildTool): ShowMessageRequestParams = { + def params(tool: VersionRecommendation): ShowMessageRequestParams = { def toFixMessage = s"To fix this problem, upgrade to $tool ${tool.recommendedVersion} " 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 75d8a447492..4f8b8286fc5 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -17,7 +17,6 @@ import scala.concurrent.TimeoutException import scala.concurrent.duration._ import scala.util.Failure import scala.util.Success -import scala.util.Try import scala.util.control.NonFatal import scala.meta.internal.bsp.BspConfigGenerationStatus._ @@ -29,14 +28,17 @@ import scala.meta.internal.bsp.BuildChange import scala.meta.internal.bsp.ConnectionBspStatus import scala.meta.internal.bsp.ScalaCliBspScope import scala.meta.internal.builds.BloopInstall +import scala.meta.internal.builds.BloopInstallProvider import scala.meta.internal.builds.BspErrorHandler import scala.meta.internal.builds.BuildServerProvider 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.SbtBuildTool import scala.meta.internal.builds.ScalaCliBuildTool import scala.meta.internal.builds.ShellRunner +import scala.meta.internal.builds.VersionRecommendation import scala.meta.internal.builds.WorkspaceReload import scala.meta.internal.decorations.PublishDecorationsParams import scala.meta.internal.decorations.SyntheticHoverProvider @@ -285,7 +287,7 @@ class MetalsLspService( ) private val onBuildChanged = - BatchedFunction.fromFuture[AbsolutePath, BuildChange]( + BatchedFunction.fromFuture[AbsolutePath, Unit]( onBuildChangedUnbatched, "onBuildChanged", ) @@ -755,7 +757,7 @@ class MetalsLspService( diagnostics, languageClient, () => bspSession, - () => bspConnector.resolve(), + () => bspConnector.resolve(buildTool), tables, clientConfig, mtagsResolver, @@ -770,7 +772,7 @@ class MetalsLspService( () => tables.buildTool.selectedBuildTool(), buildTargets, () => bspSession, - () => bspConnector.resolve(), + () => bspConnector.resolve(buildTool), buildTools, ) @@ -908,6 +910,31 @@ class MetalsLspService( val isInitialized = new AtomicBoolean(false) + def fullConnect(): Future[Unit] = { + buildTools.initialize() + for { + found <- supportedBuildTool() + chosenBuildServer = found match { + case Some(BuildTool.Found(buildServer, _)) + if buildServer.forcesBuildServer => + tables.buildServers.chooseServer(buildServer.executableName) + Some(buildServer.executableName) + case _ => tables.buildServers.selectedServer() + } + _ <- Future + .sequence( + List( + quickConnectToBuildServer(), + slowConnectToBuildServer( + forceImport = false, + found, + chosenBuildServer, + ), + ) + ) + } yield () + } + def initialized(): Future[Unit] = { registerNiceToHaveFilePatterns() @@ -918,9 +945,7 @@ class MetalsLspService( Future .sequence( List[Future[Unit]]( - Future(buildTools.initialize()), - quickConnectToBuildServer().ignoreValue, - slowConnectToBuildServer(forceImport = false).ignoreValue, + fullConnect(), Future(workspaceSymbols.indexClasspath()), Future(formattingProvider.load()), ) @@ -967,7 +992,7 @@ class MetalsLspService( if (userConfig.customProjectRoot != old.customProjectRoot) { tables.buildTool.reset() tables.buildServers.reset() - slowConnectToBuildServer(false).ignoreValue + fullConnect() } else Future.successful(()) val resetDecorations = @@ -1325,14 +1350,6 @@ class MetalsLspService( ): CompletableFuture[Unit] = { val path = AbsolutePath(event.path) val isScalaOrJava = path.isScalaOrJava - event.eventType match { - case EventType.CreateOrModify - if path.isInBspDirectory(folder) && path.extension == "json" - && isValidBspFile(path) => - scribe.info(s"Detected new build tool in $path") - quickConnectToBuildServer() - case _ => - } if (isScalaOrJava && event.eventType == EventType.Delete) { onDelete(path).asJava } else if ( @@ -1360,15 +1377,12 @@ class MetalsLspService( } }.asJava } else if (path.isBuild) { - onBuildChanged(List(path)).ignoreValue.asJava + onBuildChanged(List(path)).asJava } else { CompletableFuture.completedFuture(()) } } - private def isValidBspFile(path: AbsolutePath): Boolean = - path.readTextOpt.exists(text => Try(ujson.read(text)).toOption.nonEmpty) - private def onChange(paths: Seq[AbsolutePath]): Future[Unit] = { paths.foreach { path => fingerprints.add(path, FileIO.slurp(path, charset)) @@ -1380,8 +1394,7 @@ class MetalsLspService( Future(indexer.reindexWorkspaceSources(paths)), compilations .compileFiles(paths, Option(focusedDocumentBuildTarget.get())), - onBuildChanged(paths).ignoreValue, - Future.sequence(paths.map(onBuildToolAdded)), + onBuildChanged(paths), ) ++ paths.map(f => Future(interactiveSemanticdbs.textDocument(f))) ) .ignoreValue @@ -1474,11 +1487,10 @@ class MetalsLspService( val path = params.getTextDocument.getUri.toAbsolutePath if (path.isJava) javaFormattingProvider.format(params) - else - for { - projectRoot <- calculateOptProjectRoot().map(_.getOrElse(folder)) - res <- formattingProvider.format(path, projectRoot, token) - } yield res + else { + val projectRoot = optProjectRoot.getOrElse(folder) + formattingProvider.format(path, projectRoot, token) + } } override def onTypeFormatting( @@ -1740,8 +1752,7 @@ class MetalsLspService( } yield bloopServers.shutdownServer() case Some(session) if session.main.isSbt => for { - currentBuildTool <- buildTool() - res <- currentBuildTool match { + res <- buildTool match { case Some(sbt: SbtBuildTool) => for { _ <- disconnectOldBuildServer() @@ -2017,53 +2028,39 @@ class MetalsLspService( }) } - private def buildTool(): Future[Option[BuildTool]] = { - buildTools.loadSupported match { - case Nil => Future(None) - case buildTools => - for { - buildTool <- buildToolSelector.checkForChosenBuildTool( - buildTools - ) - } yield buildTool.filter(isCompatibleVersion) - } - } - - private def isCompatibleVersion(buildTool: BuildTool) = - SemVer.isCompatibleVersion( - buildTool.minimumVersion, - buildTool.version, - ) - - def supportedBuildTool(): Future[Option[BuildTool.Found]] = { - def isCompatibleVersion(buildTool: BuildTool) = { - val isCompatibleVersion = this.isCompatibleVersion(buildTool) - if (isCompatibleVersion) { + private def buildTool: Option[BuildTool] = + for { + name <- tables.buildTool.selectedBuildTool() + buildTool <- buildTools.loadSupported.find(_.executableName == name) + found <- isCompatibleVersion(buildTool) match { + case BuildTool.Found(bt, _) => Some(bt) + case _ => None + } + } yield found + + def isCompatibleVersion(buildTool: BuildTool): BuildTool.Verified = { + buildTool match { + case buildTool: VersionRecommendation + if !SemVer.isCompatibleVersion( + buildTool.minimumVersion, + buildTool.version, + ) => + BuildTool.IncompatibleVersion(buildTool) + case _ => buildTool.digest(folder) match { case Some(digest) => - Some(BuildTool.Found(buildTool, digest)) - case None => - scribe.warn( - s"Could not calculate checksum for ${buildTool.executableName} in $folder" - ) - None + BuildTool.Found(buildTool, digest) + case None => BuildTool.NoChecksum(buildTool, folder) } - } else { - scribe.warn(s"Unsupported $buildTool version ${buildTool.version}") - languageClient.showMessage( - Messages.IncompatibleBuildToolVersion.params(buildTool) - ) - None - } } + } + def supportedBuildTool(): Future[Option[BuildTool.Found]] = { buildTools.loadSupported match { case Nil => { if (!buildTools.isAutoConnectable()) { warnings.noBuildTool() } - // wait for a bsp file to show up - fileWatcher.start(Set(folder.resolve(".bsp"))) Future(None) } case buildTools => @@ -2072,49 +2069,77 @@ class MetalsLspService( buildTools ) } yield { - buildTool.flatMap(isCompatibleVersion) + buildTool.flatMap { bt => + isCompatibleVersion(bt) match { + case found: BuildTool.Found => Some(found) + case warn @ BuildTool.IncompatibleVersion(buildTool) => + scribe.warn(warn.message) + languageClient.showMessage( + Messages.IncompatibleBuildToolVersion.params(buildTool) + ) + None + case warn: BuildTool.NoChecksum => + scribe.warn(warn.message) + None + } + } } } } def slowConnectToBuildServer( forceImport: Boolean - ): Future[BuildChange] = - for { - possibleBuildTool <- supportedBuildTool() - chosenBuildServer = tables.buildServers.selectedServer() - isBloopOrEmpty = chosenBuildServer.isEmpty || chosenBuildServer.exists( - _ == BloopServers.name - ) - buildChange <- possibleBuildTool match { - case Some(BuildTool.Found(buildTool: ScalaCliBuildTool, _)) - if chosenBuildServer.isEmpty => - tables.buildServers.chooseServer(ScalaCliBuildTool.name) - val scalaCliBspConfigExists = - ScalaCliBuildTool.pathsToScalaCliBsp(folder).exists(_.isFile) - if (scalaCliBspConfigExists) Future.successful(BuildChange.None) - else - buildTool - .generateBspConfig( - folder, - args => bspConfigGenerator.runUnconditionally(buildTool, args), - statusBar, - ) - .flatMap(_ => quickConnectToBuildServer()) - case Some(found) if isBloopOrEmpty => - slowConnectToBloopServer(forceImport, found.buildTool, found.digest) - case Some(found) => - indexer.reloadWorkspaceAndIndex( - forceImport, - found.buildTool, - found.digest, - importBuild, - ) - case None => - Future.successful(BuildChange.None) + ): Future[BuildChange] = for { + buildTool <- supportedBuildTool() + chosenBuildServer = tables.buildServers.selectedServer() + buildChange <- slowConnectToBuildServer( + forceImport, + buildTool, + chosenBuildServer, + ) + } yield buildChange - } - } yield buildChange + def slowConnectToBuildServer( + forceImport: Boolean, + buildTool: Option[BuildTool.Found], + chosenBuildServer: Option[String], + ): Future[BuildChange] = { + val isBloopOrEmpty = chosenBuildServer.isEmpty || chosenBuildServer.exists( + _ == BloopServers.name + ) + + buildTool match { + case Some(BuildTool.Found(buildTool: BloopInstallProvider, digest)) + if isBloopOrEmpty => + slowConnectToBloopServer(forceImport, buildTool, digest) + case Some(BuildTool.Found(buildTool: ScalaCliBuildTool, _)) + if !buildTool.isBspGenerated(folder) => + tables.buildServers.chooseServer(buildTool.executableName) + buildTool + .generateBspConfig( + folder, + args => bspConfigGenerator.runUnconditionally(buildTool, args), + statusBar, + ) + .flatMap(_ => quickConnectToBuildServer()) + case Some(BuildTool.Found(buildTool, _)) + if !chosenBuildServer.exists( + _ == buildTool.executableName + ) && buildTool.forcesBuildServer => + tables.buildServers.chooseServer(buildTool.executableName) + quickConnectToBuildServer() + case Some(found) => + indexer.reloadWorkspaceAndIndex( + forceImport, + found.buildTool, + found.digest, + importBuild, + quickConnectToBuildServer, + ) + case None => + Future.successful(BuildChange.None) + } + } /** * If there is no auto-connectable build server and no supported build tool is found @@ -2131,7 +2156,7 @@ class MetalsLspService( private def slowConnectToBloopServer( forceImport: Boolean, - buildTool: BuildTool, + buildTool: BloopInstallProvider, checksum: String, ): Future[BuildChange] = for { @@ -2144,9 +2169,8 @@ class MetalsLspService( if (result.isInstalled) quickConnectToBuildServer() else if (result.isFailed) { for { - maybeProjectRoot <- calculateOptProjectRoot() change <- - if (buildTools.isAutoConnectable(maybeProjectRoot)) { + if (buildTools.isAutoConnectable(optProjectRoot)) { // TODO(olafur) try to connect but gracefully error languageClient.showMessage( Messages.ImportProjectPartiallyFailed @@ -2167,16 +2191,13 @@ class MetalsLspService( } } yield change - def calculateOptProjectRoot(): Future[Option[AbsolutePath]] = - for { - possibleBuildTool <- buildTool() - } yield possibleBuildTool.map(_.projectRoot).orElse(buildTools.bloopProject) + def optProjectRoot(): Option[AbsolutePath] = + buildTool.map(_.projectRoot).orElse(buildTools.bloopProject) def quickConnectToBuildServer(): Future[BuildChange] = for { - optRoot <- calculateOptProjectRoot() change <- - if (!buildTools.isAutoConnectable(optRoot)) { + if (!buildTools.isAutoConnectable(optProjectRoot)) { scribe.warn("Build server is not auto-connectable.") Future.successful(BuildChange.None) } else { @@ -2254,10 +2275,9 @@ class MetalsLspService( (for { _ <- disconnectOldBuildServer() - maybeProjectRoot <- calculateOptProjectRoot() maybeSession <- timerProvider.timed("Connected to build server", true) { bspConnector.connect( - maybeProjectRoot.getOrElse(folder), + buildTool, folder, userConfig, shellRunner, @@ -2342,10 +2362,16 @@ class MetalsLspService( s"Connected to Build server: ${session.main.name} v${session.version}" ) cancelables.add(session) + buildTool.foreach( + workspaceReload.persistChecksumStatus(Digest.Status.Started, _) + ) bspSession = Some(session) for { _ <- importBuild(session) _ <- indexer.profiledIndexWorkspace(runDoctorCheck) + _ = buildTool.foreach( + workspaceReload.persistChecksumStatus(Digest.Status.Installed, _) + ) _ = if (session.main.isBloop) checkRunningBloopVersion(session.version) } yield { BuildChange.Reconnected @@ -2470,30 +2496,40 @@ class MetalsLspService( private def onBuildChangedUnbatched( paths: Seq[AbsolutePath] - ): Future[BuildChange] = { + ): Future[Unit] = { val changedBuilds = paths.flatMap(buildTools.isBuildRelated) - val buildChange = for { - chosenBuildTool <- tables.buildTool.selectedBuildTool() - if (changedBuilds.contains(chosenBuildTool)) - } yield slowConnectToBuildServer(forceImport = false) - buildChange.getOrElse(Future.successful(BuildChange.None)) + tables.buildTool.selectedBuildTool() match { + // no build tool and new added + case None if changedBuilds.nonEmpty => + scribe.info(s"Detected new build tool in $path") + fullConnect() + // used build tool changed + case Some(chosenBuildTool) if changedBuilds.contains(chosenBuildTool) => + slowConnectToBuildServer(forceImport = false).ignoreValue + // maybe new build tool added + case Some(chosenBuildTool) if changedBuilds.nonEmpty => + onBuildToolsAdded(chosenBuildTool, changedBuilds) + case _ => Future.unit + } } - private def onBuildToolAdded( - path: AbsolutePath - ): Future[BuildChange] = { + private def onBuildToolsAdded( + currentBuildToolName: String, + newBuildToolsChanged: Seq[String], + ): Future[Unit] = { val supportedBuildTools = buildTools.loadSupported() val maybeBuildChange = for { - currentBuildToolName <- tables.buildTool.selectedBuildTool() currentBuildTool <- supportedBuildTools.find( _.executableName == currentBuildToolName ) - addedBuildName <- buildTools.isBuildRelated(path) - if (buildTools.newBuildTool(addedBuildName)) - if (addedBuildName != currentBuildToolName) - newBuildTool <- supportedBuildTools.find( - _.executableName == addedBuildName - ) + newBuildTool <- newBuildToolsChanged + .filter(buildTools.newBuildTool) + .flatMap(addedBuildName => + supportedBuildTools.find( + _.executableName == addedBuildName + ) + ) + .headOption } yield { buildToolSelector .onNewBuildToolAdded(newBuildTool, currentBuildTool) @@ -2501,8 +2537,8 @@ class MetalsLspService( if (switch) slowConnectToBuildServer(forceImport = false) else Future.successful(BuildChange.None) } - } - maybeBuildChange.getOrElse(Future.successful(BuildChange.None)) + }.ignoreValue + maybeBuildChange.getOrElse(Future.unit) } /** @@ -2762,9 +2798,8 @@ class MetalsLspService( def resetWorkspace(): Future[Unit] = for { - maybeProjectRoot <- calculateOptProjectRoot() _ <- disconnectOldBuildServer() - _ = maybeProjectRoot match { + _ = optProjectRoot match { case Some(path) if buildTools.isBloop(path) => bloopServers.shutdownServer() clearBloopDir(path) diff --git a/metals/src/main/scala/scala/meta/internal/metals/PopupChoiceReset.scala b/metals/src/main/scala/scala/meta/internal/metals/PopupChoiceReset.scala index dc8b63b7398..a4544eb2302 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/PopupChoiceReset.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/PopupChoiceReset.scala @@ -28,7 +28,6 @@ class PopupChoiceReset( val result = if (value == BuildTool) { scribe.info("Resetting build tool selection.") tables.buildTool.reset() - tables.buildServers.reset() slowConnect().ignoreValue } else if (value == BuildImport) { tables.dismissedNotifications.ImportChanges.reset() diff --git a/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala b/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala index 901723b3e25..a0dbcfba8a4 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala @@ -69,6 +69,18 @@ case class UserConfiguration( ) && this.showInferredType.nonEmpty showImplicitArguments || showInferredType || showImplicitConversionsAndClasses } + + def getCustomProjectRoot(workspace: AbsolutePath): Option[AbsolutePath] = + customProjectRoot + .map(relativePath => workspace.resolve(relativePath.trim())) + .filter { projectRoot => + val exists = projectRoot.toFile.exists + if (!exists) { + scribe.error(s"custom project root $projectRoot does not exist") + } + exists + } + } object UserConfiguration { diff --git a/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCli.scala b/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCli.scala index 2a54c818104..dce21f88910 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCli.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/scalacli/ScalaCli.scala @@ -429,6 +429,7 @@ object ScalaCli { def scalaCliBspJsonContent( args: List[String] = Nil, projectRoot: String = ".", + bspName: String = "scala-cli", ): String = { val argv = List( ScalaCli.javaCommand, @@ -439,7 +440,7 @@ object ScalaCli { projectRoot, ) ++ args val bsjJson = ujson.Obj( - "name" -> "scala-cli", + "name" -> bspName, "argv" -> argv, "version" -> BuildInfo.scalaCliVersion, "bspVersion" -> scalaCliBspVersion, diff --git a/tests/slow/src/test/scala/tests/MultipleBuildFilesLspSuite.scala b/tests/slow/src/test/scala/tests/MultipleBuildFilesLspSuite.scala index 3dfa69f7d8d..a24339b97c7 100644 --- a/tests/slow/src/test/scala/tests/MultipleBuildFilesLspSuite.scala +++ b/tests/slow/src/test/scala/tests/MultipleBuildFilesLspSuite.scala @@ -3,6 +3,7 @@ package tests import scala.meta.internal.builds.MillBuildTool import scala.meta.internal.builds.SbtBuildTool import scala.meta.internal.metals.Messages.ChooseBuildTool +import scala.meta.internal.metals.scalacli.ScalaCli import scala.meta.internal.metals.{BuildInfo => V} import scala.meta.io.AbsolutePath @@ -65,4 +66,24 @@ class MultipleBuildFilesLspSuite } } + test("custom-bsp") { + cleanWorkspace() + client.chooseBuildTool = actions => + actions + .find(_.getTitle == "custom") + .getOrElse(throw new Exception("no custom as build tool")) + for { + _ <- initialize( + s"""|/.bsp/custom.json + |${ScalaCli.scalaCliBspJsonContent(bspName = "custom")} + |/build.sbt + |scalaVersion := "${V.scala213}" + |""".stripMargin + ) + _ <- server.server.indexingPromise.future + _ = assert(server.server.bspSession.nonEmpty) + _ = assert(server.server.bspSession.get.main.name == "custom") + } yield () + } + } diff --git a/tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala b/tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala index e0269f152fe..640fbbf1b5d 100644 --- a/tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala +++ b/tests/slow/src/test/scala/tests/scalacli/ScalaCliSuite.scala @@ -12,6 +12,7 @@ import scala.meta.internal.metals.SlowTaskConfig import scala.meta.internal.metals.scalacli.ScalaCli import scala.meta.internal.metals.{BuildInfo => V} +import org.eclipse.{lsp4j => l} import tests.FileLayout class ScalaCliSuite extends BaseScalaCliSuite(V.scala3) { @@ -267,6 +268,18 @@ class ScalaCliSuite extends BaseScalaCliSuite(V.scala3) { _ <- server.initialized() _ = FileLayout.fromString(simpleFileLayout, workspace) _ = FileLayout.fromString(bspLayout, workspace) + _ <- server.fullServer + .didChangeWatchedFiles( + new l.DidChangeWatchedFilesParams( + List( + new l.FileEvent( + workspace.resolve(".bsp/scala-cli.json").toURI.toString(), + l.FileChangeType.Created, + ) + ).asJava + ) + ) + .asScala _ <- server.server.indexingPromise.future _ <- server.didOpen("MyTests.scala") _ <- assertDefinitionAtLocation( diff --git a/tests/unit/src/main/scala/tests/TestingClient.scala b/tests/unit/src/main/scala/tests/TestingClient.scala index a8a2d8ebf77..00beb8840ff 100644 --- a/tests/unit/src/main/scala/tests/TestingClient.scala +++ b/tests/unit/src/main/scala/tests/TestingClient.scala @@ -13,7 +13,6 @@ import scala.concurrent.Promise import scala.meta.inputs.Input import scala.meta.internal.bsp.ConnectionBspStatus -import scala.meta.internal.builds.BuildTool import scala.meta.internal.builds.BuildTools import scala.meta.internal.decorations.PublishDecorationsParams import scala.meta.internal.metals.Buffers @@ -303,20 +302,6 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) .map(tool => createParams(tool.toString())) .contains(params) } - // NOTE: (ckipp01) Just for easiness of testing, we are going to just look - // for sbt and mill builds together, which are most common. The logic however - // is identical for all build tools. - def isSameMessageFromList( - createParams: List[BuildTool] => ShowMessageRequestParams - ): Boolean = { - val buildTools = BuildTools - .default() - .allAvailable - .filter(bt => bt.executableName == "sbt" || bt.executableName == "mill") - - val targetParams = createParams(buildTools) - params == targetParams - } def isNewBuildToolDetectedMessage(): Boolean = { val buildTools = BuildTools.default().allAvailable @@ -347,7 +332,7 @@ class TestingClient(workspace: AbsolutePath, val buffers: Buffers) getDoctorInformation } else if (BspSwitch.isSelectBspServer(params)) { selectBspServer(params.getActions.asScala.toSeq) - } else if (isSameMessageFromList(ChooseBuildTool.params)) { + } else if (params.getMessage == ChooseBuildTool.message) { chooseBuildTool(params.getActions.asScala.toSeq) } else if (MissingScalafmtConf.isCreateScalafmtConf(params)) { createScalaFmtConf diff --git a/tests/unit/src/test/scala/tests/BillLspSuite.scala b/tests/unit/src/test/scala/tests/BillLspSuite.scala index 73d6611f099..7c906d39578 100644 --- a/tests/unit/src/test/scala/tests/BillLspSuite.scala +++ b/tests/unit/src/test/scala/tests/BillLspSuite.scala @@ -202,6 +202,7 @@ class BillLspSuite extends BaseLspSuite("bill") { _ = assertNoDiff( client.workspaceMessageRequests, List( + Messages.ChooseBuildTool.message, Messages.BspSwitch.message, Messages.CheckDoctor.allProjectsMisconfigured, ).mkString("\n"), From 292bf3f265d3673bbf2c89881713e839a35580c7 Mon Sep 17 00:00:00 2001 From: Sakib Hadziavdic Date: Mon, 18 Dec 2023 16:12:01 +0100 Subject: [PATCH 08/21] Update millw to 0.4.11 --- bin/update-millw.sh | 1 + metals/src/main/resources/millw | 85 ++++++++++++++++++++--------- metals/src/main/resources/millw.bat | 14 ++--- 3 files changed, 68 insertions(+), 32 deletions(-) diff --git a/bin/update-millw.sh b/bin/update-millw.sh index 34d06c9009a..f108ca29425 100755 --- a/bin/update-millw.sh +++ b/bin/update-millw.sh @@ -5,3 +5,4 @@ root_url=https://raw.githubusercontent.com/lefou/millw/main curl $root_url/millw > $PWD/metals/src/main/resources/millw curl $root_url/millw.bat > $PWD/metals/src/main/resources/millw.bat + diff --git a/metals/src/main/resources/millw b/metals/src/main/resources/millw index c4157ae613e..9d343ff64b8 100755 --- a/metals/src/main/resources/millw +++ b/metals/src/main/resources/millw @@ -5,7 +5,7 @@ # If no version is given, it falls back to the value of DEFAULT_MILL_VERSION # # Project page: https://github.com/lefou/millw -# Script Version: 0.4.7 +# Script Version: 0.4.11 # # If you want to improve this script, please also contribute your changes back! # @@ -14,7 +14,7 @@ set -e if [ -z "${DEFAULT_MILL_VERSION}" ] ; then - DEFAULT_MILL_VERSION="0.11.0" + DEFAULT_MILL_VERSION="0.11.4" fi @@ -55,12 +55,10 @@ if [ -z "${MILL_VERSION}" ] ; then fi fi +MILL_USER_CACHE_DIR="${XDG_CACHE_HOME:-${HOME}/.cache}/mill" + if [ -z "${MILL_DOWNLOAD_PATH}" ] ; then - if [ -n "${XDG_CACHE_HOME}" ] ; then - MILL_DOWNLOAD_PATH="${XDG_CACHE_HOME}/mill/download" - else - MILL_DOWNLOAD_PATH="${HOME}/.cache/mill/download" - fi + MILL_DOWNLOAD_PATH="${MILL_USER_CACHE_DIR}/download" fi # If not already set, try to fetch newest from Github @@ -106,34 +104,71 @@ fi MILL="${MILL_DOWNLOAD_PATH}/${MILL_VERSION}" try_to_use_system_mill() { + if [ "$(uname)" != "Linux" ]; then + return 0 + fi + MILL_IN_PATH="$(command -v mill || true)" if [ -z "${MILL_IN_PATH}" ]; then - return + return 0 fi - UNIVERSAL_SCRIPT_MAGIC="@ 2>/dev/null # 2>nul & echo off & goto BOF" + SYSTEM_MILL_FIRST_TWO_BYTES=$(head --bytes=2 "${MILL_IN_PATH}") + if [ "${SYSTEM_MILL_FIRST_TWO_BYTES}" = "#!" ]; then + # MILL_IN_PATH is (very likely) a shell script and not the mill + # executable, ignore it. + return 0 + fi + + SYSTEM_MILL_PATH=$(readlink -e "${MILL_IN_PATH}") + SYSTEM_MILL_SIZE=$(stat --format=%s "${SYSTEM_MILL_PATH}") + SYSTEM_MILL_MTIME=$(stat --format=%y "${SYSTEM_MILL_PATH}") - if ! head -c 128 "${MILL_IN_PATH}" | grep -qF "${UNIVERSAL_SCRIPT_MAGIC}"; then - if [ -n "${MILLW_VERBOSE}" ]; then - echo "Could not determine mill version of ${MILL_IN_PATH}, as it does not start with the universal script magic2" 1>&2 + if [ ! -d "${MILL_USER_CACHE_DIR}" ]; then + mkdir -p "${MILL_USER_CACHE_DIR}" + fi + + SYSTEM_MILL_INFO_FILE="${MILL_USER_CACHE_DIR}/system-mill-info" + if [ -f "${SYSTEM_MILL_INFO_FILE}" ]; then + parseSystemMillInfo() { + LINE_NUMBER="${1}" + # Select the line number of the SYSTEM_MILL_INFO_FILE, cut the + # variable definition in that line in two halves and return + # the value, and finally remove the quotes. + sed -n "${LINE_NUMBER}p" "${SYSTEM_MILL_INFO_FILE}" |\ + cut -d= -f2 |\ + sed 's/"\(.*\)"/\1/' + } + + CACHED_SYSTEM_MILL_PATH=$(parseSystemMillInfo 1) + CACHED_SYSTEM_MILL_VERSION=$(parseSystemMillInfo 2) + CACHED_SYSTEM_MILL_SIZE=$(parseSystemMillInfo 3) + CACHED_SYSTEM_MILL_MTIME=$(parseSystemMillInfo 4) + + if [ "${SYSTEM_MILL_PATH}" = "${CACHED_SYSTEM_MILL_PATH}" ] \ + && [ "${SYSTEM_MILL_SIZE}" = "${CACHED_SYSTEM_MILL_SIZE}" ] \ + && [ "${SYSTEM_MILL_MTIME}" = "${CACHED_SYSTEM_MILL_MTIME}" ]; then + if [ "${CACHED_SYSTEM_MILL_VERSION}" = "${MILL_VERSION}" ]; then + MILL="${SYSTEM_MILL_PATH}" + return 0 + else + return 0 + fi fi - return fi - # Roughly the size of the universal script. - MILL_VERSION_SEARCH_RANGE="2403" - MILL_IN_PATH_VERSION=$(head -c "${MILL_VERSION_SEARCH_RANGE}" "${MILL_IN_PATH}" |\ - sed -n 's/^.*-DMILL_VERSION=\([^\s]*\) .*$/\1/p' |\ - head -n 1) + SYSTEM_MILL_VERSION=$(${SYSTEM_MILL_PATH} --version | head -n1 | sed -n 's/^Mill.*version \(.*\)/\1/p') - if [ -z "${MILL_IN_PATH_VERSION}" ]; then - echo "Could not determine mill version, even though ${MILL_IN_PATH} has the universal script magic" 1>&2 - return - fi + cat < "${SYSTEM_MILL_INFO_FILE}" +CACHED_SYSTEM_MILL_PATH="${SYSTEM_MILL_PATH}" +CACHED_SYSTEM_MILL_VERSION="${SYSTEM_MILL_VERSION}" +CACHED_SYSTEM_MILL_SIZE="${SYSTEM_MILL_SIZE}" +CACHED_SYSTEM_MILL_MTIME="${SYSTEM_MILL_MTIME}" +EOF - if [ "${MILL_IN_PATH_VERSION}" = "${MILL_VERSION}" ]; then - MILL="${MILL_IN_PATH}" + if [ "${SYSTEM_MILL_VERSION}" = "${MILL_VERSION}" ]; then + MILL="${SYSTEM_MILL_PATH}" fi } try_to_use_system_mill @@ -167,7 +202,7 @@ if [ ! -s "${MILL}" ] ; then if [ "$DOWNLOAD_FROM_MAVEN" = "1" ] ; then DOWNLOAD_URL="https://repo1.maven.org/maven2/com/lihaoyi/mill-dist/${MILL_VERSION}/mill-dist-${MILL_VERSION}.jar" else - MILL_VERSION_TAG=$(echo $MILL_VERSION | sed -E 's/([^-]+)(-M[0-9]+)?(-.*)?/\1\2/') + MILL_VERSION_TAG=$(echo "$MILL_VERSION" | sed -E 's/([^-]+)(-M[0-9]+)?(-.*)?/\1\2/') DOWNLOAD_URL="${GITHUB_RELEASE_CDN}${MILL_REPO_URL}/releases/download/${MILL_VERSION_TAG}/${MILL_VERSION}${DOWNLOAD_SUFFIX}" unset MILL_VERSION_TAG fi diff --git a/metals/src/main/resources/millw.bat b/metals/src/main/resources/millw.bat index bec5bd46e81..5c3c3b2e840 100644 --- a/metals/src/main/resources/millw.bat +++ b/metals/src/main/resources/millw.bat @@ -5,7 +5,7 @@ rem You can give the required mill version with --mill-version parameter rem If no version is given, it falls back to the value of DEFAULT_MILL_VERSION rem rem Project page: https://github.com/lefou/millw -rem Script Version: 0.4.7 +rem Script Version: 0.4.11 rem rem If you want to improve this script, please also contribute your changes back! rem @@ -16,13 +16,17 @@ rem but I don't think we need to support them in 2019 setlocal enabledelayedexpansion if [!DEFAULT_MILL_VERSION!]==[] ( - set "DEFAULT_MILL_VERSION=0.11.0" + set "DEFAULT_MILL_VERSION=0.11.4" ) if [!GITHUB_RELEASE_CDN!]==[] ( set "GITHUB_RELEASE_CDN=" ) +if [!MILL_MAIN_CLI!]==[] ( + set "MILL_MAIN_CLI=%~f0" +) + set "MILL_REPO_URL=https://github.com/com-lihaoyi/mill" rem %~1% removes surrounding quotes @@ -129,7 +133,7 @@ if not exist "%MILL%" ( rem there seems to be no way to generate a unique temporary file path (on native Windows) set DOWNLOAD_FILE=%MILL%.tmp - if [!DOWNLOAD_FROM_MAVEN]==[1] ( + if [!DOWNLOAD_FROM_MAVEN!]==[1] ( set DOWNLOAD_URL=https://repo1.maven.org/maven2/com/lihaoyi/mill-dist/!MILL_VERSION!/mill-dist-!MILL_VERSION!.jar ) else ( set DOWNLOAD_URL=!GITHUB_RELEASE_CDN!%MILL_REPO_URL%/releases/download/!MILL_VERSION_TAG!/!MILL_VERSION!!DOWNLOAD_SUFFIX! @@ -164,10 +168,6 @@ set MILL_DOWNLOAD_PATH= set MILL_VERSION= set MILL_REPO_URL= -if [!MILL_MAIN_CLI!]==[] ( - set "MILL_MAIN_CLI=%0" -) - rem Need to preserve the first position of those listed options set MILL_FIRST_ARG= if [%~1%]==[--bsp] ( From 1e77550d1057a61aa31447801744c6419807a470 Mon Sep 17 00:00:00 2001 From: Sakib Hadziavdic Date: Mon, 18 Dec 2023 16:13:12 +0100 Subject: [PATCH 09/21] Update millw to 0.4.11 --- bin/update-millw.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/bin/update-millw.sh b/bin/update-millw.sh index f108ca29425..34d06c9009a 100755 --- a/bin/update-millw.sh +++ b/bin/update-millw.sh @@ -5,4 +5,3 @@ root_url=https://raw.githubusercontent.com/lefou/millw/main curl $root_url/millw > $PWD/metals/src/main/resources/millw curl $root_url/millw.bat > $PWD/metals/src/main/resources/millw.bat - From e2a3715324ac44033a6f110b9de62b983c959d46 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Tue, 19 Dec 2023 12:53:05 +0100 Subject: [PATCH 10/21] chore: Add support for Scala 3.3.2-RC1 We already publish separate module in the compiler that allows user to have right from the box support for Scala 3.3.2 and later, but not all PRs were yet backported, which means the experience for users could be worse under 3.3.1, which we would like to avoid. --- build.sbt | 2 ++ .../scala/meta/internal/pc/Compat.scala | 4 +++ .../scala/meta/internal/pc/Compat.scala | 4 +++ .../scala/meta/internal/pc/Compat.scala | 4 +++ .../scala/meta/internal/pc/Compat.scala | 4 +++ .../pc/printer/RefinedDotcPrinter.scala | 0 .../scala/meta/internal/pc/Compat.scala | 4 +++ .../pc/printer/RefinedDotcPrinter.scala | 6 ++++ .../scala/meta/internal/pc/Compat.scala | 4 +++ .../pc/printer/RefinedDotcPrinter.scala | 30 +++++++++++++++++++ .../pc/completions/ScaladocCompletions.scala | 6 ++-- .../internal/pc/printer/MetalsPrinter.scala | 2 +- project/V.scala | 2 +- .../tests/pc/CompletionOverrideSuite.scala | 2 +- .../test/scala/tests/pc/CompletionSuite.scala | 7 +++++ .../tests/pc/InsertInferredTypeSuite.scala | 2 +- .../src/main/scala/tests/BaseSuite.scala | 2 +- 17 files changed, 77 insertions(+), 8 deletions(-) create mode 100644 mtags/src/main/scala-3.0/scala/meta/internal/pc/Compat.scala create mode 100644 mtags/src/main/scala-3.1/scala/meta/internal/pc/Compat.scala create mode 100644 mtags/src/main/scala-3.2/scala/meta/internal/pc/Compat.scala create mode 100644 mtags/src/main/scala-3.3.0/scala/meta/internal/pc/Compat.scala rename mtags/src/main/{scala-3.3 => scala-3.3.0}/scala/meta/internal/pc/printer/RefinedDotcPrinter.scala (100%) create mode 100644 mtags/src/main/scala-3.3.1/scala/meta/internal/pc/Compat.scala create mode 100644 mtags/src/main/scala-3.3.1/scala/meta/internal/pc/printer/RefinedDotcPrinter.scala create mode 100644 mtags/src/main/scala-3.3.2-RC1/scala/meta/internal/pc/Compat.scala create mode 100644 mtags/src/main/scala-3.3.2-RC1/scala/meta/internal/pc/printer/RefinedDotcPrinter.scala diff --git a/build.sbt b/build.sbt index 3495085bcc4..20261f222d8 100644 --- a/build.sbt +++ b/build.sbt @@ -19,6 +19,8 @@ def isScala2(v: Option[(Long, Long)]): Boolean = v.exists(_._1 == 2) def isScala3(v: Option[(Long, Long)]): Boolean = v.exists(_._1 == 3) def isScala3WithPresentationCompiler(v: String): Boolean = (Version.parse(v), Version.parse(V.firstScala3PCVersion)) match { + case (Some(v), _) if V.supportedScalaVersions.contains(v.toString()) => + false case (Some(v), Some(firstScala3PCVersion)) => v >= firstScala3PCVersion case _ => false } diff --git a/mtags/src/main/scala-3.0/scala/meta/internal/pc/Compat.scala b/mtags/src/main/scala-3.0/scala/meta/internal/pc/Compat.scala new file mode 100644 index 00000000000..b6a4d617c5f --- /dev/null +++ b/mtags/src/main/scala-3.0/scala/meta/internal/pc/Compat.scala @@ -0,0 +1,4 @@ +package scala.meta.internal.pc + +object Compat: + val EvidenceParamName = dotty.tools.dotc.core.NameKinds.EvidenceParamName diff --git a/mtags/src/main/scala-3.1/scala/meta/internal/pc/Compat.scala b/mtags/src/main/scala-3.1/scala/meta/internal/pc/Compat.scala new file mode 100644 index 00000000000..b6a4d617c5f --- /dev/null +++ b/mtags/src/main/scala-3.1/scala/meta/internal/pc/Compat.scala @@ -0,0 +1,4 @@ +package scala.meta.internal.pc + +object Compat: + val EvidenceParamName = dotty.tools.dotc.core.NameKinds.EvidenceParamName diff --git a/mtags/src/main/scala-3.2/scala/meta/internal/pc/Compat.scala b/mtags/src/main/scala-3.2/scala/meta/internal/pc/Compat.scala new file mode 100644 index 00000000000..b6a4d617c5f --- /dev/null +++ b/mtags/src/main/scala-3.2/scala/meta/internal/pc/Compat.scala @@ -0,0 +1,4 @@ +package scala.meta.internal.pc + +object Compat: + val EvidenceParamName = dotty.tools.dotc.core.NameKinds.EvidenceParamName diff --git a/mtags/src/main/scala-3.3.0/scala/meta/internal/pc/Compat.scala b/mtags/src/main/scala-3.3.0/scala/meta/internal/pc/Compat.scala new file mode 100644 index 00000000000..b6a4d617c5f --- /dev/null +++ b/mtags/src/main/scala-3.3.0/scala/meta/internal/pc/Compat.scala @@ -0,0 +1,4 @@ +package scala.meta.internal.pc + +object Compat: + val EvidenceParamName = dotty.tools.dotc.core.NameKinds.EvidenceParamName diff --git a/mtags/src/main/scala-3.3/scala/meta/internal/pc/printer/RefinedDotcPrinter.scala b/mtags/src/main/scala-3.3.0/scala/meta/internal/pc/printer/RefinedDotcPrinter.scala similarity index 100% rename from mtags/src/main/scala-3.3/scala/meta/internal/pc/printer/RefinedDotcPrinter.scala rename to mtags/src/main/scala-3.3.0/scala/meta/internal/pc/printer/RefinedDotcPrinter.scala diff --git a/mtags/src/main/scala-3.3.1/scala/meta/internal/pc/Compat.scala b/mtags/src/main/scala-3.3.1/scala/meta/internal/pc/Compat.scala new file mode 100644 index 00000000000..b6a4d617c5f --- /dev/null +++ b/mtags/src/main/scala-3.3.1/scala/meta/internal/pc/Compat.scala @@ -0,0 +1,4 @@ +package scala.meta.internal.pc + +object Compat: + val EvidenceParamName = dotty.tools.dotc.core.NameKinds.EvidenceParamName diff --git a/mtags/src/main/scala-3.3.1/scala/meta/internal/pc/printer/RefinedDotcPrinter.scala b/mtags/src/main/scala-3.3.1/scala/meta/internal/pc/printer/RefinedDotcPrinter.scala new file mode 100644 index 00000000000..21b5271f330 --- /dev/null +++ b/mtags/src/main/scala-3.3.1/scala/meta/internal/pc/printer/RefinedDotcPrinter.scala @@ -0,0 +1,6 @@ +package scala.meta.internal.pc.printer + +import dotty.tools.dotc.core.Contexts.Context +import dotty.tools.dotc.printing.RefinedPrinter + +abstract class RefinedDotcPrinter(_ctx: Context) extends RefinedPrinter(_ctx) diff --git a/mtags/src/main/scala-3.3.2-RC1/scala/meta/internal/pc/Compat.scala b/mtags/src/main/scala-3.3.2-RC1/scala/meta/internal/pc/Compat.scala new file mode 100644 index 00000000000..1f87fbfe851 --- /dev/null +++ b/mtags/src/main/scala-3.3.2-RC1/scala/meta/internal/pc/Compat.scala @@ -0,0 +1,4 @@ +package scala.meta.internal.pc + +object Compat: + val EvidenceParamName = dotty.tools.dotc.core.NameKinds.ContextBoundParamName diff --git a/mtags/src/main/scala-3.3.2-RC1/scala/meta/internal/pc/printer/RefinedDotcPrinter.scala b/mtags/src/main/scala-3.3.2-RC1/scala/meta/internal/pc/printer/RefinedDotcPrinter.scala new file mode 100644 index 00000000000..fbd39a46ff5 --- /dev/null +++ b/mtags/src/main/scala-3.3.2-RC1/scala/meta/internal/pc/printer/RefinedDotcPrinter.scala @@ -0,0 +1,30 @@ +package scala.meta.internal.pc.printer + +import dotty.tools.dotc.core.Contexts.Context +import dotty.tools.dotc.printing.RefinedPrinter +import dotty.tools.dotc.core.Types.* +import dotty.tools.dotc.core.Flags.* +import dotty.tools.dotc.printing.Texts.Text +import dotty.tools.dotc.core.StdNames.* + +/* In 3.4.x some changes were made to printer, + but haven't managed to port all of them yet to the LTS */ +abstract class RefinedDotcPrinter(_ctx: Context) extends RefinedPrinter(_ctx): + + def toTextPrefix(tp: Type) = + tp match + case tp: NamedType => super.toTextPrefixOf(tp) + case tp => Text() + + override def toText(tp: Type): Text = + tp match + case tp: TermRef + if !tp.denotationIsCurrent && !homogenizedView || + tp.symbol.is(Module) || tp.symbol.name == nme.IMPORT => + toTextPrefix(tp.prefix) ~ selectionString(tp) ~ ".type" + case tp: TermRef => + toTextPrefix(tp.prefix) ~ selectionString(tp) + case tr: TypeRef => + super.toText(tr) + case _ => super.toText(tp) +end RefinedDotcPrinter diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/ScaladocCompletions.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/ScaladocCompletions.scala index 4875d222ac9..04f0e064343 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/ScaladocCompletions.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/ScaladocCompletions.scala @@ -1,12 +1,12 @@ package scala.meta.internal.pc.completions +import scala.meta.internal.pc.Compat import scala.meta.pc.PresentationCompilerConfig import dotty.tools.dotc.ast.tpd import dotty.tools.dotc.ast.tpd.* import dotty.tools.dotc.core.Contexts.Context import dotty.tools.dotc.core.Flags.* -import dotty.tools.dotc.core.NameKinds.* import dotty.tools.dotc.core.Symbols.NoSymbol import dotty.tools.dotc.core.Types.ExprType import dotty.tools.dotc.core.Types.MethodOrPoly @@ -112,7 +112,7 @@ object ScaladocCompletions: defdef.trailingParamss.flatten.collect { case param if !param.symbol.isOneOf(Synthetic) && - !param.name.is(EvidenceParamName) && + !param.name.is(Compat.EvidenceParamName) && param.symbol != extensionParam => param.name.show } @@ -121,7 +121,7 @@ object ScaladocCompletions: case param if !param.is(Synthetic) && !param.isTypeParam && - !param.name.is(EvidenceParamName) => + !param.name.is(Compat.EvidenceParamName) => param.name.show } case other => diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/printer/MetalsPrinter.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/printer/MetalsPrinter.scala index a7e22f878ef..6e713b466d9 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/printer/MetalsPrinter.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/printer/MetalsPrinter.scala @@ -9,6 +9,7 @@ import scala.meta.internal.jdk.CollectionConverters.* import scala.meta.internal.metals.Report import scala.meta.internal.metals.ReportContext import scala.meta.internal.mtags.MtagsEnrichments.* +import scala.meta.internal.pc.Compat.EvidenceParamName import scala.meta.internal.pc.IndexedContext import scala.meta.internal.pc.Params import scala.meta.internal.pc.printer.ShortenedNames.ShortName @@ -18,7 +19,6 @@ import scala.meta.pc.SymbolSearch import dotty.tools.dotc.core.Contexts.Context import dotty.tools.dotc.core.Flags import dotty.tools.dotc.core.Flags.* -import dotty.tools.dotc.core.NameKinds.EvidenceParamName import dotty.tools.dotc.core.NameOps.* import dotty.tools.dotc.core.Names.Name import dotty.tools.dotc.core.StdNames diff --git a/project/V.scala b/project/V.scala index 0b9575202e1..e7f672a2323 100644 --- a/project/V.scala +++ b/project/V.scala @@ -10,7 +10,7 @@ object V { val wrapperMetalsVersion = "3.4.0-RC1-bin-20231127-41e7d95-NIGHTLY" // When you can add to removedScalaVersions in MtagsResolver.scala with the last released version - val scala3RC: Option[String] = None + val scala3RC: Option[String] = Some("3.3.2-RC1") val sbtScala = "2.12.17" val ammonite212Version = "2.12.18" val ammonite213Version = "2.13.12" diff --git a/tests/cross/src/test/scala/tests/pc/CompletionOverrideSuite.scala b/tests/cross/src/test/scala/tests/pc/CompletionOverrideSuite.scala index 49d4a0ee850..81c3d9ba26d 100644 --- a/tests/cross/src/test/scala/tests/pc/CompletionOverrideSuite.scala +++ b/tests/cross/src/test/scala/tests/pc/CompletionOverrideSuite.scala @@ -450,7 +450,7 @@ class CompletionOverrideSuite extends BaseCompletionSuite { ) checkEdit( - "jlang", + "jlang".tag(IgnoreScalaVersion("3.3.2-RC1")), """|abstract class Mutable { | def foo: java.lang.StringBuilder |} diff --git a/tests/cross/src/test/scala/tests/pc/CompletionSuite.scala b/tests/cross/src/test/scala/tests/pc/CompletionSuite.scala index 2727939d0ad..2a5e6fbc458 100644 --- a/tests/cross/src/test/scala/tests/pc/CompletionSuite.scala +++ b/tests/cross/src/test/scala/tests/pc/CompletionSuite.scala @@ -1335,6 +1335,13 @@ class CompletionSuite extends BaseCompletionSuite { |higherKinds scala.languageFeature |implicitConversions scala.languageFeature |""".stripMargin, + ">=3.3.2-RC1" -> + """|dynamics languageFeature + |existentials languageFeature + |experimental languageFeature + |higherKinds languageFeature + |implicitConversions languageFeature + |""".stripMargin, ">=3.4.0-RC1-bin-20230921-3d539e6-NIGHTLY" -> """|dynamics scala.languageFeature |existentials scala.languageFeature diff --git a/tests/cross/src/test/scala/tests/pc/InsertInferredTypeSuite.scala b/tests/cross/src/test/scala/tests/pc/InsertInferredTypeSuite.scala index 8a8cbb2f883..cc6b615de81 100644 --- a/tests/cross/src/test/scala/tests/pc/InsertInferredTypeSuite.scala +++ b/tests/cross/src/test/scala/tests/pc/InsertInferredTypeSuite.scala @@ -452,7 +452,7 @@ class InsertInferredTypeSuite extends BaseCodeActionSuite { ) checkEdit( - "path", + "path".tag(IgnoreScalaVersion("3.3.2-RC1")), """|import java.nio.file.Paths |object ExplicitResultTypesPrefix { | class Path diff --git a/tests/mtest/src/main/scala/tests/BaseSuite.scala b/tests/mtest/src/main/scala/tests/BaseSuite.scala index ba74663b83c..bfa69f8da51 100644 --- a/tests/mtest/src/main/scala/tests/BaseSuite.scala +++ b/tests/mtest/src/main/scala/tests/BaseSuite.scala @@ -20,7 +20,7 @@ abstract class BaseSuite extends munit.FunSuite with Assertions { val FlakyWindows = new Tag("FlakyWindows") val scala3PresentationCompilerVersion: String = - s">=${BuildInfoVersions.firstScala3PCVersion}" + s">=3.3.3" Testing.enable() From 5955c07021ccfbc11250026a09ab28b63a5fd870 Mon Sep 17 00:00:00 2001 From: tgodzik Date: Fri, 1 Dec 2023 11:14:21 +0100 Subject: [PATCH 11/21] improvement: Support completions for implicit classes Previously, we would only automatically suggest extension methods, not implicit classes. Now, we also properly suggest implicit classes. We could follow up with support for Scala 2 also. --- .../internal/pc/CompilerSearchVisitor.scala | 8 +- .../meta/internal/pc/SemanticdbSymbols.scala | 15 ++ .../pc/completions/CompletionProvider.scala | 5 +- .../pc/completions/CompletionValue.scala | 14 ++ .../internal/pc/completions/Completions.scala | 25 ++- .../internal/mtags/ScalaToplevelMtags.scala | 140 ++++++++++--- .../pc/CompletionExtensionMethodSuite.scala | 186 ++++++++++++++++++ .../mtest/src/main/scala/tests/PCSuite.scala | 2 +- .../feature/CompletionCrossLspSuite.scala | 48 +++++ .../test/scala/tests/ScalaToplevelSuite.scala | 12 +- 10 files changed, 414 insertions(+), 41 deletions(-) diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/CompilerSearchVisitor.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/CompilerSearchVisitor.scala index aef1b57b89e..6cdc980d473 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/CompilerSearchVisitor.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/CompilerSearchVisitor.scala @@ -10,6 +10,7 @@ import scala.meta.internal.metals.ReportContext import scala.meta.pc.* import dotty.tools.dotc.core.Contexts.* +import dotty.tools.dotc.core.Flags import dotty.tools.dotc.core.Names.* import dotty.tools.dotc.core.Symbols.* @@ -21,7 +22,12 @@ class CompilerSearchVisitor( val logger: Logger = Logger.getLogger(classOf[CompilerSearchVisitor].getName) private def isAccessible(sym: Symbol): Boolean = try - sym != NoSymbol && sym.isPublic && sym.isStatic + sym != NoSymbol && sym.isPublic && sym.isStatic || { + val owner = sym.maybeOwner + owner != NoSymbol && owner.isClass && + owner.is(Flags.Implicit) && + owner.isStatic && owner.isPublic + } catch case err: AssertionError => logger.log(Level.WARNING, err.getMessage()) diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/SemanticdbSymbols.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/SemanticdbSymbols.scala index c8749645d91..92429cbdd73 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/SemanticdbSymbols.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/SemanticdbSymbols.scala @@ -48,7 +48,20 @@ object SemanticdbSymbols: // however in scalac this method is defined only in `module Files` if typeSym.is(JavaDefined) then typeSym :: owner.info.decl(termName(value)).symbol :: Nil + /** + * Looks like decl doesn't work for: + * package a: + * implicit class A (i: Int): + * def inc = i + 1 + */ + else if typeSym == NoSymbol then + val searched = typeName(value) + owner.info.allMembers + .find(_.name == searched) + .map(_.symbol) + .toList else typeSym :: Nil + end if case Descriptor.Term(value) => val outSymbol = owner.info.decl(termName(value)).symbol if outSymbol.exists @@ -91,6 +104,8 @@ object SemanticdbSymbols: .map(_.symbol) .filter(sym => symbolName(sym) == s) .toList + end match + end tryMember parentSymbol.flatMap(tryMember) try diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/CompletionProvider.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/CompletionProvider.scala index ea5c2c2ec1a..67da964a58c 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/CompletionProvider.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/CompletionProvider.scala @@ -224,7 +224,7 @@ class CompletionProvider( def mkItemWithImports( v: CompletionValue.Workspace | CompletionValue.Extension | - CompletionValue.Interpolator + CompletionValue.Interpolator | CompletionValue.ImplicitClass ) = val sym = v.symbol path match @@ -277,7 +277,8 @@ class CompletionProvider( end mkItemWithImports completion match - case v: (CompletionValue.Workspace | CompletionValue.Extension) => + case v: (CompletionValue.Workspace | CompletionValue.Extension | + CompletionValue.ImplicitClass) => mkItemWithImports(v) case v: CompletionValue.Interpolator if v.isWorkspace || v.isExtension => mkItemWithImports(v) diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/CompletionValue.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/CompletionValue.scala index 8f02ebd3bd0..03173154ef9 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/CompletionValue.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/CompletionValue.scala @@ -130,6 +130,20 @@ object CompletionValue: override def description(printer: MetalsPrinter)(using Context): String = s"${printer.completionSymbol(symbol)} (extension)" + /** + * CompletionValue for old implicit classes methods via SymbolSearch + */ + case class ImplicitClass( + label: String, + symbol: Symbol, + override val snippetSuffix: CompletionSuffix, + override val importSymbol: Symbol, + ) extends Symbolic: + override def completionItemKind(using Context): CompletionItemKind = + CompletionItemKind.Method + override def description(printer: MetalsPrinter)(using Context): String = + s"${printer.completionSymbol(symbol)} (implicit)" + /** * @param shortenedNames shortened type names by `Printer`. This field should be used for autoImports * @param start Starting position of the completion diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/Completions.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/Completions.scala index 7d0b823b5c9..7a3ac0d3491 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/Completions.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/Completions.scala @@ -595,14 +595,35 @@ class Completions( Some(search.search(query, buildTargetIdentifier, visitor)) case CompletionKind.Members => val visitor = new CompilerSearchVisitor(sym => - if sym.is(ExtensionMethod) && + def isExtensionMethod = sym.is(ExtensionMethod) && qualType.widenDealias <:< sym.extensionParam.info.widenDealias - then + + def isImplicitClass(owner: Symbol) = + val constructorParam = + owner.info.allMembers + .find(_.symbol.isAllOf(Flags.PrivateParamAccessor)) + .map(_.info) + owner.isClass && owner.is(Flags.Implicit) && + constructorParam.exists(p => + qualType.widenDealias <:< p.widenDealias + ) + end isImplicitClass + + def isImplicitClassMethod = sym.is(Flags.Method) && + isImplicitClass(sym.maybeOwner) + + if isExtensionMethod then completionsWithSuffix( sym, sym.decodedName, CompletionValue.Extension(_, _, _), ).map(visit).forall(_ == true) + else if isImplicitClassMethod then + completionsWithSuffix( + sym, + sym.decodedName, + CompletionValue.ImplicitClass(_, _, _, sym.maybeOwner), + ).map(visit).forall(_ == true) else false, ) Some(search.searchMethods(query, buildTargetIdentifier, visitor)) diff --git a/mtags/src/main/scala/scala/meta/internal/mtags/ScalaToplevelMtags.scala b/mtags/src/main/scala/scala/meta/internal/mtags/ScalaToplevelMtags.scala index c6cafdbb439..0dba337b8b2 100644 --- a/mtags/src/main/scala/scala/meta/internal/mtags/ScalaToplevelMtags.scala +++ b/mtags/src/main/scala/scala/meta/internal/mtags/ScalaToplevelMtags.scala @@ -79,8 +79,16 @@ class ScalaToplevelMtags( expectTemplate: Option[ExpectTemplate], prevWasDot: Boolean = false ): Unit = { - def newExpectTemplate: Some[ExpectTemplate] = - Some(ExpectTemplate(indent, currentOwner, false, false)) + def newExpectTemplate(isImplicit: Boolean = false): Some[ExpectTemplate] = + Some( + ExpectTemplate( + indent, + currentOwner, + false, + false, + isImplicit = isImplicit + ) + ) def newExpectCaseClassTemplate: Some[ExpectTemplate] = Some( ExpectTemplate( @@ -92,20 +100,27 @@ class ScalaToplevelMtags( isCaseClassConstructor = true ) ) - def newExpectClassTemplate: Some[ExpectTemplate] = + def newExpectClassTemplate( + isImplicit: Boolean = false + ): Some[ExpectTemplate] = Some( ExpectTemplate( indent, currentOwner, false, false, - isClassConstructor = true + isClassConstructor = true, + isImplicit = isImplicit ) ) def newExpectPkgTemplate: Some[ExpectTemplate] = Some(ExpectTemplate(indent, currentOwner, true, false)) def newExpectExtensionTemplate(owner: String): Some[ExpectTemplate] = Some(ExpectTemplate(indent, owner, false, true)) + def newExpectImplicitTemplate: Some[ExpectTemplate] = + Some( + ExpectTemplate(indent, currentOwner, false, false, isImplicit = true) + ) def newExpectIgnoreBody: Some[ExpectTemplate] = Some( ExpectTemplate( @@ -130,6 +145,8 @@ class ScalaToplevelMtags( def needEmitTermMember(): Boolean = includeMembers && !prevWasDot + def srcName = input.filename.stripSuffix(".scala") + if (!isDone) { val data = scanner.curr val currRegion = @@ -147,7 +164,7 @@ class ScalaToplevelMtags( val nextRegion = new Region.Package(currentOwner, currRegion) loop(indent, false, nextRegion, newExpectPkgTemplate) } else - loop(indent, false, currRegion, newExpectTemplate) + loop(indent, false, currRegion, newExpectTemplate()) case IDENTIFIER if dialect.allowExtensionMethods && data.name == "extension" => val nextOwner = @@ -177,21 +194,43 @@ class ScalaToplevelMtags( newExpectExtensionTemplate(nextOwner) ) case CLASS | TRAIT | OBJECT | ENUM if needEmitMember(currRegion) => - emitMember(false, currRegion.owner) + /* Scala 3 allows for toplevel implicit classes, but generates + * artificial package object. Scala 2 doesn't allow for it. + */ + val isScala3Implicit = + dialect.allowExtensionMethods && currRegion.produceSourceToplevel && + expectTemplate.exists(_.isImplicit) + val owner = if (isScala3Implicit) { + val name = s"$srcName$$package" + val pos = newPosition + val owner = withOwner(currRegion.owner) { + term(name, pos, Kind.OBJECT, 0) + } + owner + } else currRegion.owner + emitMember(isPackageObject = false, owner) val template = expectTemplate match { case Some(expect) if expect.isCaseClassConstructor => newExpectCaseClassTemplate - case _ => newExpectClassTemplate + case Some(expect) => + newExpectClassTemplate(expect.isImplicit) + case _ => + newExpectClassTemplate(isImplicit = false) } loop(indent, isAfterNewline = false, currRegion, template) // also covers extension methods because of `def` inside case DEF // extension group - if (includeMembers && dialect.allowExtensionMethods && currRegion.isExtension) => + if (includeMembers && (dialect.allowExtensionMethods && currRegion.isExtension || currRegion.isImplicit)) => acceptTrivia() newIdentifier.foreach { name => withOwner(currRegion.owner) { - term(name.name, name.pos, Kind.METHOD, EXTENSION) + method( + name.name, + region.overloads.disambiguator(name.name), + name.pos, + EXTENSION + ) } } loop(indent, isAfterNewline = false, currRegion, newExpectIgnoreBody) @@ -209,7 +248,12 @@ class ScalaToplevelMtags( acceptTrivia() newIdentifier.foreach { name => withOwner(expect.owner) { - term(name.name, name.pos, Kind.METHOD, EXTENSION) + method( + name.name, + region.overloads.disambiguator(name.name), + name.pos, + EXTENSION + ) } } loop(indent, isAfterNewline = false, currRegion, None) @@ -218,7 +262,6 @@ class ScalaToplevelMtags( if dialect.allowToplevelStatements && needEmitFileOwner(currRegion) => val pos = newPosition - val srcName = input.filename.stripSuffix(".scala") val name = s"$srcName$$package" val owner = withOwner(currRegion.owner) { term(name, pos, Kind.OBJECT, 0) @@ -306,7 +349,10 @@ class ScalaToplevelMtags( case COLON if dialect.allowSignificantIndentation => (expectTemplate, nextIsNL()) match { case (Some(expect), true) if needToParseBody(expect) => - val next = expect.startIndentedRegion(currRegion) + val next = expect.startIndentedRegion( + currRegion, + isImplicitClass = expect.isImplicit + ) resetRegion(next) scanner.nextToken() loop(0, isAfterNewline = true, next, None) @@ -340,7 +386,11 @@ class ScalaToplevelMtags( case Some(expect) if needToParseBody(expect) || needToParseExtension(expect) => val next = - expect.startInBraceRegion(currRegion, expect.isExtension) + expect.startInBraceRegion( + currRegion, + expect.isExtension, + expect.isImplicit + ) resetRegion(next) scanner.nextToken() loop(indent, isAfterNewline = false, next, None) @@ -351,7 +401,7 @@ class ScalaToplevelMtags( } case RBRACE => val nextRegion = currRegion match { - case Region.InBrace(_, prev, _, _) => resetRegion(prev) + case Region.InBrace(_, prev, _, _, _) => resetRegion(prev) case r => r } scanner.nextToken() @@ -391,7 +441,10 @@ class ScalaToplevelMtags( indent, isAfterNewline = false, currRegion.prev, - newExpectTemplate + newExpectTemplate( + // we still need the information if the current template is implicit + expectTemplate.exists(_.isImplicit) + ) ) case COMMA => val nextExpectTemplate = expectTemplate.filter(!_.isPackageBody) @@ -422,7 +475,7 @@ class ScalaToplevelMtags( val (shouldCreateClassTemplate, isAfterNewline) = emitEnumCases(region, nextIsNewLine) val nextExpectTemplate = - if (shouldCreateClassTemplate) newExpectClassTemplate + if (shouldCreateClassTemplate) newExpectClassTemplate() else expectTemplate.filter(!_.isPackageBody) loop( indent, @@ -431,6 +484,15 @@ class ScalaToplevelMtags( if (scanner.curr.token == CLASS) newExpectCaseClassTemplate else nextExpectTemplate ) + case IMPLICIT => + scanner.nextToken() + loop( + indent, + isAfterNewline, + currRegion, + newExpectImplicitTemplate, + prevWasDot + ) case t => val nextExpectTemplate = expectTemplate.filter(!_.isPackageBody) scanner.nextToken() @@ -832,7 +894,8 @@ object ScalaToplevelMtags { isExtension: Boolean = false, ignoreBody: Boolean = false, isCaseClassConstructor: Boolean = false, - isClassConstructor: Boolean = false + isClassConstructor: Boolean = false, + isImplicit: Boolean = false ) { /** @@ -845,15 +908,29 @@ object ScalaToplevelMtags { private def adjustRegion(r: Region): Region = if (isPackageBody) r.prev else r - def startInBraceRegion(prev: Region, extension: Boolean = false): Region = - new Region.InBrace(owner, adjustRegion(prev), extension) + def startInBraceRegion( + prev: Region, + extension: Boolean = false, + isImplicitClass: Boolean = false + ): Region = + new Region.InBrace(owner, adjustRegion(prev), extension, isImplicitClass) def startInParenRegion(prev: Region, isCaseClass: Boolean): Region = if (isCaseClass) Region.InParenCaseClass(owner, adjustRegion(prev), true) else Region.InParenClass(owner, adjustRegion(prev)) - def startIndentedRegion(prev: Region, extension: Boolean = false): Region = - new Region.Indented(owner, indent, adjustRegion(prev), extension) + def startIndentedRegion( + prev: Region, + extension: Boolean = false, + isImplicitClass: Boolean = false + ): Region = + new Region.Indented( + owner, + indent, + adjustRegion(prev), + extension, + isImplicitClass: Boolean + ) } @@ -863,6 +940,7 @@ object ScalaToplevelMtags { def acceptMembers: Boolean def produceSourceToplevel: Boolean = termOwner.isPackage def isExtension: Boolean = false + def isImplicit: Boolean = false val overloads: OverloadDisambiguator = new OverloadDisambiguator() def termOwner: String = owner // toplevel terms are wrapped into an artificial Object @@ -898,39 +976,43 @@ object ScalaToplevelMtags { owner: String, prev: Region, extension: Boolean = false, - override val termOwner: String + override val termOwner: String, + override val isImplicit: Boolean ) extends Region { def this( owner: String, prev: Region, - extension: Boolean - ) = this(owner, prev, extension, owner) + extension: Boolean, + isImplicit: Boolean + ) = this(owner, prev, extension, owner, isImplicit) def acceptMembers: Boolean = owner.endsWith("/") override def isExtension = extension override val withTermOwner: String => InBrace = termOwner => - InBrace(owner, prev, extension, termOwner) + InBrace(owner, prev, extension, termOwner, isImplicit) } final case class Indented( owner: String, exitIndent: Int, prev: Region, extension: Boolean = false, - override val termOwner: String + override val termOwner: String, + override val isImplicit: Boolean ) extends Region { def this( owner: String, exitIndent: Int, prev: Region, - extension: Boolean - ) = this(owner, exitIndent, prev, extension, owner) + extension: Boolean, + isImplicit: Boolean + ) = this(owner, exitIndent, prev, extension, owner, isImplicit) def acceptMembers: Boolean = owner.endsWith("/") override def isExtension = extension override val withTermOwner: String => Indented = termOwner => - Indented(owner, exitIndent, prev, extension, termOwner) + Indented(owner, exitIndent, prev, extension, termOwner, isImplicit) } final case class InParenClass( diff --git a/tests/cross/src/test/scala/tests/pc/CompletionExtensionMethodSuite.scala b/tests/cross/src/test/scala/tests/pc/CompletionExtensionMethodSuite.scala index c75c9b433fb..480f9c9f317 100644 --- a/tests/cross/src/test/scala/tests/pc/CompletionExtensionMethodSuite.scala +++ b/tests/cross/src/test/scala/tests/pc/CompletionExtensionMethodSuite.scala @@ -21,6 +21,20 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { |""".stripMargin ) + check( + "simple-old-syntax", + """|package example + | + |object Test: + | implicit class TestOps(a: Int): + | def testOps(b: Int): String = ??? + | + |def main = 100.test@@ + |""".stripMargin, + """|testOps(b: Int): String (implicit) + |""".stripMargin + ) + check( "simple2", """|package example @@ -36,6 +50,21 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { filter = _.contains("(extension)") ) + check( + "simple2-old-syntax", + """|package example + | + |object enrichments: + | implicit class TestOps(a: Int): + | def testOps(b: Int): String = ??? + | + |def main = 100.t@@ + |""".stripMargin, + """|testOps(b: Int): String (implicit) + |""".stripMargin, + filter = _.contains("(implicit)") + ) + check( "simple-empty", """|package example @@ -51,6 +80,21 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { filter = _.contains("(extension)") ) + check( + "simple-empty-old", + """|package example + | + |object enrichments: + | implicit class TestOps(a: Int): + | def testOps(b: Int): String = ??? + | + |def main = 100.@@ + |""".stripMargin, + """|testOps(b: Int): String (implicit) + |""".stripMargin, + filter = _.contains("(implicit)") + ) + check( "filter-by-type", """|package example @@ -68,6 +112,23 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { filter = _.contains("(extension)") ) + check( + "filter-by-type-old", + """|package example + | + |object enrichments: + | implicit class A(num: Int): + | def identity2: Int = num + 1 + | implicit class B(str: String): + | def identity: String = str + | + |def main = "foo".iden@@ + |""".stripMargin, + """|identity: String (implicit) + |""".stripMargin // incr won't be available + + ) + check( "filter-by-type-subtype", """|package example @@ -86,6 +147,24 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { filter = _.contains("(extension)") ) + check( + "filter-by-type-subtype-old", + """|package example + | + |class A + |class B extends A + | + |object enrichments: + | implicit class Test(a: A): + | def doSomething: A = a + | + |def main = (new B).do@@ + |""".stripMargin, + """|doSomething: A (implicit) + |""".stripMargin, + filter = _.contains("(implicit)") + ) + checkEdit( "simple-edit", """|package example @@ -108,6 +187,28 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { |""".stripMargin ) + checkEdit( + "simple-edit-old", + """|package example + | + |object enrichments: + | implicit class A (num: Int): + | def incr: Int = num + 1 + | + |def main = 100.inc@@ + |""".stripMargin, + """|package example + | + |import example.enrichments.A + | + |object enrichments: + | implicit class A (num: Int): + | def incr: Int = num + 1 + | + |def main = 100.incr + |""".stripMargin + ) + checkEdit( "simple-edit-suffix", """|package example @@ -157,6 +258,28 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { | val plus = 100.plus(19) | val y = 19.plus($0) |} + """ + ) + + checkEdit( + "simple-edit-suffix-old", + """|package example + | + |object enrichments: + | implicit class A (num: Int): + | def plus(other: Int): Int = num + other + | + |def main = 100.pl@@ + |""".stripMargin, + """|package example + | + |import example.enrichments.A + | + |object enrichments: + | implicit class A (num: Int): + | def plus(other: Int): Int = num + other + | + |def main = 100.plus($0) |""".stripMargin ) @@ -176,6 +299,20 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { |""".stripMargin ) + check( + "directly-in-pkg1-old".tag(IgnoreScalaVersion.forLessThan("3.2.2")), + """| + |package examples: + | implicit class A(num: Int): + | def incr: Int = num + 1 + | + |package examples2: + | def main = 100.inc@@ + |""".stripMargin, + """|incr: Int (implicit) + |""".stripMargin + ) + check( "directly-in-pkg2".tag(IgnoreScalaVersion.forLessThan("3.2.2")), """|package example: @@ -190,6 +327,20 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { |""".stripMargin ) + check( + "directly-in-pkg2-old".tag(IgnoreScalaVersion.forLessThan("3.2.2")), + """|package examples: + | object X: + | def fooBar(num: Int) = num + 1 + | implicit class A (num: Int) { def incr: Int = num + 1 } + | + |package examples2: + | def main = 100.inc@@ + |""".stripMargin, + """|incr: Int (implicit) + |""".stripMargin + ) + checkEdit( "directly-in-pkg3".tag(IgnoreScalaVersion.forLessThan("3.2.2")), """|package example: @@ -207,6 +358,23 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { |""".stripMargin ) + checkEdit( + "directly-in-pkg3-old".tag(IgnoreScalaVersion.forLessThan("3.2.2")), + """|package examples: + | implicit class A (num: Int) { def incr: Int = num + 1 } + | + |package examples2: + | def main = 100.inc@@ + |""".stripMargin, + """|import examples.A + |package examples: + | implicit class A (num: Int) { def incr: Int = num + 1 } + | + |package examples2: + | def main = 100.incr + |""".stripMargin + ) + check( "nested-pkg".tag(IgnoreScalaVersion.forLessThan("3.2.2")), """|package a: // some comment @@ -225,4 +393,22 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { |""".stripMargin ) + check( + "nested-pkg-old".tag(IgnoreScalaVersion.forLessThan("3.2.2")), + """|package aa: // some comment + | package cc: + | implicit class A (num: Int): + | def increment2 = num + 2 + | implicit class A (num: Int): + | def increment = num + 1 + | + | + |package bb: + | def main: Unit = 123.incre@@ + |""".stripMargin, + """|increment: Int (implicit) + |increment2: Int (implicit) + |""".stripMargin + ) + } diff --git a/tests/mtest/src/main/scala/tests/PCSuite.scala b/tests/mtest/src/main/scala/tests/PCSuite.scala index 79f40b5b226..d74e0ec1d9e 100644 --- a/tests/mtest/src/main/scala/tests/PCSuite.scala +++ b/tests/mtest/src/main/scala/tests/PCSuite.scala @@ -100,7 +100,7 @@ trait PCSuite { case NonFatal(e) => println(s"warn: ${e.getMessage}") } - workspace.inputs(filename) = (code2, dialect) + workspace.inputs(file.toURI.toString()) = (code2, dialect) } } diff --git a/tests/slow/src/test/scala/tests/feature/CompletionCrossLspSuite.scala b/tests/slow/src/test/scala/tests/feature/CompletionCrossLspSuite.scala index d2a14f6bb1f..991aa42f4ec 100644 --- a/tests/slow/src/test/scala/tests/feature/CompletionCrossLspSuite.scala +++ b/tests/slow/src/test/scala/tests/feature/CompletionCrossLspSuite.scala @@ -106,6 +106,54 @@ class CompletionCrossLspSuite } yield () } + test("implicit-class") { + cleanWorkspace() + for { + _ <- initialize( + s"""/metals.json + |{ + | "a": { "scalaVersion": "${V.scala3}" } + |} + |/a/src/main/scala/a/B.scala + |package b + |implicit class B (num: Int): + | def plus(other: Int) = num + other + |/a/src/main/scala/a/A.scala + |package a + | + |object A { + | // @@ + |} + |""".stripMargin + ) + _ <- server.didOpen("a/src/main/scala/a/B.scala") + _ = assertNoDiagnostics() + _ <- assertCompletionEdit( + "1.p@@", + """|package a + | + |import b.B + | + |object A { + | 1.plus($0) + |} + |""".stripMargin, + filter = _.contains("plus"), + ) + _ <- assertCompletion( + "1.pl@@", + """|plus(other: Int): Int (implicit) + |""".stripMargin, + filter = _.contains("plus"), + ) + _ <- assertCompletion( + "\"plus is not available for string\".plu@@", + "", + filter = _.contains("plus"), + ) + } yield () + } + test("basic-scala3") { cleanWorkspace() for { diff --git a/tests/unit/src/test/scala/tests/ScalaToplevelSuite.scala b/tests/unit/src/test/scala/tests/ScalaToplevelSuite.scala index ae9116ea5b6..169f1de83fc 100644 --- a/tests/unit/src/test/scala/tests/ScalaToplevelSuite.scala +++ b/tests/unit/src/test/scala/tests/ScalaToplevelSuite.scala @@ -348,8 +348,8 @@ class ScalaToplevelSuite extends BaseSuite { List( "a/", "a/A.", - "a/A.bar.", - "a/A.foo.", + "a/A.bar().", + "a/A.foo().", ), mode = All, dialect = dialects.Scala3, @@ -369,8 +369,8 @@ class ScalaToplevelSuite extends BaseSuite { List( "a/", "a/Test$package.", - "a/Test$package.bar.", - "a/Test$package.foo.", + "a/Test$package.bar().", + "a/Test$package.foo().", ), mode = All, dialect = dialects.Scala3, @@ -386,8 +386,8 @@ class ScalaToplevelSuite extends BaseSuite { | def baz: Long = ??? |""".stripMargin, List( - "a/", "a/Test$package.", "a/Test$package.foo.", "a/Test$package.bar.", - "a/Test$package.baz.", + "a/", "a/Test$package.", "a/Test$package.foo().", "a/Test$package.bar().", + "a/Test$package.baz().", ), mode = All, dialect = dialects.Scala3, From 5237d3a8e5f04db8d8e5cafd0c664e1d7ff573aa Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Fri, 15 Dec 2023 19:40:39 +0100 Subject: [PATCH 12/21] bugfix: Only generate package object once --- .../meta/internal/mtags/ScalaToplevelMtags.scala | 15 +++++++++++---- .../tests/pc/CompletionExtensionMethodSuite.scala | 4 ++-- .../main/scala-3/example/ToplevelImplicit.scala | 10 ++++++++++ .../decorations3/example/EndMarker.scala | 2 +- .../decorations3/example/ToplevelImplicit.scala | 10 ++++++++++ .../example/ToplevelImplicit.scala | 10 ++++++++++ .../example/ToplevelImplicit.scala | 10 ++++++++++ .../test/resources/expect/toplevels-scala3.expect | 6 +++++- .../mtags-scala3/example/ToplevelImplicit.scala | 10 ++++++++++ .../semanticTokens3/example/EndMarker.scala | 2 +- .../example/ToplevelImplicit.scala | 10 ++++++++++ .../example/ToplevelImplicit.scala | 10 ++++++++++ .../example/ToplevelImplicit.scala | 10 ++++++++++ 13 files changed, 100 insertions(+), 9 deletions(-) create mode 100644 tests/input/src/main/scala-3/example/ToplevelImplicit.scala create mode 100644 tests/unit/src/test/resources/decorations3/example/ToplevelImplicit.scala create mode 100644 tests/unit/src/test/resources/definition-scala3/example/ToplevelImplicit.scala create mode 100644 tests/unit/src/test/resources/documentSymbol-scala3/example/ToplevelImplicit.scala create mode 100644 tests/unit/src/test/resources/mtags-scala3/example/ToplevelImplicit.scala create mode 100644 tests/unit/src/test/resources/semanticTokens3/example/ToplevelImplicit.scala create mode 100644 tests/unit/src/test/resources/semanticdb-scala3/example/ToplevelImplicit.scala create mode 100644 tests/unit/src/test/resources/toplevel-with-inner-scala3/example/ToplevelImplicit.scala diff --git a/mtags/src/main/scala/scala/meta/internal/mtags/ScalaToplevelMtags.scala b/mtags/src/main/scala/scala/meta/internal/mtags/ScalaToplevelMtags.scala index 0dba337b8b2..418ae3f197d 100644 --- a/mtags/src/main/scala/scala/meta/internal/mtags/ScalaToplevelMtags.scala +++ b/mtags/src/main/scala/scala/meta/internal/mtags/ScalaToplevelMtags.scala @@ -197,17 +197,19 @@ class ScalaToplevelMtags( /* Scala 3 allows for toplevel implicit classes, but generates * artificial package object. Scala 2 doesn't allow for it. */ - val isScala3Implicit = + val needsToGenerateFileClass = dialect.allowExtensionMethods && currRegion.produceSourceToplevel && expectTemplate.exists(_.isImplicit) - val owner = if (isScala3Implicit) { + val owner = if (needsToGenerateFileClass) { val name = s"$srcName$$package" val pos = newPosition val owner = withOwner(currRegion.owner) { term(name, pos, Kind.OBJECT, 0) } owner - } else currRegion.owner + } else if (expectTemplate.exists(_.isImplicit)) { + currRegion.termOwner + } else { currRegion.owner } emitMember(isPackageObject = false, owner) val template = expectTemplate match { case Some(expect) if expect.isCaseClassConstructor => @@ -217,7 +219,12 @@ class ScalaToplevelMtags( case _ => newExpectClassTemplate(isImplicit = false) } - loop(indent, isAfterNewline = false, currRegion, template) + loop( + indent, + isAfterNewline = false, + if (needsToGenerateFileClass) currRegion.withTermOwner(owner) else currRegion, + template + ) // also covers extension methods because of `def` inside case DEF // extension group diff --git a/tests/cross/src/test/scala/tests/pc/CompletionExtensionMethodSuite.scala b/tests/cross/src/test/scala/tests/pc/CompletionExtensionMethodSuite.scala index 480f9c9f317..05e0c2ead35 100644 --- a/tests/cross/src/test/scala/tests/pc/CompletionExtensionMethodSuite.scala +++ b/tests/cross/src/test/scala/tests/pc/CompletionExtensionMethodSuite.scala @@ -258,9 +258,9 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { | val plus = 100.plus(19) | val y = 19.plus($0) |} - """ + """.stripMargin ) - + checkEdit( "simple-edit-suffix-old", """|package example diff --git a/tests/input/src/main/scala-3/example/ToplevelImplicit.scala b/tests/input/src/main/scala-3/example/ToplevelImplicit.scala new file mode 100644 index 00000000000..7651f8aee7b --- /dev/null +++ b/tests/input/src/main/scala-3/example/ToplevelImplicit.scala @@ -0,0 +1,10 @@ +package example + +// This wasn't possible in Scala 2 +implicit class Xtension(number: Int) { + def increment: Int = number + 1 +} + +implicit class XtensionAnyVal(private val number: Int) extends AnyVal { + def double: Int = number * 2 +} diff --git a/tests/unit/src/test/resources/decorations3/example/EndMarker.scala b/tests/unit/src/test/resources/decorations3/example/EndMarker.scala index 26c56797083..cb7489c8a8b 100644 --- a/tests/unit/src/test/resources/decorations3/example/EndMarker.scala +++ b/tests/unit/src/test/resources/decorations3/example/EndMarker.scala @@ -3,4 +3,4 @@ package example object EndMarker: def foo/*: Int*/ = 1 - end foo + end foo \ No newline at end of file diff --git a/tests/unit/src/test/resources/decorations3/example/ToplevelImplicit.scala b/tests/unit/src/test/resources/decorations3/example/ToplevelImplicit.scala new file mode 100644 index 00000000000..d9de637fac5 --- /dev/null +++ b/tests/unit/src/test/resources/decorations3/example/ToplevelImplicit.scala @@ -0,0 +1,10 @@ +package example + +// This wasn't possible in Scala 2 +implicit class Xtension(number: Int) { + def increment: Int = number + 1 +} + +implicit class XtensionAnyVal(private val number: Int) extends AnyVal { + def double: Int = number * 2 +} \ No newline at end of file diff --git a/tests/unit/src/test/resources/definition-scala3/example/ToplevelImplicit.scala b/tests/unit/src/test/resources/definition-scala3/example/ToplevelImplicit.scala new file mode 100644 index 00000000000..a98666153ea --- /dev/null +++ b/tests/unit/src/test/resources/definition-scala3/example/ToplevelImplicit.scala @@ -0,0 +1,10 @@ +package example + +// This wasn't possible in Scala 2 +implicit class Xtension/**/(number/**/: Int/*Int.scala*/) { + def increment/**/: Int/*Int.scala*/ = number/**/ +/*Int.scala*/ 1 +} + +implicit class XtensionAnyVal/**/(private val number/**/: Int/*Int.scala*/) extends AnyVal/*AnyVal.scala*/ { + def double/**/: Int/*Int.scala*/ = number/**/ */*Int.scala*/ 2 +} diff --git a/tests/unit/src/test/resources/documentSymbol-scala3/example/ToplevelImplicit.scala b/tests/unit/src/test/resources/documentSymbol-scala3/example/ToplevelImplicit.scala new file mode 100644 index 00000000000..19cfa235c8a --- /dev/null +++ b/tests/unit/src/test/resources/documentSymbol-scala3/example/ToplevelImplicit.scala @@ -0,0 +1,10 @@ +/*example(Package):10*/package example + +// This wasn't possible in Scala 2 +/*example.Xtension(Class):6*/implicit class Xtension(number: Int) { + /*example.Xtension#increment(Method):5*/def increment: Int = number + 1 +} + +/*example.XtensionAnyVal(Class):10*/implicit class XtensionAnyVal(private val number: Int) extends AnyVal { + /*example.XtensionAnyVal#double(Method):9*/def double: Int = number * 2 +} diff --git a/tests/unit/src/test/resources/expect/toplevels-scala3.expect b/tests/unit/src/test/resources/expect/toplevels-scala3.expect index 39a344acf53..96c318a2e61 100644 --- a/tests/unit/src/test/resources/expect/toplevels-scala3.expect +++ b/tests/unit/src/test/resources/expect/toplevels-scala3.expect @@ -41,6 +41,10 @@ example/PatternMatching.scala -> example/PatternMatching# example/Scalalib.scala -> example/Scalalib# example/StructuralTypes.scala -> example/StructuralTypes. example/ToplevelDefVal.scala -> example/ToplevelDefVal$package. +example/ToplevelImplicit.scala -> example/ToplevelImplicit$package. +example/ToplevelImplicit.scala -> example/ToplevelImplicit$package. +example/ToplevelImplicit.scala -> example/ToplevelImplicit$package.Xtension# +example/ToplevelImplicit.scala -> example/ToplevelImplicit$package.XtensionAnyVal# example/TryCatch.scala -> example/TryCatch# example/TypeEnum.scala -> example/TypeEnum# example/TypeParameters.scala -> example/TypeParameters# @@ -53,4 +57,4 @@ example/nested/LocalClass.scala -> example/nested/LocalClass# example/nested/package.scala -> example/PackageObjectSibling# example/nested/package.scala -> example/nested/package. example/package.scala -> example/package. -example/type/Backtick.scala -> example/type/Backtick# +example/type/Backtick.scala -> example/type/Backtick# \ No newline at end of file diff --git a/tests/unit/src/test/resources/mtags-scala3/example/ToplevelImplicit.scala b/tests/unit/src/test/resources/mtags-scala3/example/ToplevelImplicit.scala new file mode 100644 index 00000000000..0df96f53532 --- /dev/null +++ b/tests/unit/src/test/resources/mtags-scala3/example/ToplevelImplicit.scala @@ -0,0 +1,10 @@ +package example + +// This wasn't possible in Scala 2 +implicit class Xtension/*example.Xtension().*//*example.Xtension#*/(number/*example.Xtension#number.*/: Int) { + def increment/*example.Xtension#increment().*/: Int = number + 1 +} + +implicit class XtensionAnyVal/*example.XtensionAnyVal().*//*example.XtensionAnyVal#*/(private val number/*example.XtensionAnyVal#number.*/: Int) extends AnyVal { + def double/*example.XtensionAnyVal#double().*/: Int = number * 2 +} diff --git a/tests/unit/src/test/resources/semanticTokens3/example/EndMarker.scala b/tests/unit/src/test/resources/semanticTokens3/example/EndMarker.scala index 7e4e14ab074..0c9bc254a5a 100644 --- a/tests/unit/src/test/resources/semanticTokens3/example/EndMarker.scala +++ b/tests/unit/src/test/resources/semanticTokens3/example/EndMarker.scala @@ -3,4 +3,4 @@ <>/*keyword*/ <>/*class*/: <>/*keyword*/ <>/*method,definition*/ = <<1>>/*number*/ - <>/*keyword*/ <>/*method,definition*/ + <>/*keyword*/ <>/*method,definition*/ \ No newline at end of file diff --git a/tests/unit/src/test/resources/semanticTokens3/example/ToplevelImplicit.scala b/tests/unit/src/test/resources/semanticTokens3/example/ToplevelImplicit.scala new file mode 100644 index 00000000000..ce84a8602c7 --- /dev/null +++ b/tests/unit/src/test/resources/semanticTokens3/example/ToplevelImplicit.scala @@ -0,0 +1,10 @@ +<>/*keyword*/ <>/*namespace*/ + +<>/*comment*/ +<>/*modifier*/ <>/*keyword*/ <>/*class*/(<>/*variable,declaration,readonly*/: <>/*class,abstract*/) { + <>/*keyword*/ <>/*method,definition*/: <>/*class,abstract*/ = <>/*variable,readonly*/ <<+>>/*method*/ <<1>>/*number*/ +} + +<>/*modifier*/ <>/*keyword*/ <>/*class*/(<>/*modifier*/ <>/*keyword*/ <>/*variable,declaration,readonly*/: <>/*class,abstract*/) <>/*keyword*/ <>/*class,abstract*/ { + <>/*keyword*/ <>/*method,definition*/: <>/*class,abstract*/ = <>/*variable,readonly*/ <<*>>/*method*/ <<2>>/*number*/ +} \ No newline at end of file diff --git a/tests/unit/src/test/resources/semanticdb-scala3/example/ToplevelImplicit.scala b/tests/unit/src/test/resources/semanticdb-scala3/example/ToplevelImplicit.scala new file mode 100644 index 00000000000..180e4a65ddf --- /dev/null +++ b/tests/unit/src/test/resources/semanticdb-scala3/example/ToplevelImplicit.scala @@ -0,0 +1,10 @@ +package example + +// This wasn't possible in Scala 2 +implicit class Xtension/*example.ToplevelImplicit$package.Xtension#*/(number/*example.ToplevelImplicit$package.Xtension#number.*/: Int/*scala.Int#*/) { + def increment/*example.ToplevelImplicit$package.Xtension#increment().*/: Int/*scala.Int#*/ = number/*example.ToplevelImplicit$package.Xtension#number.*/ +/*scala.Int#`+`(+4).*/ 1 +} + +implicit class XtensionAnyVal/*example.ToplevelImplicit$package.XtensionAnyVal#*/(private val number/*example.ToplevelImplicit$package.XtensionAnyVal#number.*/: Int/*scala.Int#*/) extends AnyVal/*scala.AnyVal#*/ { + def double/*example.ToplevelImplicit$package.XtensionAnyVal#double().*/: Int/*scala.Int#*/ = number/*example.ToplevelImplicit$package.XtensionAnyVal#number.*/ */*scala.Int#`*`(+3).*/ 2 +} diff --git a/tests/unit/src/test/resources/toplevel-with-inner-scala3/example/ToplevelImplicit.scala b/tests/unit/src/test/resources/toplevel-with-inner-scala3/example/ToplevelImplicit.scala new file mode 100644 index 00000000000..2a6ab582f9f --- /dev/null +++ b/tests/unit/src/test/resources/toplevel-with-inner-scala3/example/ToplevelImplicit.scala @@ -0,0 +1,10 @@ +package example + +// This wasn't possible in Scala 2 +implicit class/*example.ToplevelImplicit$package.*/ Xtension/*example.ToplevelImplicit$package.Xtension#*/(number: Int) { + def increment: Int = number + 1 +} + +implicit class XtensionAnyVal/*example.ToplevelImplicit$package.XtensionAnyVal#*/(private val number: Int) extends AnyVal { + def double: Int = number * 2 +} From e39acff1eba9eb77e101a8cd3d90328657d94ec9 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Fri, 15 Dec 2023 20:01:51 +0100 Subject: [PATCH 13/21] refactor: Tweaks after reviews --- .../internal/pc/CompilerSearchVisitor.scala | 14 ++++--- .../meta/internal/pc/SemanticdbSymbols.scala | 8 +--- .../internal/pc/completions/Completions.scala | 9 ++++- .../internal/mtags/ScalaToplevelMtags.scala | 3 +- .../src/main/scala/tests/BasePCSuite.scala | 1 + .../pc/CompletionExtensionMethodSuite.scala | 40 ++++++++++++------- .../resources/expect/toplevels-scala3.expect | 1 - 7 files changed, 46 insertions(+), 30 deletions(-) diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/CompilerSearchVisitor.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/CompilerSearchVisitor.scala index 6cdc980d473..8cfa3cb2fd0 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/CompilerSearchVisitor.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/CompilerSearchVisitor.scala @@ -21,13 +21,15 @@ class CompilerSearchVisitor( val logger: Logger = Logger.getLogger(classOf[CompilerSearchVisitor].getName) + private def isAccessibleImplicitClass(sym: Symbol) = + val owner = sym.maybeOwner + owner != NoSymbol && owner.isClass && + owner.is(Flags.Implicit) && + owner.isStatic && owner.isPublic + private def isAccessible(sym: Symbol): Boolean = try - sym != NoSymbol && sym.isPublic && sym.isStatic || { - val owner = sym.maybeOwner - owner != NoSymbol && owner.isClass && - owner.is(Flags.Implicit) && - owner.isStatic && owner.isPublic - } + sym != NoSymbol && sym.isPublic && sym.isStatic || + isAccessibleImplicitClass(sym) catch case err: AssertionError => logger.log(Level.WARNING, err.getMessage()) diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/SemanticdbSymbols.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/SemanticdbSymbols.scala index 92429cbdd73..3c8bcfa732b 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/SemanticdbSymbols.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/SemanticdbSymbols.scala @@ -51,15 +51,11 @@ object SemanticdbSymbols: /** * Looks like decl doesn't work for: * package a: - * implicit class A (i: Int): + * implicit class <> (i: Int): * def inc = i + 1 */ else if typeSym == NoSymbol then - val searched = typeName(value) - owner.info.allMembers - .find(_.name == searched) - .map(_.symbol) - .toList + owner.info.member(typeName(value)).symbol :: Nil else typeSym :: Nil end if case Descriptor.Term(value) => diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/Completions.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/Completions.scala index 7a3ac0d3491..d51d8307bd2 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/Completions.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/Completions.scala @@ -599,9 +599,14 @@ class Completions( qualType.widenDealias <:< sym.extensionParam.info.widenDealias def isImplicitClass(owner: Symbol) = + val constructorParam = - owner.info.allMembers - .find(_.symbol.isAllOf(Flags.PrivateParamAccessor)) + owner.info + .membersBasedOnFlags( + Flags.ParamAccessor, + Flags.EmptyFlags, + ) + .headOption .map(_.info) owner.isClass && owner.is(Flags.Implicit) && constructorParam.exists(p => diff --git a/mtags/src/main/scala/scala/meta/internal/mtags/ScalaToplevelMtags.scala b/mtags/src/main/scala/scala/meta/internal/mtags/ScalaToplevelMtags.scala index 418ae3f197d..46c4ba40a72 100644 --- a/mtags/src/main/scala/scala/meta/internal/mtags/ScalaToplevelMtags.scala +++ b/mtags/src/main/scala/scala/meta/internal/mtags/ScalaToplevelMtags.scala @@ -222,7 +222,8 @@ class ScalaToplevelMtags( loop( indent, isAfterNewline = false, - if (needsToGenerateFileClass) currRegion.withTermOwner(owner) else currRegion, + if (needsToGenerateFileClass) currRegion.withTermOwner(owner) + else currRegion, template ) // also covers extension methods because of `def` inside diff --git a/tests/cross/src/main/scala/tests/BasePCSuite.scala b/tests/cross/src/main/scala/tests/BasePCSuite.scala index 84874519f08..f7ef495f650 100644 --- a/tests/cross/src/main/scala/tests/BasePCSuite.scala +++ b/tests/cross/src/main/scala/tests/BasePCSuite.scala @@ -225,6 +225,7 @@ abstract class BasePCSuite extends BaseSuite with PCSuite { } object IgnoreScalaVersion { + def apply(version: String): IgnoreScalaVersion = { IgnoreScalaVersion(_ == version) } diff --git a/tests/cross/src/test/scala/tests/pc/CompletionExtensionMethodSuite.scala b/tests/cross/src/test/scala/tests/pc/CompletionExtensionMethodSuite.scala index 05e0c2ead35..e1413bf9430 100644 --- a/tests/cross/src/test/scala/tests/pc/CompletionExtensionMethodSuite.scala +++ b/tests/cross/src/test/scala/tests/pc/CompletionExtensionMethodSuite.scala @@ -22,7 +22,7 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { ) check( - "simple-old-syntax", + "simple-old-syntax".tag(IgnoreForScala3CompilerPC), """|package example | |object Test: @@ -51,7 +51,7 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { ) check( - "simple2-old-syntax", + "simple2-old-syntax".tag(IgnoreForScala3CompilerPC), """|package example | |object enrichments: @@ -81,7 +81,7 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { ) check( - "simple-empty-old", + "simple-empty-old".tag(IgnoreForScala3CompilerPC), """|package example | |object enrichments: @@ -113,7 +113,7 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { ) check( - "filter-by-type-old", + "filter-by-type-old".tag(IgnoreForScala3CompilerPC), """|package example | |object enrichments: @@ -125,7 +125,7 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { |def main = "foo".iden@@ |""".stripMargin, """|identity: String (implicit) - |""".stripMargin // incr won't be available + |""".stripMargin // identity2 won't be available ) @@ -148,7 +148,7 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { ) check( - "filter-by-type-subtype-old", + "filter-by-type-subtype-old".tag(IgnoreForScala3CompilerPC), """|package example | |class A @@ -188,7 +188,7 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { ) checkEdit( - "simple-edit-old", + "simple-edit-old".tag(IgnoreForScala3CompilerPC), """|package example | |object enrichments: @@ -262,11 +262,11 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { ) checkEdit( - "simple-edit-suffix-old", + "simple-edit-suffix-old".tag(IgnoreForScala3CompilerPC), """|package example | |object enrichments: - | implicit class A (num: Int): + | implicit class A (val num: Int): | def plus(other: Int): Int = num + other | |def main = 100.pl@@ @@ -276,7 +276,7 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { |import example.enrichments.A | |object enrichments: - | implicit class A (num: Int): + | implicit class A (val num: Int): | def plus(other: Int): Int = num + other | |def main = 100.plus($0) @@ -300,7 +300,10 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { ) check( - "directly-in-pkg1-old".tag(IgnoreScalaVersion.forLessThan("3.2.2")), + "directly-in-pkg1-old" + .tag( + IgnoreScalaVersion.forLessThan("3.2.2").and(IgnoreForScala3CompilerPC) + ), """| |package examples: | implicit class A(num: Int): @@ -328,7 +331,10 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { ) check( - "directly-in-pkg2-old".tag(IgnoreScalaVersion.forLessThan("3.2.2")), + "directly-in-pkg2-old" + .tag( + IgnoreScalaVersion.forLessThan("3.2.2").and(IgnoreForScala3CompilerPC) + ), """|package examples: | object X: | def fooBar(num: Int) = num + 1 @@ -359,7 +365,10 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { ) checkEdit( - "directly-in-pkg3-old".tag(IgnoreScalaVersion.forLessThan("3.2.2")), + "directly-in-pkg3-old" + .tag( + IgnoreScalaVersion.forLessThan("3.2.2").and(IgnoreForScala3CompilerPC) + ), """|package examples: | implicit class A (num: Int) { def incr: Int = num + 1 } | @@ -394,7 +403,10 @@ class CompletionExtensionMethodSuite extends BaseCompletionSuite { ) check( - "nested-pkg-old".tag(IgnoreScalaVersion.forLessThan("3.2.2")), + "nested-pkg-old" + .tag( + IgnoreScalaVersion.forLessThan("3.2.2").and(IgnoreForScala3CompilerPC) + ), """|package aa: // some comment | package cc: | implicit class A (num: Int): diff --git a/tests/unit/src/test/resources/expect/toplevels-scala3.expect b/tests/unit/src/test/resources/expect/toplevels-scala3.expect index 96c318a2e61..2d139454969 100644 --- a/tests/unit/src/test/resources/expect/toplevels-scala3.expect +++ b/tests/unit/src/test/resources/expect/toplevels-scala3.expect @@ -42,7 +42,6 @@ example/Scalalib.scala -> example/Scalalib# example/StructuralTypes.scala -> example/StructuralTypes. example/ToplevelDefVal.scala -> example/ToplevelDefVal$package. example/ToplevelImplicit.scala -> example/ToplevelImplicit$package. -example/ToplevelImplicit.scala -> example/ToplevelImplicit$package. example/ToplevelImplicit.scala -> example/ToplevelImplicit$package.Xtension# example/ToplevelImplicit.scala -> example/ToplevelImplicit$package.XtensionAnyVal# example/TryCatch.scala -> example/TryCatch# From 6473e3864cf5c149ead68fe8baee92a1bfd2b74f Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Tue, 19 Dec 2023 15:28:54 +0100 Subject: [PATCH 14/21] chore: Add missing tests to TestGroups --- project/TestGroups.scala | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/project/TestGroups.scala b/project/TestGroups.scala index e7a6c15fdab..f4db472b250 100644 --- a/project/TestGroups.scala +++ b/project/TestGroups.scala @@ -46,7 +46,10 @@ object TestGroups { "tests.DidFocusWhileCompilingLspSuite", "tests.BloopJavaHomeLspSuite", "tests.testProvider.ScalatestFinderSuite", "tests.ScalaCliSuite", "tests.FoldingRangeScala3LineFolingOnlySuite", - "tests.debug.BreakpointScalaCliDapSuite", "tests.CallHierarchyLspSuite"), + "tests.debug.BreakpointScalaCliDapSuite", "tests.CallHierarchyLspSuite", + "tests.BspStatusSuite", "tests.ServerLivenessMonitorLspSuite", + "tests.ToplevelWithInnerScala2Suite", "tests.ScaladocSymbolsSuite", + "tests.SkipCommentsSuite", "tests.JarSourcesProviderSuite"), Set("tests.AmmoniteSuite", "tests.debug.BreakpointDapSuite", "tests.OnTypeFormattingSuite", "tests.ReferenceLspSuite", "tests.SuperMethodLspSuite", "tests.SyntaxErrorLspSuite", @@ -115,7 +118,11 @@ object TestGroups { "tests.RunProviderLensLspSuite", "tests.SemanticTokensLspSuite", "tests.ToplevelsScala3Suite", "tests.codeactions.InlineValueLspSuite", "tests.JavaToplevelSuite", "tests.ToplevelLibrarySuite", - "tests.FoldingRangeScala3LineFoldingOnlySuite"), + "tests.FoldingRangeScala3LineFoldingOnlySuite", "tests.LogBackupSuite", + "tests.decorations.SyntheticDecorationsExpectSuite", + "tests.codeactions.ConvertSingleLineCommentLspSuite", + "tests.ServerLivenessMonitorSuite", "tests.ResetWorkspaceLspSuite", + "tests.ToplevelWithInnerScala3Suite"), ) } From d596f6499fd52a3426f3181a5b4fe560d13ac2bf Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Tue, 19 Dec 2023 16:45:53 +0100 Subject: [PATCH 15/21] bugfix: Retry check for the latest MEtals version Fixes https://github.com/scalameta/metals/issues/5952 --- metals-docs/src/main/scala/docs/Snapshot.scala | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/metals-docs/src/main/scala/docs/Snapshot.scala b/metals-docs/src/main/scala/docs/Snapshot.scala index 555a7fa9042..cea96fcdd8f 100644 --- a/metals-docs/src/main/scala/docs/Snapshot.scala +++ b/metals-docs/src/main/scala/docs/Snapshot.scala @@ -25,11 +25,17 @@ object Snapshot { private implicit val localDateTimeOrdering: Ordering[LocalDateTime] = Ordering.fromLessThan[LocalDateTime]((a, b) => a.compareTo(b) < 0) - def latest(repo: String, binaryVersion: String): Snapshot = { + def latest(repo: String, binaryVersion: String, retry: Int = 5): Snapshot = { if (System.getenv("CI") != null) { try { fetchLatest(repo, binaryVersion) } catch { + case NonFatal(e) if retry > 0 => + scribe.error( + "unexpected error fetching SNAPSHOT version, retrying...", + e, + ) + latest(repo, binaryVersion, retry - 1) case NonFatal(e) => scribe.error("unexpected error fetching SNAPSHOT version", e) current From 89540f8a738260b8090690d428972e50955e1a1e Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Wed, 20 Dec 2023 15:08:43 +0100 Subject: [PATCH 16/21] chore: delete remote language server --- build.sbt | 2 +- docs/integrations/remote-language-server.md | 169 ------------------ .../internal/metals/DefinitionProvider.scala | 15 +- .../internal/metals/MetalsLspService.scala | 10 -- .../internal/metals/MetalsServerConfig.scala | 5 - .../internal/metals/ReferenceProvider.scala | 13 +- .../internal/metals/UserConfiguration.scala | 15 -- .../remotels/RemoteLanguageServer.scala | 136 -------------- .../remotels/RemoteLocationResult.scala | 7 - project/TestGroups.scala | 1 - .../RemoteLanguageServerLspSuite.scala | 39 ---- .../TestingRemoteLanguageServer.scala | 70 -------- website/sidebars.json | 1 - 13 files changed, 5 insertions(+), 478 deletions(-) delete mode 100644 docs/integrations/remote-language-server.md delete mode 100644 metals/src/main/scala/scala/meta/internal/remotels/RemoteLanguageServer.scala delete mode 100644 metals/src/main/scala/scala/meta/internal/remotels/RemoteLocationResult.scala delete mode 100644 tests/unit/src/test/scala/tests/remotels/RemoteLanguageServerLspSuite.scala delete mode 100644 tests/unit/src/test/scala/tests/remotels/TestingRemoteLanguageServer.scala diff --git a/build.sbt b/build.sbt index 20261f222d8..8f0b4fc5c87 100644 --- a/build.sbt +++ b/build.sbt @@ -501,7 +501,7 @@ lazy val metals = project "com.outr" %% "scribe-slf4j" % V.scribe, // needed for flyway database migrations // for JSON formatted doctor "com.lihaoyi" %% "ujson" % "3.1.3", - // For remote language server + // For fetching projects' templates "com.lihaoyi" %% "requests" % "0.8.0", // for producing SemanticDB from Scala source files, to be sure we want the same version of scalameta "org.scalameta" %% "scalameta" % V.semanticdb(scalaVersion.value), diff --git a/docs/integrations/remote-language-server.md b/docs/integrations/remote-language-server.md deleted file mode 100644 index 457ef037d53..00000000000 --- a/docs/integrations/remote-language-server.md +++ /dev/null @@ -1,169 +0,0 @@ ---- -id: remote-language-server -title: Remote Language Servers ---- - -Metals has experimental support to offload certain requests to a remote language -server. This feature can be used to navigate large codebases when it's not -possible to index all the code on a local computer. - -## Difference from local language servers - -There are some important differences between local and remote language servers: - -- Instead of JSON-RPC, a remote language server responds to HTTP POST requests - with an `application/json` header and a JSON-formatted body. The reason HTTP - is chosen over JSON-RPC is because it makes the remote language server - accessible from more clients, for example via `curl`. A caveat with using HTTP - instead of JSON-RPC is that it's not possible for the remote language server - to push notification down to the client. In the future, we could consider - using JSON-RPC via websockets instead. -- Instead of using absolute `file://` URIs, a remote language server uses - relative `source://` URIs. For example, the absolute URI - `file://path/to/workspace/src/main/scala/Address.scala` becomes the relative - URI `source://src/main/scala/Address.scala` when communicating with a remote - language server. - -## Methods - -Each remote language server method expects a JSON-formatted body of type -`JsonRpcRequest`. - -```ts -interface JsonRpcRequest { - /** The JSON-RPC method name, for example textDocument/definition */ - method: string; - - /** The parameter for the JSON-RPC method, for example `TextDocumentPositionParams` */ - params: T; - - /** The ID of this request, can be any integer number. */ - id: number; -} -``` - -### `textDocument/definition` - -The `textDocument/definition` request is sent from the client to the server to -get the list of definitions for a given position. - -_Request_: - -- method: `textDocument/definition` -- params: `JsonRpcRequest`, where - `TextDocumentPositionParams` is defined in LSP. - -_Response_: - -- result: `Location[]`, as defined in LSP. - -_Example request_: - -```sh -curl --location --request POST 'http://remote-language-server.com' \ ---header 'Content-Type: application/json' \ ---data-raw '{ - "method": "textDocument/definition", - "params": { - "textDocument": { - "uri": "source://src/main/scala/Address.scala" - }, - "position": { - "line": 5, - "character": 10 - } - }, - "id": 10 -}' -``` - -_Example response_: - -```json -[ - { - "uri": "source://src/main/scala/User.scala", - "range": { - "start": { "line": 61, "character": 15 }, - "end": { "line": 61, "character": 31 } - } - } -] -``` - -### `textDocument/references` - -The `textDocument/references` request is sent from the client to the server to -get the list of all references to a symbol at a given position. - -_Request_: - -- method: `textDocument/references` -- params: `JsonRpcRequest`, where `ReferenceParams` is defined - in LSP. - -_Response_: - -- result: `Location[]`, as defined in LSP. - -_Example request_: - -```sh -curl --location --request POST 'http://remote-language-server.com' \ ---header 'Content-Type: application/json' \ ---data-raw '{ - "method": "textDocument/references", - "params": { - "textDocument": { - "uri": "source://src/main/scala/Address.scala" - }, - "position": { - "line": 5, - "character": 10 - }, - "context": { - "includeDeclaration": true - } - }, - "id": 10 -}' -``` - -_Example response_: - -```json -[ - { - "uri": "source://src/main/scala/User.scala", - "range": { - "start": { "line": 61, "character": 15 }, - "end": { "line": 61, "character": 31 } - } - }, - { - "uri": "source://src/main/scala/Country.scala", - "range": { - "start": { "line": 62, "character": 16 }, - "end": { "line": 62, "character": 32 } - } - } -] -``` - -## Open questions - -The protocol for remote language servers is new and likely to have breaking -changes in upcoming releases. The protocol in this document should be considered -as a proof-of-concept that demonstrates the feasibility of this approach. There -remain a few open questions in order to extend remote language servers to -support richer functionality: - -- how do we ensure that results from the remote server are synchronized with the - file changes to the local disk? -- how do we combine local and remote `workspace/symbol` results? -- how should `textDocument/definition` return results to library dependency - sources that are not present on local disk? - -Given these open questions and the experimental status of remote language -servers, this functionality may be removed from Metals in future releases -without notice. diff --git a/metals/src/main/scala/scala/meta/internal/metals/DefinitionProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/DefinitionProvider.scala index 1569101fc1a..f7ed20e9b29 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/DefinitionProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/DefinitionProvider.scala @@ -16,7 +16,6 @@ import scala.meta.internal.mtags.Symbol import scala.meta.internal.mtags.SymbolDefinition import scala.meta.internal.parsing.TokenEditDistance import scala.meta.internal.parsing.Trees -import scala.meta.internal.remotels.RemoteLanguageServer import scala.meta.internal.semanticdb import scala.meta.internal.semanticdb.IdTree import scala.meta.internal.semanticdb.OriginalTree @@ -59,7 +58,6 @@ final class DefinitionProvider( semanticdbs: Semanticdbs, warnings: Warnings, compilers: () => Compilers, - remote: RemoteLanguageServer, trees: Trees, buildTargets: BuildTargets, scalaVersionSelector: ScalaVersionSelector, @@ -96,22 +94,15 @@ final class DefinitionProvider( case _ => DefinitionResult.empty } - val fromIndex = - if (fromSnapshot.isEmpty && remote.isEnabledForPath(path)) { - remote.definition(params).map(_.getOrElse(fromSnapshot)) - } else { - Future.successful(fromSnapshot) - } - val fromCompilerOrSemanticdb = fromIndex.flatMap { result => - if (result.isEmpty && path.isScalaFilename) { + val fromCompilerOrSemanticdb = + if (fromSnapshot.isEmpty && path.isScalaFilename) { compilers().definition(params, token) } else { if (fromSemanticdb.isEmpty) { warnings.noSemanticdb(path) } - Future.successful(result) + Future.successful(fromSnapshot) } - } fromCompilerOrSemanticdb.map { definition => if (definition.isEmpty && !definition.symbol.endsWith("/")) { 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 4f8b8286fc5..73e3cee60e6 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -78,7 +78,6 @@ import scala.meta.internal.parsing.DocumentSymbolProvider import scala.meta.internal.parsing.FoldingRangeProvider import scala.meta.internal.parsing.TokenEditDistance import scala.meta.internal.parsing.Trees -import scala.meta.internal.remotels.RemoteLanguageServer import scala.meta.internal.rename.RenameProvider import scala.meta.internal.semver.SemVer import scala.meta.internal.tvp._ @@ -238,13 +237,6 @@ class MetalsLspService( () => userConfig, buildTargets, ) - private val remote = new RemoteLanguageServer( - () => folder, - () => userConfig, - initialServerConfig, - buffers, - buildTargets, - ) val compilations: Compilations = new Compilations( buildTargets, @@ -472,7 +464,6 @@ class MetalsLspService( semanticdbs, warnings, () => compilers, - remote, trees, buildTargets, scalaVersionSelector, @@ -557,7 +548,6 @@ class MetalsLspService( semanticdbs, buffers, definitionProvider, - remote, trees, buildTargets, ) diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsServerConfig.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsServerConfig.scala index 094132cbb17..5da255d443a 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsServerConfig.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsServerConfig.scala @@ -27,7 +27,6 @@ import scala.meta.pc.PresentationCompilerConfig.OverrideDefFormat * dialogues that don't implement window/showMessageRequest yet. * @param isInputBoxEnabled whether the client supports the `metals/inputBox` extension. * @param isVerbose turn on verbose logging. - * @param remoteTimeout timeout period for retrieving references while using `RemoteLanguageServer`. * @param openFilesOnRenames whether or not file should be opened when a rename occurs * in an unopened file. * @param renameFileThreshold amount of files that should be opened during a rename @@ -74,10 +73,6 @@ final case class MetalsServerConfig( "metals.verbose", default = false, ), - remoteTimeout: String = System.getProperty( - "metals.timeout", - "1 minute", - ), openFilesOnRenames: Boolean = false, renameFileThreshold: Int = 300, askToReconnect: Boolean = MetalsServerConfig.binaryOption( diff --git a/metals/src/main/scala/scala/meta/internal/metals/ReferenceProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/ReferenceProvider.scala index f90d7b78b2e..2b5f794d65e 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ReferenceProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/ReferenceProvider.scala @@ -17,7 +17,6 @@ import scala.meta.internal.mtags.Semanticdbs import scala.meta.internal.mtags.Symbol import scala.meta.internal.parsing.TokenEditDistance import scala.meta.internal.parsing.Trees -import scala.meta.internal.remotels.RemoteLanguageServer import scala.meta.internal.semanticdb.Scala._ import scala.meta.internal.semanticdb.SymbolInformation import scala.meta.internal.semanticdb.SymbolOccurrence @@ -38,7 +37,6 @@ final class ReferenceProvider( semanticdbs: Semanticdbs, buffers: Buffers, definition: DefinitionProvider, - remote: RemoteLanguageServer, trees: Trees, buildTargets: BuildTargets, ) extends SemanticdbFeatureProvider { @@ -168,16 +166,7 @@ final class ReferenceProvider( ) ReferencesResult(occurrence.symbol, locations) } - case None => - scribe.debug(s"No semanticdb for $source") - // NOTE(olafur): we block here instead of returning a Future because it - // requires a significant refactoring to make the reference provider and - // its dependencies (including rename provider) asynchronous. The remote - // language server returns `Future.successful(None)` when it's disabled - // so this isn't even blocking for normal usage of Metals. - List( - remote.referencesBlocking(params).getOrElse(ReferencesResult.empty) - ) + case None => Nil } } diff --git a/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala b/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala index a0dbcfba8a4..48151de1926 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala @@ -44,7 +44,6 @@ case class UserConfiguration( showInferredType: Option[String] = None, showImplicitArguments: Boolean = false, showImplicitConversionsAndClasses: Boolean = false, - remoteLanguageServer: Option[String] = None, enableStripMarginOnTypeFormatting: Boolean = true, enableIndentOnPaste: Boolean = false, enableSemanticHighlighting: Boolean = true, @@ -274,17 +273,6 @@ object UserConfiguration { |try to adjust the indentation to that of the current cursor. |""".stripMargin, ), - UserConfigurationOption( - "remote-language-server", - """empty string `""`.""", - """"https://language-server.company.com/message"""", - "Remote language server", - """A URL pointing to an endpoint that implements a remote language server. - | - |See https://scalameta.org/metals/docs/integrations/remote-language-server for - |documentation on remote language servers. - |""".stripMargin, - ), UserConfigurationOption( "fallback-scala-version", BuildInfo.scala3, @@ -527,8 +515,6 @@ object UserConfiguration { getBooleanKey("show-implicit-arguments").getOrElse(false) val showImplicitConversionsAndClasses = getBooleanKey("show-implicit-conversions-and-classes").getOrElse(false) - val remoteLanguageServer = - getStringKey("remote-language-server") val enableStripMarginOnTypeFormatting = getBooleanKey("enable-strip-margin-on-type-formatting").getOrElse(true) val enableIndentOnPaste = @@ -588,7 +574,6 @@ object UserConfiguration { showInferredType, showImplicitArguments, showImplicitConversionsAndClasses, - remoteLanguageServer, enableStripMarginOnTypeFormatting, enableIndentOnPaste, enableSemanticHighlighting, diff --git a/metals/src/main/scala/scala/meta/internal/remotels/RemoteLanguageServer.scala b/metals/src/main/scala/scala/meta/internal/remotels/RemoteLanguageServer.scala deleted file mode 100644 index a54712498e2..00000000000 --- a/metals/src/main/scala/scala/meta/internal/remotels/RemoteLanguageServer.scala +++ /dev/null @@ -1,136 +0,0 @@ -package scala.meta.internal.remotels - -import java.{util => ju} - -import scala.concurrent.Await -import scala.concurrent.ExecutionContext -import scala.concurrent.Future -import scala.concurrent.duration.Duration -import scala.util.Try - -import scala.meta.internal.metals.Buffers -import scala.meta.internal.metals.BuildTargets -import scala.meta.internal.metals.DefinitionResult -import scala.meta.internal.metals.JsonParser._ -import scala.meta.internal.metals.MetalsEnrichments._ -import scala.meta.internal.metals.MetalsServerConfig -import scala.meta.internal.metals.ReferencesResult -import scala.meta.internal.metals.UserConfiguration -import scala.meta.internal.mtags.MD5 -import scala.meta.internal.semanticdb.Scala.Symbols -import scala.meta.io.AbsolutePath - -import com.google.gson.JsonObject -import com.google.gson.JsonPrimitive -import org.eclipse.lsp4j.Location -import org.eclipse.{lsp4j => l} - -class RemoteLanguageServer( - workspace: () => AbsolutePath, - userConfig: () => UserConfiguration, - serverConfig: MetalsServerConfig, - buffers: Buffers, - buildTargets: BuildTargets, -)(implicit ec: ExecutionContext) { - val timeout: Duration = Duration(serverConfig.remoteTimeout) - def isEnabledForPath(path: AbsolutePath): Boolean = - userConfig().remoteLanguageServer.isDefined && - buildTargets.inverseSources(path).isEmpty - - def referencesBlocking( - params: l.ReferenceParams - ): Option[ReferencesResult] = { - // NOTE(olafur): we provide this blocking implementation because it requires - // a significant refactoring to make the reference provider and its - // dependencies (including rename provider) asynchronous. The remote - // language server returns `Future.successful(None)` when it's disabled so - // this isn't even blocking for normal usage of Metals. - Await.result(references(params), timeout) - } - def references( - params: l.ReferenceParams - ): Future[Option[ReferencesResult]] = - blockingRequest { url => - for { - locations <- postLocationRequest( - url, - params.toJsonObject, - "textDocument/references", - ) - } yield ReferencesResult(Symbols.None, locations.asScala.toSeq) - } - - def definition( - params: l.TextDocumentPositionParams - ): Future[Option[DefinitionResult]] = - blockingRequest { url => - for { - locations <- postLocationRequest( - url, - params.toJsonObject, - "textDocument/definition", - ) - } yield DefinitionResult(locations, Symbols.None, None, None) - } - - private def blockingRequest[T](fn: String => Option[T]): Future[Option[T]] = { - userConfig().remoteLanguageServer match { - case Some(url) => Future(concurrent.blocking(fn(url))) - case None => Future.successful(None) - } - } - - private def postLocationRequest( - url: String, - params: JsonObject, - method: String, - ): Option[ju.List[Location]] = { - val maybeResponse = Try( - requests.post( - url, - data = asRemoteParameters(params, method).toString(), - headers = List("Content-Type" -> "application/json"), - ) - ) - maybeResponse.toEither.left.foreach { error => - scribe.error(s"remote: request failed '${error.getMessage()}'") - } - for { - response <- maybeResponse.toOption - if response.statusCode == 200 - result <- response.text().parseJson.as[RemoteLocationResult].toOption - locations <- Option(result.result) - } yield { - locations.forEach { location => - val relativeUri = location.getUri().stripPrefix("source://") - val absoluteUri = workspace().resolve(relativeUri).toURI.toString - location.setUri(absoluteUri) - } - locations - } - } - - /** - * Creates a JSON request according to https://scalameta.org/metals/docs/contributors/remote-language-server.html - */ - private def asRemoteParameters( - params: JsonObject, - method: String, - ): JsonObject = { - val textDocument = params.get("textDocument").getAsJsonObject() - val absolutePath = textDocument.get("uri").getAsString.toAbsolutePath - val relativeUri = - absolutePath.toRelative(workspace()).toURI(isDirectory = false) - val md5 = MD5.compute(buffers.get(absolutePath).getOrElse("")) - - textDocument.add("uri", new JsonPrimitive(s"source://$relativeUri")) - textDocument.add("md5", new JsonPrimitive(md5)) - - val result = new JsonObject() - result.add("method", new JsonPrimitive(method)) - result.add("params", params) - result.add("id", new JsonPrimitive(10)) - result - } - -} diff --git a/metals/src/main/scala/scala/meta/internal/remotels/RemoteLocationResult.scala b/metals/src/main/scala/scala/meta/internal/remotels/RemoteLocationResult.scala deleted file mode 100644 index 1379441428a..00000000000 --- a/metals/src/main/scala/scala/meta/internal/remotels/RemoteLocationResult.scala +++ /dev/null @@ -1,7 +0,0 @@ -package scala.meta.internal.remotels - -import java.{util => ju} - -import org.eclipse.{lsp4j => l} - -class RemoteLocationResult(val result: ju.List[l.Location]) diff --git a/project/TestGroups.scala b/project/TestGroups.scala index f4db472b250..8419cb3e683 100644 --- a/project/TestGroups.scala +++ b/project/TestGroups.scala @@ -19,7 +19,6 @@ object TestGroups { "tests.DidFocusLspSuite", "tests.BuildServerConnectionLspSuite", "tests.BuildTargetsLspSuite", "tests.FileWatcherLspSuite", "tests.CurrentProjectCompileLspSuite", - "tests.remotels.RemoteLanguageServerLspSuite", "tests.WindowStateDidChangeLspSuite", "tests.DocumentSymbolLspSuite", "tests.WorkspaceSymbolExpectSuite", "tests.digest.DigestsSuite", "tests.MtagsSuite", "tests.ChosenBuildServerSuite", diff --git a/tests/unit/src/test/scala/tests/remotels/RemoteLanguageServerLspSuite.scala b/tests/unit/src/test/scala/tests/remotels/RemoteLanguageServerLspSuite.scala deleted file mode 100644 index 1868dbaae7b..00000000000 --- a/tests/unit/src/test/scala/tests/remotels/RemoteLanguageServerLspSuite.scala +++ /dev/null @@ -1,39 +0,0 @@ -package tests.remotels - -import scala.meta.internal.metals.UserConfiguration - -class RemoteLanguageServerLspSuite extends tests.BaseLspSuite("remotels") { - - private val remote = TestingRemoteLanguageServer() - - override def beforeAll(): Unit = remote.start() - override def afterAll(): Unit = remote.stop() - override def userConfig: UserConfiguration = - super.userConfig.copy(remoteLanguageServer = Some(remote.address)) - - test("basic") { - for { - _ <- initialize( - """|/metals.json - |{ - | "a": { } - |} - |/Foo.scala - |object Foo { - | val x = 42 - |} - |""".stripMargin - ) - _ <- server.didOpen("Foo.scala") - _ = assertNoDiff( - server.workspaceDefinitions, - """|/Foo.scala - |object Foo/*L0*/ { - | val x/*L1*/ = 42 - |} - |""".stripMargin, - ) - } yield () - } - -} diff --git a/tests/unit/src/test/scala/tests/remotels/TestingRemoteLanguageServer.scala b/tests/unit/src/test/scala/tests/remotels/TestingRemoteLanguageServer.scala deleted file mode 100644 index b6c39c989d5..00000000000 --- a/tests/unit/src/test/scala/tests/remotels/TestingRemoteLanguageServer.scala +++ /dev/null @@ -1,70 +0,0 @@ -package tests.remotels - -import java.nio.charset.StandardCharsets -import java.{util => ju} - -import scala.util.control.NonFatal - -import scala.meta.internal.io.InputStreamIO -import scala.meta.internal.metals.JsonParser._ -import scala.meta.internal.metals.MetalsHttpServer -import scala.meta.internal.remotels.RemoteLocationResult - -import io.undertow.Handlers.path -import io.undertow.Undertow -import org.eclipse.lsp4j.TextDocumentPositionParams -import org.eclipse.{lsp4j => l} - -class TestingRemoteLanguageServer(server: Undertow) { - def address: String = { - MetalsHttpServer.address(server) + "/message" - } - def start(): Unit = { - server.start() - } - def stop(): Unit = { - server.stop() - } -} -object TestingRemoteLanguageServer { - def apply(): TestingRemoteLanguageServer = { - val host = "localhost" - val port = MetalsHttpServer.freePort(host, 9876) - val server = Undertow - .builder() - .addHttpListener(port, host) - .setHandler( - path().addExactPath( - "/message", - MetalsHttpServer.textHandler( - "application/json", - e => { - try { - val request = new String( - InputStreamIO.readBytes(e.getInputStream()), - StandardCharsets.UTF_8, - ).parseJson.toJsonObject - val params = - request.get("params").as[TextDocumentPositionParams].get - val response = new RemoteLocationResult( - ju.Collections.singletonList( - new l.Location( - params.getTextDocument().getUri(), - new l.Range(params.getPosition(), params.getPosition()), - ) - ) - ) - response.toJson.toString() - } catch { - case NonFatal(e) => - scribe.error(e) - "" - } - }, - ), - ) - ) - .build() - new TestingRemoteLanguageServer(server) - } -} diff --git a/website/sidebars.json b/website/sidebars.json index cef3b90c889..df1f54f81b6 100644 --- a/website/sidebars.json +++ b/website/sidebars.json @@ -28,7 +28,6 @@ "contributors/releasing" ], "Integrating with Metals": [ - "integrations/remote-language-server", "integrations/new-build-tool", "integrations/new-editor", "integrations/test-explorer", From 6b9babd60ebd641d1358e4456453861f615ee165 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Thu, 21 Dec 2023 11:02:56 +0100 Subject: [PATCH 17/21] improvement: Only log if failed to resolve mtags at all Previously, we would log an error and then try to download stable presentation compiler, which could be misleading. Now we only print an error if we fail to resolve any mtags. --- .../meta/internal/metals/MtagsResolver.scala | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/MtagsResolver.scala b/metals/src/main/scala/scala/meta/internal/metals/MtagsResolver.scala index cc27321116d..03e874620f3 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MtagsResolver.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MtagsResolver.scala @@ -100,17 +100,6 @@ object MtagsResolver { original: Option[String], resolveType: ResolveType.Value = ResolveType.Regular, ): Option[MtagsBinaries] = { - def logError(e: Throwable): Unit = { - val msg = s"Failed to fetch mtags for ${scalaVersion}" - e match { - case _: SimpleResolutionError => - // no need to log traces for coursier error - // all explanation is in message - scribe.error(msg + "\n" + e.getMessage()) - case _ => - scribe.error(msg, e) - } - } def fetch(tries: Int = 0): State = logResolution { try { @@ -138,8 +127,7 @@ object MtagsResolver { ) } catch { case NonFatal(e) => - logError(e) - State.Failure(System.currentTimeMillis(), tries) + State.Failure(System.currentTimeMillis(), tries, e) } } def shouldResolveAgain(failure: State.Failure): Boolean = { @@ -196,6 +184,18 @@ object MtagsResolver { }, ) + def logError(e: Throwable): Unit = { + val msg = s"Failed to fetch mtags for ${scalaVersion}" + e match { + case _: SimpleResolutionError => + // no need to log traces for coursier error + // all explanation is in message + scribe.error(msg + "\n" + e.getMessage()) + case _ => + scribe.error(msg, e) + } + } + computed match { case State.Success(v) => Some(v) @@ -226,8 +226,10 @@ object MtagsResolver { ResolveType.Nightly, ) } - case _ => + case failure: State.Failure => + logError(failure.exception) None + case _ => None } } } @@ -291,7 +293,8 @@ object MtagsResolver { object State { val maxTriesInARow: Int = 2 case class Success(v: MtagsBinaries.Artifacts) extends State - case class Failure(lastTryMillis: Long, tries: Int) extends State + case class Failure(lastTryMillis: Long, tries: Int, exception: Throwable) + extends State } } From 1d0b9ba9de9953948f9f9016410506070e475065 Mon Sep 17 00:00:00 2001 From: Jakub Ciesluk <323892@uwr.edu.pl> Date: Thu, 21 Dec 2023 13:57:03 +0100 Subject: [PATCH 18/21] chore: Remove Pants specific checks Since https://github.com/scalameta/metals/pull/5791 Pants can be used as custom BSP. This should be more reliable, since most of the support was alredy removed. --- .../meta/internal/builds/BuildTools.scala | 5 ----- .../scala/meta/internal/builds/Digest.scala | 2 -- .../internal/metals/MetalsLspService.scala | 4 +--- .../mtags/ScalametaCommonEnrichments.scala | 2 -- project/TestGroups.scala | 22 ++++++++----------- 5 files changed, 10 insertions(+), 25 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala b/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala index bc36c2fdc34..ee4c8759cc3 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala @@ -114,10 +114,6 @@ final class BuildTools( _.resolve("pom.xml").isFile ) def isMaven: Boolean = mavenProject.isDefined - def pantsProject: Option[AbsolutePath] = searchForBuildTool( - _.resolve("pants.ini").isFile - ) - def isPants: Boolean = pantsProject.isDefined def bazelProject: Option[AbsolutePath] = searchForBuildTool( _.resolve("WORKSPACE").isFile ) @@ -190,7 +186,6 @@ final class BuildTools( if (isMill) buf += "Mill" if (isGradle) buf += "Gradle" if (isMaven) buf += "Maven" - if (isPants) buf += "Pants" if (isBazel) buf += "Bazel" buf.result() } diff --git a/metals/src/main/scala/scala/meta/internal/builds/Digest.scala b/metals/src/main/scala/scala/meta/internal/builds/Digest.scala index bb2fc11a5b7..1808a837f45 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/Digest.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/Digest.scala @@ -101,8 +101,6 @@ object Digest { digestGeneralJvm(path, digest) } else if (isXml) { digestXml(path, digest) - } else if (path.isBuild) { - digestFileBytes(path, digest) } else { true } 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 73e3cee60e6..df96d6b5986 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -1322,7 +1322,7 @@ class MetalsLspService( */ private def fileWatchFilter(path: Path): Boolean = { val abs = AbsolutePath(path) - abs.isScalaOrJava || abs.isSemanticdb || abs.isBuild || + abs.isScalaOrJava || abs.isSemanticdb || abs.isInBspDirectory(folder) } @@ -1366,8 +1366,6 @@ class MetalsLspService( semanticDBIndexer.onOverflow(semanticdbPath) } }.asJava - } else if (path.isBuild) { - onBuildChanged(List(path)).asJava } else { CompletableFuture.completedFuture(()) } diff --git a/mtags/src/main/scala/scala/meta/internal/mtags/ScalametaCommonEnrichments.scala b/mtags/src/main/scala/scala/meta/internal/mtags/ScalametaCommonEnrichments.scala index af5ee1d9eec..0ae0c88e8a2 100644 --- a/mtags/src/main/scala/scala/meta/internal/mtags/ScalametaCommonEnrichments.scala +++ b/mtags/src/main/scala/scala/meta/internal/mtags/ScalametaCommonEnrichments.scala @@ -344,8 +344,6 @@ trait ScalametaCommonEnrichments extends CommonMtagsEnrichments { case None => path.toURI.toString } - def isBuild: Boolean = - path.filename.startsWith("BUILD") def isInBspDirectory(workspace: AbsolutePath): Boolean = path.toNIO.startsWith(workspace.resolve(".bsp").toNIO) diff --git a/project/TestGroups.scala b/project/TestGroups.scala index 8419cb3e683..c9ca1eea37b 100644 --- a/project/TestGroups.scala +++ b/project/TestGroups.scala @@ -24,8 +24,7 @@ object TestGroups { "tests.MtagsSuite", "tests.ChosenBuildServerSuite", "tests.SemanticdbSuite", "tests.digest.MillDigestSuite", "tests.DocumentSymbolSuite", "tests.FoldingRangeSuite", - "tests.pants.PantsSuite", "tests.JavadocSuite", - "tests.MtagsEnrichmentsSuite", "tests.FuzzySuite", + "tests.JavadocSuite", "tests.MtagsEnrichmentsSuite", "tests.CreateDirectoriesSuite", "tests.SbtOptsSuite", "tests.LineListenerSuite", "tests.debug.MessageIdAdapterSuite", "tests.MetalsEnrichmentsSuite", "tests.DefinitionDirectorySuite", @@ -36,7 +35,7 @@ object TestGroups { "tests.codeactions.RewriteBracesParensLspSuite", "tests.debug.CompletionDapSuite", "tests.debug.EvaluationDapSuite", "tests.FindTextInDependencyJarsSuite", "tests.TestSuitesProviderSuite", - "tests.classFinder.FindAllClassesSuite", + "tests.classFinder.FindAllClassesSuite", "tests.FuzzySuite", "tests.codeactions.ExtractValueLspSuite", "tests.clients.CommandSuite", "tests.JavaInteractiveSemanticdbSuite", "tests.SemVerSuite", "tests.codeactions.CompanionObjectSuite", @@ -55,9 +54,8 @@ object TestGroups { "tests.codeactions.StringActionsLspSuite", "tests.RangeFormattingSuite", "tests.CodeLensesLspSuite", "tests.DefinitionLspSuite", "tests.RelatedSuite", "tests.DocumentHighlightLspSuite", - "tests.NewProjectLspSuite", + "tests.NewProjectLspSuite", "tests.WorkspaceSymbolLspSuite", "tests.codeactions.ImplementAbstractMembersLspSuite", - "tests.WorkspaceSymbolLspSuite", "tests.RangeFormattingWhenSelectingSuite", "tests.AddPackageLspSuite", "tests.WarningsLspSuite", "tests.BspSwitchLspSuite", "tests.codeactions.CreateNewSymbolLspSuite", @@ -70,13 +68,12 @@ object TestGroups { "tests.JdkSourcesSuite", "tests.digest.SbtDigestSuite", "tests.digest.MavenDigestSuite", "tests.UserConfigurationSuite", "tests.digest.GradleDigestSuite", "tests.DetectionSuite", - "tests.digest.PantsDigestSuite", "tests.NewFileTemplateSuite", - "tests.ScalaVersionsSuite", "tests.HttpServerSuite", - "tests.BatchedFunctionSuite", "tests.SbtVersionSuite", - "tests.MessagesSuite", "tests.pants.PantsGlobsSuite", - "tests.SelectBspServerSuite", "tests.InverseDependenciesSuite", - "tests.pantsbuild.PantsProjectNameSuite", "tests.TrigramSubstringsSuite", - "tests.TimerSuite", "tests.FoldingRangesSuite", + "tests.NewFileTemplateSuite", "tests.ScalaVersionsSuite", + "tests.HttpServerSuite", "tests.BatchedFunctionSuite", + "tests.SbtVersionSuite", "tests.MessagesSuite", + "tests.TrigramSubstringsSuite", "tests.SelectBspServerSuite", + "tests.InverseDependenciesSuite", "tests.TimerSuite", + "tests.FoldingRangesSuite", "tests.SelectionRangeLspSuite", "tests.rangeFormatting.MultilineStringRangeFormattingWhenPastingSuite", "tests.codeactions.ConvertPatternMatchLspSuite", "tests.ScalaToplevelLibrarySuite", @@ -84,7 +81,6 @@ object TestGroups { "tests.rangeFormatting.MultilineStringRangeFormattingWhenSelectingSuite", "tests.rangeFormatting.IndentWhenPastingSuite", "tests.DebugDiscoverySuite", "tests.SemanticdbScala2Suite", - "tests.SelectionRangeLspSuite", "tests.codeactions.OrganizeImportsLspSuite", "tests.codeactions.InsertInferredTypeLspSuite", "tests.FoldingRangeScala2Suite", "tests.SystemProcessSuite", From a7f95d700da2d53d17e23f5b52193a8a0e40bc0f Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Thu, 16 Nov 2023 10:35:24 +0100 Subject: [PATCH 19/21] bugfix: Fix java support for different files Previously, we wouldn't include the classpath, so completions/hover would only work locally. Now, we add the compiled classpath, so things should work as expected. Fixes https://github.com/scalameta/metals/issues/5829 --- .../metals/JavaInteractiveSemanticdb.scala | 2 +- .../parsing/JavaFoldingRangeExtractor.scala | 2 +- .../internal/pc/JavaCompletionProvider.scala | 2 +- .../meta/internal/pc/JavaHoverProvider.scala | 2 +- .../meta/internal/pc/JavaMetalsGlobal.scala | 21 ++++++++--- .../pc/JavaPresentationCompiler.scala | 5 +-- .../feature/CompletionCrossLspSuite.scala | 2 +- .../src/main/scala/tests/TestingServer.scala | 2 +- .../test/scala/tests/CompletionLspSuite.scala | 37 +++++++++++++++++++ 9 files changed, 61 insertions(+), 14 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/metals/JavaInteractiveSemanticdb.scala b/metals/src/main/scala/scala/meta/internal/metals/JavaInteractiveSemanticdb.scala index bc1d4f50896..a3fc8b80f25 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/JavaInteractiveSemanticdb.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/JavaInteractiveSemanticdb.scala @@ -72,7 +72,7 @@ class JavaInteractiveSemanticdb( // so can't use Metals SourceJavaFileObject val javaFileObject = JavaMetalsGlobal.makeFileObject(localSource.toFile) - val javacTask = JavaMetalsGlobal.compilationTask( + val javacTask = JavaMetalsGlobal.classpathCompilationTask( javaFileObject, Some(printWriter), allOptions, diff --git a/metals/src/main/scala/scala/meta/internal/parsing/JavaFoldingRangeExtractor.scala b/metals/src/main/scala/scala/meta/internal/parsing/JavaFoldingRangeExtractor.scala index c69274ce88a..b4d8ee7c722 100644 --- a/metals/src/main/scala/scala/meta/internal/parsing/JavaFoldingRangeExtractor.scala +++ b/metals/src/main/scala/scala/meta/internal/parsing/JavaFoldingRangeExtractor.scala @@ -122,7 +122,7 @@ final object JavaFoldingRangeExtractor { text: String, path: AbsolutePath, ): Option[(Trees, CompilationUnitTree)] = { - val javacTask = JavaMetalsGlobal.compilationTask(text, path.toURI) + val javacTask = JavaMetalsGlobal.baseCompilationTask(text, path.toURI) val elems = javacTask.parse() javacTask.analyze() val trees = Trees.instance(javacTask) diff --git a/mtags-java/src/main/scala/scala/meta/internal/pc/JavaCompletionProvider.scala b/mtags-java/src/main/scala/scala/meta/internal/pc/JavaCompletionProvider.scala index b1fe39e9382..e6602b293e0 100644 --- a/mtags-java/src/main/scala/scala/meta/internal/pc/JavaCompletionProvider.scala +++ b/mtags-java/src/main/scala/scala/meta/internal/pc/JavaCompletionProvider.scala @@ -46,7 +46,7 @@ class JavaCompletionProvider( params.text().substring(params.offset()) else params.text() val task: JavacTask = - JavaMetalsGlobal.compilationTask(textWithSemicolon, params.uri()) + compiler.compilationTask(textWithSemicolon, params.uri()) val scanner = JavaMetalsGlobal.scanner(task) val position = CursorPosition(params.offset(), params.offset(), params.offset()) diff --git a/mtags-java/src/main/scala/scala/meta/internal/pc/JavaHoverProvider.scala b/mtags-java/src/main/scala/scala/meta/internal/pc/JavaHoverProvider.scala index 9378cd2bf90..b793bde3986 100644 --- a/mtags-java/src/main/scala/scala/meta/internal/pc/JavaHoverProvider.scala +++ b/mtags-java/src/main/scala/scala/meta/internal/pc/JavaHoverProvider.scala @@ -50,7 +50,7 @@ class JavaHoverProvider( def hoverOffset(params: OffsetParams): Option[HoverSignature] = { val task: JavacTask = - JavaMetalsGlobal.compilationTask(params.text(), params.uri()) + compiler.compilationTask(params.text(), params.uri()) val scanner = JavaMetalsGlobal.scanner(task) val types = task.getTypes() val elements = task.getElements() diff --git a/mtags-java/src/main/scala/scala/meta/internal/pc/JavaMetalsGlobal.scala b/mtags-java/src/main/scala/scala/meta/internal/pc/JavaMetalsGlobal.scala index ed7bdd58ee4..f37f817df5f 100644 --- a/mtags-java/src/main/scala/scala/meta/internal/pc/JavaMetalsGlobal.scala +++ b/mtags-java/src/main/scala/scala/meta/internal/pc/JavaMetalsGlobal.scala @@ -3,6 +3,7 @@ package scala.meta.internal.pc import java.io.File import java.io.Writer import java.net.URI +import java.nio.file.Path import javax.tools.Diagnostic import javax.tools.DiagnosticListener import javax.tools.JavaCompiler @@ -19,7 +20,8 @@ import com.sun.source.util.TreePath class JavaMetalsGlobal( val search: SymbolSearch, - val metalsConfig: PresentationCompilerConfig + val metalsConfig: PresentationCompilerConfig, + val classpath: Seq[Path] ) { var lastVisitedParentTrees: List[TreePath] = Nil @@ -31,6 +33,16 @@ class JavaMetalsGlobal( lastVisitedParentTrees = scanner.lastVisitedParentTrees lastVisitedParentTrees.headOption } + + def compilationTask(sourceCode: String, uri: URI): JavacTask = { + val javaFileObject = SourceJavaFileObject.make(sourceCode, uri) + JavaMetalsGlobal.classpathCompilationTask( + javaFileObject, + None, + List("-classpath", classpath.mkString(File.pathSeparator)) + ) + } + } object JavaMetalsGlobal { @@ -49,12 +61,11 @@ object JavaMetalsGlobal { files.iterator().next() } - def compilationTask(sourceCode: String, uri: URI): JavacTask = { + def baseCompilationTask(sourceCode: String, uri: URI): JavacTask = { val javaFileObject = SourceJavaFileObject.make(sourceCode, uri) - compilationTask(javaFileObject, None, Nil) + JavaMetalsGlobal.classpathCompilationTask(javaFileObject, None, Nil) } - - def compilationTask( + def classpathCompilationTask( javaFileObject: JavaFileObject, out: Option[Writer], allOptions: List[String] diff --git a/mtags-java/src/main/scala/scala/meta/internal/pc/JavaPresentationCompiler.scala b/mtags-java/src/main/scala/scala/meta/internal/pc/JavaPresentationCompiler.scala index 6a4b7cb42c4..4f61cd168dd 100644 --- a/mtags-java/src/main/scala/scala/meta/internal/pc/JavaPresentationCompiler.scala +++ b/mtags-java/src/main/scala/scala/meta/internal/pc/JavaPresentationCompiler.scala @@ -9,7 +9,6 @@ import java.util.concurrent.CompletableFuture import java.util.concurrent.ExecutorService import java.util.concurrent.ScheduledExecutorService -import scala.collection.Seq import scala.concurrent.ExecutionContext import scala.concurrent.ExecutionContextExecutor import scala.jdk.CollectionConverters._ @@ -46,7 +45,7 @@ case class JavaPresentationCompiler( workspace: Option[Path] = None ) extends PresentationCompiler { - private val javaCompiler = new JavaMetalsGlobal(search, config) + private val javaCompiler = new JavaMetalsGlobal(search, config, classpath) override def complete( params: OffsetParams @@ -186,7 +185,7 @@ case class JavaPresentationCompiler( ): PresentationCompiler = copy( buildTargetIdentifier = buildTargetIdentifier, - classpath = classpath.asScala, + classpath = classpath.asScala.toSeq, options = options.asScala.toList ) diff --git a/tests/slow/src/test/scala/tests/feature/CompletionCrossLspSuite.scala b/tests/slow/src/test/scala/tests/feature/CompletionCrossLspSuite.scala index 991aa42f4ec..785c6c7aaee 100644 --- a/tests/slow/src/test/scala/tests/feature/CompletionCrossLspSuite.scala +++ b/tests/slow/src/test/scala/tests/feature/CompletionCrossLspSuite.scala @@ -43,7 +43,7 @@ class CompletionCrossLspSuite "extends Serializable@@", """|DefaultSerializable - scala.collection.generic |NotSerializableException - java.io - |Serializable a + |Serializable a |Serializable - java.io |SerializablePermission - java.io |""".stripMargin, diff --git a/tests/unit/src/main/scala/tests/TestingServer.scala b/tests/unit/src/main/scala/tests/TestingServer.scala index ed664d0c72f..cfe27c5554a 100644 --- a/tests/unit/src/main/scala/tests/TestingServer.scala +++ b/tests/unit/src/main/scala/tests/TestingServer.scala @@ -1106,7 +1106,7 @@ final case class TestingServer( val shouldIncludeDetail = item.getDetail != null && includeDetail val detail = if (shouldIncludeDetail && !label.contains(item.getDetail)) - item.getDetail + " " + item.getDetail else "" label + detail } diff --git a/tests/unit/src/test/scala/tests/CompletionLspSuite.scala b/tests/unit/src/test/scala/tests/CompletionLspSuite.scala index 5157f0fb8e0..ad27c7e6b4e 100644 --- a/tests/unit/src/test/scala/tests/CompletionLspSuite.scala +++ b/tests/unit/src/test/scala/tests/CompletionLspSuite.scala @@ -343,4 +343,41 @@ class CompletionLspSuite extends BaseCompletionLspSuite("completion") { ) } yield () } + + test("java") { + cleanWorkspace() + for { + _ <- initialize( + """/metals.json + |{ + | "a": {} + |} + |/a/src/main/java/a/A.java + |package a; + | + |public class A { + | String name = ""; + |} + |/a/src/main/java/a/B.java + |package a; + | + |public class B { + | A a = new A(); + | public void main(){ + | // @@ + | } + |} + |""".stripMargin + ) + _ <- server.didSave("a/src/main/java/a/B.java")(identity) + _ <- assertCompletion( + "a.n@@", + """|name java.lang.String + |notify() void + |notifyAll() void + |""".stripMargin, + filename = Some("a/src/main/java/a/B.java"), + ) + } yield () + } } From a7bb6eb04bc16df9ce97e5f560b8d07929fe01a5 Mon Sep 17 00:00:00 2001 From: Scalameta Bot Date: Fri, 22 Dec 2023 00:35:24 +0000 Subject: [PATCH 20/21] build(deps): Update guava from 32.1.3-jre to 33.0.0-jre --- project/V.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/V.scala b/project/V.scala index e7f672a2323..8f6669678bd 100644 --- a/project/V.scala +++ b/project/V.scala @@ -47,7 +47,7 @@ object V { val scribe = "3.13.0" val qdox = "2.0.3" - val guava = "com.google.guava" % "guava" % "32.1.3-jre" + val guava = "com.google.guava" % "guava" % "33.0.0-jre" val lsp4j = "org.eclipse.lsp4j" % "org.eclipse.lsp4j" % lsp4jV val dap4j = "org.eclipse.lsp4j" % "org.eclipse.lsp4j.debug" % lsp4jV From dc2917183c395c1e2ac6788d62bb65ff2be2b110 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Thu, 21 Dec 2023 16:14:17 +0100 Subject: [PATCH 21/21] bugfix: Don't show all classes when searching for implementations of abtract type Previously, for all types we would try to dealias them, but if the type was abtract we would get Any, which is the parent of all classes, which would not be useful. Now, we will show implementations of that abstract type. --- .../ImplementationProvider.scala | 19 ++++++++++++++----- .../scala/tests/ImplementationLspSuite.scala | 14 ++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/implementation/ImplementationProvider.scala b/metals/src/main/scala/scala/meta/internal/implementation/ImplementationProvider.scala index f36257b9ebb..66940c8604a 100644 --- a/metals/src/main/scala/scala/meta/internal/implementation/ImplementationProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/implementation/ImplementationProvider.scala @@ -79,7 +79,7 @@ final class ImplementationProvider( document <- documents.documents symbolInfo <- document.symbols } { - if (isClassLike(symbolInfo)) { + if (isClassLike(symbolInfo) || symbolInfo.isType) { parentImplLocationPairs ++= parentsFromSignature( symbolInfo.symbol, symbolInfo.signature, @@ -125,7 +125,9 @@ final class ImplementationProvider( val symbolSearch = defaultSymbolSearch(source, currentDocument) val sym = symbolOccurrence.symbol val dealiased = - if (sym.desc.isType) dealiasClass(sym, symbolSearch) else sym + if (sym.desc.isType) + dealiasClass(sym, symbolSearch) + else sym val definitionDocument = if (currentDocument.definesSymbol(dealiased)) { @@ -307,7 +309,8 @@ final class ImplementationProvider( implLocation, ) if !findSymbol(implDocument, implSymbol).exists( - _.kind == SymbolInformation.Kind.TYPE + // we should not show types if we are looking for a class implementations + sym => sym.isType && !parentSymbol.isType ) implOccurrence <- findDefOccurrence( implDocument, @@ -452,7 +455,13 @@ object ImplementationProvider { ): String = { if (symbol.desc.isType) { findSymbol(symbol) - .map(inf => dealiasClass(inf, findSymbol).symbol) + .map { inf => + val isAbstractType = inf.isAbstract && inf.isType + // abstract type will always have Any as upper bound + if (isAbstractType) symbol + else dealiasClass(inf, findSymbol).symbol + + } .getOrElse(symbol) } else { symbol @@ -527,6 +536,6 @@ object ImplementationProvider { } def isClassLike(info: SymbolInformation): Boolean = - info.isObject || info.isClass || info.isTrait || info.isType || info.isInterface + info.isObject || info.isClass || info.isTrait || info.isInterface } diff --git a/tests/unit/src/test/scala/tests/ImplementationLspSuite.scala b/tests/unit/src/test/scala/tests/ImplementationLspSuite.scala index c55f8c2a820..771698e3461 100644 --- a/tests/unit/src/test/scala/tests/ImplementationLspSuite.scala +++ b/tests/unit/src/test/scala/tests/ImplementationLspSuite.scala @@ -536,6 +536,20 @@ class ImplementationLspSuite extends BaseRangesSuite("implementation") { |} |""".stripMargin, ) + check( + "type-implementation", + """|/a/src/main/scala/a/Main.scala + |trait Test { + | type C@@C + |} + | + |trait Other extends Test { + | type <> = String + |} + | + |trait Unrelated + |""".stripMargin, + ) check( "java-implementation",