support BSP formatting

[danielleontiev]

When editing scala-cli scripts in a text editor with enabled Metals, attempting to invoke the "format file" feature will lead to Metals asking to create a .scalafmt.conf file because Metals uses scalafmt under the hood to perform formatting.

Metals asks to provide formatting configuration

On the other hand, scala-cli is capable of formatting Scala sources using scalafmt. Unlike plain scalafmt, scala-cli provides reasonable defaults and therefore does not require the creation of a .scalafmt.conf configuration file to work.

So, I originally created the following issue in Metals: scalameta/metals-feature-requests#334. In this issue, I initially proposed invoking scala-cli fmt from the Metals side if scala-cli is used as the BSP server. This would allow formatting files without creating a .scalafmt.conf configuration.

Instead, I was offered to extend the BSP protocol to natively support formatting driven by the BSP server. This would avoid special-casing build tools on the Metals side. An ongoing discussion is taking place on how to support the change in the protocol: build-server-protocol/build-server-protocol#459

Currently, two approaches have been suggested:

• buildTarget/format method in BSP. The protocol changes for this method have been implemented in the aforementioned PR (feat: add buildTarget/format method and canFormat capability build-server-protocol/build-server-protocol#459)
• textDocument/format method in BSP. Unlike the previous method, this method operates on an arbitrary set of files/directories inside the workspace (buildTarget/format operates on buildTarget).

Both approaches have pros and cons, and since there is still no consensus in the discussion, I have created draft implementations for both. I kindly ask for your feedback because I hope you will be able to provide insights that will help to choose the correct approach.

• buildTarget/format implementation danielleontiev/scala-cli#1
• textDocument/format implementation danielleontiev/scala-cli#2

[danielleontiev]

Hi @Gedochao @MaciejG604!
Did you have a chance to take a look at PRs?

[lwronski]

Hi @danielleontiev, I wanted to discuss your intentions regarding the implementation of the textDocument/format endpoint. I have the same considerations for the buildTarget/format endpoint.

For the textDocument/format endpoint, should it reformat files specified in the params by scalafmt and return a response indicating whether it was successful or not? Alternatively, should it return a list of TextEdit objects containing the formatted content of the files, which Metals can then apply as changes in the files?

In the first approach, the bsp implementation in scala-cli would format the file by scalafmt and override its content. However, in the second approach, the bsp endpoint would return the reformatted content of the file via TextEdit objects and not override file content, it will be done in Metals.

[danielleontiev]

Hi!
In the current implementation both buildTarget/format and textDocument/format return empty response meaning that formatting has been completed successfully - or rise the error. So, my intent was to call scalafmt under the hood on targeted files.

The pros are

1. It's more simple, because BSP protocol does not have anything like TextEdit
2. There are two use cases: first is formatting individual files and second is formatting everything, and in second case if there are plenty of files to format, doing so by applying edits on language server side may have performance implications
3. Maybe a little harder to implement in language server which may be a problematic for other LSP implementations

I suspect that the most common use-case for the feature is formatting individual files from editor and the target opened file may be unsaved when user triggers the formatting, so I agree with you point, probably, we need to consider adding something like TextEdit to the protocol.

The more simple but less user-friendly option would be to forbid user to format unsaved files when formatting is backed by BSP, but it's of course not perfect from the UX perspective

[lwronski]

Thank you for the explanation. I will need to discuss this internally, and I will provide a response tomorrow.

[tgodzik]

We discussed it internally and there are some issues with the approach. If you format things in the background the editor might get out of sync very easily and you will get local conflicts. Instead it would be best to use TextEdits (or a single one for the entire file), which can be forwarded to the editor. We could run format for a single file in that case, but not for the entire workspace since this would require opening all the files that changed.

I wonder if this is not a bit of an overengineered approach. What we could do instead in Metals is just default to .scala-build/.scalafmt.conf if it exist or add an option to create it there if Scala CLI is used (though I think it's best to add it to the workspace to avoid having different versions used by users). Even with the first step, just using the Scala CLI config file would probably improve the situation for you.

You can also set a global config for your script workspaces to avoid having the scalafmt.conf file near them.

I understand this required a lot of work and I should have mentioned my reservations earlier, which I am very sorry about. But all the current tools use scalafmt API for formatting, so using it from Metals is really exactly the same as adding BSP format request and doing it via Scala CLI.

[danielleontiev]

Ok, no problem!

What we could do instead in Metals is just default to .scala-build/.scalafmt.conf if it exist or add an option to create it there if Scala CLI is used

I am not against this one but it seems to be a build-tool aware approach which is not something necessary bad but something initially we've tried to avoid while discussing scalameta/metals-feature-requests#334. I think that setting a global config for script workspaces path as you mentioned is a good option

[tgodzik]

I added the detection of .scalafmt file in .scala-build by default. scalameta/metals#5559