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

Improve E0433, so that it suggests missing imports #72923

Merged
merged 2 commits into from
Jun 4, 2020

Conversation

Patryk27
Copy link
Contributor

@Patryk27 Patryk27 commented Jun 2, 2020

Closes #52468

@rust-highfive
Copy link
Collaborator

r? @ecstatic-morse

(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 Jun 2, 2020
@ecstatic-morse
Copy link
Contributor

r? @estebank

@estebank
Copy link
Contributor

estebank commented Jun 2, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Jun 2, 2020

📌 Commit d31d215 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 Jun 2, 2020
Co-authored-by: Esteban Kuber <estebank@users.noreply.github.com>
@estebank
Copy link
Contributor

estebank commented Jun 3, 2020

@bors r+

@bors
Copy link
Contributor

bors commented Jun 3, 2020

📌 Commit c55d55e has been approved by estebank

bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 4, 2020
Rollup of 6 pull requests

Successful merges:

 - rust-lang#72718 (Add regression test for rust-lang#72554)
 - rust-lang#72782 (rustc_target: Remove `pre_link_args_crt`)
 - rust-lang#72923 (Improve E0433, so that it suggests missing imports)
 - rust-lang#72950 (fix `AdtDef` docs)
 - rust-lang#72951 (Add Camelid per request)
 - rust-lang#72964 (Bump libc dependency to latest version (0.2.71))

Failed merges:

r? @ghost
@bors bors merged commit 085c16d into rust-lang:master Jun 4, 2020
@Patryk27 Patryk27 deleted the fix/52468 branch June 4, 2020 15:50
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 19, 2021
Fix popping singleton paths in when generating E0433

Fixes rust-lang#82156

---

This was introduced with rust-lang#72923, so pinging `@Patryk27` for reviews.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Nov 1, 2022
Fix E0433 No Typo Suggestions

Fixes rust-lang#48676
Fixes rust-lang#87791
Fixes rust-lang#96625
Fixes rust-lang#95462
Fixes rust-lang#101637
Follows up PR rust-lang#72923

Several open issues refer to the problem that E0433 does not suggest typos like other errors normally do. This fix augments the implementation of PR rust-lang#72923.

**Background**
When the path of a function call, e.g. `Struct::foo()`, involves names that cannot be resolved, there are two errors that could be emitted by the compiler:
 - If `Struct` is not found, it is ``E0433: failed to resolve: use of undeclared type `Struct` ``.
 - If `foo` is not found in `Struct`, it is ``E0599: no function or associated item named `foo` found for struct `Struct` in the current scope``

When a name is used as a type, `e.g. fn foo() -> Struct`, and the name cannot be resolved, it is ``E0412: cannot find type `Struct` in this scope``.

Before rust-lang#72923, `E0433` does not implement any suggestions, and the PR introduces suggestions for missing `use`s. When a resolution error occurs in the path of a function call, it tries to smart resolve just the type part of the path, e.g. `module::Struct` of a call to `module::Struct::foo()`. However, along with the suggestions, the smart-resolve function will report `E0412` since it only knows that it is a type that we cannot resolve instead of being a part of the path. So, the original implementation swap out `E0412` errors returned by the smart-resolve function with the real `E0433` error, but keeps the "missing `use`" suggestions to be reported to the programmer.

**Issue**
The current implementation only reports if there are "missing `use`" suggestions returned by the smart-resolve function; otherwise, it would fall back the normal reporting, which does not emit suggestions. But the smart-resolve function could also produce typo suggestions, which are omitted currently.

Also, it seems like that not all info has been swapped out when there are missing suggestions. The error message underlining the name in the snippet still says ``not found in this scope``, which is a `E0412` messages, if there are `use` suggestions, but says the normal `use of undeclared type` otherwise.

**Fixes**
This fix swaps out all fields in `Diagnostic` returned by the smart-resolve function except for `suggestions` with the current error, and merges the `suggestions` of the returned error and that of the current error together. If there are `use` suggestions, the error is saved to `use_injection` to be reported at the end; otherwise, the error is emitted immediately as `Resolver::report_error` does.

Some tests are updated to use the correct underlining error messages, and one additional test for typo suggestion is added to the test suite.

r? rust-lang/diagnostics
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.

Foo::bar() invokes do not give suggestions for Foo when it is not in scope
5 participants