From 6694a541418692ed079ca435224faad861373ffc Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Thu, 7 Sep 2023 14:01:39 +0200 Subject: [PATCH] refactor: simplify `searchForBuildTool` function --- .../meta/internal/bsp/BspConnector.scala | 18 ++++++++-- .../meta/internal/builds/BuildTools.scala | 36 +++++++------------ .../scala/meta/internal/metals/Messages.scala | 1 + .../internal/metals/MetalsLspService.scala | 19 +++++----- .../meta/internal/metals/ProjectRoot.scala | 29 --------------- .../scala/meta/internal/metals/Tables.scala | 2 -- .../internal/metals/UserConfiguration.scala | 34 +++--------------- .../scala/tests/FileWatcherLspSuite.scala | 3 +- 8 files changed, 46 insertions(+), 96 deletions(-) delete mode 100644 metals/src/main/scala/scala/meta/internal/metals/ProjectRoot.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 711a2736f50..489d64d7020 100644 --- a/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala +++ b/metals/src/main/scala/scala/meta/internal/bsp/BspConnector.scala @@ -89,7 +89,12 @@ class BspConnector( Future.successful(None) case ResolvedBloop => bloopServers - .newServer(projectRoot, bspTraceRoot, userConfiguration, addLivenessMonitor) + .newServer( + projectRoot, + bspTraceRoot, + userConfiguration, + addLivenessMonitor, + ) .map(Some(_)) case ResolvedBspOne(details) if details.getName() == SbtBuildTool.name => @@ -114,7 +119,9 @@ class BspConnector( .map(Some(_)) case ResolvedBspOne(details) => tables.buildServers.chooseServer(details.getName()) - bspServers.newServer(projectRoot, bspTraceRoot, details, addLivenessMonitor).map(Some(_)) + bspServers + .newServer(projectRoot, bspTraceRoot, details, addLivenessMonitor) + .map(Some(_)) case ResolvedMultiple(_, availableServers) => val distinctServers = availableServers .groupBy(_.getName()) @@ -146,7 +153,12 @@ class BspConnector( ) ) _ = tables.buildServers.chooseServer(item.getName()) - conn <- bspServers.newServer(projectRoot, bspTraceRoot, item, addLivenessMonitor) + conn <- bspServers.newServer( + projectRoot, + bspTraceRoot, + item, + addLivenessMonitor, + ) } yield Some(conn) } } 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 f7c79516720..d4b887940c8 100644 --- a/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala +++ b/metals/src/main/scala/scala/meta/internal/builds/BuildTools.scala @@ -117,29 +117,19 @@ final class BuildTools( private def searchForBuildTool( isProjectRoot: AbsolutePath => Boolean - ): Option[AbsolutePath] = { - def recIsProjectRoot( - path: AbsolutePath, - level: Int = 0, - ): Option[AbsolutePath] = - if ( - path.isDirectory && - !path.toNIO.filename.startsWith(".") - ) { - if (isProjectRoot(path)) Some(path) - else if (level < 1) - path.toNIO - .toFile() - .listFiles() - .collectFirst(root => - recIsProjectRoot(AbsolutePath(root), level + 1) match { - case Some(root) => root - } - ) - else None - } else None - recIsProjectRoot(workspace) - } + ): Option[AbsolutePath] = + if (isProjectRoot(workspace)) Some(workspace) + else + workspace.toNIO + .toFile() + .listFiles() + .collectFirst { + case file + if file.isDirectory && + !file.getName.startsWith(".") && + isProjectRoot(AbsolutePath(file.toPath())) => + AbsolutePath(file.toPath()) + } def allAvailable: List[BuildTool] = { List( 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 d8f8a05cc0a..cdd710306b7 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Messages.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Messages.scala @@ -1009,6 +1009,7 @@ object Messages { params } } + } object FileOutOfScalaCliBspScope { 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 9cecc45436a..9c35566b7a9 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala @@ -887,7 +887,7 @@ class MetalsLspService( def connectTables(): Connection = tables.connect() - def initialized(): Future[Unit] = { + def initialized(): Future[Unit] = for { _ <- maybeSetupScalaCli() _ <- @@ -902,7 +902,6 @@ class MetalsLspService( ) ) } yield () - } def onShutdown(): Unit = { tables.fingerprints.save(fingerprints.getAllFingerprints()) @@ -1656,7 +1655,7 @@ class MetalsLspService( } yield bloopServers.shutdownServer() case Some(session) if session.main.isSbt => for { - currentBuildTool <- supportedBuildTool + currentBuildTool <- supportedBuildTool() res <- currentBuildTool match { case Some(sbt: SbtBuildTool) => for { @@ -1948,7 +1947,9 @@ class MetalsLspService( }) } - def supportedBuildTool(): Future[Option[BuildTool]] = { + def supportedBuildTool( + withStartFileWatcher: Boolean = false + ): Future[Option[BuildTool]] = { def isCompatibleVersion(buildTool: BuildTool) = { val isCompatibleVersion = SemVer.isCompatibleVersion( buildTool.minimumVersion, @@ -1970,8 +1971,10 @@ class MetalsLspService( if (!buildTools.isAutoConnectable()) { warnings.noBuildTool() } - // wait for a bsp file to show up - fileWatcher.start(Set(folder.resolve(".bsp"))) + if (withStartFileWatcher) { + // wait for a bsp file to show up + fileWatcher.start(Set(folder.resolve(".bsp"))) + } Future(None) } case buildTools => @@ -1987,7 +1990,7 @@ class MetalsLspService( forceImport: Boolean ): Future[BuildChange] = for { - possibleBuildTool <- supportedBuildTool + possibleBuildTool <- supportedBuildTool(withStartFileWatcher = true) chosenBuildServer = tables.buildServers.selectedServer() isBloopOrEmpty = chosenBuildServer.isEmpty || chosenBuildServer.exists( _ == BloopServers.name @@ -2078,7 +2081,7 @@ class MetalsLspService( def calculateOptProjectRoot(): Future[Option[AbsolutePath]] = for { - possibleBuildTool <- supportedBuildTool + possibleBuildTool <- supportedBuildTool() } yield possibleBuildTool.map(_.projectRoot).orElse(buildTools.bloopProject) def quickConnectToBuildServer(): Future[BuildChange] = diff --git a/metals/src/main/scala/scala/meta/internal/metals/ProjectRoot.scala b/metals/src/main/scala/scala/meta/internal/metals/ProjectRoot.scala deleted file mode 100644 index 31e30ca2b6e..00000000000 --- a/metals/src/main/scala/scala/meta/internal/metals/ProjectRoot.scala +++ /dev/null @@ -1,29 +0,0 @@ -package scala.meta.internal.metals - -import java.sql.Connection - -import scala.meta.internal.metals.JdbcEnrichments._ - -class ProjectRoot(conn: () => Connection) { - def relativePath(): Option[String] = { - conn() - .query( - "select * from project_root LIMIT 1;" - )(_ => ()) { _.getString("relative_path") } - .headOption - } - def set(relativePath: Option[String]): Int = synchronized { - reset() - relativePath - .map { relativePath => - conn().update { - "insert into project_root values (?);" - } { stmt => stmt.setString(1, relativePath) } - } - .getOrElse(0) - } - - def reset(): Unit = { - conn().update("delete from project_root;") { _ => () } - } -} diff --git a/metals/src/main/scala/scala/meta/internal/metals/Tables.scala b/metals/src/main/scala/scala/meta/internal/metals/Tables.scala index 58cb9db878a..61e6accb1da 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/Tables.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/Tables.scala @@ -37,8 +37,6 @@ final class Tables( new ChosenBuildTool(() => connection) val fingerprints = new Fingerprints(() => connection) - val projectRoot = - new ProjectRoot(() => connection) private val ref: AtomicReference[ConnectionState] = new AtomicReference(ConnectionState.Empty) 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 431b9416a42..e9707c42b89 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala @@ -313,19 +313,6 @@ object UserConfiguration { |launcher, not available in PATH. |""".stripMargin, ), - UserConfigurationOption( - "projects-roots", - """empty map""", - """"{ - "project1": "backend", - "project2": "backend", - }"""", - "Project roots", - """Optional map of Scala projects' roots as relative path from workspace folder root. - |The project root should contain the build tool configuration if not passed workspace root is assumed. - |For a single root project a the key should be set to "root" or simply a plain string can be passed. - |""".stripMargin, - ), ) def fromJson( @@ -365,16 +352,12 @@ object UserConfiguration { ) }, ) - def getStringKey( - key: String, - customError: JsonElement => Option[String] = _ => None, - ): Option[String] = - getStringKeyOnObj(key, json, customError) + def getStringKey(key: String): Option[String] = + getStringKeyOnObj(key, json) def getStringKeyOnObj( key: String, currentObject: JsonObject, - customError: JsonElement => Option[String] = _ => None, ): Option[String] = getKey( key, @@ -383,9 +366,7 @@ object UserConfiguration { Try(value.getAsString) .fold( _ => { - errors += customError(value).getOrElse( - s"json error: key '$key' should have value of type string but obtained $value" - ) + errors += s"json error: key '$key' should have value of type string but obtained $value" None }, Some(_), @@ -443,10 +424,7 @@ object UserConfiguration { }, ) - def getStringMap( - key: String, - collectError: Boolean = true, - ): Option[Map[String, String]] = + def getStringMap(key: String): Option[Map[String, String]] = getKey( key, json, @@ -461,9 +439,7 @@ object UserConfiguration { } }.fold( _ => { - if (collectError) { - errors += s"json error: key '$key' should have be object with string values but obtained $value" - } + errors += s"json error: key '$key' should have be object with string values but obtained $value" None }, entries => Some(entries.toMap), diff --git a/tests/unit/src/test/scala/tests/FileWatcherLspSuite.scala b/tests/unit/src/test/scala/tests/FileWatcherLspSuite.scala index 74b23495266..64ae906678c 100644 --- a/tests/unit/src/test/scala/tests/FileWatcherLspSuite.scala +++ b/tests/unit/src/test/scala/tests/FileWatcherLspSuite.scala @@ -170,8 +170,7 @@ class FileWatcherLspSuite extends BaseLspSuite("file-watcher") { .asScala _ = assertNoDiff( client.workspaceMessageRequests, - s"""|${Messages.ImportBuild.params("sbt").getMessage()} - |""".stripMargin, + Messages.ImportBuild.params("sbt").getMessage(), ) } yield () }