Stuttering

[dtolnay]

Should it be log::Level or log::LogLevel? This comes up all the time and we've never really settled it as a community. So far we have tended to encourage importing types rather than modules:

use log::LogLevel; // encouraged

let level: log::LogLevel; // discouraged

But this works less well if the type relies on the module name to disambiguate what it means, as in the case of io::Result.

use std::io::Result; // probably not what you want in most cases

To add to the mess, we have tended to discourage renaming imports.

use std::io::Result as IoResult; // discouraged

How do we reconcile all of these?

[dtolnay]

Clippy currently has a lint against types that stutter, but it is disabled by default.

[dtolnay]

There might be some guidance from the C++ world. In particular, Google's C++ style guide disallows using namespace so they must have conventions to avoid stutter.

[dpc]

IMO, Go forcing everything imported to be qualified by import name did the right thing.

[seanmonstar]

An interesting example is the std::fmt module. You typically are not encouraged to import types from the module, but just import the module itself and qualify all usage. For example:

impl fmt::Display for Foo {
    fn fmt(&self, f: &muy fmt::Formatter) -> fmt::Result {
        // ...
    }
}

[dtolnay]

That's an interesting one. For whatever reason, in my code I have found that I prefer this:

use std::fmt::{self, Debug, Display};

impl Display for Foo {
    fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
        /* ... */
    }
}

[daboross]

I think this should provide at least one canonical guidance for this: https://github.com/rust-lang/rfcs/blob/master/text/0356-no-module-prefixes.md. It's about modules rather than crates, but I think the same reasons apply?

I originally saw this convention at https://aturon.github.io/style/naming/README.html#avoid-redundant-prefixes-[rfc-356] too.

Note that the RFC mentions use io::Error as IoError; too - I believe as a "good" workaround?

[seanmonstar]

As for renaming imports, the most common place I do that is use std::error::Error as StdError;.

[Kixunil]

I prefer use std::fmt. Same with io. I dislike stuttering.

[budziq]

Hmm, in reqwest I can see
redirect::{RedirectPolicy, RedirectAction, RedirectAttempt};
This could also qualify as stuttering.

[seanmonstar]

True, though the naming was because I don't expect to expose the internal "redirect" module, so they can just be 'Policy' and so on. I suppose they could internally, with a public reexport introducing the prefix, but either option doesn't seem much better in the end.

[budziq]

So going further with reqwest as an example. Assuming that Redirect module is made public seanmonstar/reqwest#105 should its types be "destuttered" by default along with the export?

Or is certain level of repetition is still encouraged as is the case Client::ClientBuilder / Request::RequestBuilder?

[jonas-schievink]

This came up in mlua-rs/rlua#15, too.

To add to the mess, we have tended to discourage renaming imports.

That'd make rluas API a lot more noisy to use, since it would export a lot of generic types (including String) if the Lua prefix is removed.

[Kixunil]

@jonas-schievink LuaString, LuaResult, etc seem like a reasonable exceptions to the rule to me.

[carllerche]

Any conclusion? I need answers, but have no thoughts :)

[tshepang]

Not stuttering is good... I like:

