scalameta / metals

Scala language server with rich IDE features 🚀
https://scalameta.org/metals/
Apache License 2.0
2.07k stars 323 forks source link

Respect rootUri in initialize request #216

Closed olafurpg closed 5 years ago

olafurpg commented 6 years ago

Currently, the server infers the workspace root uri from the working directory (sys.props("user.dir")). In LSP, the initialize request includes a rootUri field that should be used instead.

A workaround for this problem is to make sure the metals binary is started from the correct working directory.

The workspace root is important since it needs to match the root of the build in order to correctly support definition/references. The semanticdb-scalac compiler plugin emits *.semanticdb files that contain relative filepaths, which are absolutized with the workspace directory.

I suspect fixing this will require some refactoring since we currently assume that the workspace root is already available once the server starts. https://github.com/scalameta/metals/blob/76c239f35a82086870e7cd83a6b2609c376c6737/metals/src/main/scala/scala/meta/metals/Main.scala#L16 However, most services accept a cwd: AbsolutePath parameter so there should not be any hidden usages of sys.props("user.dir").

olafurpg commented 6 years ago

I think a good solution to fix this issue would be to extend lsp4s to support request handlers that return new Services.

Services.empty.requestServices(Lifecycle.initialize) { params =>
  val cwd = params.rootUri
  val newServices = constructNewServices(cwd)
  val response = InitializeResult(...)
  (response, newServices)
}

The lsp4s LanguageServer could use newServices to update this map here https://github.com/scalameta/lsp4s/blob/0b0387a5eb769df008c74490f78d16ea4cf7210d/jsonrpc/src/main/scala/scala/meta/jsonrpc/LanguageServer.scala#L49 so that the following requests are handled by newServices

gabro commented 5 years ago

Is this still relevant, now that the LSP code is gone?

olafurpg commented 5 years ago

Yes, I want to fix it from the start with the new lsp server implementation.

olafurpg commented 5 years ago

Fixed in #337