RFC: Convention: do not prefix exports with the module's name #356

Merged
merged 1 commit into from Oct 15, 2014

Conversation

Projects
None yet
9 participants
@aturon
Member

aturon commented Oct 6, 2014

This is a conventions RFC that proposes that the items exported from a module
should never be prefixed with that module name. For example, we should have
io::Error, not io::IoError.

(An alternative design is included that special-cases overlap with the
prelude.)

Rendered

@aturon aturon changed the title from RFC: Convention: do no prefix exports with the module's name to RFC: Convention: do not prefix exports with the module's name Oct 6, 2014

@aturon

This comment has been minimized.

Show comment
Hide comment
@aturon

aturon Oct 6, 2014

Member

You could also imagine yet another alternative: special case only Result, which is usually re-exported in specalized form and therefore may benefit from a prefix for clarity. Compare:

fn open(path: &Path) -> Result<File>
fn open(path: &Path) -> IoResult<File>

In the first version, it's clear that Result has been specialized since it's taking only a single parameter. But that's easy to miss. It may be worth a bit of redundancy to signal the specialization here, as in the second line.

It is likely the Result is the only item in the prelude that is routinely re-exported in specialized form, so treating it as a truly special case might make sense.

(Still, I personally prefer the proposed version of the rule, with no special cases.)

Member

aturon commented Oct 6, 2014

You could also imagine yet another alternative: special case only Result, which is usually re-exported in specalized form and therefore may benefit from a prefix for clarity. Compare:

fn open(path: &Path) -> Result<File>
fn open(path: &Path) -> IoResult<File>

In the first version, it's clear that Result has been specialized since it's taking only a single parameter. But that's easy to miss. It may be worth a bit of redundancy to signal the specialization here, as in the second line.

It is likely the Result is the only item in the prelude that is routinely re-exported in specialized form, so treating it as a truly special case might make sense.

(Still, I personally prefer the proposed version of the rule, with no special cases.)

@Ericson2314

This comment has been minimized.

Show comment
Hide comment
@Ericson2314

Ericson2314 Oct 6, 2014

Contributor

@aturon I've been thinking about this, and while I agree it is a good goal, certain parts of Rust push users in the opposite direction. The basic pattern I've seen in the std libraries is to prefix the type name, and then provide a naked impl for just about all functionality, then users can write:

use std::io::net::tcp::TcpSocket;
use std::io::net::udp::UdpSocket;

TcpSocket::bind(...);
UdpSocket::bind(...);

and not worry about importing the io module or any other, just types. (Yeah, I know there is no TcpSocket, but suppose there was.)

With these guidelines

use std::io::net::tcp::Socket;
use std::io::net::udp::Socket;

won't work, so it would natural to write:

use std::io::net::tcp;
use std::io::net::udp;

udp::Socket::bind(...);
tcp::Socket::bind(...);

If Socket were the only public/"important" type in each of those modules however, it almost makes sense to go full ML, and write:

use std::io::net::tcp_socket;
use std::io::net::udp_socket;

udp_socket::bind(...); // returns udp_socket::T;
tcp_socket::bind(...); // returns tcp_socket::T;

The point of all this, is naked impls introduce multiple ways to structure code with the exact same semantics. On on extreme there is the io::IoResult redundancy, on the other there is ML's ugly io::result::T. The middle ground of io::Result::method or io::net::UdpSocket::method means moving many types into modules higher up on the hierarchy (what happens to UdpSocket), which means bigger modules/files, or breaking things up into hidden smaller modules per each type, and then pub use temp_mod::*ing them in the parent module.

All these have downsides, and even if one is chosen, it seems likely that code following the other patterns will emerge in practice.

Contributor

Ericson2314 commented Oct 6, 2014

@aturon I've been thinking about this, and while I agree it is a good goal, certain parts of Rust push users in the opposite direction. The basic pattern I've seen in the std libraries is to prefix the type name, and then provide a naked impl for just about all functionality, then users can write:

