Add support for Resolve Macro As of clojure-lsp

[bpringe]

@ericdallo mentioned we'll need to write something for this to work. I'm just adding this issue to track it for later.

[bpringe]

I was made aware by Eric that the "resolve macro" code action is showing up even though we don't support it yet. We need to soon either implement this or make that code action not show in the UI.

[ericdallo]

That code action need 2 additional parameters to work that Calva could ask for user to input:

• the macro to resolve as: clojure.core/defn, clojure.core/def etc
• The clj-kondo config.edn file to persist that setting, lsp-mode suggests both project/.clj-kondo/config.edn and ~/.clj-kondo/config.edn

More info here: https://clojure-lsp.github.io/clojure-lsp/features/#resolve-macro-as

[bpringe]

@ericdallo https://github.com/emacs-lsp/lsp-mode/blob/73b127f4cf09a443e1353aa6c40b2379b59c1bd6/clients/lsp-clojure.el#L206

In the above line, if the workspace root is nil, then the literal project is used in the path? Can you explain why this is like this? In that case it would write to the literal path relative to the current open file - project/.clj-kondo/config.edn, right? If so, how does this help? Thanks!

[ericdallo]

yeah, that was a just a test that won't work 😅 we should probably check if the (lsp-workspace-root) exists, if not, suggest only the home dir.

(lsp-workspace-root) will return nil when server is not connected

[bpringe]

Oh I see... so Calva should get that from the server-info command?

[ericdallo]

Calva already send the rootPath on the initializeRequest, right? you can use the same

[bpringe]

Actually we don't send that in the initialize request... should we be? I do see that clojure-lsp sets the root path itself anyway, it seems. I see it with the server-info command.

[ericdallo]

I think calva send the project root, otherwise, clojure-lsp wouldn't scan the project: https://github.com/clojure-lsp/clojure-lsp/blob/master/src/clojure_lsp/handlers.clj#L40-L41

[ericdallo]

You can confirm that checking the log between client<->server

[bpringe]

I can see it's sent, but we don't set it ourselves. I think the client library we use is sending it. I can probably just use vscode.workspace.rootPath anyway. Thanks.

Edit: rootPath is deprecated, but we can use workspaceFolders instead (first index holds the root path).

[ericdallo]

exactly!

[bpringe]

@ericdallo I'm not sure if this is an issue but I noticed when I only have a single file open (not a project), the resolve-macro-as code action doesn't get returned by the server when the cursor is on a deftest, for example, but with a project open, it does.

Request and response when a project is open and cursor is on deftest:

[Trace - 4:25:37 PM] Sending request 'textDocument/codeAction - (9)'.
Params: {
    "textDocument": {
        "uri": "file:///home/brandon/development/clojure-test/test/core_test.clj"
    },
    "range": {
        "start": {
            "line": 9,
            "character": 7
        },
        "end": {
            "line": 9,
            "character": 7
        }
    },
    "context": {
        "diagnostics": []
    }
}


[Trace - 4:25:37 PM] Received response 'textDocument/codeAction - (9)' in 22ms.
Result: [
    {
        "title": "Resolve macro 'clojure.test/deftest' as...",
        "kind": "quickfix",
        "command": {
            "title": "Resolve macro as...",
            "command": "resolve-macro-as",
            "arguments": [
                "file:///home/brandon/development/clojure-test/test/core_test.clj",
                10,
                8
            ]
        }
    },
    {
        "title": "Clean namespace",
        "kind": "source.organizeImports",
        "command": {
            "title": "Clean namespace",
            "command": "clean-ns",
            "arguments": [
                "file:///home/brandon/development/clojure-test/test/core_test.clj",
                9,
                7
            ]
        }
    }
]

Request and response when only one file is open (same file as above) and cursor is on deftest:

