Private dependencies in public interface lint rework

[mladedav]

• I did not use an LLM to create a change in this PR.
• I used an LLM to create a change in this PR, and I have explained below how it was used.

Tracking issue: #44663
Closes #119428

This reworks the lint so that it does not fire based on where an item is defined but based on how it is imported. I think this is best illustrated in the following test:

tests/ui/privacy/pub-priv-dep/diamond_deps.rs

The test here is edited for brevity. Note that Shared is the same type and the lint fires only when imported through the private dependency.

// A diamond dependency:
//
//           diamond_reepxort
//                  /\
//        (public) /  \ (PRIVATE)
//                /    \
//   diamond_pub_dep  diamond_priv_dep
//                \    /
//        (public) \  /  (public)
//                  \/
//                shared

#![deny(exported_private_dependencies)]

extern crate diamond_priv_dep;
extern crate diamond_pub_dep;

pub fn leaks_priv() -> diamond_priv_dep::Shared {
    //~^ ERROR type `Shared` from private dependency 'diamond_priv_dep' in public interface
    diamond_priv_dep::Shared
}

// This does NOT produce a lint because the dependency is public
pub fn leaks_pub() -> diamond_pub_dep::Shared {
    diamond_pub_dep::Shared
}

This is my first contribution of this scope and as such, I'm not sure about several things I did:

• is the new query necessary? The current is_private_dep is not enough since it handles transitively public dependencies as public
• is it necessary to save this separately into crate metadata? I've modeled it after private_dep so I just did the same things I've seen there.
• should I use tcx.hir_module_items(mod_id).par_items(..) instead of tcx.hir_visit_all_item_likes_in_crate
  • I think so, but I'd appreciate some guidance here
• can the old PrivateItemsInPublicInterfacesChecker be trimmed down more so that there are not two passes for this? It still emits hard errors for types in associated types and I'm not sure how to check for this with the visitor.

@epage I'd appreciate if you could check the test changes in the first commit. The state now is how I understand the desired state based on the discussion in #119428 but I'd rather get a final ok on this.

[rustbot]

r? @JohnTitor

rustbot has assigned @JohnTitor.
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: compiler
• compiler expanded to 75 candidates
• Random selection from 19 candidates

[rustbot]

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

[JohnTitor]

@rustbot reroll
sorry, I'm not familiar with this

[mati865]

I'm not familiar either. @rustbot reroll

[petrochenkov]

Purely from the description, checking for how it is imported and rustc_privacy is generally not a good combination.
Some stuff will probably need to be moved to rustc_resolve, like in #143856.
I'll look at this, but only next week, at best.

[mladedav]

Right, I've found some cases where this rework doesn't work, it just happened to work for the tests that were here but using fully qualified paths vs imports has effect on whether the lint fires so this is broken.

I'll try to rework it so that the needed information is collected I guess in the late pass of resolve.

@rustbot label +S-waiting-on-author -S-waiting-on-review

[rust-bors]

☔ The latest upstream changes (presumably #161043) made this pull request unmergeable. Please resolve the merge conflicts by rebasing.