OrganizeImports CodeAction, how does it work

[mdbergmann]

Hi.

I'm using Emacs with eglot.
eglot supports the 'source.organizeImports' CodeAction.
But it seems that Metals doesn't send such when the clients asks for available CodeActions.
I've read that organizeImports is done using scalafmt on the backend side. But I'm not sure how to enable or get that triggered.

Regards

[ckipp01]

Thanks for the question @mdbergmann.

But it seems that Metals doesn't send such when the clients asks for available CodeActions.

So after the initialize comes in, Metals declares that it supports the organize imports code action here:

metals/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala

Lines 726 to 738 in a5bb60d

	if (initializeParams.supportsCodeActionLiterals) {
	capabilities.setCodeActionProvider(
	new CodeActionOptions(
	List(
	CodeActionKind.QuickFix,
	CodeActionKind.Refactor,
	CodeActionKind.SourceOrganizeImports
	).asJava
	)
	)
	} else {
	capabilities.setCodeActionProvider(true)
	}

So eglot should pick that up and see that Metals supports it.

But it seems that Metals doesn't send such when the clients asks for available CodeActions.

There should be a general way to just trigger this for a file with an editor.action.organizeImport or something similar.

I've read that organizeImports is done using scalafmt on the backend side.

Just a small correction, this feature in Metals is actually implemented with Scalafix and the scalafix-organize-imports rule whereas the formatting feature is done with Scalafmt.

[ckipp01]

Also as a total aside, our eglot section probably hasn't been touched in quite some time. Please feel free to let us know if there is anything out of date on there and send in a pr to either fix anything, add anything you find useful, etc.

[mdbergmann]

Yep, thanks. I'll have a look if there is something out of date for eglot.

There should be a general way to just trigger this for a file with an editor.action.organizeImport or something similar.

Is this part of the LSP protocol to trigger such a thing?

[tgodzik]

It is, it's part of https://microsoft.github.io/language-server-protocol/specification#textDocument_codeAction - there is even a specific code action for organize imports.

[mdbergmann]

So, OK. Please help me understand.

A CodeAction request by the client makes the server respond with available CodeActions for the given Range.
Those actions can include

• 'edit' instruction, which is something the client can do itself and just sends a change notification to the server after the edit.
• 'command' instruction, which the client sends to the server to apply the code action.

It can also be both.
The 'command', as I understood, is something the server provides to the client to be sent back to the server as a 'Command' request. The 'command' can be actually something arbitrary, individual from LSP server to LSP server.
So in case of the 'source.organizeImports' CodeAction, which command should the client send to Metals to trigger?

I am tracing the RPC communication, so I could see what command it might be, but Metals doesn't seem to send a CodeAction for organizing imports to eglot even after explicitly adding unused imports.

[ckipp01]

So the way that it's worked thus far with other clients is that a code action request is sent and inside of the CodeActionContext only is used to specify that they are only wanting the organizeImports action. Here is an example of what it looks like in trace. This example is using Vim and coc.nvim:

[Trace - 11:32:44 AM] Received request 'textDocument/codeAction - (25)'
Params: {
  "textDocument": {
    "uri": "file:///Users/ckipp/Documents/scala-workspace/tester/src/main/scala/example/Main.scala"
  },
  "range": {
    "start": {
      "line": 0,
      "character": 0
    },
    "end": {
      "line": 12,
      "character": 0
    }
  },
  "context": {
    "diagnostics": [
      {
        "range": {
          "start": {
            "line": 2,
            "character": 0
          },
          "end": {
            "line": 2,
            "character": 34
          }
        },
        "severity": 2,
        "source": "bloop",
        "message": "Unused import"
      },
      {
        "range": {
          "start": {
            "line": 3,
            "character": 0
          },
          "end": {
            "line": 3,
            "character": 32
          }
        },
        "severity": 2,
        "source": "bloop",
        "message": "Unused import"
      }
    ],
    "only": [
      "source.organizeImports"
    ]
  }
}

And since only is used here, Metals will recognize that it's the only one requested and therefore return it:

[Trace - 11:32:44 AM] Sending response 'textDocument/codeAction - (25)'. Processing request took 184ms
Result: [
  {
    "title": "Organize imports",
    "kind": "source.organizeImports",
    "edit": {
      "changes": {
        "file:///Users/ckipp/Documents/scala-workspace/tester/src/main/scala/example/Main.scala": [
          {
            "range": {
              "start": {
                "line": 0,
                "character": 0
              },
              "end": {
                "line": 12,
                "character": 0
              }
            },
            "newText": "package example\n\n\n\nobject Main extends App {\n\n  val x: Option[String] \u003d Some(\"hi\")\n\n  val y \u003d 3\n\n}\n"
          }
        ]
      }
    }
  }
]

[mdbergmann]

OK, thanks. I kind of got this working.
I've sent a CodeAction request to Metals with a range of the full document instructing it to only add 'source.organizeImports' CodeAction.
That works. However, it sent me back an 'edit' with a reorganized document where the unused import was just reorder, but it's still there.
From your trace I can see that you actually have some diagnostics that you had received from Metals for the unused imports.
But I don't see that here. So I have unused imports, but Metals doesn't seem to recognize them. While on the other hand I get other diagnostics for the same document like:

   85  37 note     bloop: [scapegoat] List.size is O(n)
  List.size is O(n). Consider using a different data type with O(1) size lookup such as Vector or an Array.
   92  68 warning  bloop: [scapegoat] Variable shadowing
  Variable shadowing is very useful, but can easily lead to nasty bugs in your code. Shadowed variables can be potentially confusing to other maintainers when the same name is adopted to have a new meaning in a nested scope.

  162  57 note     bloop: [scapegoat] List.size is O(n)
  List.size is O(n). Consider using a different data type with O(1) size lookup such as Vector or an Array.
  235  20 warning  bloop: non-variable type argument java.util.UUID in type pattern List[java.util.UUID] (the underlying of List[java.util.UUID]) is unchecked since it is eliminated by erasure

So, I'm not sure what's wrong here.
Maybe I should try to clean and rebuild.

[ckipp01]

Are you using scalacOptions += -Ywarn-unused` (that differs a bit per scala version), since you need that in order for the compiler to detect that the import is unused. This will also produce a diagnostic for it.

[mdbergmann]

OK, the -Ywarn-unused did the trick.

I think this is sufficient info for me.
Many thanks.