-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Implement public/private dependency feature #57586
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
8b6f3dd
Initial implementation work
Aaron1011 09ff0ba
Fix emitting lint
Aaron1011 afb1921
Fixup code
Aaron1011 23014b4
Properly register lint
Aaron1011 93d872d
Add UI test
Aaron1011 08c901f
Always treat 'std' and 'core' as public
Aaron1011 12f9b79
Improve UI tests
Aaron1011 d60214c
Clippy fixes, rename stuff to match RFC
Aaron1011 bc2221f
Add test for 'std' crate being public
Aaron1011 d8fc630
Track extern_public command-line argument
Aaron1011 dfd2669
Delete dead code
Aaron1011 3fa3647
Rename external_private_dependency to exported_private_dependencies
Aaron1011 173f5cf
Remove feature gate
Aaron1011 fe15f71
Move --extern-public behind -Z unstable-options
Aaron1011 45486cc
Tidy fixes
Aaron1011 b29a21f
Remove feature from test
Aaron1011 48ec29d
Replace --extern-public with --extern-private
Aaron1011 a05bfc6
Test allowing individual struct field
Aaron1011 24f3595
Remove unnecessary is_local() check
Aaron1011 bfcd14d
Add future compat lint declaration
Aaron1011 541d315
Update tests for future-compat warning removal
Aaron1011 369faae
Cleanup unecessary code
Aaron1011 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ | |
|
||
#[macro_use] extern crate rustc; | ||
#[macro_use] extern crate syntax; | ||
#[macro_use] extern crate log; | ||
extern crate rustc_typeck; | ||
extern crate syntax_pos; | ||
extern crate rustc_data_structures; | ||
|
@@ -1451,13 +1452,15 @@ impl<'a, 'tcx> Visitor<'tcx> for ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> { | |
|
||
struct SearchInterfaceForPrivateItemsVisitor<'a, 'tcx: 'a> { | ||
tcx: TyCtxt<'a, 'tcx, 'tcx>, | ||
item_id: ast::NodeId, | ||
item_def_id: DefId, | ||
span: Span, | ||
/// The visitor checks that each component type is at least this visible. | ||
required_visibility: ty::Visibility, | ||
has_pub_restricted: bool, | ||
has_old_errors: bool, | ||
in_assoc_ty: bool, | ||
private_crates: FxHashSet<CrateNum> | ||
} | ||
|
||
impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> { | ||
|
@@ -1492,6 +1495,16 @@ impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> { | |
} | ||
|
||
fn check_def_id(&mut self, def_id: DefId, kind: &str, descr: &dyn fmt::Display) -> bool { | ||
if self.leaks_private_dep(def_id) { | ||
self.tcx.lint_node(lint::builtin::EXPORTED_PRIVATE_DEPENDENCIES, | ||
self.item_id, | ||
self.span, | ||
&format!("{} `{}` from private dependency '{}' in public \ | ||
interface", kind, descr, | ||
self.tcx.crate_name(def_id.krate))); | ||
|
||
} | ||
|
||
let node_id = match self.tcx.hir().as_local_node_id(def_id) { | ||
Some(node_id) => node_id, | ||
None => return false, | ||
|
@@ -1514,9 +1527,23 @@ impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> { | |
self.tcx.lint_node(lint::builtin::PRIVATE_IN_PUBLIC, node_id, self.span, | ||
&format!("{} (error {})", msg, err_code)); | ||
} | ||
|
||
} | ||
|
||
false | ||
} | ||
|
||
/// An item is 'leaked' from a private dependency if all | ||
/// of the following are true: | ||
/// 1. It's contained within a public type | ||
/// 2. It comes from a private crate | ||
fn leaks_private_dep(&self, item_id: DefId) -> bool { | ||
let ret = self.required_visibility == ty::Visibility::Public && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will report things used in mod private_details {
pub fn details() -> PrivateDep { ... } // Will be linted
} , but that's probably okay for a start. |
||
self.private_crates.contains(&item_id.krate); | ||
|
||
debug!("leaks_private_dep(item_id={:?})={}", item_id, ret); | ||
return ret; | ||
} | ||
} | ||
|
||
impl<'a, 'tcx> DefIdVisitor<'a, 'tcx> for SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> { | ||
|
@@ -1530,6 +1557,7 @@ struct PrivateItemsInPublicInterfacesVisitor<'a, 'tcx: 'a> { | |
tcx: TyCtxt<'a, 'tcx, 'tcx>, | ||
has_pub_restricted: bool, | ||
old_error_set: &'a NodeSet, | ||
private_crates: FxHashSet<CrateNum> | ||
} | ||
|
||
impl<'a, 'tcx> PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> { | ||
|
@@ -1560,12 +1588,14 @@ impl<'a, 'tcx> PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> { | |
|
||
SearchInterfaceForPrivateItemsVisitor { | ||
tcx: self.tcx, | ||
item_id, | ||
item_def_id: self.tcx.hir().local_def_id(item_id), | ||
span: self.tcx.hir().span(item_id), | ||
required_visibility, | ||
has_pub_restricted: self.has_pub_restricted, | ||
has_old_errors, | ||
in_assoc_ty: false, | ||
private_crates: self.private_crates.clone() | ||
} | ||
} | ||
|
||
|
@@ -1690,6 +1720,7 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Lrc<AccessLevels> { | |
fn check_mod_privacy<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) { | ||
let empty_tables = ty::TypeckTables::empty(None); | ||
|
||
|
||
// Check privacy of names not checked in previous compilation stages. | ||
let mut visitor = NamePrivacyVisitor { | ||
tcx, | ||
|
@@ -1725,6 +1756,12 @@ fn privacy_access_levels<'tcx>( | |
queries::check_mod_privacy::ensure(tcx, tcx.hir().local_def_id(module)); | ||
} | ||
|
||
let private_crates: FxHashSet<CrateNum> = tcx.sess.opts.extern_private.iter() | ||
.flat_map(|c| { | ||
tcx.crates().iter().find(|&&krate| &tcx.crate_name(krate) == c).cloned() | ||
}).collect(); | ||
|
||
|
||
// Build up a set of all exported items in the AST. This is a set of all | ||
// items which are reachable from external crates based on visibility. | ||
let mut visitor = EmbargoVisitor { | ||
|
@@ -1767,6 +1804,7 @@ fn privacy_access_levels<'tcx>( | |
tcx, | ||
has_pub_restricted, | ||
old_error_set: &visitor.old_error_set, | ||
private_crates | ||
}; | ||
krate.visit_all_item_likes(&mut DeepVisitor::new(&mut visitor)); | ||
} | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
pub struct PubType; |
20 changes: 20 additions & 0 deletions
20
src/test/ui/feature-gates/feature-gate-public_private_dependencies.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
// This test is different from other feature gate tests. | ||
// Instead of checking that an error occurs without the feature gate, | ||
// it checks that *no* errors/warnings occurs without the feature gate. | ||
// This is due to the fact that 'public_private_dependencies' just enables | ||
// a lint, so disabling it shouldn't cause any code to stop compiling. | ||
|
||
// run-pass | ||
// aux-build:pub_dep.rs | ||
|
||
// Without ![feature(public_private_dependencies)], | ||
// this should do nothing/ | ||
#![deny(exported_private_dependencies)] | ||
|
||
extern crate pub_dep; | ||
|
||
pub struct Foo { | ||
pub field: pub_dep::PubType | ||
} | ||
|
||
fn main() {} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
pub struct OtherType; | ||
pub trait OtherTrait {} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
pub struct PubType; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
// aux-build:priv_dep.rs | ||
// aux-build:pub_dep.rs | ||
// compile-flags: --extern-private priv_dep | ||
#![deny(exported_private_dependencies)] | ||
|
||
// This crate is a private dependency | ||
extern crate priv_dep; | ||
// This crate is a public dependenct | ||
extern crate pub_dep; | ||
|
||
use priv_dep::{OtherType, OtherTrait}; | ||
use pub_dep::PubType; | ||
|
||
// Type from private dependency used in private | ||
// type - this is fine | ||
struct PrivateType { | ||
field: OtherType | ||
} | ||
|
||
pub struct PublicType { | ||
pub field: OtherType, | ||
//~^ ERROR type `priv_dep::OtherType` from private dependency 'priv_dep' in public interface | ||
priv_field: OtherType, // Private field - this is fine | ||
pub other_field: PubType // Type from public dependency - this is fine | ||
} | ||
|
||
impl PublicType { | ||
pub fn pub_fn(param: OtherType) {} | ||
//~^ ERROR type `priv_dep::OtherType` from private dependency 'priv_dep' in public interface | ||
|
||
fn priv_fn(param: OtherType) {} | ||
} | ||
|
||
pub trait MyPubTrait { | ||
type Foo: OtherTrait; | ||
} | ||
//~^^^ ERROR trait `priv_dep::OtherTrait` from private dependency 'priv_dep' in public interface | ||
|
||
pub struct AllowedPrivType { | ||
#[allow(exported_private_dependencies)] | ||
pub allowed: OtherType | ||
} | ||
|
||
|
||
|
||
fn main() {} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
error: type `priv_dep::OtherType` from private dependency 'priv_dep' in public interface | ||
--> $DIR/pub-priv1.rs:21:5 | ||
| | ||
LL | pub field: OtherType, | ||
| ^^^^^^^^^^^^^^^^^^^^ | ||
| | ||
note: lint level defined here | ||
--> $DIR/pub-priv1.rs:4:9 | ||
| | ||
LL | #![deny(exported_private_dependencies)] | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
error: type `priv_dep::OtherType` from private dependency 'priv_dep' in public interface | ||
--> $DIR/pub-priv1.rs:28:5 | ||
| | ||
LL | pub fn pub_fn(param: OtherType) {} | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
error: trait `priv_dep::OtherTrait` from private dependency 'priv_dep' in public interface | ||
--> $DIR/pub-priv1.rs:34:1 | ||
| | ||
LL | / pub trait MyPubTrait { | ||
LL | | type Foo: OtherTrait; | ||
LL | | } | ||
| |_^ | ||
|
||
error: aborting due to 3 previous errors | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
// The 'std' crates should always be implicitly public, | ||
// without having to pass any compiler arguments | ||
|
||
// run-pass | ||
|
||
#![deny(exported_private_dependencies)] | ||
|
||
pub struct PublicType { | ||
pub field: Option<u8> | ||
} | ||
|
||
fn main() {} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.