Skip to content
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

CARGO: fetch actual info about stdlb #6417

Merged
merged 7 commits into from Dec 7, 2020
Merged

CARGO: fetch actual info about stdlb #6417

merged 7 commits into from Dec 7, 2020

Conversation

Undin
Copy link
Member

@Undin Undin commented Nov 20, 2020

Previously, we always used hardcoded info about stdlib: its packages and their relationships.
But it prevented us from getting actual info about stdlib packages: dependencies, edition, features, etc.
As a result, some code in stdlib that depends on third-party crates (like cfg-if) cannot be properly taken into account while name resolution and type inference.

These changes introduce an option (as an experimental feature for now) to fetch actual info about stdlib packages.
Due to the fact that stdlib sources contain Cargo.toml, it's possible just to run cargo metadata to get all necessary info.
It allows us to get proper info about:

  • edition. When stdlib started to use 2018 edition, plugin name resolution was broken in some places because of it. See Cannot find declaration of BTreeMap and BTreeSet #3835. We solved this issue by hardcoded edition for root stdlib packages depending on rustc version. Actual info about edition allows avoiding similar issues for future editions like edition 2021.
  • dependencies. It fixes name resolution in cases when third-party libraries are used in stdlib packages. The most known case is the usage of cfg-if lib in std::os module that provides different API for different operating systems. Now name resolution and completion for child modules of std::os should work as expected.
  • features. They also are taken into account while name resolution

2020-11-20 13 41 58

Restrictions:

Known issues and future improvements:

  • There isn't UI option to enable it. Currently, the feature can be enabled via org.rust.cargo.fetch.actual.stdlib.metadata experimental feature
  • stdlib dependencies are shown as project dependencies. They should be part of stdlib node
  • Progress is not shown in Sync tool window
  • Name resolution doesn't work properly for stdlib items inside stdlib dependencies because of rustc-std-workspace-core dependency and its friends. Not sure that we want to fix it

Closes #3864

Main step to fix the following issues: #3957 #4960 #5881 #6081
Part of #6104

changelog: provide a way to fetch actual info about stdlib packages like dependencies, edition, Cargo features. It should help the plugin better understand stdlib structure, for example, provide proper completion and navigation for items defined in std::os module. Note, the corresponding feature works since Rust 1.41 and disabled by default for now. You can enabled it via org.rust.cargo.fetch.actual.stdlib.metadata option in Experimental Feature dialog (Help | Find Action and type Experimental features to open the dialog)

@Undin Undin added the feature label Nov 20, 2020
@Undin Undin force-pushed the undin/stdlib-metadata branch 2 times, most recently from 95dead3 to 83cb266 Compare November 23, 2020 20:13
bors bot added a commit that referenced this pull request Nov 26, 2020
6438: STUB: create stubs for lifetime parameters only if needed r=vlad20012 a=Undin

Original issue was found in [gimli](https://github.com/gimli-rs/gimli/blob/d36fcf11c878c8ebfb4da4cad300773869b5b5e4/examples/simple.rs#L38-L41) project that now is part of stdlib sources (stdlib dependency).

Also, print file url in `RsLazyBlockStubCreationTestBase` tests. It may help to find file with problem code and even open it in your IDE if it's located in local file system.

Fix the corresponding test in #6374 and #6417

Co-authored-by: Arseniy Pendryak <a.pendryak@yandex.ru>

if (!stdlibDependenciesDir.exists()) {
try {
cargo.vendorDependencies(project, testPackageSrcDir, stdlibDependenciesDir)
Copy link
Member

Choose a reason for hiding this comment

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

Looks like testPackageSrcDir is needed only in this branch. Let's move its calculations into the branch

Copy link
Member

Choose a reason for hiding this comment

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

Let's add a short comment about why libtest is used as a root for vendor.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure it's a good idea to mutate rust src directory (i.e. store "vendor" dir there if it doesn't exist). If rustup is not used, it can be any user-specified directory, maybe even located in a read-only filesystem. If rustup is used, well, latter versions may decide to delete it, for example.

I think it's ok for now, but we should rethink it before enabling this feature by default.

We don't support old Cargo versions that cannot provide `workspace_members` field in `cargo metadata`
Previously, we always used hardcoded info about stdlib: its packages and their relationships.
But it prevented us from getting actual info about stdlib packages: dependencies, edition, features, etc.
As a result, some code in stdlib that depends on third-party crates (like `cfg-if`) cannot be properly taken into account while name resolution and type inference.

These changes introduce an option (as an experimental feature for now) to fetch actual info about stdlib packages.
Due to the fact that stdlib sources contain `Cargo.toml`, it's possible just to run `cargo metadata` to get all necessary info.
It allows us to get proper info about:
- edition. When stdlib started to use `2018` edition, plugin name resolution was broken in some places because of it. See #3835. We solved this issue by hardcoded edition for root stdlib packages depending on `rustc` version. Actual info about edition allows avoiding similar issues for future editions like edition 2021.
- dependencies. It fixes name resolution in cases when third-party libraries are used in stdlib packages. The most known case is the usage of `cfg-if` lib in `std::os` module that provides different API for different operating systems. Now name resolution and completion for child modules of `std::os` should work as expected.
- features. They also are taken into account while name resolution
Otherwise, some name resolution doesn't work correctly
Previously, we used hardcoded options that prevented us from checking os specific things
@vlad20012
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Dec 7, 2020

Build succeeded:

@bors bors bot merged commit 4012ce1 into master Dec 7, 2020
@bors bors bot deleted the undin/stdlib-metadata branch December 7, 2020 19:51
@github-actions github-actions bot added this to the v137 milestone Dec 7, 2020
@Kobzol
Copy link
Member

Kobzol commented Dec 16, 2020

Just a heads-up: rust-lang/rust#78790 is being (probably temporarily) removed (rust-lang/rust#80082). Not sure if it relates to this PR.

@Undin
Copy link
Member Author

Undin commented Dec 16, 2020

@Kobzol thanks for the worry!
I've already known about this revert. It's a pity, of course, but implementation from this PR can download dependencies for stdlib packages (i.e. invoke cargo vendor) itself if vendor lib cannot be found in stdlib sources so the revert shouldn't break this PR, only slows the first call of project structure loading

bors bot added a commit that referenced this pull request Jan 12, 2021
6609: CARGO: download stdlib dependencies into system dir r=vlad20012 a=Undin

Previously, the plugin downloaded stdlib dependencies (call `cargo vendor`) into src directory.
But such approach didn't take into account read-only file systems and stdlib updates properly.

Now, stdlib dependencies are downloaded into IDE system dir and its path depends on rustc version

Continuation of #6417
Fixes #6603

changelog: TODO


Co-authored-by: Arseniy Pendryak <a.pendryak@yandex.ru>
bors bot added a commit that referenced this pull request Jan 12, 2021
6609: CARGO: download stdlib dependencies into system dir r=vlad20012 a=Undin

Previously, the plugin downloaded stdlib dependencies (call `cargo vendor`) into src directory.
But such approach didn't take into account read-only file systems and stdlib updates properly.

Now, stdlib dependencies are downloaded into IDE system dir and its path depends on rustc version

Continuation of #6417
Fixes #6603

changelog: TODO


Co-authored-by: Arseniy Pendryak <a.pendryak@yandex.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants