rust-lang / rust-analyzer

A Rust compiler front-end for IDEs
https://rust-analyzer.github.io/
Apache License 2.0
14.09k stars 1.57k forks source link

Proper support for standalone/detached files #14318

Open Veykril opened 1 year ago

Veykril commented 1 year ago

https://github.com/rust-lang/rust-analyzer/blob/9fca0a4afefead3daf8f66fd357999d7cd520880/crates/project-model/src/workspace.rs#L86-L96

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.

Fixing this would be beneficial to the rustlings project, removing the need for the rust-project.json generation step they currently employ.

HKalbasi commented 1 year ago

It might also make sense to adding a syntax for declaring dependencies inline via comments in detached files. It will also helps for #9343 as evcxr currently handles dependencies in jupyter notebooks via :deps dep = "x.y.z" commands.

Veykril commented 1 year ago

We can't really support that, to resolve dependencies we need cargo which in turns needs a proper cargo workspace

HKalbasi commented 1 year ago

We probably need to have a mirror of the project in a temp directory with a "normal" project structure at that point, which is what evcxr is doing if I understand correctly.

Veykril commented 1 year ago

https://github.com/rust-lang/rfcs/pull/3424 will be interesting to us here. Unsure how we can support this but we probably would need to handle it similar to build scripts, that is we analyze the file without dep info first/invoke it initially with cargo metadata once, and afterwards keep track of the dependencies comment. If it changes re-run metadata.

victor-timofei commented 1 year ago

Is anyone working on this? If not, I'd love to pick it up.

Veykril commented 1 year ago

No one is working on this right now, though it's also not quite clear how to tackle this. I personally don't really understand the idea described in the comment tbh, I took a look at this at some point and the comment didn't really make sense to me in terms of how things are working in r-a right now.

victor-timofei commented 1 year ago

I did a little bit of experimentation and the way I found that this currently works (tried it on Neovim with lspconfig) is like:

local lsp_conf = require('lspconfig')

lsp_conf.rust_analyzer.setup {
    -- Other setup stuff
    -- ...

    -- To be able to start rust-analyzer at all a root_dir is needed,
    -- I added a star match for testing purposes to match on any file
    -- This could be an lspconfig/Neovim only issue
    root_dir = lsp_conf.util.root_pattern("*"),

    -- LSP Client needs to know the detached files ahead of time on
    -- initialization and pass them to the r-a on startup
    init_options = { detachedFiles = { "/tmp/file.rs" } }
}

r-a Client for VSCode does something similar here.

And the PR for the initial support is #8955

victor-timofei commented 1 year ago

What I currently do not understand yet why would this require a change on the salsa-level(does this mean on the upstream salsa crate?).

The way I understand this it would require to hide the CrateGraph behind an API that would query by default the current CrateGraph and on a failure to fallback to querying the detached files I guess.

I would guess at the time of writing reload.rs was the one responsible for discovering the files and adding the to the CrateGraph?

Veykril commented 1 year ago

Maybe @matklad can explain what they meant with the comment there since it was written by them

matklad commented 1 year ago

This has to do with durabilities :

https://rust-analyzer.github.io/blog/2023/07/24/durable-incrementality.html

if we add an ability to open files in an ad-how way, we should make sure that opening such a file does not require to revalidate the entire graph of queries.

This is in contrast to assign a new dependency via cargo add —- in that case, it’s OK to revalidate everything, because that’s a rare explicit operation, that could be slow.

right now, each detached file is treated as a separate crate, and all crates are a part of a singleton crate graph query.s thwt means that:

To fix this, we need to go from a single crate graph query to something more fine grained, so that we can add new detached files without disturbing Durable parts of the query graph.

HKalbasi commented 1 year ago

Opening a detached file (in case of cargo scripts) is going to change the crate graph anyway. One thing we can do is separating the query graphs of workspaces. Currently GlobalState can track multiple ProjectWorkspaces, but it merges all of their crate graphs. I guess it is possible to split those crate graphs, so that each ProjectWorkspace get its own crate graph (It wastes a huge amount of memory on loading multiple stds, and it makes some challenges on ide features for files inside multiple workspaces, but I think it is doable) so opening a new detached file won't invalidate analysis on the old files and workspaces. But does it matter? After opening a file, the user probably needs analysis in that file, not somewhere else, and that is going to take same amount of time, if not more.

Opening a detached file as a cargo script is as slow as opening a new cargo project, and I don't see how preventing things being invalidated will make it fast. By opening a new cargo script, we will discover a whole new cargo toml which some of its dependencies might not be even downloaded yet, I don't think any change in salsa can make that fast.

That said, we may still be able to do something. We can try to detect which files are going to be opened in the detached mode (for example by looking at the shebang) and constructing the correct project workspace and crate graph from the beginning. It won't help in opening random files from random places (and I don't think we can do anything for that), but will help in projects that are multiple detached files in a folder, similar to the rustling.

Veykril commented 5 months ago

While the crate graph issues is problematic in terms of perf, I think it should be fine for us to skip that optimization for now (at least in favor of cargo script workspaces). The main issue with these detached files / cargo script workspaces are that we cannot eagerly support them, as we would need to built the full module graph for the project to figure out if a file is detached or not which is too much up front work. So at best we can support these lazily, which while not perfect is still decent I think. That is, once such a file is opened we can permanently add it to the known workspaces (I wonder if we should do the same for undiscovered workspaces).

mo8it commented 2 months ago

Small note about the issue description: Rustlings wouldn't benefit from this anymore after the release of version 6. It uses an automatically updated Cargo.toml file that contains all exercises and solutions as binaries.

davidbarsky commented 2 months ago

Small note about the issue description: Rustlings wouldn't benefit from this anymore after the release of version 6. It uses an automatically updated Cargo.toml file that contains all exercises and solutions as binaries.

@mo8it Congratulations on releasing Rustlings 6.0! Depending on your interest/desire to maintain that Cargo.toml file, https://github.com/rust-lang/rust-analyzer/pull/17246 might be helpful here, as that'd make finding/loading rust-project.json files much more Cargo-like.

(rust-analyzer still very much needs better standalone file support for the reasons I described in https://github.com/rust-lang/rust-analyzer/issues/17537.)