[Trace - 4:29:10 PM] Sending request 'textDocument/codeAction - (17)'.
Params: {
    "textDocument": {
        "uri": "file:///home/brandon/development/clojure-test/test/core_test.clj"
    },
    "range": {
        "start": {
            "line": 14,
            "character": 7
        },
        "end": {
            "line": 14,
            "character": 7
        }
    },
    "context": {
        "diagnostics": []
    }
}


[Trace - 4:29:10 PM] Received response 'textDocument/codeAction - (17)' in 7ms.
Result: []

[ericdallo]

@bpringe, yes, it seems this is an issue with the analysis as when you open a single file, clojure-lsp just call clj-kondo for the current file and not a classpath, so it doesn't have the analysis of clojure.test ns.
We could investigate how to improve it, but, IMHO is a corner case with low priority.

[bpringe]

Thanks for explaining. I agree that it's a corner case. 👍

[bpringe]

@ericdallo Sorry for yet another question on this 😄. I've implemented the code action part of this, and also started implementing a Calva command. The command gets the doc uri, line, and character and calls the command that handles the code action, where the rest of the args are retrieved. However, if no macro actually exists at the cursor position, clojure-lsp adds nil as the macro to the config, which breaks parsing of the file.

Should clojure-lsp handle this nil case and not edit the config file if the macro-at-cursor is nil?

;; Cursor here |
(deftest hello ..

;; Config result
{:lint-as {nil clojure.core/def}}

[bpringe]

This is the parsing error, if you're interested. 🤷‍♂️

{
	"resource": "/home/brandon/development/clojure-test/test/core_test.clj",
	"owner": "_generated_diagnostic_collection_name_#0",
	"code": "syntax",
	"severity": 8,
	"message": "Can't parse /home/brandon/development/clojure-test/test/core_test.clj, java.lang.NullPointerException",
	"source": "clj-kondo",
	"startLineNumber": 1,
	"startColumn": 1,
	"endLineNumber": 1,
	"endColumn": 1
}

[ericdallo]

Sorry for yet another question on this smile

Np, I like to help :)

yes, we can fix it on clojure-lsp, this doesn't happen ATM with emacs since we return the code action only when it works, but anyway we should avoid those things when user calls manually the resolve-macro.

The command gets the doc uri, line, and character and calls the command that handles the code action

Just to make it clear, It should call the command with 5 args: uri, line, character, macro-to-resolve, config-location

[bpringe]

Thanks. Yeah, the code action is only returned when it works in VS Code as well, but I added a global (enabled in Clojure files) command for Calva. It looked as though lsp-mode did this, but maybe I was mistaken.

It should call the command with 5 args: uri, line, character, macro-to-resolve, config-location

I do call the clojure-lsp command like that in the resolve-macro command/handler, where clojure-lsp provides those things to the handler, but for the global Calva command (not the code action), those first 3 args have to be gotten by Calva, then passed to the resolve-macro command/handler, similar to these args here.

[bpringe]

Wondering if I should just leave it as a code action and remove the command... simpler that way. 🤔

[ericdallo]

Oh you are right, there is a manual command for that indeed on lsp-mode!

I do call the clojure-lsp command like that in the resolve-macro command/handler, where clojure-lsp provides those things to the handler, but for the global Calva command (not the code action), those first 3 args have to be gotten by Calva, then passed to the resolve-macro command/handler, similar to these args here.

yes it makes sense, this is how lsp-mode as well, so I think it's following the same pattern correctly

[bpringe]

I'm don't know how lsp-mode behaves but I noticed in Calva if just a single file is open, the command results in nil being added as the macro in the config file.

Note that this is not the code action, as I know that does not show up for single files, as we discussed above.

Does clojure-lsp not find the macro at cursor correctly if just a single file is open? If it can't easily be done, maybe I could disable the command if no project is open.

[bpringe]

Never mind, I'm guessing the issue for code actions is the same in this situation too. I just disabled the command if no folder is open.

[ericdallo]

I confirmed the resolve macro works on lsp-mode for a simple file /tmp/sample.clj 🤔

[bpringe]

Hmm... I made a file - /tmp/test.clj - and put the cursor right after deftest, then ran the command.

(ns core-test
  (:require [clojure.test :refer [is deftest testing]]))

(deftest| one-equals-one
  (is (= 1 1)))

Request + response:

[Trace - 4:36:45 PM] Sending request 'workspace/executeCommand - (204)'.
Params: {
    "command": "resolve-macro-as",
    "arguments": [
        "file:///tmp/test.clj",
        4,
        9,
        "clojure.core/def",
        "/home/brandon/.config/clj-kondo/config.edn"
    ]
}


[Trace - 4:36:45 PM] Received response 'workspace/executeCommand - (204)' in 2ms.
No result returned.

I don't see any errors in the clojure-lsp log file. I see 2021-04-13T23:36:45.973Z brandon-desktop INFO [clojure-lsp.feature.resolve-macro:52] - Resolving macro as clojure.core/def. Saving setting on /home/brandon/.config/clj-kondo/config.edn which looks like it worked, though it's saving nil.

Is that log statement supposed to say the name of the macro it's resolving as? Like Resolving macro clojure.test/deftest as clojure.core/def? Guessing if so then the nil explains why it's missing, but still don't know why clojure-lsp is not finding the macro at cursor.

Edit: Looks like it does not log which macro it's resolving as the new macro.

[bpringe]

@ericdallo It looks like clojure-lsp's db is used for getting the function name at the cursor. That's supposed to work when only a single file is open?"

If that fails, then this when-let body doesn't run. In that case, the full-symbol here would be nil, which explains what I'm seeing in the config.

[ericdallo]

@bpringe the deftest issue is because of this, because the macro comes from another namespace non analyzed because you opened as not a project.
Maybe you can make Calva resolve macro only work for projects as users will probably want to use on macro from other ns.

[ericdallo]

@ericdallo It looks like clojure-lsp's db is used for getting the function name at the cursor. That's supposed to work when only a single file is open?"

That function should always return even for single files, It'll only return nil if it doesn't know the document uri, which is if client send it incorrectly

If that fails, then this when-let body doesn't run. In that case, the full-symbol here would be nil, which explains what I'm seeing in the config.

Yep, I'll check it and try to fix it

[ericdallo]

Fixed here @bpringe, it was returning nil indeed because it could not find the definition of the macro, because it was not analyzed the whole classpath that would include the definition of clojure.core/deftest.

I fixed improving the log and error to user.

[bpringe]

@ericdallo Thanks! So, did resolving this same macro work in your single file test with lsp-mode? If so, why did it work there and not for me?

Another question - are there cases where resolving a macro with a single file would work? I'm confused as to when this would work with a single file given what you said above (unless the macro definition was in the current namespace). If it wouldn't work in most cases, I wonder if I should disable it unless a folder is open... but I guess even a "folder" being open doesn't mean a "project" is open, so the issue could still exist if a "folder" is open.

[ericdallo]

@ericdallo Thanks! So, did resolving this same macro work in your single file test with lsp-mode? If so, why did it work there and not for me?

Not, it will not work for deftest or any macro that belongs to another namespace not in the single file, as clojure-lsp don't scan the classpath for single file tests, what I did was just to avoid changing the config to nil and printing a error log to user for those cases.

Another question - are there cases where resolving a macro with a single file would work? I'm confused as to when this would work with a single file given what you said above (unless the macro definition was in the current namespace). If it wouldn't work in most cases, I wonder if I should disable it unless a folder is open... but I guess even a "folder" being open doesn't mean a "project" is open, so the issue could still exist if a "folder" is open.

yes, IMO we should allow resolve the macro only when the rootPath is passed to the initialize request method, which means, client send a project root to clojure-lsp and clojure-lsp scan the classpath for definitions.