Does the "avoid prefixing the type with the name of the module" style rule still apply?

Before 1.0, we landed an RFC that suggested that instead of

mod io {
 struct IoError;
}

we should do

mod io {
   struct Error;
}

with the expectation that folks would use io; and sprinkle io::Error through their code.

In my experience, Rust code doesn't really follow this. Instead, Rust code tends to avoid using multi-level paths (foo::bar) inline (except for free functions, which presumably people want to strongly signal come from outside the module), and people often will do use io::Error as IoError instead.

What are people's thoughts on this? Should we perhaps change this style guideline?

3 Likes

Honestly, I think it depends on the situation. Something like Error should almost always be named exactly that, letting the user use the path (or import alias) as necessary. But for other structs/enums? I find myself frequently silencing clippy — to the point of just blanket #![allow]ing it crate-wide normally.

So perhaps just eliminate the style guideline entirely? Unless others can think of a better way to express what I'm saying.

2 Likes

I prefer the "no repeats" style. I also use things like io::Error often enough (but not always).

5 Likes

I have no idea what is “normal“ in Rust code generally, but I very much prefer the module-name-based approach, and that is how I write all of my code. It means that in general, code within a module can be much briefer, and effectively self-descriptive, and then its public usage can be clarified by name-spacing as appropriate. (I do exactly the same in other languages as well, to the point where languages would you do not allow that kind of easy module-based name spacing – looking at you, Swift! – frustrate me at times!)

3 Likes

I prefer the module-name approach, so that there's one consistent prefix. For instance, git2::Repository and git2::Tree, not git2::GitRepository and git2::GitTree. If I need to disambiguate, I can qualify with the module. In some cases, I don't have to; for instance, a project primarily focused on git can just use Tree and Repository.

The only time I tend to use renaming on import is with use std::io::Write as IoWrite; and use std::fmt::Write as FmtWrite;. (And that's only if I need to impl those; if I just need to use them I sometimes use std::io::Write as _;.) Those two traits should not have had the same name.

16 Likes

Yeah, I also prefer the module name approach for git2::Repository, but for very generic names like Error I feel differently.

2 Likes

It’s actually on exactly those places where I most like the module-scoped version, actually! But some of that may be because I nearly always design things (and use things) with this naming pattern in mind, so I nearly always use io and almost never use io::Error unless it’s in a context where it is sufficiently scoped and unambiguous that it doesn’t matter. :thinking:

2 Likes

I think std is unusual with its deeply nested modules. It's probably because std is many times larger than an average crate, and most crates just aren't big enough to need to finely subdivide their public API.

I am mildly annoyed by ambiguous Result aliases. result::Result has two args, io::Result has one, fmt::Result has zero. Then every crate has its own Result flavor with its own Error kind, so use foo::* can lead to baffling Result breakage.

However, I think it could be fixable with better rustdoc output (with intelligently shortened path prefixes) and maybe fine-tuning of compiler error messages.

5 Likes

Reason #1302 to not use glob imports on external crates…

Admittedly, for internal usage globs can be useful, but you should be well aware of what's happening in that situation.

4 Likes

This is specifically targetable (in libraries exposing a new Result alias), at least.

Rather than

type Result<T> = std::result::Result<T, crate::Error>;

always prefer

type Result<T, E=crate::Error> = std::result::Result<T, E>;

This doesn't solve the collision case, but it does allow you to actually import the name and still use a specific error.

(And the collision case is more an argument against glob imports of modules not designed for glob importing.)

I think in general libraries should try to optimize for legibility of use as module::Type, because for people who want to use it with the prefix, they have to use the library's name. But when imported unprefixed, you have the opportunity to change the name.

As the example, if std provided io::IoError, I would have to import it directly as IoError. But because it's io::Error, I can use it as io::Error or as IoError.

(Interesting side idea: allow modules to mark certain names as not being brought into scope by a glob import? Probably more surprising than useful, tbh.)

8 Likes

but it also applies to use std::io::* and use std::fmt::*. I know there's std::io::prelude::*. I'm not sure if there's one for fmt, but also that area seems under-designed a bit. There's magical global prelude and there are DIY informal preludes all over the place without language support, while the shortest most supported syntax is a known footgun.

I‘d say it depends on the design of a particular library/module.

Sometimes the library encompasses many types, and has a classy_name::, so it makes sense to design it for prefixed use (good example here is importing fmt module to manually implement Debug).

Sometime, a library exports a couple of vocabulary types, which are better imported (Mutexes and atomics, for example)

And the hard case is when you have both. My worst example here is HashMap, which you generally use unqualified, unless you need to match on hash_map::Entry. You either need to import module as well (inconsistently using imports) or to use unqualified Entry, which just reads as too general, and might clash with another Entry in scope.

2 Likes

Consider the situation of once_cell but also image and others. Both have a similar style, a bunch of top-level modules for grouping but each module has similar or even the same content and a high overlap of names. Basically, you can not import items from both modules without renaming. In image we thus have gone the route of applying a prefix against the style rules (i.e. image::png::PngDecoder) since this is somewhat of an expected case and the module names alone are generic enough to collide with names in their own right, when importing them alone as a prefix.

I wonder if it were possible to combine these two approaches, e.g. by adding some feature of glob imports with automatic prefix or prefixes per scoping. Then renaming them to the style rules would be more comfortable.

// Rough sketch for possible syntax
use image::png::Decoder as Png*;
let _ = PngDecoder::new(...);
4 Likes

The prefix seems fine for "classy" package names, but that isn't always possible. For example, icu_plural_rules::Error is not as nice as something like PluralsError. The prefix on the type name can be quite a bit shorter than the crate name because it doesn't need to be unique in the global crate namespace.

This pretty much describes where I'm at. I'm using (1 level of) module prefixes more and more, and think that's actually a great way to clean up use imports.

But that said, when you wind up with a lot of types with the same name flying around, it can get confusing.

Another place where it's problematic is rustdoc. It may be very clear when you're looking at actual code what's happening with a bunch of module prefixes, but that information is lost in the rustdoc, and when you see Error it becomes trickier to answer the question of "which Error?" (you have to at least mouseover and read the link, or click through)

8 Likes

There is an open issue for this: https://github.com/rust-lang/rust/issues/74924

2 Likes

The general strategy I've taken for this is:

  • If a module has a single error type, name it Error and reference it as mod::Error elsewhere. Exposing a Result is optional (I'm not a fan of custom Result aliases).
  • If a module needs to expose multiple errors, name them FooError and reference them as FooError (I try to avoid this).
  • Avoid ::* like the plague because it makes it really hard to find where a name came from. (The way rustc does this drives me nuts.) super::* is obviously ok for inline modules, and it's probably ok to use when doing re-exports.
  • Avoid preludes. Writing five lines of use foo::Trait as _; never hurt anyone.

I think Go's flat package namespace a bit insane, but the idea that all external names are referred to as package.Name is a pretty solid one. I also never import free functions directly, and always write mem::size_of etc.

4 Likes

You can always rename it while importing so eg you can do use std::io::IoError as Error; if you so desire.

In that case, why not do use hash_map::Entry as HashMapEntry; or something like it? When it isn't clear from context that Entry is specifically tied to HashMap, that's what I'd probably do.

In that case, why not do use hash_map::Entry as HashMapEntry; or something like it?

In my aesthetics, this would be worse than both qualified hash_map::Entry and unqualified Entry, I assign high cost to adding synonyms.

2 Likes

May I ask why?

I ask because personally I've never attached such a cost, and I think that's mostly because I can use rust-analyzer to jump to a definition, and when that works (which sadly it doesn't all the time) it's not really ambiguous for me what's what.