Skip to content

feature: Support standalone Rust files #8955

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
May 24, 2021

Conversation

SomeoneToIgnore
Copy link
Contributor

standalone

Closes #6388

Caveats:

  • I've decided to support multiple detached files in the code (anticipating the scratch files), but I found no way to open multiple files in VSCode at once: running code *.rs makes the plugin to register in the vscode.workspace.textDocuments only the first file, while code actually displays all files later.
    Apparently what happens is the same as when you have VSCode open at some workplace already and then run code some_other_file.rs: it gets opened in the same workspace of the same VSCode with no server to support it.
    If there's a way to override it, I'd appreciate the pointer.

  • No way to toggle inlay hints, since the setting is updated for the workspace (which does not exist for a single file opened)

[2021-05-24 00:22:49.100] [exthost] [error] Error: Unable to write to Workspace Settings because no workspace is opened. Please open a workspace first and try again.

  • No runners/lens to run or check the code are implemented for this mode.
    In theory, we can detect rustc, run it on a file and run the resulting binary, but not sure if worth doing it at this stage.

Otherwise imports, hints, completion and other features work.

Copy link
Member

@matklad matklad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this!

But let's add the following as a comment on DetachedFiles, as a sort of extended TODO:

The primary limitation of this approach is that the set of detached files needs to be fixed at the beginning. That's not the end user experience we should strive for. Ideally, you should be able to just open a random detached file in existing cargo projects, and get the basic features working. That needs some changes on the salsa-level though. In particular, we should split the unified CrateGraph (which currently has maximal durability) into proper crate graph, and a set of ad hoc roots (with minimal durability). Then, we need to hide the graph behind the queries such that most queries look only at the proper crate graph, and fall back to ad hoc roots only if there's no results. After this, we should be able to tweak the logic in reload.rs to add newly opened files, which don't belong to any existing crates, to the set of the detached files.

