Stream: t-compiler/wg-rls-2.0

Topic: add qualified function assist


Timo Freiberg (Apr 16 2020 at 19:00, on Zulip):

I'm starting work on the next feature of the add_function assist: generating the function in another module if a qualified path is used! (inspired by matklad's feature request)
And I'm obviously getting stuck.

I assume I want to find the target module via code_model::Module::find_use_path?
It would be nice to pass a ModuleDefId::FunctionId for the item argument, only the given function doesn't exist yet.
I could probably remove the last segment from the path and try to pass that to find_use_path (as a ModuleDefId::ModuleId), does that sound reasonable?
If yes, do you also have some tips on how to do that path manipulation?

matklad (Apr 16 2020 at 19:02, on Zulip):

No, find_use_path is a reverse operation, you want Semantic::resolve_path

Timo Freiberg (Apr 16 2020 at 19:02, on Zulip):

ah. thanks!

Timo Freiberg (Apr 16 2020 at 19:03, on Zulip):

will that resolve a path to foo::bar, if the module foo exists but the function bar doesn't?

Timo Freiberg (Apr 16 2020 at 19:07, on Zulip):

nope.
again, do i want to try to resolve foo in that example to find the module? and then generate fn bar ... in there?

matklad (Apr 16 2020 at 19:11, on Zulip):

You'll need to get only the foo path

matklad (Apr 16 2020 at 19:11, on Zulip):

ast::Path::qualifier will give you a parent path

Timo Freiberg (Apr 16 2020 at 19:17, on Zulip):

oh. i'm already using path.qualifier().is_some() to purposefully disable the assist :face_palm:
looks like i can make some progress, thanks for the support!

Timo Freiberg (Apr 17 2020 at 18:18, on Zulip):

so generating the function in another module in the same file works: https://github.com/rust-analyzer/rust-analyzer/pull/3998
but generating it in another file doesn't, it inserts it in the original file with the offset calculated from the other file...
does that require changes in the whole assist infrastructure?

Timo Freiberg (Apr 17 2020 at 18:50, on Zulip):

alright, i see that in ra_ide::assists::assists, the AssistAction in ResolvedAssist.action that doesn't contain any file information is turned into a SourceChange which does contain a FileId (for each SourceFileEdit).
I think I might add a new field to ra_assists::AssistAction, something like enum AssistTargetFile {CurrentFile, TargetFile(FileId)}

Timo Freiberg (Apr 18 2020 at 21:31, on Zulip):

alright, that worked, functions can be added to other files now. what's missing is the assist navigating to the changed file though...
the file shows up in the codeAction response in command.arguments.0.cursorPosition.textDocument, so is that functionality missing in the lsp clients?

right now, the response looks like this:

{
        "command": {
            "arguments": [
                {
                    "cursorPosition": {
                        "position": {
                            "character": 4,
                            "line": 8
                        },
                        "textDocument": {
                            "uri": "file:///tmp/bla/src/main.rs"
                        }
                    },
                    "label": "Add function",
                    "workspaceEdit": {
                        "documentChanges": [
                            {
                                "edits": [
                                    {
                                        "newText": "\n\nfn bar() {\n    todo!()\n}",
                                        "range": {
                                            "end": {
                                                "character": 1,
                                                "line": 5
                                            },
                                            "start": {
                                                "character": 1,
                                                "line": 5
                                            }
                                        }
                                    }
                                ],
                                "textDocument": {
                                    "uri": "file:///tmp/bla/src/main.rs",
                                    "version": null
                                }
                            }
                        ]
                    }
                }
            ],
            "command": "rust-analyzer.applySourceChange",
            "title": "Add function"
        },
        "title": "Add function"
    }
matklad (Apr 18 2020 at 21:56, on Zulip):

that functionality missing in the lsp clients?

Which client do you use? It works with VS Code (the "add module" extension makes use of it)

Timo Freiberg (Apr 18 2020 at 22:03, on Zulip):

Hmm then I'm missing something, it doesn't work for me with nvim(coc-rust-analyzer) and with vs code. Gonna take another look tomorrow :)

matklad (Apr 18 2020 at 22:04, on Zulip):

A good way to check would be to apply the fixit for mod some_non_existent_module; It should move you to the new file

matklad (Apr 18 2020 at 22:19, on Zulip):

Upstream issue: https://github.com/microsoft/language-server-protocol/issues/724

Timo Freiberg (Apr 19 2020 at 09:16, on Zulip):

Hmm... the codeAction response for the "add module" assist looks like this:

{
        "command": {
            "arguments": [
                {
                    "cursorPosition": null,
                    "label": "create module",
                    "workspaceEdit": {
                        "documentChanges": [
                            {
                                "kind": "create",
                                "uri": "file:///tmp/bla/src/foo.rs"
                            }
                        ]
                    }
                }
            ],
            "command": "rust-analyzer.applySourceChange",
            "title": "create module"
        },
        "title": "create module"
    }

I don't think I can use this for add_function :/

Timo Freiberg (Apr 19 2020 at 14:15, on Zulip):

the plot thickens!
cursorPosition.textDocument.uri is read here, but if it differs from the currently opened URI(?), the function exits early
i guess this needs to be changed as well?

Timo Freiberg (Apr 20 2020 at 17:37, on Zulip):

Alright, the only thing really missing is having unit tests for generating the function in another file. Unfortunately, the assist helper always applies the assist to the starting file and only compares the expected with that file.
I guess I see where changes need to be made... This has proven to be more complex than I thought!

Last update: Sep 30 2020 at 16:00UTC