Be more specific when flagging imports as redundant due to the extern prelude

[fmease]

There are multiple distinct kinds of preludes. Be more specific when flagging imports as redundant due to the extern prelude.

r? Nilstrieb or compiler

[Noratrieb]

@bors r+ rollup

[bors]

📌 Commit 1acd048 has been approved by Nilstrieb

It is now in the queue for this repository.

[petrochenkov]

There are multiple distinct kinds of preludes.

And this diagnostic may fire for all of them, not just extern prelude.
(Except libstd prelude, which has non-dummy spans.)

[fmease]

I see
@bors r-

[fmease]

Std Library Prelude

Span is hopefully never dummy, so we can assume it's unreachable in this code path.

Language Prelude

use std::primitive::u8; //~ WARN the item `u8` is imported redundantly
const _: u8 = 0;

You are right, this one is reachable and this PR currently misclassifies the prelude.

macro_use Prelude

Seems like the code path that emits redundant import won't get reached in this case since unused `#[macro_use]` import takes precedence:

// ::: dep.rs :::   rustc dep.rs --crate-type=lib
#[macro_export] macro_rules! m { () => {} }

// ::: usr.rs :::   rustc usr.rs --crate-tyype=lib -L.
#[macro_use] extern crate dep; //~ WARN unused `#[macro_use]` import
use dep::m;
m!();

Ergo it might not be reachable in this code path.

Tool Prelude

Doesn't apply to this code path.

[fmease]

All that to say, I will look into emitting extern prelude or language prelude depending on the prelude kind. I'm in favor making it more explicit since I presume that most users will probably think of the libstd prelude when they see the word prelude.

[fmease]

Ah, and if we do use the #[macro_use] import and import sth. redundantly due to the #[macro_use], we currently don't emit any warnings which could be considered a bug I guess 🤔

// ::: dep.rs :::   rustc dep.rs --crate-type=lib
#[macro_export]
macro_rules! m { () => {} }
#[macro_export]
macro_rules! g { () => {} }

// ::: usr.rs :::   rustc usr.rs --crate-type=lib -L.
#[macro_use] extern crate dep;
use dep::m; // <-- this is actually redundant but we don't emit any warnings
m!();
g!();

[petrochenkov]

Ah, and if we do use the #[macro_use] import and import sth. redundantly due to the #[macro_use], we currently don't emit any warnings which could be considered a bug I guess 🤔

The lint is just conservative with macros in general.
We don't set Used::Scope for early_resolve_ident_in_lexical_scope in finalize_macro_resolutions, so m!(); isn't classified as a scope-relative use.
Maybe we should do that.

In any case, the span won't be dummy in this scenario.
It will point to #[macro_use] or to the imported macro declaration itself.

[petrochenkov]

Tool Prelude

Doesn't apply to this code path.

Yeah, seems right, it produces an error in that case https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=8718593e4c9d888ae72a8ac08164b38d

[petrochenkov]

So we have 3 prelude cases with dummy spans:

• Extern prelude
• Built-in types
• Built-in attributes

use allow; // warning: the item `allow` is imported redundantly

#[allow(unused)]
fn main() {}

From those, extern prelude should be most common.
Maybe be it's fine to just say "extern prelude" in the diagnostics.
I don't want add any extra logic for tracking prelude kinds just for this message.

[petrochenkov]

@fmease
Could you add test cases for non-extern preludes (including the erroring ones, like tool modules)?

[fmease]

Sorry for the delay.

We don't set Used::Scope for early_resolve_ident_in_lexical_scope in finalize_macro_resolutions, so m!(); isn't classified as a scope-relative use.
Maybe we should do that. [emphasis mine]

I've added that to my internal rustc issue tracker to be revisited by me at some point.

Could you add test cases for non-extern preludes (including the erroring ones, like tool modules)?

For the #[macro_use] prelude case, we already have tests/ui/imports/unused-macro-use.rs and tests/ui/rust-2018/macro-use-warned-against.rs. Do they suffice? They obviously don't match the snippet I posted above 100%.

For the tool prelude case, we already have tests/ui/rust-2018/uniform-paths/cross-crate.rs and tests/ui/rust-2018/uniform-paths/prelude-fail-2.rs. They should sufficiently cover this case, I think.

---

I've noticed that we currently have tests for redundant imports in two places, namely tests/ui/lint/use-redundant/ and tests/ui/imports/ (as tests/ui/imports/redundant-import-*.rs). I can move the newly added tests as well as the preexisting ones into tests/ui/lint/use-redundant/ if you'd like me to.

[petrochenkov]

@bors r+

[bors]

📌 Commit 8a24ddf has been approved by petrochenkov

It is now in the queue for this repository.