`#[expect(unused_imports)]` does not work correctly on grouped imports about rust HOT 8 OPEN

cc @xFrednet who has been handling expect

from rust.

Thank you for the ping. So my educated guess is that this issue comes from how rustc handles group imports. The import from the issue is internally desugared into three separate imports, like this (Playground "Show HIR"):

#[expect(unused_imports)]
use ::{};
#[expect(unused_imports)]
use std::io;
#[expect(unused_imports)]
use std::fs;

From the text output of HIR it looks like the attributes are duplicated. This in turn creates additional expectations which are not fulfilled.

The question is now, hot to figure out, that these items and attributes all originate from the same item.

@cjgillot do you maybe have an idea on this one?

from rust.

That's not the only place where we duplicate attributes. I had to partially work around it in #127313, but that's not a good solution in general.

Right now, the diagnostic infra stores the stable LintExpectationId, we update unstable -> stable before emitting diagnostics, and we try to match the stable id.

What if we did the other way?

I mean:

• In the DiagCtxtInner, store keep fulfilled_expectations: Vec<LintExpectationId> but stop stashing diagnostics to convert unstable->stable version, just emit them. Emitting an unstable id should only happen from pre-lowering code, which is not incremental, so we won't try to decode such a diagnostic to re-emit it.
• In lint_expectations(()), stop computing the unstable -> stable mapping.
• In check_expectations(()), make a FxHashSet<(AttrId, u16)> from the global fulfilled expectations, and use it to check what lint_expectations(()) returns.

Step 3 should be ok for incr comp, as check_expectations is eval_always.

Does that make sense to you?

from rust.

I'm not sure, I fully understand you. Does this rephrasing sound reasonably close?

1. The pre-lowering code, that can emit unstable LintExpectationIDs is always executed. Instead of transforming these ID's we just don't store between compilations as they will be emitted again during the next run.
2. lint_expectations() returns all lint expectations defined by the user.
3. check_expectations() will take the stable IDs and map them to their unstable versions. These unstable ID's are then used to fulfill the expectations from lint_expectations(). And this should fix this issue since the AST hasn't duplicated any attributes yet.

from rust.

Yes.

from rust.

On what level does lint_expectations() operate? Is it on the AST or HIR?

Because, if it's on the HIR, we still need to find a way to map from duplicated attributes back to their original single ones. Don't we?

from rust.

lint_expectations() can operate on either, depending on what is easier to code. You could also have the early lint pass feed it.

check_expectations() can always look at the attribute in HIR to do this mapping. Given id: LintExpectationId, something like:

match id {
    Unstable { attr_id, lint_index: Some(lint_index) } => (attr_id, lint_index),
    Stable { hir_id, attr_index, lint_index: Some(lint_index) } => {
        // We are an `eval_always` query, so looking at the attribute's `AttrId` is ok.
        let attr_id = tcx.hir().attrs(hir_id)[attr_index].id;
        (attr_id, lint_index)
    }
    _ => bug!(),
}

When we duplicate the attributes, we don't change the AttrId, so relying on it is ok.

from rust.

Interesting, yep, then this should work :D

Is this something that can/should be done with #127313?

If we want to make it a followup PR, we should probably ping the reviewer or reroll 🤔

from rust.