use std::io::net::tcp::TcpSocket;
use std::io::net::udp::UdpSocket;

TcpSocket::bind(...);
UdpSocket::bind(...);

and not worry about importing the io module or any other, just types. (Yeah, I know there is no TcpSocket, but suppose there was.)

With these guidelines

use std::io::net::tcp::Socket;
use std::io::net::udp::Socket;

won't work, so it would natural to write:

use std::io::net::tcp;
use std::io::net::udp;

udp::Socket::bind(...);
tcp::Socket::bind(...);

If Socket were the only public/"important" type in each of those modules however, it almost makes sense to go full ML, and write:

use std::io::net::tcp_socket;
use std::io::net::udp_socket;

udp_socket::bind(...); // returns udp_socket::T;
tcp_socket::bind(...); // returns tcp_socket::T;

The point of all this, is naked impls introduce multiple ways to structure code with the exact same semantics. On on extreme there is the io::IoResult redundancy, on the other there is ML's ugly io::result::T. The middle ground of io::Result::method or io::net::UdpSocket::method means moving many types into modules higher up on the hierarchy (what happens to UdpSocket), which means bigger modules/files, or breaking things up into hidden smaller modules per each type, and then pub use temp_mod::*ing them in the parent module.

All these have downsides, and even if one is chosen, it seems likely that code following the other patterns will emerge in practice.

@aturon

This comment has been minimized.

Show comment
Hide comment
@aturon

aturon Oct 6, 2014

Member

@Ericson2314

I'm not entirely following your comment. In particular, you say:

With these guidelines,

use std::io::net::tcp::Socket;
use std::io::net::udp::Socket;

won't work ...

but as the guidelines mention, you're free to import types with a prefix to resolve ambiguity:

use std::io::net::tcp::Socket as TcpSocket;
use std::io::net::udp::Socket as UdpSocket;

Essentially, this pushes resolving overlaps between names to the client of an API (where the overlaps are actually created) rather than the provider of an API (who would otherwise have to guess at which overlaps will occur).

Member

aturon commented Oct 6, 2014

@Ericson2314

I'm not entirely following your comment. In particular, you say:

With these guidelines,

use std::io::net::tcp::Socket;
use std::io::net::udp::Socket;

won't work ...

but as the guidelines mention, you're free to import types with a prefix to resolve ambiguity:

use std::io::net::tcp::Socket as TcpSocket;
use std::io::net::udp::Socket as UdpSocket;

Essentially, this pushes resolving overlaps between names to the client of an API (where the overlaps are actually created) rather than the provider of an API (who would otherwise have to guess at which overlaps will occur).

@aturon

This comment has been minimized.

Show comment
Hide comment
@aturon

aturon Oct 6, 2014

Member

@Ericson2314

As an aside, it's almost possible, using associated items, for modules with a single "important" type to just be that type, which would actually be quite nice. Unfortunately, without HKT, this isn't sufficiently expressive to handle common cases (e.g. iterator types).

Member

aturon commented Oct 6, 2014

@Ericson2314

As an aside, it's almost possible, using associated items, for modules with a single "important" type to just be that type, which would actually be quite nice. Unfortunately, without HKT, this isn't sufficiently expressive to handle common cases (e.g. iterator types).

@blaenk

This comment has been minimized.

Show comment
Hide comment
@blaenk

blaenk Oct 6, 2014

Contributor

I completely agree with the proposed design; no special casing please!

Good show 👏

Contributor

blaenk commented Oct 6, 2014

I completely agree with the proposed design; no special casing please!

Good show 👏

@CloudiDust

This comment has been minimized.

Show comment
Hide comment
@CloudiDust

CloudiDust Oct 12, 2014

Contributor

As we can alias the imports, I don't see the need to duplicate information. So +1 to this, with no special cases.

Contributor

CloudiDust commented Oct 12, 2014

As we can alias the imports, I don't see the need to duplicate information. So +1 to this, with no special cases.

@Ericson2314

This comment has been minimized.

Show comment
Hide comment
@Ericson2314

Ericson2314 Oct 14, 2014

Contributor

@aturon I'm sorry. I guess my comment just boils down to that it would be nice if there was only one possible way of doing things, and no need to choose one of client- or provider-side ambiguity in many places. The ML ::t pattern, while ugly, does resolve the latter issue if one assumes nobody would be tempted to open the module and use t directly.

What I should have said first is: A. given the current state of Rust, I think your conventions are the best ones to have. B. I am curious how you interpret your conventions regarding names like std::vec::Vec.

Contributor

Ericson2314 commented Oct 14, 2014

@aturon I'm sorry. I guess my comment just boils down to that it would be nice if there was only one possible way of doing things, and no need to choose one of client- or provider-side ambiguity in many places. The ML ::t pattern, while ugly, does resolve the latter issue if one assumes nobody would be tempted to open the module and use t directly.

What I should have said first is: A. given the current state of Rust, I think your conventions are the best ones to have. B. I am curious how you interpret your conventions regarding names like std::vec::Vec.

@brendanzab

This comment has been minimized.

Show comment
Hide comment
@brendanzab

brendanzab Oct 14, 2014

Member

It's great that you are tackling this @aturon.

How does this work with traits? Referring to stuff prefixed by the module name is useful, but you need to import traits directly in order to access their methods.

Member

brendanzab commented Oct 14, 2014

It's great that you are tackling this @aturon.

How does this work with traits? Referring to stuff prefixed by the module name is useful, but you need to import traits directly in order to access their methods.

@CloudiDust

This comment has been minimized.

Show comment
Hide comment
@CloudiDust

CloudiDust Oct 14, 2014

Contributor

@bjz, I think aliasing can help here.

Contributor

CloudiDust commented Oct 14, 2014

@bjz, I think aliasing can help here.

@alexcrichton alexcrichton referenced this pull request in rust-lang/rust Oct 15, 2014

Closed

Remove module prefixes from type names #18073

@alexcrichton alexcrichton merged commit 4484680 into rust-lang:master Oct 15, 2014

@aturon aturon referenced this pull request in rust-lang/rust Nov 26, 2014

Merged

std: Rewrite the `sync` module #19274

@glaebhoerl

This comment has been minimized.

Show comment
Hide comment
@glaebhoerl

glaebhoerl Dec 7, 2014

Contributor

I largely agree with the principle here, but a notable drawback of the completely purist approach (where IoError becomes io::Error and TcpSocket becomes tcp::Socket) in practice is that the names are much less informative when taken out of context. An IoError or a TcpSocket more-or-less uniquely identifies what the given thing is meant to be. Error and Socket, less so.

This is an issue, for example, in generated documentation. An extreme illustration of the problem is provided by Haskell's numeric-prelude package, which is widely reviled for its author's preferred convention of naming all types T and classes C, and using the module system to disambiguate. I suspect a large part of the sentiment can be attributed to the resulting Haddocks.

If we're going to go down the same route, we need a solution. Clearly in some cases rustdoc should display more than just the name of a referenced item, but also some of the preceding modules in its path. But from where should it know this?

Contributor

glaebhoerl commented Dec 7, 2014

I largely agree with the principle here, but a notable drawback of the completely purist approach (where IoError becomes io::Error and TcpSocket becomes tcp::Socket) in practice is that the names are much less informative when taken out of context. An IoError or a TcpSocket more-or-less uniquely identifies what the given thing is meant to be. Error and Socket, less so.

This is an issue, for example, in generated documentation. An extreme illustration of the problem is provided by Haskell's numeric-prelude package, which is widely reviled for its author's preferred convention of naming all types T and classes C, and using the module system to disambiguate. I suspect a large part of the sentiment can be attributed to the resulting Haddocks.

