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

suggest crate::... for "local" paths in 2018 #54230

Closed
nikomatsakis opened this issue Sep 14, 2018 · 5 comments · Fixed by #54391
Closed

suggest crate::... for "local" paths in 2018 #54230

nikomatsakis opened this issue Sep 14, 2018 · 5 comments · Fixed by #54391
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-edition-2018 Area: The 2018 edition A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-resolve Area: Name/path resolution done by `rustc_resolve` specifically T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

The following code, when compiled in Rust 2018, errors out (as expected). However, the error could be more helpful.

mod foo {
    type Bar = u32;
}

use foo::Bar;

fn main() {
    let x: Bar = 22;
}

Error:

error[E0463]: can't find crate for `foo`
 --> src/main.rs:5:5
  |
5 | use foo::Bar;
  |     ^^^ can't find crate

error: aborting due to previous error

but I think we should suggest something like

did you mean `crate::foo::Bar`?
@nikomatsakis nikomatsakis added A-resolve Area: Name/path resolution done by `rustc_resolve` specifically T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-edition-2018-lints labels Sep 14, 2018
@csmoe csmoe added the A-diagnostics Area: Messages for errors, warnings, and lints label Sep 14, 2018
@davidtwco davidtwco self-assigned this Sep 14, 2018
@nikomatsakis
Copy link
Contributor Author

nikomatsakis commented Sep 14, 2018

Some thoughts:

Ideally, we would compare against two things:
- the names of crates in the prelude etc
- the names of items defined in the root module (to some approximation)

and offer appropriate suggestions. Not sure if there is code that tries to do either of those already. I'd have to look around.

cc @petrochenkov @eddyb

@eddyb
Copy link
Member

eddyb commented Sep 14, 2018

I'm mostly aware of the one speculative lookup I've introduced:

let binding = self.resolve_ident_in_module_unadjusted(
ModuleOrUniformRoot::Module(self_module),
ident,
ns,
restricted_shadowing,
record_used,
path_span,
);
// FIXME(eddyb) This may give false negatives, specifically
// if a crate with the same name is found in `extern_prelude`,
// preventing the check below this one from returning `binding`
// in all cases.
//
// That is, if there's no crate with the same name, `binding`
// is always returned, which is the result of doing the exact
// same lookup of `ident`, in the `self` module.
// But when a crate does exist, it will get chosen even when
// macro expansion could result in a success from the lookup
// in the `self` module, later on.
//
// NB. This is currently alleviated by the "ambiguity canaries"
// (see `is_uniform_paths_canary`) that get introduced for the
// maybe-relative imports handled here: if the false negative
// case were to arise, it would *also* cause an ambiguity error.
if binding.is_ok() {
return binding;
}

You need to set module to self.resolve_crate_root(ident) and record_used to false.

Not sure what you mean by "the names of crates in the prelude" - that's easy enough, self.extern_prelude.contains(&ident.name) - but if it's in there, it wouldn't have failed?

@davidtwco
Copy link
Member

Between starting to look at this and now, I've noticed that the error has changed to:

error[E0432]: unresolved import `foo`
  --> src/test/ui/rust-2018/local-path-suggestions.rs:17:5
   |
17 | use foo::Bar;
   |     ^^^ Did you mean `self::foo`?

error: aborting due to previous error

@nikomatsakis
Copy link
Contributor Author

@davidtwco hmm, yeah, there is technically ambiguity here I guess, in that one could write with use self or use crate. I don't know which would be considered "more idiomatic" -- I lean towards absolute paths myself, but I'm sure opinions will vary. =)

e.g., if one is doing something like this, then surely you want use self::..

enum Foo { .. }
pub use self::Foo::*;

@nikomatsakis
Copy link
Contributor Author

Also, if we modify the test ever so slightly to be more realistic:

mod foo {
    type Bar = u32;
}

mod baz {
    use foo::Bar;

    fn baz() {
        let x: Bar = 22;
    }
}

fn main() { }

then I assume you get no suggestion?

pietroalbini added a commit to pietroalbini/rust that referenced this issue Oct 2, 2018
suggest `crate::...` for "local" paths in 2018

Fixes rust-lang#54230.

This commit adds suggestions for unresolved imports in the cases where
there could be a missing `crate::`, `super::`, `self::` or a missing
external crate name before an import.

r? @nikomatsakis
bors added a commit that referenced this issue Oct 3, 2018
suggest `crate::...` for "local" paths in 2018

Fixes #54230.

This commit adds suggestions for unresolved imports in the cases where
there could be a missing `crate::`, `super::`, `self::` or a missing
external crate name before an import.

r? @nikomatsakis
@fmease fmease added A-edition-2018 Area: The 2018 edition A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. and removed A-edition-2018-lints labels Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-edition-2018 Area: The 2018 edition A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. A-resolve Area: Name/path resolution done by `rustc_resolve` specifically T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants