Issue 81508 fix #83669

kwj2104 · 2021-03-30T03:08:22Z

Problem: When variable name is used incorrectly as path, error and warning point to undeclared/unused name, when in fact the name is used, just incorrectly (should be used as a variable, not part of a path).

Summary for fix: When path resolution errs, diagnostics checks for variables in ValueNS that have the same name (e.g., variable rather than path named Foo), and adds additional suggestion that user may actually intend to use the variable name rather than a path.

The fix does not suppress or otherwise change the warning that results. I did not find a straightforward way in the code to modify this, but would love to make changes here as well with any guidance.

rust-highfive · 2021-03-30T03:08:25Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @varkor (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

JohnTitor

Also, could you rebase instead of merging as per https://rustc-dev-guide.rust-lang.org/git.html#no-merge-policy?

JohnTitor · 2021-03-30T05:55:52Z

compiler/rustc_resolve/src/lib.rs

+                            {
+                                suggestion = Some((
+                                    vec![(path_span, format!("{}", ident))],
+                                    String::from("try using variable instead of path"),


IMO:

Suggested change

String::from("try using variable instead of path"),

String::from("try using the similarly named variable instead"),

Thanks for the feedback - I've made the wording edits and rebased.

JohnTitor · 2021-03-30T19:27:48Z

(Note that you messed up the submodules, you should drop them from the commits.)

JohnTitor · 2021-03-31T07:47:29Z

Okay, thanks for rebasing, the last thing I'd like to ask is to squash commits into one, could you?

varkor · 2021-03-31T15:52:35Z

src/test/ui/resolve/issue-81508.stderr

+   |                        ^^^-----
+   |                        |
+   |                        use of undeclared type `Foo`
+   |                        help: try using the similarly named variable instead: `Foo`


I think this message is confusing, because it seems to suggest using Foo, which the user is already doing. I think we should instead point to the declaration of Foo and say something like:

`Foo` is defined here, but is not a type

This makes sense. So we're on the same page, I'm imagining (for the example above):

error[E0433]: failed to resolve: use of undeclared type `Baz` --> main.rs:4:20 | 4 | println!("{}", Baz::Bar); | ^^^----- | | | use of undeclared type `Baz` | help: Baz is defined but not a type | 3 | let Baz: &str = ""; | ^^^ error: aborting due to previous error

EDIT: Think I found a path forward

@kwj2104: yes, that looks like the sort of thing I had in mind!

New format for errors now points to declaration line:

error[E0433]: failed to resolve: use of undeclared type `Baz` --> $DIR/issue-81508.rs:11:20 | LL | let Baz: &str = ""; | --- help: Baz is defined here, but is not a type LL | LL | println!("{}", Baz::Bar); | ^^^ use of undeclared type `Baz`

varkor

This is looking pretty good now! Most of my comments are very minor.

compiler/rustc_resolve/src/lib.rs

compiler/rustc_resolve/src/late.rs

compiler/rustc_resolve/src/lib.rs

varkor · 2021-04-11T15:28:06Z

Thanks @kwj2104: this looks great!

@bors r+

bors · 2021-04-11T15:28:08Z

📌 Commit f51f25a has been approved by varkor

Rollup of 7 pull requests Successful merges: - rust-lang#83669 (Issue 81508 fix) - rust-lang#84014 (Improve trait/impl method discrepancy errors) - rust-lang#84059 (Bump libc dependency of std to 0.2.93) - rust-lang#84067 (clean up example on read_to_string) - rust-lang#84079 (Improve test for `rustdoc::bare_urls` lint) - rust-lang#84094 (Remove FixedSizeArray) - rust-lang#84101 (rustdoc: Move crate loader to collect_intra_doc_links::early ) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

bors · 2021-04-12T03:13:25Z

⌛ Testing commit f51f25a with merge e41f378...

rust-highfive assigned varkor Mar 30, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 30, 2021