From 6e8f9dde3c26cf1fff31e38932078848d85f8ac9 Mon Sep 17 00:00:00 2001 From: Shoyu Vanilla Date: Thu, 12 Dec 2024 03:47:48 +0900 Subject: [PATCH 1/2] fix: Implement missing logics in `exported_private_dependencies` lint --- compiler/rustc_privacy/src/lib.rs | 57 +++++++++++++ library/std/Cargo.toml | 2 +- tests/ui/privacy/pub-priv-dep/pub-priv1.rs | 22 ++--- .../ui/privacy/pub-priv-dep/pub-priv1.stderr | 84 +++++++++++++++---- 4 files changed, 138 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 5271d03a6f617..bd8bab1d93366 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -1351,6 +1351,7 @@ struct SearchInterfaceForPrivateItemsVisitor<'tcx> { required_effective_vis: Option, in_assoc_ty: bool, in_primary_interface: bool, + check_private_dep_leaks_only: bool, } impl SearchInterfaceForPrivateItemsVisitor<'_> { @@ -1411,6 +1412,10 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { ); } + if self.check_private_dep_leaks_only { + return false; + } + let Some(local_def_id) = def_id.as_local() else { return false; }; @@ -1530,6 +1535,23 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'_, 'tcx> { required_effective_vis, in_assoc_ty: false, in_primary_interface: true, + check_private_dep_leaks_only: false, + } + } + + fn check_private_dep_leaks_only( + &self, + def_id: LocalDefId, + required_visibility: ty::Visibility, + ) -> SearchInterfaceForPrivateItemsVisitor<'tcx> { + SearchInterfaceForPrivateItemsVisitor { + tcx: self.tcx, + item_def_id: def_id, + required_visibility, + required_effective_vis: None, + in_assoc_ty: false, + in_primary_interface: true, + check_private_dep_leaks_only: true, } } @@ -1732,6 +1754,14 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'_, 'tcx> { .generics() .predicates(); } + + // normally, a public item can implement a private trait, but this should be linted for leakages of + // private dependencies. + if let Some(trait_ref) = tcx.impl_trait_ref(item.owner_id.def_id) { + self.check_private_dep_leaks_only(item.owner_id.def_id, impl_vis) + .visit_trait(trait_ref.instantiate_identity()); + } + for impl_item_ref in impl_.items { let impl_item_vis = if impl_.of_trait.is_none() { min( @@ -1759,6 +1789,33 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'_, 'tcx> { } } } + DefKind::Use => { + let item = tcx.hir_item(id); + if let hir::ItemKind::Use(path, use_kind) = item.kind + // List imports are desugared as single ones so skip `ListStem`s + && use_kind != rustc_hir::UseKind::ListStem + { + if let Some(def_id) = path.res.iter().filter_map(Res::opt_def_id).last() { + // normally, public items in a private modules can be re-exported but this should be linted for + // leakages of a private dependencies + self.check_private_dep_leaks_only(item.owner_id.def_id, item_visibility) + .check_def_id( + def_id, + item.kind.descr(), + &LazyDefPathStr { def_id, tcx }, + ); + } + } + } + DefKind::ExternCrate => { + let item = tcx.hir_item(id); + if let Some(cnum) = tcx.extern_mod_stmt_cnum(item.owner_id.def_id) { + let def_id = cnum.as_def_id(); + // `pub extern some_dep` should be linted if and only if `some_dep` is a private dependency + self.check_private_dep_leaks_only(item.owner_id.def_id, item_visibility) + .visit_def_id(def_id, item.kind.descr(), &LazyDefPathStr { def_id, tcx }); + } + } _ => {} } } diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index a0a28e8079601..8dea13f0d55bc 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -25,7 +25,7 @@ hashbrown = { version = "0.15", default-features = false, features = [ ] } std_detect = { path = "../stdarch/crates/std_detect", default-features = false, features = [ 'rustc-dep-of-std', -] } +], public = true } # Dependencies of the `backtrace` crate rustc-demangle = { version = "0.1.24", features = ['rustc-dep-of-std'] } diff --git a/tests/ui/privacy/pub-priv-dep/pub-priv1.rs b/tests/ui/privacy/pub-priv-dep/pub-priv1.rs index 112eaf528be27..46383dab31d91 100644 --- a/tests/ui/privacy/pub-priv-dep/pub-priv1.rs +++ b/tests/ui/privacy/pub-priv-dep/pub-priv1.rs @@ -7,10 +7,11 @@ // dependency or a private one. #![deny(exported_private_dependencies)] +#![allow(hidden_glob_reexports)] // This crate is a private dependency -// FIXME: This should trigger. pub extern crate priv_dep; +//~^ ERROR extern crate `priv_dep` from private dependency 'priv_dep' in public interface // This crate is a public dependency extern crate pub_dep; // This crate is a private dependency @@ -77,31 +78,30 @@ pub type Alias = OtherType; pub struct PublicWithPrivateImpl; -// FIXME: This should trigger. -// See https://github.com/rust-lang/rust/issues/71043 impl OtherTrait for PublicWithPrivateImpl {} +//~^ ERROR trait `OtherTrait` from private dependency 'priv_dep' in public interface pub trait PubTraitOnPrivate {} -// FIXME: This should trigger. -// See https://github.com/rust-lang/rust/issues/71043 impl PubTraitOnPrivate for OtherType {} +//~^ ERROR type `OtherType` from private dependency 'priv_dep' in public interface pub struct AllowedPrivType { #[allow(exported_private_dependencies)] pub allowed: OtherType, } -// FIXME: This should trigger. pub use priv_dep::m; -// FIXME: This should trigger. +//~^ ERROR `use` import `m` from private dependency 'priv_dep' in public interface pub use pm::fn_like; -// FIXME: This should trigger. +//~^ ERROR `use` import `fn_like` from private dependency 'pm' in public interface pub use pm::PmDerive; -// FIXME: This should trigger. +//~^ ERROR `use` import `PmDerive` from private dependency 'pm' in public interface pub use pm::pm_attr; - -// FIXME: This should trigger. +//~^ ERROR `use` import `pm_attr` from private dependency 'pm' in public interface pub use priv_dep::E::V1; +//~^ ERROR `use` import `V1` from private dependency 'priv_dep' in public interface +pub use priv_dep::*; +//~^ ERROR `use` import `priv_dep` from private dependency 'priv_dep' in public interface fn main() {} diff --git a/tests/ui/privacy/pub-priv-dep/pub-priv1.stderr b/tests/ui/privacy/pub-priv-dep/pub-priv1.stderr index 53d461a5774a0..6476ae42e6ada 100644 --- a/tests/ui/privacy/pub-priv-dep/pub-priv1.stderr +++ b/tests/ui/privacy/pub-priv-dep/pub-priv1.stderr @@ -1,8 +1,8 @@ -error: type `OtherType` from private dependency 'priv_dep' in public interface - --> $DIR/pub-priv1.rs:29:5 +error: extern crate `priv_dep` from private dependency 'priv_dep' in public interface + --> $DIR/pub-priv1.rs:13:1 | -LL | pub field: OtherType, - | ^^^^^^^^^^^^^^^^^^^^ +LL | pub extern crate priv_dep; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ | note: the lint level is defined here --> $DIR/pub-priv1.rs:9:9 @@ -11,64 +11,118 @@ LL | #![deny(exported_private_dependencies)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: type `OtherType` from private dependency 'priv_dep' in public interface - --> $DIR/pub-priv1.rs:36:5 + --> $DIR/pub-priv1.rs:30:5 + | +LL | pub field: OtherType, + | ^^^^^^^^^^^^^^^^^^^^ + +error: type `OtherType` from private dependency 'priv_dep' in public interface + --> $DIR/pub-priv1.rs:37:5 | LL | pub fn pub_fn_param(param: OtherType) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: type `OtherType` from private dependency 'priv_dep' in public interface - --> $DIR/pub-priv1.rs:39:5 + --> $DIR/pub-priv1.rs:40:5 | LL | pub fn pub_fn_return() -> OtherType { OtherType } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: trait `OtherTrait` from private dependency 'priv_dep' in public interface - --> $DIR/pub-priv1.rs:46:5 + --> $DIR/pub-priv1.rs:47:5 | LL | type Foo: OtherTrait; | ^^^^^^^^^^^^^^^^^^^^ error: trait `OtherTrait` from private dependency 'priv_dep' in public interface - --> $DIR/pub-priv1.rs:50:1 + --> $DIR/pub-priv1.rs:51:1 | LL | pub trait WithSuperTrait: OtherTrait {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: type `OtherType` from private dependency 'priv_dep' in public interface - --> $DIR/pub-priv1.rs:59:5 + --> $DIR/pub-priv1.rs:60:5 | LL | type X = OtherType; | ^^^^^^ error: trait `OtherTrait` from private dependency 'priv_dep' in public interface - --> $DIR/pub-priv1.rs:63:1 + --> $DIR/pub-priv1.rs:64:1 | LL | pub fn in_bounds(x: T) { unimplemented!() } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: type `OtherType` from private dependency 'priv_dep' in public interface - --> $DIR/pub-priv1.rs:66:1 + --> $DIR/pub-priv1.rs:67:1 | LL | pub fn private_in_generic() -> std::num::Saturating { unimplemented!() } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: type `OtherType` from private dependency 'priv_dep' in public interface - --> $DIR/pub-priv1.rs:69:1 + --> $DIR/pub-priv1.rs:70:1 | LL | pub static STATIC: OtherType = OtherType; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: type `OtherType` from private dependency 'priv_dep' in public interface - --> $DIR/pub-priv1.rs:72:1 + --> $DIR/pub-priv1.rs:73:1 | LL | pub const CONST: OtherType = OtherType; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: type `OtherType` from private dependency 'priv_dep' in public interface - --> $DIR/pub-priv1.rs:75:1 + --> $DIR/pub-priv1.rs:76:1 | LL | pub type Alias = OtherType; | ^^^^^^^^^^^^^^ -error: aborting due to 11 previous errors +error: trait `OtherTrait` from private dependency 'priv_dep' in public interface + --> $DIR/pub-priv1.rs:81:1 + | +LL | impl OtherTrait for PublicWithPrivateImpl {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: type `OtherType` from private dependency 'priv_dep' in public interface + --> $DIR/pub-priv1.rs:86:1 + | +LL | impl PubTraitOnPrivate for OtherType {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: `use` import `m` from private dependency 'priv_dep' in public interface + --> $DIR/pub-priv1.rs:94:9 + | +LL | pub use priv_dep::m; + | ^^^^^^^^^^^ + +error: `use` import `fn_like` from private dependency 'pm' in public interface + --> $DIR/pub-priv1.rs:96:9 + | +LL | pub use pm::fn_like; + | ^^^^^^^^^^^ + +error: `use` import `PmDerive` from private dependency 'pm' in public interface + --> $DIR/pub-priv1.rs:98:9 + | +LL | pub use pm::PmDerive; + | ^^^^^^^^^^^^ + +error: `use` import `pm_attr` from private dependency 'pm' in public interface + --> $DIR/pub-priv1.rs:100:9 + | +LL | pub use pm::pm_attr; + | ^^^^^^^^^^^ + +error: `use` import `V1` from private dependency 'priv_dep' in public interface + --> $DIR/pub-priv1.rs:102:9 + | +LL | pub use priv_dep::E::V1; + | ^^^^^^^^^^^^^^^ + +error: `use` import `priv_dep` from private dependency 'priv_dep' in public interface + --> $DIR/pub-priv1.rs:104:9 + | +LL | pub use priv_dep::*; + | ^^^^^^^^ + +error: aborting due to 20 previous errors From 73b15e48ab3218baa4024c25e1292ac010e2d9a4 Mon Sep 17 00:00:00 2001 From: Shoyu Vanilla Date: Wed, 26 Feb 2025 02:03:19 +0900 Subject: [PATCH 2/2] Remove detection of private dependency leaks for `use` and `extern` from `rustc_privacy` --- compiler/rustc_privacy/src/lib.rs | 27 ---------- library/std/Cargo.toml | 2 +- tests/ui/privacy/pub-priv-dep/pub-priv1.rs | 15 +++--- .../ui/privacy/pub-priv-dep/pub-priv1.stderr | 52 ++----------------- 4 files changed, 14 insertions(+), 82 deletions(-) diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index bd8bab1d93366..c9b361a8a3504 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -1789,33 +1789,6 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'_, 'tcx> { } } } - DefKind::Use => { - let item = tcx.hir_item(id); - if let hir::ItemKind::Use(path, use_kind) = item.kind - // List imports are desugared as single ones so skip `ListStem`s - && use_kind != rustc_hir::UseKind::ListStem - { - if let Some(def_id) = path.res.iter().filter_map(Res::opt_def_id).last() { - // normally, public items in a private modules can be re-exported but this should be linted for - // leakages of a private dependencies - self.check_private_dep_leaks_only(item.owner_id.def_id, item_visibility) - .check_def_id( - def_id, - item.kind.descr(), - &LazyDefPathStr { def_id, tcx }, - ); - } - } - } - DefKind::ExternCrate => { - let item = tcx.hir_item(id); - if let Some(cnum) = tcx.extern_mod_stmt_cnum(item.owner_id.def_id) { - let def_id = cnum.as_def_id(); - // `pub extern some_dep` should be linted if and only if `some_dep` is a private dependency - self.check_private_dep_leaks_only(item.owner_id.def_id, item_visibility) - .visit_def_id(def_id, item.kind.descr(), &LazyDefPathStr { def_id, tcx }); - } - } _ => {} } } diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index 8dea13f0d55bc..a0a28e8079601 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -25,7 +25,7 @@ hashbrown = { version = "0.15", default-features = false, features = [ ] } std_detect = { path = "../stdarch/crates/std_detect", default-features = false, features = [ 'rustc-dep-of-std', -], public = true } +] } # Dependencies of the `backtrace` crate rustc-demangle = { version = "0.1.24", features = ['rustc-dep-of-std'] } diff --git a/tests/ui/privacy/pub-priv-dep/pub-priv1.rs b/tests/ui/privacy/pub-priv-dep/pub-priv1.rs index 46383dab31d91..06e4f12924485 100644 --- a/tests/ui/privacy/pub-priv-dep/pub-priv1.rs +++ b/tests/ui/privacy/pub-priv-dep/pub-priv1.rs @@ -10,8 +10,8 @@ #![allow(hidden_glob_reexports)] // This crate is a private dependency +// FIXME: This should trigger. pub extern crate priv_dep; -//~^ ERROR extern crate `priv_dep` from private dependency 'priv_dep' in public interface // This crate is a public dependency extern crate pub_dep; // This crate is a private dependency @@ -91,17 +91,18 @@ pub struct AllowedPrivType { pub allowed: OtherType, } +// FIXME: This should trigger. pub use priv_dep::m; -//~^ ERROR `use` import `m` from private dependency 'priv_dep' in public interface +// FIXME: This should trigger. pub use pm::fn_like; -//~^ ERROR `use` import `fn_like` from private dependency 'pm' in public interface +// FIXME: This should trigger. pub use pm::PmDerive; -//~^ ERROR `use` import `PmDerive` from private dependency 'pm' in public interface +// FIXME: This should trigger. pub use pm::pm_attr; -//~^ ERROR `use` import `pm_attr` from private dependency 'pm' in public interface + +// FIXME: This should trigger. pub use priv_dep::E::V1; -//~^ ERROR `use` import `V1` from private dependency 'priv_dep' in public interface +// FIXME: This should trigger. pub use priv_dep::*; -//~^ ERROR `use` import `priv_dep` from private dependency 'priv_dep' in public interface fn main() {} diff --git a/tests/ui/privacy/pub-priv-dep/pub-priv1.stderr b/tests/ui/privacy/pub-priv-dep/pub-priv1.stderr index 6476ae42e6ada..87c845e52ef1c 100644 --- a/tests/ui/privacy/pub-priv-dep/pub-priv1.stderr +++ b/tests/ui/privacy/pub-priv-dep/pub-priv1.stderr @@ -1,8 +1,8 @@ -error: extern crate `priv_dep` from private dependency 'priv_dep' in public interface - --> $DIR/pub-priv1.rs:13:1 +error: type `OtherType` from private dependency 'priv_dep' in public interface + --> $DIR/pub-priv1.rs:30:5 | -LL | pub extern crate priv_dep; - | ^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | pub field: OtherType, + | ^^^^^^^^^^^^^^^^^^^^ | note: the lint level is defined here --> $DIR/pub-priv1.rs:9:9 @@ -10,12 +10,6 @@ note: the lint level is defined here LL | #![deny(exported_private_dependencies)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: type `OtherType` from private dependency 'priv_dep' in public interface - --> $DIR/pub-priv1.rs:30:5 - | -LL | pub field: OtherType, - | ^^^^^^^^^^^^^^^^^^^^ - error: type `OtherType` from private dependency 'priv_dep' in public interface --> $DIR/pub-priv1.rs:37:5 | @@ -88,41 +82,5 @@ error: type `OtherType` from private dependency 'priv_dep' in public interface LL | impl PubTraitOnPrivate for OtherType {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: `use` import `m` from private dependency 'priv_dep' in public interface - --> $DIR/pub-priv1.rs:94:9 - | -LL | pub use priv_dep::m; - | ^^^^^^^^^^^ - -error: `use` import `fn_like` from private dependency 'pm' in public interface - --> $DIR/pub-priv1.rs:96:9 - | -LL | pub use pm::fn_like; - | ^^^^^^^^^^^ - -error: `use` import `PmDerive` from private dependency 'pm' in public interface - --> $DIR/pub-priv1.rs:98:9 - | -LL | pub use pm::PmDerive; - | ^^^^^^^^^^^^ - -error: `use` import `pm_attr` from private dependency 'pm' in public interface - --> $DIR/pub-priv1.rs:100:9 - | -LL | pub use pm::pm_attr; - | ^^^^^^^^^^^ - -error: `use` import `V1` from private dependency 'priv_dep' in public interface - --> $DIR/pub-priv1.rs:102:9 - | -LL | pub use priv_dep::E::V1; - | ^^^^^^^^^^^^^^^ - -error: `use` import `priv_dep` from private dependency 'priv_dep' in public interface - --> $DIR/pub-priv1.rs:104:9 - | -LL | pub use priv_dep::*; - | ^^^^^^^^ - -error: aborting due to 20 previous errors +error: aborting due to 13 previous errors