if (rustDocuments.length > 0) {
ctx = await Ctx.create(config, context, serverPath, { kind: 'Detached Files', files: rustDocuments });
} else {
throw new Error("no rust files are opened");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, didn't realize we still have this vscode.workspace.workspaceFolders?.[0] logic on the client side. It's wrong and should be removed (unrelated to this PR of course)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Time to break the client again, I guess.

What should be the alternative?
const target = vscode.workspace.workspaceFolders![0]; // safe, see main activate()
?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As usual, just don't do it at all :-)

We use workspace folder for two things:

  • to set cwd for the server
  • in task provider

The serever is not relying on cwd (we are careful with using AbsPaths everywhere), so, presumably, that logic can be removed.

The tasks should be fixed to collect the task from all workspace folders, and not just from the first one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to set cwd for the server

I feel that could be connected with the debug.ts logic (it's a 3rd place where we use vscode.workspace.workspaceFolders), but I'll give it a try and check how it goes.

Not doing anything in this PR, just in case.

@SomeoneToIgnore
Copy link
Contributor Author

That's not the end user experience we should strive for.

Agree, that's why I added multiple files into the DetachedFiles variant.
Yet I'm not sure how to react on the dynamic file opening in VSCode to start hacking with the server side 😞
I'll dig more later.

Meanwhile, the FIXME is added.

@matklad
Copy link
Member

matklad commented May 24, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented May 24, 2021

@bors bors bot merged commit 05fc97e into rust-lang:master May 24, 2021
@SomeoneToIgnore SomeoneToIgnore deleted the open-single-files branch May 24, 2021 12:50
bors bot added a commit that referenced this pull request May 26, 2021
8995: fix: Create tasks for all workspaces r=matklad a=SomeoneToIgnore

Follow-up of #8955 (comment)

Before: 
<img width="593" alt="image" src="https://user-images.githubusercontent.com/2690773/119575267-712b5300-bdbf-11eb-833c-f688f7a7dd0f.png">

After: 
<img width="643" alt="image" src="https://user-images.githubusercontent.com/2690773/119575273-74264380-bdbf-11eb-8283-a78bbcb7346e.png">

This is the first time I've used multiple workspaces feature in VSCode, but so far looks like
* opening detached files works
* run and debug lens work
* Rust Analyzer: Run action works
* run task works and now shows tasks for all workspaces
* there are no platform-specific changes involved

Co-authored-by: Kirill Bulatov <[email protected]>
@boehs
Copy link

boehs commented May 31, 2021

Thank you. Thank you so much. I am on the verge of tears.

@runiq
Copy link

runiq commented Jun 10, 2021

So I tried to enable this in Neovim; and I'm still getting "Failed to discover workspace" with the newest r-a release (which should have #9091 included). It seems that #9091 implies some required support in the LSP client?

@SomeoneToIgnore
Copy link
Contributor Author

Currently the r-a server requires detachedFiles with the absolute paths to the standalone files passed as a field of the initialization options: https://github.com/rust-analyzer/rust-analyzer/pull/8955/files#diff-bca0ed357f47caaadd008edc9b2df19f36869583b78c2c0265b9f0db3650df2eR54

@boehs
Copy link

boehs commented Jun 11, 2021

Currently the r-a server requires detachedFiles with the absolute paths to the standalone files passed as a field of the initialization options: https://github.com/rust-analyzer/rust-analyzer/pull/8955/files#diff-bca0ed357f47caaadd008edc9b2df19f36869583b78c2c0265b9f0db3650df2eR54

How can I, as a rust analyzer user, fix this? I use RA vscode

@SomeoneToIgnore
Copy link
Contributor Author

SomeoneToIgnore commented Jun 11, 2021

I don't think there's anything to fix in my quote, this is the way it works in vscode currently: if you use a server that's new enough, it's able to accept and process this parameter; if you use vscode plugin that's new enough, it's able to send this parameter when a detached file is opened with the code foo.rs way.
On my OS, it currently works with the freshmost nightly plugin and server.

If you have any issues with opening a detached file via code command, I suggest filing a separate issue with the details.

If you want to support arbitrary detached files being dynamically added to an already open vscode instance, that's not supported currently and will require plenty of changes along the way (protocol extensions to let the server dynamically know about the new files, server side refactoring for better dynamic project update and the feature implementation itself) and won't happen in the near future unless somebody works on that.

@boehs
Copy link

boehs commented Jun 12, 2021

Thank you, the dynamic opening is likely my issue, good to know the solution.

simrat39 added a commit to simrat39/neovim that referenced this pull request Jul 19, 2021
* Some language servers *cough*rust-analyzer*cough* need an empty/custom
  workspaceFolders for certain usecases. For example, rust-analyzer
  needs an empty workspaceFolders table for standalone file support
  (See rust-lang/rust-analyzer#8955).

* This can also be useful for other languages that need to commonly
  open a certain directory (like flutter or lua), which would help
  prevent spinning up a new language server altogether.

* In case no workspaceFolders are passed, we fallback to what we had
  before.
mfussenegger pushed a commit to neovim/neovim that referenced this pull request Jul 20, 2021
…15132)

Some language servers *cough*rust-analyzer*cough* need an empty/custom
workspaceFolders for certain usecases. For example, rust-analyzer
needs an empty workspaceFolders table for standalone file support
(See rust-lang/rust-analyzer#8955).

This can also be useful for other languages that need to commonly
open a certain directory (like flutter or lua), which would help
prevent spinning up a new language server altogether.

In case no workspaceFolders are passed, we fallback to what we had
before.
@oblitum
Copy link
Contributor

oblitum commented Sep 9, 2021

I have standalone cargo-less files working in coc-rust-analyzer, added dynamically (got just a single r-a instance running for multiple, file-path unrelated, rust files opened after the first one: cd /tmp/a/, nvim foo.rs, :e ../b/bar.rs, :e ../c/baz.rs, etc).

coc-rust-analyzer has once added support for this newer DetachedFiles feature, but it has been eventually amended on top of a late more fundamental change at workspace detection in core coc.nvim that would send null when a workspace folder isn't inferred (which is backed by spec at: "It can be null if the client supports workspace folders but none are configured").

So I assume r-a works just fine for standalone files when receiving null as workspaceFolders? Regardless of the DetachedFiles feature? It seems to me these two points in the spec intersect somehow.

EDIT:

See newer test bellow, this one is actually invalid.

@SomeoneToIgnore
Copy link
Contributor Author

I've experimented with similar thing in VSCode: it can react on any file that's open in the current editor and try to resolve that opened file's path against current workspace, either returning the relative(?) path for workspace files or null for failed resolutions.

If I'm not mistaken, you describe the same functionality.
The main culprit here is the fact, that there's files that are not the part of the VSCode workspace, that are part of cargo workspace: any stdlib file sources, any crate file sources or their dependencies' sources: they are usually all installed into directories that are different from what the current VSCode workspace includes, so null will be returned for such.

But rust-analyzer already has a proper crate metadata for such files, and adding those as standalone files would be weird at best, if not breaking.

More complexity adds the fact that VSCode API operates FS pathes whereas current cargo graph (same as the majority things in rust-analyzer) is pretty distant from any FS abstractions: to support a "is this detached file actually from some crate in the tree" check, you'll have to change the server codebase.

justinmk pushed a commit to justinmk/neovim that referenced this pull request Sep 26, 2021
…eovim#15132)

Some language servers *cough*rust-analyzer*cough* need an empty/custom
workspaceFolders for certain usecases. For example, rust-analyzer
needs an empty workspaceFolders table for standalone file support
(See rust-lang/rust-analyzer#8955).

This can also be useful for other languages that need to commonly
open a certain directory (like flutter or lua), which would help
prevent spinning up a new language server altogether.

In case no workspaceFolders are passed, we fallback to what we had
before.
@oblitum
Copy link
Contributor

oblitum commented Oct 23, 2021

I want to amend the information in the test above, it's invalid, and repeat the test with valid information, I think this is potentially pretty relevant to r-a's support of standalone cargo-less files. r-a seems to actually fit almost the same case as metals, except that it exceptionally works for files opened on start (nvim a/foo.rs), but not for files opened afterwards (:e b/bar.rs). What happened in my previous test is that coc.nvim actually has sent /tmp/a as the workspace directory, instead of sending null, and this seems the reason that made r-a "to work" on all the files, this behavior of coc.nvim is actually an issue (see neoclide/coc.nvim#3375), I expected it to have sent null, but it did not, it just do that in a hard-coded case at the moment, which is when files are opened from home directory. So, without further ado, let's redo this test while opening files from home, which allows us to asses r-a's behavior when actually no workspace is being provided:

  1. cd ~
  2. mkdir a
  3. touch a/foo.rs
  4. mkdir b
  5. touch b/bar.rs
  6. nvim a/foo.rs
  7. :CocCommand workspace.workspaceFolders (no workspace folder reported, on previous test this was /tmp/a)
  8. type some code (in foo.rs), you'll see rust analyzer up and working
  9. :e b/bar.rs
  10. :CocCommand workspace.workspaceFolders (no workspace folder reported, on previous test this was /tmp/a)
  11. type some code (in bar.rs), you'll see rust analyzer is absent (differently from previous test on /tmp)

If you apply this test with coc-sh instead of coc-rust-analyzer (for shell files of course), bash-lsp works without problems, because it has no issues providing its services on null workspaces, for standalone files, and without any protocol extension.

(If for some reason the above isn't clear enough, feel free to mail me if you wish a link with a very detailed description of what this is about, which is what made me revisit r-a's case. It may make it clearer where r-a lies in this problem scope)

cc @fannheyward

lewis6991 pushed a commit to lewis6991/neovim that referenced this pull request Dec 12, 2021
…eovim#15132)

Some language servers *cough*rust-analyzer*cough* need an empty/custom
workspaceFolders for certain usecases. For example, rust-analyzer
needs an empty workspaceFolders table for standalone file support
(See rust-lang/rust-analyzer#8955).

This can also be useful for other languages that need to commonly
open a certain directory (like flutter or lua), which would help
prevent spinning up a new language server altogether.

In case no workspaceFolders are passed, we fallback to what we had
before.
@mikayla-maki mikayla-maki mentioned this pull request Oct 28, 2022
40 tasks
@arryhere
Copy link

arryhere commented May 21, 2023

is there a guide on how to enable all this is vs-code ? no one seems to talk about it on how to actually use it..

@SomeoneToIgnore
Copy link
Contributor Author

all this

There's nothing but the ability to open a standalone file with code, as code foo.rs
Works for me still:
image

Due to impl limitations described above, all consequent standalone file opens in the same editor won't work.

@UnkwUsr
Copy link

UnkwUsr commented Jul 11, 2023

Note for neovim users there: you probably should use rust-tools.nvim, as lspconfig won't support this language-specific feature.

@srtnnm
Copy link

srtnnm commented Jan 19, 2024

image
nice job! yeah, it really works
but i can't really understand why rust-analyzer still gives an errors
i think it must be an warning or something like "not in workspace"

@koiralakiran1
Copy link

koiralakiran1 commented Mar 27, 2024

I see that for standalone files, the auto-complete works but some errors/warnings aren't detected. I'm not sure if I'm missing something. For standalone files, some errors like missing semicolons do show up though but others like unknown functions don't.

For cargo project:
image

For standalone/detached files opened with code test.rs.
image

I'm running the project on WSL if that matters. I've tried on windows & mac as well to see if there's a difference. I'll use a cargo project moving forward but I'd love to have RA work on a standalone file.

rust-analyzer version: 0.3.1896-standalone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow opening rust-analyzer on a single file
9 participants