use std::io;
fn foo() -> io::Result<()> {
...

[tshepang]

Renames are fine too, so long as they don't stutter. Also, I would not love seeing this (which actually stutters too):

use std::error::Error as StdError;

[dtolnay]

People will use module-qualified paths if they are sensible and pervasive in your documentation and usage examples. "Sensible" is a function of number of characters typed and how unique the name is.

In my experience people are fine typing 2-3 character modules and mostly fine with 4 character modules. More than that is asking too much, meaning that items in a 5+ character module or library better have a name that can stand alone.

• carl::Options -- if your documentation writes it this way pervasively, users typically will as well.
• zx::Options -- practically all users will be okay with this.
• carl::options::Options -- this is probably the worst. Users cannot import carl::options and then deal with options::Options because options::Options is dumb. Users cannot import carl::options::Options because it looks too much like prelude's Option. Users are forced to import and rename to CarlOptions or something.
• carl_framework::Options -- few people will type this any more often than twice in a file, even if your documentation uses it. Framework will need to give Options a more unique name so users can write an import.
• carl_framework::CarlOptions -- this is fine, users will import. Better than carl_framework::Options.
• carl::CarlOptions -- worse than carl::Options.

Certain items have somewhat higher thresholds between sensible and not sensible. For Error and Result types I would say the threshold is 8-10 characters in the library name (the length up to which users happily write a qualified path, even if used several times in a file). For free functions the threshold is 12-15. Again, documentation needs to be consistent in order for them to do it that way.

[softprops]

I've recently been thinking about this specifically in the context of errors https://twitter.com/softprops/status/1038234080305967105?s=19

Being my own devils advocate what this can lead to is overscoping of errors on focused methods. In other words a method in a foo crate that returns foo::Error but in practice may only yield a subset of the total types of errors foo::Error represents. This means clients are asked to handle a representation of errors that won't ever occur in a specific context. Yes this encourages much more focused crates where foo::Error enumerates a very small set of cases but in practice this seems rarely the case. foo::Error typically represents all the cases.

[dtolnay]

The discussion of fine-grained vs broader error types is in #46.

If a library or module uses fine-grained errors like the ones returned by std FromStr impls that you linked to, I would expect that they would not all be called Error. They would also not be called foo::FooError because by definition of being fine-grained they are characterized by more than simply "the error associated with library foo" -- so you probably wouldn't run into a problem of stutter.

[burdiyan]

FWIW, I’ve been very deep into Go language for the past 6 years or more. I’m so used to concise package names and qualified imports.

Stuttering and the tendency to import types rather than modules in most open source Rust code I’ve seen is probably the single most annoying reason for me to start digging into Rust more. I just can’t stand it.

I can never understand if a type is a third party type or is defined in the same module. I constantly have to scroll to the top to check it. And don’t get me started about obsolete imports (when people forget to remove the type from the import when they don’t need it anymore) :)

I really-really wish the community was encouraged to import modules most of the time, and avoid adding module names to types. I guess occasionally importing types is fine, especially when they are very few. But the norm should be using the module as a qualifier, IMHO.

[Kixunil]

Such huge amount indicates that probably the file in which it is is also very big. If so, it's generally poor coding practice.

[dpc]

I don't really see much wrong with this file. It uses a lot of types from it's own crate, meaning in it's own context. It doesn't use any primitive types and wraps things like channel senders so it increases number of types used (https://github.com/hashintel/hash/blob/fab19f235f89b18e3e28a8a26cfec745aea0b38e/packages/engine/src/workerpool/comms/top.rs#L17). I have no problem reading this code, though it's non-trivial at it's core, as it's some worker pool or whatever.

What I'd like is more documentation so it's easier to understand what is what and for what. Other than this, no issues.

[dpc]

Refactoring wise, possibly it would be better if the general architecture of this "worker-pool" was decoupled from the actual business logic that utilizes it, so one can analyze the communication paths, spawns, etc. separately. But that's also kind of debatable and additional effort.

[Kixunil]

Lol, missed the link before. Yeah, from very light look doesn't look that bad.

[dpc]

I have projects that have an internal prelude, that I import willy-nilly with use crate::prelude::*;, liberally mixed with use submodule::* and use super::*.

Not only my code editor shows me little box with docstring & definition of the type under the cursor, but if I want to see more it's just one "jump of definition" away anyway. So it's never been an issue. I don't know - do people print source code on paper and review while eating breakfast?

Though recently my code editor (kakoune) got lsp actions support so I can use "expand the import wildcard" and improve on that front, to help these poor souls who read my shitty code.

[wookietreiber]

What I've also seen is to de-stutter in the module where it's defined, as in:

// projects/foo/src/result.rs
pub struct Result;

... and then use the then non-stuttered prefix with a re-export:

// projects/foo/src/lib.rs
pub mod result;
pub use result::{Result as FooResult};