edition lint: modules shadowing extern crates

[nrc]

AFAIK, we don't have a plan for how to handle this. We should make one!

cc @rust-lang/compiler

[oli-obk]

So the issue is

mod foo {
    extern crate foo;
}

Because in the 2018 edition

mod foo {
    use foo;
}

Because that seems conflict free.

Having both extern crate foo; and mod foo; in the root is already a conflict in 2015.

So I'm not sure what this issue is about

[alexcrichton]

@nrc do you have thoughts on @oli-obk's comment? I'm trying to see what I could do to help move this a long but it's not clear to me what it is!

[Mark-Simulacrum]

mod serde;
use crate::serde::Serialize;

This is I believe ambiguous as implemented today because of implicit prelude; both extern crate serde and mod serde are defined in the "root" namespace so the import will fail or behave unexpectedly. I wasn't able to quickly reproduce on the playground; haven't tried locally. But I think something along these lines is possibly at least a problem.

[alexcrichton]

@Mark-Simulacrum I don't think there's an implicit prelude of extern crate declarations though? use crate::serde always uses the mod serde; or extern crate serde *if defined explicitly, and use serde::...` always uses an external crate

[nikomatsakis]

The issue I believe is not that there is some inherent ambiguity -- e.g., with @Mark-Simulacrum's example, in fact, the use can only refer to the module (in particular, with relative paths, the prelude doesn't even come into play at all).

A more interesting case is a case like this:

mod serde;
mod foo {
    // In Rust 2015, this comes from the module at crate root.
    //
    // In Rust 2018, this comes from the crate `serde` (presuming there is one), or otherwise
    // is an error.
    use serde::bar;
}

I believe our fear was that we would sometimes make incorrect rewrite suggestions in the migration lints. I can think of a few possibilities.

Incorrect use

This may not be an issue, actually, but the idea would be that you have a use like this which currently refers to the serde crate:

mod serde;

mod bar {
    extern crate serde;
    use self::serde::Foo;
}

If we were to try and rewrite this path to use serde::Foo, then we'd have a problem, but I don't think we attempt that now. I think we will only rewrite a use in the case where the extern crate is at the root, in which case module shadowing is a non-issue -- well, that is true for the current proposal. @aturon was talking about extending things last I heard.

Outside of a use

extern crate serde;

mod foo {
mod serde;

fn foo() -> ::serde::Foo { }
}

I don't recall what kind of rewrites we do in these cases, but perhaps we would rewrite ::serde::Foo to serde::Foo? If so, that'd be bad.

[nikomatsakis]

(On a related note, we will currently suggest removing extern crate in some cases, but shadowing is an example of the kind of case where we have to be a bit careful.)

[alexcrichton]

@nikomatsakis I fear I'm left even more confused after reading your comment than before :(

For your "incorrect use" example we don't currently lint anything starting with self or super, only paths that don't start with one of those (and they don't start with an extern crate).

For the "outside of a use" example I'm not really sure what's going on, but we don't make any suggestions today.

It sounds like this issue is mostly "if we get aggressive about suggestions we'll have problems", but I don't think we're that aggressive today? It's today just a pure "your code will stop compiling, please fix it" lint rather than "your code isn't idiomatic, please let me help you" lint

[aturon]

This issue is not currently actionable. I've removed from the milestone but tagged for @rust-lang/lang team discussion.

[joshtriplett]

We do need to fix the edition lints and rustfix suggestions for the edition, especially with uniform_paths.