fix [std_instead_of_core]: false positives for core::io/MSRV

[bushrat011899]

Objective

• Fix false positive identified by @rktaxe in this comment on MSRV-aware std_instead_of_core #13158.
• Fix MSRV-aware std_instead_of_core #13158

Solution

Previously the lint had an exception for all instances of a stable item in an unstable module, primarily to allow certain intrinsics such as copy to be accessible. Instead, I check for the presence of rustc_allowed_through_unstable_modules to handle those exceptions, and allow the is_stable check within the lint to early out as soon as any part of its path is unstable.

I believe this was the last piece required to resolve #13158, and have added tests for all examples listed in the issue. If there are other examples of this lint failing, I'd greatly appreciate seeing them!

---

Notes

• No AI tooling of any kind was used during the creation of this PR.
• Originally opened (in error) as fix(clippy): std_instead_of_core false positive for core::io rust#156164

---

Please write a short comment explaining your change (or "none" for internal only changes)

changelog: fix certain false positives for [std_instead_of_core] for stable items in an unstable module (e.g., core::io::ErrorKind), and other MSRV-unaware issues.

[rustbot]

r? @Jarcho

rustbot has assigned @Jarcho.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: 7 candidates
• 7 candidates expanded to 7 candidates
• Random selection from Jarcho, dswij, llogiq, samueltardieu

[github-actions]

Lintcheck changes for d9517a6

Lint	Added	Removed	Changed
clippy::std_instead_of_alloc	0	1	12
clippy::std_instead_of_core	1	84	3

This comment will be updated if you push new changes

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[bushrat011899]

It was a little more involved than I anticipated, but I believe I have resolved the false positive I was intending to, and also solved a couple of additional multi-import ones. I've added additional test cases for the lint which should catch any regressions. This also solved some MSRV issues too. For example, serde 1.0.204 had an MSRV of 1.31 and imports AtomicBool, AtomicI8, and AtomicU8 in a single line. Current clippy suggests importing all from core, even though AtomicI8 and AtomicU8 were only added to core in 1.34.0.

Because I was struggling to understand how the lint worked initially, I've refactored it probably more than it strictly had to be. However I believe it is now easier to understand (at least to my new-to-clippy eyes), with additional comments and internal documentation.

@rustbot ready

[Jarcho]

The only change this needed for the title is in is_stable which could be:

fn is_stable(cx: &LateContext<'_>, mut def_id: DefId, msrv: Msrv) -> bool {
    loop {
        if let Some(stability) = cx.tcx.lookup_stability(def_id) {
            match stability.level {
                // Workaround for items from `core::intrinsics` with a stable export in a different module.
                // Not that we ignore the `since` field as we are already accessing the item in question.
                StabilityLevel::Stable { allowed_through_unstable_modules: Some(_), .. } => return true,
                StabilityLevel::Stable { since, .. } => match since {
                    StableSince::Version(v) if !msrv.meets(cx, v) => return false,
                    StableSince::Current if msrv.current(cx).is_none() => return false,
                    StableSince::Err(_) => return false,
                    StableSince::Version(_) | StableSince::Current => {},
                },
                StabilityLevel::Unstable { .. } => return false,
            }
        }

        match cx.tcx.opt_parent(def_id) {
            Some(parent) => def_id = parent,
            None => return true,
        }
    }
}

The rest of the changes should be a separate PR.

[rktaxe]

With the lint now firing on 1.97-beta, I presume this will also need backporting before that stabilizes.

[bushrat011899]

@Jarcho thanks for the feedback! I've reduced this PR down to just your suggestion, but I believe it's led to a much larger multi-import false-positive rate. Namely, instances such as std::sync::{Arc, Mutex} now apply to the whole statement, which it should be specific to the Arc component. I'm basing this on the results from this lintcheck run.

[bushrat011899]

I've added my original changes as a followup commit within this PR and confirmed it resolves the multi-import regressions. With the regressions in mind, I feel it is appropriate to consider this PR in its current entirety. However, I'm happy to try and resolve the regressions differently if that's desirable.

[Jarcho]

Please do split it into multiple PRs. I haven't reviewed that part and the multi-import fixes are less important. The whole thing likely won't be backported either.

[bushrat011899]

Understood, I'll open up a PR shortly with the multi-import fix.

[bushrat011899]

Please see #17252 for the split-off commit fixing the multi-import issue discussed here.

[Jarcho]

Thank you.

View changes since this review