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

Emit diagnostics for unresolved imports and extern crates #6016

Merged
merged 7 commits into from
Sep 17, 2020
Merged

Emit diagnostics for unresolved imports and extern crates #6016

merged 7 commits into from
Sep 17, 2020

Conversation

jonas-schievink
Copy link
Contributor

AFAIK, we don't have any major bugs in name resolution that would cause a lot of false positives here (except procedural attribute macro support and some rare issues around #[path] on module files), so these are not marked as experimental diagnostics right now.

I noticed that diagnostics in a file sometimes don't get displayed after opening, but require some edit to be performed. This seems like a preexisting issue though.

@bjorn3
Copy link
Member

bjorn3 commented Sep 16, 2020

Would it be possible to suppress only errors about unknown extern crate and all unresolved imports that would go through those extern crate? That make it possible to enable the unresolved imports diagnostic without giving error about imports of rustc_*.

@jonas-schievink
Copy link
Contributor Author

Hmm, that doesn't sound easy to implement. However, I just checked with an old project of mine, and you can use the same trick that worked for IntelliJ back in the day to teach rust-analyzer about dependencies on rustc crates:

[target.'cfg(NOT_A_PLATFORM)'.dependencies]
rustc = { path = "../../r2c2-rust/src/librustc" }
rustc_mir = { path = "../../r2c2-rust/src/librustc_mir" }
rustc_apfloat = { path = "../../r2c2-rust/src/librustc_apfloat" }
rustc_target = { path = "../../r2c2-rust/src/librustc_target" }
rustc_data_structures = { path = "../../r2c2-rust/src/librustc_data_structures" }
rustc_incremental = { path = "../../r2c2-rust/src/librustc_incremental" }
rustc_index = { path = "../../r2c2-rust/src/librustc_index" }
rustc_codegen_utils = { path = "../../r2c2-rust/src/librustc_codegen_utils" }
rustc_codegen_ssa = { path = "../../r2c2-rust/src/librustc_codegen_ssa" }
syntax = { path = "../../r2c2-rust/src/libsyntax" }
syntax_pos = { path = "../../r2c2-rust/src/libsyntax_pos" }
rustc_errors = { path = "../../r2c2-rust/src/librustc_errors" }
serialize = { path = "../../r2c2-rust/src/libserialize" }

(replace r2c2-rust with a rust-lang/rust checkout that matches your rust-toolchain file)

@bjorn3
Copy link
Member

bjorn3 commented Sep 16, 2020

That trick will only work on my system. Everyone has a different place they check out projects. Also most of the time my rust checkout is very outdated.

Hmm, that doesn't sound easy to implement.

If base of import is unresolved => ignore error. That is basically what rustc does. If you do use foo; use foo::bar; it will only error on use foo and probably leave something like a placeholder for foo to prevent errors on any further uses of foo.

@jonas-schievink
Copy link
Contributor Author

If base of import is unresolved => ignore error.

I know how to implement it in theory, but we don't record this information, so it's harder. I guess I can add a heuristic that checks if the first path segment corresponds to a crate we've already warned about, even though that won't catch renamed crates.

One other issue is that we'd presumably still want to keep the current behavior for crates in the extern prelude, which would make this behavior somewhat inconsistent, but if we're only trying to make use of "sourceless sysroot crates" easier this should be fine.

That trick will only work on my system. Everyone has a different place they check out projects. Also most of the time my rust checkout is very outdated.

That's fair. I can think of a few other possible solutions:

  • Add a submodule
  • Distribute compiler sources as part of the rustc-dev component and teach rust-analyzer about them (which would also enable all other functionality for them)

Of course, you can also turn off the warning entirely thanks to #5682.

@jonas-schievink
Copy link
Contributor Author

bors r+

@bors
Copy link
Contributor

bors bot commented Sep 17, 2020

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.

2 participants