If we're going to go down the same route, we need a solution. Clearly in some cases rustdoc should display more than just the name of a referenced item, but also some of the preceding modules in its path. But from where should it know this?

@pnkfelix

This comment has been minimized.

Show comment
Hide comment
@pnkfelix

pnkfelix Dec 7, 2014

Member

@glaebhoerl good point.

could rustdoc pick up a preferred name via an attribute? That sounds straight-forward to me, though I have not hacked on rustdoc's source much recently. E.g.

mod io {
    #[rustdoc_name="io::Error"]
    /// <description of io::Error>
    enum Error {
        ...
    }
    ...
}
Member

pnkfelix commented Dec 7, 2014

@glaebhoerl good point.

could rustdoc pick up a preferred name via an attribute? That sounds straight-forward to me, though I have not hacked on rustdoc's source much recently. E.g.

mod io {
    #[rustdoc_name="io::Error"]
    /// <description of io::Error>
    enum Error {
        ...
    }
    ...
}
@glaebhoerl

This comment has been minimized.

Show comment
Hide comment
@glaebhoerl

glaebhoerl Dec 7, 2014

Contributor

That was my first thought as well. A more automated approach which just occurred to me might be that if a given page contains references to more than one item with the same name, prepend path components to each until they differ. That would probably help in a lot of cases, though there may still be ones where, while only one is referenced, more than one exists in the wider universe, and it's not obvious which of them is referred to. But even in that case, if the reader knows about the rule, she could infer that all references on the page are to the same item, and by hovering one of them discover which it is, which is less bad than having to hover all of them in case one of them is different. (Perhaps there could also be more sophisticated approaches like checking whether more than one item with that name exists in the entire crate-it's-from or crate-being-documented.)

Contributor

glaebhoerl commented Dec 7, 2014

That was my first thought as well. A more automated approach which just occurred to me might be that if a given page contains references to more than one item with the same name, prepend path components to each until they differ. That would probably help in a lot of cases, though there may still be ones where, while only one is referenced, more than one exists in the wider universe, and it's not obvious which of them is referred to. But even in that case, if the reader knows about the rule, she could infer that all references on the page are to the same item, and by hovering one of them discover which it is, which is less bad than having to hover all of them in case one of them is different. (Perhaps there could also be more sophisticated approaches like checking whether more than one item with that name exists in the entire crate-it's-from or crate-being-documented.)

@Ericson2314

This comment has been minimized.

Show comment
Hide comment
@Ericson2314

Ericson2314 Dec 8, 2014

Contributor

+1 For the automatic approach.

I would love it if the exact algorithm takes account of both what's on the page and what's in the crate. Consider that it will probably be common to libraries to make Error enums where one variant wraps another library's Error enum (serialize::json::ParserError is almost an example of this). It would be most convenient to see io::Error here too, even if Io::Error is the only type called Error in std.

Contributor

Ericson2314 commented Dec 8, 2014

+1 For the automatic approach.

I would love it if the exact algorithm takes account of both what's on the page and what's in the crate. Consider that it will probably be common to libraries to make Error enums where one variant wraps another library's Error enum (serialize::json::ParserError is almost an example of this). It would be most convenient to see io::Error here too, even if Io::Error is the only type called Error in std.

@sfackler

This comment has been minimized.

Show comment
Hide comment
@sfackler

sfackler Dec 8, 2014

Member

The #[rustdoc_name="foo"] strategy seems a bit risky - I could see the name rotting as an item moves around. Maybe something like #[doc(name_depth="1")] would work (i.e. add one module of context to the name).

Member

sfackler commented Dec 8, 2014

The #[rustdoc_name="foo"] strategy seems a bit risky - I could see the name rotting as an item moves around. Maybe something like #[doc(name_depth="1")] would work (i.e. add one module of context to the name).

@chriskrycho chriskrycho referenced this pull request in rust-lang-nursery/reference Mar 29, 2017

Closed

Document all features #9

18 of 48 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment