Rustc does not warn about `use` with paths incompatible with `uniform_paths` for edition 2018

[orium]

Updated Description:

This code generates no warnings when compiled with the 2015 edition:

#![deny(rust_2018_compatibility)]
#![allow(unused_imports)]

extern crate rayon;

mod foo {
    mod rayon {}
    
    use rayon::Scope;
}

but when compiled with the 2018 edition fails to compile:

error[E0432]: unresolved import `rayon::Scope`
 --> src/lib.rs:9:9
  |
9 |     use rayon::Scope;
  |         ^^^^^^^^^^^^ no `Scope` in `foo::rayon`

error[E0659]: `rayon` is ambiguous (name vs any other name during import resolution)
 --> src/lib.rs:9:9
  |
9 |     use rayon::Scope;
  |         ^^^^^ ambiguous name
  |
note: `rayon` could refer to the module defined here
 --> src/lib.rs:7:5
  |
7 |     mod rayon {}
  |     ^^^^^^^^^^^^
  = help: use `self::rayon` to refer to this module unambiguously
note: `rayon` could also refer to the extern crate imported here
 --> src/lib.rs:4:1
  |
4 | extern crate rayon;
  | ^^^^^^^^^^^^^^^^^^^
  = help: use `::rayon` to refer to this extern crate unambiguously

error: aborting due to 2 previous errors

Some errors occurred: E0432, E0659.
For more information about an error, try `rustc --explain E0432`.

Old Description

Rustc does not warn about use with paths incompatible with uniform_paths for edition 2018. This means that rustfix it will not fix your code for edition 2018.

To reproduce create the following project:

Cargo.toml:

[package]
name = "foobar"
version = "0.0.0"
authors = ["Foo <example@example.com>"]

[dependencies]
criterion = "0.2"

src/lib.rs:

#![allow(unused)]
#![feature(rust_2018_preview, uniform_paths)]
extern crate criterion;
use criterion::Criterion;

If you run cargo +nightly check you will not get any warning, which means that when you run cargo +nightly fix --edition nothing will change, but the code is not compilable in rust 2018. Change Cargo.toml to enable edition 2018 (by adding cargo-features = ["edition"] and package.edition = "2018") and run cargo +nightly build:

$ cargo +nightly build
[...]
error: `criterion` import is ambiguous
 --> src/lib.rs:4:5
  |
3 | extern crate criterion;
  | ----------------------- can refer to `self::criterion`
4 | use criterion::Criterion;
  |     ^^^^^^^^^ can refer to external crate `::criterion`
  |
  = help: write `::criterion` or `self::criterion` explicitly instead
  = note: relative `use` paths enabled by `#![feature(uniform_paths)]`
[...]

cargo +nightly fix --edition should have changed line 4 to use ::criterion::Criterion.

Ref: Bug in cargo: rust-lang/cargo#5905

[zackmdavis]

The error is emitted during name resolution:

rust/src/librustc_resolve/resolve_imports.rs

Lines 721 to 733 in ee73f80

	// Special-case the error when `self::x` finds its own `use x;`.
	if has_external_crate &&
	results.module_scope == Some(span) &&
	results.block_scopes.is_empty() {
	let msg = format!("`{}` import is redundant", name);
	this.session.struct_span_err(span, &msg)
	.span_label(span,
	format!("refers to external crate `::{}`", name))
	.span_label(span,
	format!("defines `self::{}`, shadowing itself", name))
	.help(&format!("remove or write `::{}` explicitly instead", name))
	.note("relative `use` paths enabled by `#![feature(uniform_paths)]`")
	.emit();

To make this rustfix-able, we would want to reformulate the .help note as a structured suggestion instead. (With the caveat that rustfix can't yet handle overlapping/alternative suggestions, so hopefully one of the "remove" or "::cratename" alternatives can be the canonical fix.)

[eddyb]

@zackmdavis That's not enough. What needs to happen is the canaries need to be injected in Rust 2015 before switching to Rust 2018, and that's where the correct suggestions can be generated, because we know what the user meant (in the working Rust 2015 code).

I've been meaning to do this ever since I realized it's actually plausible with the way the canaries are implemented, because we can inject them in more situations, and provide different diagnostics.

[nikomatsakis]

@eddyb what is the story with this one?

[eddyb]

@nikomatsakis I didn't get to it, but also I've started to doubt the idea of reusing canaries.
Maybe it'd be better to have a separate check for the name resolving in the same module, whenever the suggestions to add crate:: are emitted?

cc @petrochenkov

[pnkfelix]

Discussed at compiler team meeting.

This seems important enough to resolve that it should remain on the RC2 milestone.

[nikomatsakis]

I just tested this locally and the most important instance (criterion conflicting with itself) was fixed in the meantime.

[eddyb]

The example works now, as the extern crate and the extern prelude entry with the same name are the same entity - we still might need to do some automatic migration for ambiguities involving local items, but it's lower priority I reckon.

[nikomatsakis]

Closing for now — the problem described in this issue is fixed, though there may be a more general bug (we should try to make a test case for that?).

[eddyb]

You can still reproduce this by having a local item (in the type/module namespace), with the same name, e.g. this triggers the error on the playground, on beta & nightly:

extern crate rayon;
mod foo {
    mod rayon {}
    use rayon::Scope;
}

I added #![deny(rust_2018_compatibility)] and got no errors, so rustfix wouldn't do anything.

[nikomatsakis]

OK, I'm going to re-open, but move to Rust 2018 Release milestone, since I think that we can probably address this with some targeted tweaks and it seems like it will not affect The Entire World.

[nikomatsakis]

BTW, there is discussion on Zulip and in particular starting around here we are talking about the new reproduction

[pnkfelix]

removing old nomination tag.

[pnkfelix]

visited for T-compiler triage. Marking P-high, since it is on the 2018 Release milestone.

[nikomatsakis]

@petrochenkov does your PR address this issue?

[petrochenkov]

does your PR address this issue?

No, it doesn't.

[petrochenkov]

I haven't added any new lints on 2015 edition (which is this issue about), only uniform path reimplementation on 2018 edition.

[alexcrichton]

I've updated the OP to reflect the current status here

@nikomatsakis it sounds like on the discussion linked you had a strawman of how to fix this. Did you get a chance to test that out? Or is someone lined up to fix this perhaps in a different fashion?

[nikomatsakis]

I do think this is plausible to fix with a "quick hack". I might try to pull it off today, though I'm not sure how important it is. The original example no longer applies, and it's not clear how common the pattern described above is in real code.

[alexcrichton]

I think I'd agree as well that this, while still a bug, is likely no longer edition-critical. Any opposition to removing it from the milestone and removing P-high?

[nikomatsakis]

@alexcrichton sgtm, I didn't get to do any work on this today