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

suggestion for typoed crate or module #89347

Merged
merged 1 commit into from Oct 14, 2021

Conversation

TaKO8Ki
Copy link
Member

@TaKO8Ki TaKO8Ki commented Sep 29, 2021

Previously, the compiler didn't suggest similarly named crates or modules. This pull request adds a suggestion for typoed crates or modules.

#76208

before:

error[E0433]: failed to resolve: use of undeclared type or module `chono`
 --> src/main.rs:2:5
  |
2 | use chono::prelude::*;
  |     ^^^^^ use of undeclared type or module `chono`

after:

error[E0433]: failed to resolve: use of undeclared type or module `chono`
 --> src/main.rs:2:5
  |
2 | use chono::prelude::*;
  |     ^^^^^ 
  |     |
  |     use of undeclared crate or module `chono`
  |     help: a similar crate or module exists: `chrono`

@rust-highfive
Copy link
Collaborator

r? @wesleywiser

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 29, 2021
@rust-log-analyzer

This comment has been minimized.

@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Sep 29, 2021

@wesleywiser
Only test/ui/hygiene/extern-prelude-from-opaque-fail.rs failed. Could you give me some advice to solve this problem?

What I want to solve is:

In Zulip

In test/ui/hygiene/extern-prelude-from-opaque-fail.rs, a decl_macro a declares extern crate core as my_core internally and my_core::mem::drop(0) is called in a f() function outside the decl_macro. So my_core::mem::drop(0) causes a failed to resolve error, but the compiler suggests a similar crate name like the following which the function cannot use.

error[E0433]: failed to resolve: use of undeclared crate or module `my_core`
  --> $DIR/extern-prelude-from-opaque-fail.rs:24:14
   |
LL |     fn f() { my_core::mem::drop(0); }
   |              ^^^^^^^
   |              |
   |              use of undeclared crate or module `my_core`
   |              help: a similar crate or module exists: `my_core`

@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Oct 1, 2021

r? @estebank

@rust-highfive rust-highfive assigned estebank and unassigned wesleywiser Oct 1, 2021
@wesleywiser
Copy link
Member

I think to fully solve that issue, you'll need to do something very similar to what the identifier resolution does and traverse the scope to see what modules are currently in scope (see https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_resolve/lib.rs.html#1729-1740 which is probably what you need to call). That seems very messy though.

@estebank do you know of a better way?

@estebank
Copy link
Contributor

estebank commented Oct 1, 2021

@wesleywiser I think that you are right that that is the "principled" approach, but I think we should land this PR as is as a stopgap (without closing the original report so we can clean this up later). What do you think?

@wesleywiser
Copy link
Member

Oh, yes absolutely! This seems like an obvious improvement over the status quo even if there are some edge cases.

@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Oct 1, 2021

@estebank @wesleywiser Thank you for your advice!

@TaKO8Ki TaKO8Ki requested a review from estebank October 2, 2021 04:23
@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Oct 4, 2021

@estebank So should I open a new issue about that?

Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

So should I open a new issue about that?

Please do.

.map(|sugg| {
(
vec![(ident.span, sugg.to_string())],
String::from("a similar crate or module exists"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
String::from("a similar crate or module exists"),
String::from("there is a crate or module with a similar name"),

Ideally we would get the type of what the ident is from find_similarly_named_module_or_crate so we can be more specific in the message. We can do that later.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok. I'll do it in a new PR.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Oct 6, 2021

@estebank I opened a issue about that! #89592

@estebank
Copy link
Contributor

estebank commented Oct 6, 2021

There are three tests that are failing with the same ICE:

------------------------------------------
stderr:
------------------------------------------
thread 'rustc' panicked at '`ModuleData::def_id` is called on a block module', compiler/rustc_resolve/src/lib.rs:607:27
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.57.0-nightly (86650600b 2021-10-06) running on x86_64-unknown-linux-gnu

note: compiler flags: -Z threads=1 -Z ui-testing -Z deduplicate-diagnostics=no -Z emit-future-incompat-report -Z save-analysis -C codegen-units=1 -C prefer-dynamic -C rpath -C debuginfo=0

query stack during panic:
end of query stack

------------------------------------------



failures:
    [ui] ui/proc-macro/weird-hygiene.rs
    [ui] ui/save-analysis/issue-59134-0.rs
    [ui] ui/save-analysis/issue-59134-1.rs

This can likely be prevented by looking at the module kind before calling def_id() or using opt_def_id() instead.

@TaKO8Ki TaKO8Ki requested a review from estebank October 6, 2021 12:57
compiler/rustc_resolve/src/lib.rs Outdated Show resolved Hide resolved
compiler/rustc_resolve/src/lib.rs Outdated Show resolved Hide resolved
@petrochenkov
Copy link
Contributor

I think that you are right that that is the "principled" approach, but I think we should land this PR as is as a stopgap

I understand this when we are talking about high-priority bugfixes, but with diagnostics this approach already turned rustc_parser into a nuclear garbage dump.

@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Oct 12, 2021

@estebank @petrochenkov
Then, should I solve this problem in this PR?

@estebank
Copy link
Contributor

If this is a temporary state of affairs, I'm ok landing this as is.

@petrochenkov The "principled" approach will complicate resolve and be potentially more intrusive compared to what is being put forward here. It is the principled approach because it is more thorough. The changes here are relegated into the diagnostics module and a single method call.

@TaKO8Ki can you please commit to follow up on this soon?

@bors r+

@bors
Copy link
Contributor

bors commented Oct 12, 2021

📌 Commit f0a611b6b08397f41505fc0085e11f8cb0943fe6 has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 12, 2021
@Mark-Simulacrum
Copy link
Member

Could we squash commits first, please? There's a few typo fixes and similar that seem like they would be nice to squash away. @bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 12, 2021
avoid suggesting the same name

sort candidates

fix a message

use `opt_def_id` instead of `def_id`

move `find_similarly_named_module_or_crate` to rustc_resolve/src/diagnostics.rs
@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Oct 13, 2021

@estebank

can you please commit to follow up on this soon?

Yes.

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 13, 2021

📌 Commit f819e6d has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 13, 2021
the8472 added a commit to the8472/rust that referenced this pull request Oct 13, 2021
…ebank

suggestion for typoed crate or module

Previously, the compiler didn't suggest similarly named crates or modules. This pull request adds a suggestion for typoed crates or modules.

rust-lang#76208

before:

```
error[E0433]: failed to resolve: use of undeclared type or module `chono`
 --> src/main.rs:2:5
  |
2 | use chono::prelude::*;
  |     ^^^^^ use of undeclared type or module `chono`
```

after:

```
error[E0433]: failed to resolve: use of undeclared type or module `chono`
 --> src/main.rs:2:5
  |
2 | use chono::prelude::*;
  |     ^^^^^
  |     |
  |     use of undeclared crate or module `chono`
  |     help: a similar crate or module exists: `chrono`
```
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 13, 2021
…askrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#89347 (suggestion for typoed crate or module)
 - rust-lang#89670 (Improve `std::thread::available_parallelism` docs)
 - rust-lang#89757 (Use shallow clones for submodules)
 - rust-lang#89759 (Assemble the compiler when running `x.py build`)
 - rust-lang#89846 (Add `riscv32imc-esp-espidf` to 1.56 changelog)
 - rust-lang#89853 (Update the 1.56.0 release header for consistency)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit efac68b into rust-lang:master Oct 14, 2021
@rustbot rustbot added this to the 1.57.0 milestone Oct 14, 2021
@TaKO8Ki TaKO8Ki deleted the crate-or-module-typo branch October 15, 2021 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants