From de2fc0b7850962616fb8e1531b84b9b14d2a7edd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Lehel?= Date: Tue, 24 Jun 2014 23:02:54 +0200 Subject: [PATCH 1/5] RFC: Ban private items in public APIs --- active/0000-no-privates-in-public.md | 230 +++++++++++++++++++++++++++ 1 file changed, 230 insertions(+) create mode 100644 active/0000-no-privates-in-public.md diff --git a/active/0000-no-privates-in-public.md b/active/0000-no-privates-in-public.md new file mode 100644 index 00000000000..58f0dfeb934 --- /dev/null +++ b/active/0000-no-privates-in-public.md @@ -0,0 +1,230 @@ +- Start Date: 2014-06-24 +- RFC PR #: (leave this empty) +- Rust Issue #: (leave this empty) + +# Summary + +Make it illegal to expose private items in public APIs. + + +# Motivation + +Seeing private items in the types of public items is weird. + +It leads to various "what if" scenarios we need to think about and deal with, +and it's easier to avoid them altogether. + +It's the safe choice for 1.0, because we can liberalize things later if we +choose to, but not make them more restrictive. + +If I see an item mentioned in rust-doc generated documentation, I should be +able to click it to see its documentation in turn. + + +# Detailed design + +## Overview + +The general idea is that: + + * If an item is publicly exposed by a module `module`, items referred to in + the public-facing parts of that item (e.g. its type) must themselves be + public. + + * An item referred to in `module` is considered to be public if it is visible + to clients of `module`. + +Details follow. + + +## The rules + +An item is considered to be publicly exposed by a module if it is declared `pub` +by that module, or if it is re-exported using `pub use` by that module. + +For items which are publicly exposed by a module, the rules are that: + + * If it is a `static` declaration, items referred to in its type must be public. + + * If it is an `fn` declaration, items referred to in its trait bounds, argument + types, and return type must be public. + + * If it is a `struct` or `enum` declaration, items referred to in its trait + bounds and in the types of its `pub` fields must be public. + + * If it is a `type` declaration, items referred to in its definition must be + public. + + * If it is a `trait` declaration, items referred to in its super-traits, in the + trait bounds of its type parameters, and in the signatures of its methods + (see `fn` case above) must be public. + + +## What does "public" mean? + +An item `Item` referred to in the module `module` is considered to be public if: + + * The qualified name used by `module` to refer to `Item`, when recursively + resolved through `use` declarations back to the original declaration of + `Item`, resolves along the way to at least one `pub` declaration, whether a + `pub use` declaration or a `pub` original declaration; and + + * For at least one of the above resolved-to `pub` declarations, all ancestor + modules of the declaration, up to the deepest common ancestor module of the + declaration with `module`, are `pub`. + +In all other cases, an `Item` referred to in `module` is not considered to be +public, or `module` itself cannot refer to `Item` and the distinction is +irrelevant. + +### Examples + +In the following examples, the item `Item` referred to in the module `module` +is considered to be public: + +```` +pub mod module { + pub struct Item { ... } +} +```` + +```` +pub struct Item { ... } +pub mod module { + use Item; +} +```` + +```` +pub mod x { + pub struct Item { ... } +} +pub mod module { + use x::Item; +} +```` + +```` +pub mod module { + pub mod x { + pub struct Item { ... } + } + use self::x::Item; +} +```` + +```` +struct Item { ... } +pub mod module { + pub use Item; +} +```` + +```` +struct Foo { ... } +pub use Item = Foo; +pub mod module { + use Item; +} +```` + +```` +struct Foo { ... } +pub use Bar = Foo; +use Item = Bar; +pub mod module { + use Item; +} +```` + +```` +struct Item { ... } +pub mod x { + pub use Item; + pub mod y { + use x::Item; + pub mod module { + use super::Item; + } + } +} +```` + +In the above examples, it is assumed that `module` will refer to `Item` as +simply `Item`, but the same thing holds true if `module` refrains from importing +`Item` explicitly with a private `use` declaration, and refers to it directly by +qualifying it with a path instead. + + +In the below examples, the item `Item` referred to in the module `module` is +*not* considered to be public: + +```` +pub mod module { + struct Item { ... } +} +```` + +```` +struct Item { ... } +pub mod module { + use Item; +} +```` + +```` +mod x { + pub struct Item { ... } +} +pub mod module { + use x::Item; +} +```` + +```` +pub mod module { + use self::x::Item; + mod x { + pub struct Item { ... } + } +} +```` + +```` +struct Item { ... } +pub use Alias = Item; +pub mod x { + pub use Item; + pub mod module { + use Item; // refers to top-level `Item` + } +} +```` + + +# Drawbacks + +Requires effort to implement. + +May break existing code. + +It may turn out that there are use cases which become inexpressible. I don't +expect this, but if there are, we should consider solutions to them on a +case-by-case basis. + + +# Alternatives + +The alternative is the status quo, and the impact of not doing this is that +we'll have to live with it forever. *(dramatic music)* + + +# Unresolved questions + +Is this the right set of rules to apply? + +Did I describe them correctly in the "Detailed design"? + +Did I miss anything? Are there any holes or contradictions? + +Is there a simpler, easier, and/or more logical formulation? From eb1eb06b09f9456df9717e8a555ab61601446dbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Lehel?= Date: Thu, 26 Jun 2014 13:11:35 +0200 Subject: [PATCH 2/5] expand the Motivation and Drawbacks sections with examples --- active/0000-no-privates-in-public.md | 60 ++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 3 deletions(-) diff --git a/active/0000-no-privates-in-public.md b/active/0000-no-privates-in-public.md index 58f0dfeb934..68b1f9a1bdd 100644 --- a/active/0000-no-privates-in-public.md +++ b/active/0000-no-privates-in-public.md @@ -20,6 +20,37 @@ choose to, but not make them more restrictive. If I see an item mentioned in rust-doc generated documentation, I should be able to click it to see its documentation in turn. +## Examples + +(See also https://github.com/rust-lang/rust/issues/10573.) + + * Being able to use a given type but not *write* the type is disconcerting. + For example: + + struct Foo { ... } + pub fn foo() -> Foo { ... } + + As a client of this module, I can write `let my_foo = foo();`, but not + `let my_foo: Foo = foo();`. This is a logical consequence of the rules, but + it doesn't make any sense. + + * Can I access public fields of a private type? For instance: + + struct Foo { pub x: int, ... } + pub fn foo() -> Foo { ... } + + Can I now write `foo().x`, even though the struct `Foo` itself is not visible + to me, and in rust-doc, I couldn't see its documentation? In other words, + when the only way I could learn about the field `x` is by looking at the + source code for the module? + + * Can I call methods on a private type? Can I "know about" its trait `impl`s? + +Again, it's surely possible to formulate rules such that answers to these +questions can be deduced from them mechanically. But even thinking about it +gives me the creeps. If the results arrived at are absurd, then our assumptions +should be revisited. In this case, it's cleaner to just say "don't do that". + # Detailed design @@ -208,10 +239,33 @@ Requires effort to implement. May break existing code. -It may turn out that there are use cases which become inexpressible. I don't -expect this, but if there are, we should consider solutions to them on a -case-by-case basis. +It may turn out that there are use cases which become inexpressible. If there +are, we should consider solutions to them on a case-by-case basis. +One such use case is constraining types in the public interface to a trait, +but not permitting anyone outside the module to implement the trait. For +instance: + + trait Private {} + pub fn public() { ... } + +Similarly, you may want a public trait which is closed to external +implementations: + + pub trait MyClosedTrait: Private { ... } + +I believe that if these use cases are deemed important, then they should be +addressed directly, by allowing traits to be declared closed to external +implementations explicitly. Possible syntaxes include: + +```` +#[no_external_impls] +pub trait MyClosedTrait { ... } +```` + +```` +pub closed trait MyClosedTrait { ... } +```` # Alternatives From 7cb74e20560075d3e225b1ba92182995b6c530f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Lehel?= Date: Thu, 26 Jun 2014 13:14:28 +0200 Subject: [PATCH 3/5] markdown fix --- active/0000-no-privates-in-public.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/active/0000-no-privates-in-public.md b/active/0000-no-privates-in-public.md index 68b1f9a1bdd..0bbc7cc4e26 100644 --- a/active/0000-no-privates-in-public.md +++ b/active/0000-no-privates-in-public.md @@ -27,8 +27,8 @@ able to click it to see its documentation in turn. * Being able to use a given type but not *write* the type is disconcerting. For example: - struct Foo { ... } - pub fn foo() -> Foo { ... } + struct Foo { ... } + pub fn foo() -> Foo { ... } As a client of this module, I can write `let my_foo = foo();`, but not `let my_foo: Foo = foo();`. This is a logical consequence of the rules, but @@ -36,8 +36,8 @@ able to click it to see its documentation in turn. * Can I access public fields of a private type? For instance: - struct Foo { pub x: int, ... } - pub fn foo() -> Foo { ... } + struct Foo { pub x: int, ... } + pub fn foo() -> Foo { ... } Can I now write `foo().x`, even though the struct `Foo` itself is not visible to me, and in rust-doc, I couldn't see its documentation? In other words, From 2346d7e1f90a6ce765ecf5e07ff7a51df38697f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Lehel?= Date: Thu, 26 Jun 2014 13:38:00 +0200 Subject: [PATCH 4/5] clarify a thing --- active/0000-no-privates-in-public.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/active/0000-no-privates-in-public.md b/active/0000-no-privates-in-public.md index 0bbc7cc4e26..60d29a021aa 100644 --- a/active/0000-no-privates-in-public.md +++ b/active/0000-no-privates-in-public.md @@ -49,7 +49,8 @@ able to click it to see its documentation in turn. Again, it's surely possible to formulate rules such that answers to these questions can be deduced from them mechanically. But even thinking about it gives me the creeps. If the results arrived at are absurd, then our assumptions -should be revisited. In this case, it's cleaner to just say "don't do that". +should be revisited. In these cases, it would be cleaner to simply say, +"don't do that". # Detailed design @@ -267,6 +268,10 @@ pub trait MyClosedTrait { ... } pub closed trait MyClosedTrait { ... } ```` +Adding this is not an integral part of this RFC, and my preference would be to +discuss it separately. It's only an option in case the mentioned functionality +is considered too valuable to lose even temporarily, which I personally doubt. + # Alternatives The alternative is the status quo, and the impact of not doing this is that From 20c571410cbad1f8ec1b99d99088cee60f1e3d86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Lehel?= Date: Fri, 8 Aug 2014 18:19:09 +0200 Subject: [PATCH 5/5] greatly expand and sharpen the Motivation section based on discussion in the comment thread --- active/0000-no-privates-in-public.md | 277 +++++++++++++++++++++------ 1 file changed, 215 insertions(+), 62 deletions(-) diff --git a/active/0000-no-privates-in-public.md b/active/0000-no-privates-in-public.md index 60d29a021aa..5076dadd45e 100644 --- a/active/0000-no-privates-in-public.md +++ b/active/0000-no-privates-in-public.md @@ -4,23 +4,31 @@ # Summary -Make it illegal to expose private items in public APIs. +Require a feature gate to expose private items in public APIs, until we grow the +appropriate language features to be able to remove the feature gate and forbid +it entirely. # Motivation -Seeing private items in the types of public items is weird. +Only those language features should be a part of 1.0 which we've intentionally +committed to supporting, and to preserving their behavior indefinitely. -It leads to various "what if" scenarios we need to think about and deal with, -and it's easier to avoid them altogether. +The ability to mention private types in public APIs, and the apparent usefulness +of this ability for various purposes, is something that happened and was +discovered by accident. It also gives rise to various bizarre +[questions][questions] and interactions which we would need to expend effort +thinking about and answering, and which it would be much preferable to avoid +having to do entirely. -It's the safe choice for 1.0, because we can liberalize things later if we -choose to, but not make them more restrictive. +The only intentionally designed mechanism for representation hiding in the +current language is private fields in `struct`s (with newtypes as a specific +case), and this is the mechanism which should be used for it. -If I see an item mentioned in rust-doc generated documentation, I should be -able to click it to see its documentation in turn. +[questions]: https://github.com/rust-lang/rust/issues/10573 -## Examples + +## Examples of strangeness (See also https://github.com/rust-lang/rust/issues/10573.) @@ -35,22 +43,184 @@ able to click it to see its documentation in turn. it doesn't make any sense. * Can I access public fields of a private type? For instance: - + struct Foo { pub x: int, ... } pub fn foo() -> Foo { ... } Can I now write `foo().x`, even though the struct `Foo` itself is not visible - to me, and in rust-doc, I couldn't see its documentation? In other words, + to me, and in `rust-doc`, I couldn't see its documentation? In other words, when the only way I could learn about the field `x` is by looking at the - source code for the module? + source code for the given module? - * Can I call methods on a private type? Can I "know about" its trait `impl`s? + * Can I call public methods on a private type? + + * Can I "know about" `trait`s the private type `impl`s? Again, it's surely possible to formulate rules such that answers to these -questions can be deduced from them mechanically. But even thinking about it -gives me the creeps. If the results arrived at are absurd, then our assumptions -should be revisited. In these cases, it would be cleaner to simply say, -"don't do that". +questions can be deduced from them mechanically. But that doesn't mean it's a +good idea to do so. If the results are bizarre, then our assumptions should be +reconsidered. In these cases, it would be wiser to simply say, "don't do that". + + +## Properties + +By restricting public APIs to only mentioning public items, we can guarantee that: + +*Only public definitions are reachable through the public surface area of an API.* + +Or in other words: for any item I see mentioned in `rust-doc` generated +documentation, I can *always* click it to see *its* documentation, in turn. + +Or, dually: + +*The presence or absence of private definitions should not be observable or +discoverable through the public API.* + +As @aturon put it: + +> One concrete problem with allowing private items to leak is that you lose some +> local reasoning. You might expect that if an item is marked private, you can +> refactor at will without breaking clients. But with leakage, you can't make +> this determination based on the item alone: you have to look at the entire API +> to spot leakages (or, I guess, have the lint do so for you). Perhaps not a +> huge deal in practice, but worrying nonetheless. + + +## Use cases for exposing private items, and preferable solutions + +### Abstract types + +One may wish to use a private type in a public API to hide its implementation, +either by using the private type in the API directly, or by defining a +`pub type` synonym for it. + +The correct solution in this case is to use a newtype instead. However, this can +currently be an unacceptably heavyweight solution in some cases, because one +must manually write all of the trait `impl`s to forward from the newtype to the +old type. This should be resolved by adding a [newtype deriving feature][gntd] +along the same lines as GHC (based on the same infrastructure as +[`Transmute`][91], née `Coercible`), or possibly with first-class module-scoped +existential types a la ML. + +[gntd]: https://www.haskell.org/ghc/docs/7.8.1/html/users_guide/deriving.html#newtype-deriving +[91]: https://github.com/rust-lang/rfcs/pull/91 + + +### Private supertraits + +A use case for private supertraits currently is to prevent outside modules from +implementing the given trait, and potentially to have a private interface for +the given types which is accessible only from within the given module. For +example: + + trait PrivateInterface { + fn internal_id(&self) -> uint; + } + + pub trait PublicInterface: PrivateInterface { + fn name(&self) -> String; + ... + } + + pub struct Foo { ... } + pub struct Bar { ... } + + impl PrivateInterface for Foo { ... } + impl PublicInterface for Foo { ... } + impl PrivateInterface for Bar { ... } + impl PublicInterface for Bar { ... } + + pub fn do_thing_with(x: &T) { + // PublicInterface implies PrivateInterface! + let id = x.internal_id(); + ... + } + +Here `PublicInterface` may only be implemented by us, because it requires +`PrivateInterface` as a supertrait, which is not exported outside the module. +Thus `PublicInterface` is only implemented by a closed set of types which we +specify. Public functions may require `PublicInterface` to be generic over this +closed set of types, and in their implementations, they may also use the methods +of the private `PrivateInterface` supertrait. + +The better solution for this use case, which doesn't require exposing +a `PrivateInterface` in the public-facing parts of the API, would be to have +private trait methods. This can be seen by considering the analogy of `trait`s +as generic `struct`s and `impl`s as `static` instances of those `struct`s (with +the compiler selecting the appropriate instance based on type inference). +Supertraits can also be modelled as additional fields. + +For example: + + pub trait Eq { + fn eq(&self, other: &Self) -> bool; + fn ne(&self, other: &Self) -> bool; + } + + impl Eq for Foo { + fn eq(&self, other: &Foo) -> bool { /* def eq */ } + fn ne(&self, other: &Foo) -> bool { /* def ne */ } + } + +This corresponds to: + + pub struct Eq { + pub eq: fn(&Self, &Self) -> bool, + pub ne: fn(&Self, &Self) -> bool + } + + pub static EQ_FOR_FOO: Eq = { + eq: |&this, &other| { /* def eq */ }, + ne: |&this, &other| { /* def ne */ } + }; + +Now if we consider the private supertrait example from above, that becomes: + + struct PrivateInterface { + pub internal_id: fn(&Self) -> uint + } + + pub struct PublicInterface { + pub super0: PrivateInterface, + pub name: fn(&Self) -> String + }; + +We can see that this solution is analogous to the same kind of +private-types-in-public-APIs situation which we want to forbid. And it sheds +light on a hairy question which had been laying hidden beneath the surface: +outside modules can't see `PrivateInterface`, but can they see `internal_id`? +We had been assuming "no", because that was convenient, but rigorously thinking +it through, `trait` methods are conceptually public, so this wouldn't +*necessarily* be the "correct" answer. + +The *right* solution here is the same as for `struct`s: private fields, or +correspondingly, private methods. In other words, if we were working with +`struct`s and `static`s directly, we would write: + + pub struct PublicInterface { + pub name: fn(&Self) -> String, + internal_id: fn(&Self) -> uint + } + +so the public data is public and the private data is private, no mucking around +with the visibility of their *types*. Correspondingly, we would like to write +something like: + + pub trait PublicInterface { + fn name(&self) -> String; + priv fn internal_id(&self) -> uint; + } + +(Note that this is **not** a suggestion for particular syntax.) + +If we can write this, everything we want falls out straightforwardly. +`internal_id` is only visible inside the given module, and outside modules can't +access it. Furthermore, just as you can't construct a (`static` or otherwise) +instance of a `struct` if it has inaccessible private fields, you also can't +construct an `impl` of a `trait` if it has inaccessible private methods. + +So private supertraits should also be put behind a feature gate, like everything +else, until we figure out how to add private `trait` methods. # Detailed design @@ -60,10 +230,10 @@ should be revisited. In these cases, it would be cleaner to simply say, The general idea is that: * If an item is publicly exposed by a module `module`, items referred to in - the public-facing parts of that item (e.g. its type) must themselves be + the public-facing parts of that item (e.g. its type) must themselves be public. - * An item referred to in `module` is considered to be public if it is visible + * An item referred to in `module` is considered to be public if it is visible to clients of `module`. Details follow. @@ -71,6 +241,9 @@ Details follow. ## The rules +These rules apply as long as the feature gate is not enabled. After the feature +gate has been removed, they will apply always. + An item is considered to be publicly exposed by a module if it is declared `pub` by that module, or if it is re-exported using `pub use` by that module. @@ -81,14 +254,14 @@ For items which are publicly exposed by a module, the rules are that: * If it is an `fn` declaration, items referred to in its trait bounds, argument types, and return type must be public. - * If it is a `struct` or `enum` declaration, items referred to in its trait + * If it is a `struct` or `enum` declaration, items referred to in its trait bounds and in the types of its `pub` fields must be public. - * If it is a `type` declaration, items referred to in its definition must be + * If it is a `type` declaration, items referred to in its definition must be public. * If it is a `trait` declaration, items referred to in its super-traits, in the - trait bounds of its type parameters, and in the signatures of its methods + trait bounds of its type parameters, and in the signatures of its methods (see `fn` case above) must be public. @@ -97,16 +270,16 @@ For items which are publicly exposed by a module, the rules are that: An item `Item` referred to in the module `module` is considered to be public if: * The qualified name used by `module` to refer to `Item`, when recursively - resolved through `use` declarations back to the original declaration of + resolved through `use` declarations back to the original declaration of `Item`, resolves along the way to at least one `pub` declaration, whether a `pub use` declaration or a `pub` original declaration; and - * For at least one of the above resolved-to `pub` declarations, all ancestor + * For at least one of the above resolved-to `pub` declarations, all ancestor modules of the declaration, up to the deepest common ancestor module of the declaration with `module`, are `pub`. - + In all other cases, an `Item` referred to in `module` is not considered to be -public, or `module` itself cannot refer to `Item` and the distinction is +public, or `module` itself cannot refer to `Item` and the distinction is irrelevant. ### Examples @@ -182,13 +355,13 @@ pub mod x { } ```` -In the above examples, it is assumed that `module` will refer to `Item` as +In the above examples, it is assumed that `module` will refer to `Item` as simply `Item`, but the same thing holds true if `module` refrains from importing `Item` explicitly with a private `use` declaration, and refers to it directly by qualifying it with a path instead. -In the below examples, the item `Item` referred to in the module `module` is +In the below examples, the item `Item` referred to in the module `module` is *not* considered to be public: ```` @@ -236,46 +409,26 @@ pub mod x { # Drawbacks -Requires effort to implement. - -May break existing code. - -It may turn out that there are use cases which become inexpressible. If there -are, we should consider solutions to them on a case-by-case basis. - -One such use case is constraining types in the public interface to a trait, -but not permitting anyone outside the module to implement the trait. For -instance: - - trait Private {} - pub fn public() { ... } - -Similarly, you may want a public trait which is closed to external -implementations: +Adds a (temporary) feature gate. - pub trait MyClosedTrait: Private { ... } +Requires some existing code to opt-in to the feature gate before transitioning +to saner alternatives. -I believe that if these use cases are deemed important, then they should be -addressed directly, by allowing traits to be declared closed to external -implementations explicitly. Possible syntaxes include: +Requires effort to implement. -```` -#[no_external_impls] -pub trait MyClosedTrait { ... } -```` -```` -pub closed trait MyClosedTrait { ... } -```` +# Alternatives -Adding this is not an integral part of this RFC, and my preference would be to -discuss it separately. It's only an option in case the mentioned functionality -is considered too valuable to lose even temporarily, which I personally doubt. +If we stick with the status quo, we'll have to resolve several bizarre questions +and keep supporting its behavior indefinitely after 1.0. -# Alternatives +Instead of a feature gate, we could just ban these things outright right away, +at the cost of temporarily losing some convenience and a small amount of +expressiveness before the more principled replacement features are implemented. -The alternative is the status quo, and the impact of not doing this is that -we'll have to live with it forever. *(dramatic music)* +We could make an exception for private supertraits, as these are not quite as +problematic as the other cases. However, especially given that a more principled +alternative is known (private methods), I would rather not make any exceptions. # Unresolved questions @@ -286,4 +439,4 @@ Did I describe them correctly in the "Detailed design"? Did I miss anything? Are there any holes or contradictions? -Is there a simpler, easier, and/or more logical formulation? +Is there a simpler, easier, and/or more logical formulation of the rules?