rust-lang / style-team

Home of the Rust style team
Apache License 2.0
453 stars 56 forks source link

imports (`use`) #24

Closed nrc closed 6 years ago

jimmycuadra commented 7 years ago

Here is what I do for my code:

alexcrichton commented 7 years ago

I personally also follow @jimmycuadra's "three sections" rule as well, and I find it very beneficial for knowing what's imported at a glance. I imagine it could be hard for rustfmt to distinguish between internal/external crate imports, however :(, so it may not end up making it all the way through.

Another rule I personally follow is:

And finally, I personally prefer to never split an import across multiple lines, my preference is to:

use foo::{...}; // up to the character limit
use foo::{...}; // remaining imports

Again though, just personal preferences and/or ideas!

jimmycuadra commented 7 years ago

Since rustfmt only operates on a file (at least as far as I understand), distinguishing between internal modules and modules in external crates might not be possible, but I could imagine a Cargo integration being able to do it by having Cargo pass information to rustfmt about external crates.

lu-zero commented 7 years ago

I also find good having "three sections", possibly 1-line separated.

And I like a lot @alexcrichton way to repeat use foo::{...} to keep the lines within limit.

jimmycuadra commented 7 years ago

The reason I like to use the one-line-per-item format for multi-item imports is that it makes for very clean diffs in addition to making it easy to read. It's simply a line removed for any item removed and a line added for any item added. If you use either of these styles:

use foo::{a, b, c, d, e, f, g};
use foo::{h, i, j, k, l, m, n};
use foo::{a, b, c, d, e, f, g,
          h, i, j, k, l, m, n};

then changing the items imported often results in a shuffling/reordering of items across lines that makes it less obvious what changed in a diff.

joshtriplett commented 7 years ago

I follow the same style @alexcrichton described.

I do, often, import a module and use a one-level-qualified name at the call site, especially when a module uses highly generic names that assume namespacing. I prefer foo::init() over foo::foo_init().

petrochenkov commented 7 years ago

The "three sections" rule seems to be popular and have precedents in other language style guides.

I usually use the rule "no more than three segments in a path" and and use imports respectively. Functions are in one-level qualified form m::func(), types (including enums) in non-qualified form MyType or m::MyType if ambiguous, variants in enum-qualified form (Enum::Variant).

strega-nil commented 7 years ago

What I do is:

// if they all fit, do this:
use foo::{a, b, c};
// if they fit on one line, but not with the beginning part, do this:
use foo::{
  a, b, c
};
// and if they don't fit on one line at all, do this:
use foo::{
  a,
  b,
  c,
};

basically following rustic style guide for function args. I do like the three sections.

The one thing I really disagree with from @jimmycuadra is not importing the module itself. I very often import modules in addition to items:

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

impl Display for MyType {
  fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
    write!(f, "hello, world")
  }
}
steveklabnik commented 7 years ago

For a while, I tried to do

extern crate foo;

use foo::bar;
use foo::baz;

extern crate bar;

use bar::foo;

etc, but in the end, didn't like it as much as what @jimmycuadra suggested.

Always import the item itself, not the parent module.

The "standard style" here (in my understanding) is to import types directly, but import the parent modules for functions. I really, really like this.

I also tend to not use {}, but it seems I'm very much in the minority here. If I'm working with code that already has it, I'll use it, but also tend to prefer

use foo;
use foo::{bar, baz};

over

use foo::{self, bar, baz};

But it seems that I'm in the minority here as well.

skade commented 7 years ago

I tend to be in the "explode everything" crowd, and rarely use "{...}". I find it confusing to manipulate and it gets noisy quick.

I do glob imports, though, especially for prelude modules, such as std::io::prelude. That shouldn't be disallowed.

luser commented 7 years ago

The "standard style" here (in my understanding) is to import types directly, but import the parent modules for functions. I really, really like this.

I believe I generally follow this as well, with one notable exception: I always qualify std::io::Result by doing use std::io; and referring to it as io::Result rather than importing it because otherwise it shadows std::result::Result.

steveklabnik commented 7 years ago

@luser yes, agree 100%. Nice catch. So this is something like "except types which may shadow Prelude types"

jimmycuadra commented 7 years ago

The "standard style" here (in my understanding) is to import types directly, but import the parent modules for functions. I really, really like this.

Although I don't do this currently, I like that, too, and wouldn't be disappointed if rustfmt ended up standardizing on that style.

Not using glob imports is also not exactly a "style" rule, so I'm not sure that is applicable to rustfmt, although it is part of my personal practice.

softprops commented 7 years ago

I'm pro what @ubsan said though what @jimmycuadra said about one line per import for clean diffs is is weighing on me and is the first technical argument rather than subjective I've seen so far. Whether stylistically I agree or not ( I do in this case ) the stronger technical argument should win. The diff thing is also just not a case of personally preference, is a better case for making it easier to work in teams on rust code bases.

Another thing possibly consider is perhaps instead of two or three conditionally styled cases. Just pick one. I like that about gofmt. Its consistent because the default only has one option for import line breakage.

I'm also pro the 3 sections of use clauses.

Im not sure if extern lines are out of the scope or not for this discussion but if the aren't, I like all my externs at the top of the file followed by use lines. No technical argument to back that up though. Just my current habit.

pub mod foo; is probably in the same arena, maybe for another topic thread...

strega-nil commented 7 years ago

@softprops the idea of my style (beside being easy to read) is that a change in the number of arguments is one line diff, while not having so many lines :)

nrc commented 7 years ago

Since rustfmt only operates on a file (at least as far as I understand), distinguishing between internal modules and modules in external crates might not be possible, but I could imagine a Cargo integration being able to do it by having Cargo pass information to rustfmt about external crates.

With changes to the compiler that are currently underway, Rustfmt certainly could get this information without any extra input from Cargo.

nrc commented 7 years ago

Absolutely no glob imports. I like to be very explicit, even if it's verbose.

I'd be happy to recommend this, but I don't think Rustfmt should enforce it (it is really easy to implement as a refactoring - rustw and the RLS both support it).

Always import the item itself, not the parent module. If the item has some generic name that makes it hard to identify at the site of use, rename it to something more descriptive.

I tend to import traits directly (because you have to) and types directly, but not functions, these I prefer to import the module and use one level of scoping.

Each import section has its lines sorted alphabetically (or more accurately, by whatever sort's rules are.)

Is it worth putting either lower case of upper case - initialled names first? I sometimes do this and think it looks neater, but often don't.

If present, self should come before other names in an import, not in alphabetical order.

If a large number of items are imported from one module, such that the line would wrap past whatever column width you're using, list one item on each line, with the lines sorted, e.g.:

Personally I don't find this useful - it seriously uses up vertical space and I rarely need to scan imports - they are mostly boilerplate.

nrc commented 7 years ago

Never import multiple modules one one line (e.g. use std::{f32, f64}), instead always split them on multiple lines.

Interesting, seems like a good recommendation, I think we might be able to enforce it too, but not until some big name resolution changes land in the compiler

And finally, I personally prefer to never split an import across multiple lines, my preference is to:

I used to do this, but Rustfmt takes the opposite approach, using visual indentation, and I think I prefer that style, feels more consistent with other formatting.

nrc commented 7 years ago

I do glob imports, though, especially for prelude modules, such as std::io::prelude. That shouldn't be disallowed.

Yeah, prelude modules are a good use for glob imports, IMO.

joshtriplett commented 7 years ago

If present, self should come before other names in an import, not in alphabetical order.

Agreed.

Also, the term I've seen used for "whatever sort does" (implicitly, "in LC_COLLATE=C"): ASCIIbetically.

solson commented 7 years ago

I agree that glob imports should mostly be avoided, but not always. One rule I follow more stringently is to never have more than one glob import in scope.

With one glob, if I have an unknown name, I know it's either in my list of explicit imports or comes from the one glob, but with multiple globs I have no way of knowing where it comes from without tool support.

+1 for self followed by names in ASCIIbetical order.

strega-nil commented 7 years ago

Never import multiple modules one one line (e.g. use std::{f32, f64}), instead always split them on multiple lines.

I do not like this. I always write use std::{f32, f64};, when the modules are related. Pulling them out into multiple lines just feels like overkill.

kornelski commented 7 years ago

I'm OK with single-line use …::{…}, but multi-line use { looks weird to me, especially with one ident per line. I think I expect :: to be visually near each identifier for it to look distinct from struct/enum declarations at the first glance.

use foo::{bar, baz};
use foo::VeryLongIdentifier;
use foo::ThatWouldMakeLineTooLong;
luser commented 7 years ago

We use multi-line import in some Mozilla Python code, and I thought maybe that was specified by PEP-8, but it's not. The PEP-8 guidelines are pretty similar to what's being suggested here: http://pep8.org/#imports

The Django coding style guidelines do recommend using from foo import ( ... ) for long lists of imports, but not an imported-item-per-line: https://docs.djangoproject.com/en/dev/internals/contributing/writing-code/coding-style/#imports

They even recommend a tool specifically for formatting Python imports: https://github.com/timothycrosley/isort#readme

Screwtapello commented 7 years ago

One advantage to never using {} is that it's easier to grep a codebase for uses of some module: you can just grep for "std::f64" instead of "std::.*\bf64\b" (and even then I'm not sure that would find all the relevant matches).

One advantage to always importing a module (possibly with a short alias), never individual functions or types: people are less likely to manually namespace things if they're already namespaced. For example, in a world where people import types directly, I might declare database.DatabaseConnection and websocket.WebSocketConnection so that you can refer to both of them in the same code without confusion. If the convention is to only import modules, I can just call them database.Connection and websocket.Connection without risk of confusion—and if that's too verbose, you can alias them to db.Connection and ws.Connection.

joshtriplett commented 7 years ago

One additional item that came up in today's rust-style meeting:

Don't use super:: to reference names outside the current file. Using super:: to access names from the module in the same file from within a mod tests { seems fine, but using it to access names from other files seems excessively confusing; use absolute paths instead.

joshtriplett commented 7 years ago

Note that we also need guidelines for using names renamed with as.

One extremely common name conflict I've seen handled multiple ways:

use std::fmt; // and use fmt::Write
use std::io; // and use io::Write

versus

use std::fmt::Write as FmtWrite;
use std::io::Write as IoWrite;
solson commented 7 years ago

I'd also like to bring up this pattern, which uses use self::.

enum Foo { A, B, C }

fn bar(f: Foo) {
    use self::Foo::*;
    match f {
        A => { ... }
        B => { ... }
        C => { ... }
    }
}

I've used it in my code to eliminate LongEnumNameHere:: from the start of dozens of match arms, but I'm still not sure if it's worth it. It hasn't ever caused me any confusion, so I don't consider it problematic in that way, at least.

joshtriplett commented 7 years ago

@solson That one seems fine to me; it's super:: that can get excessively confusing.

nrc commented 7 years ago

I would like to recommend against importing enum variants - I think it is always clearer to use the qualified form (in fact the only reason you can import enum variants at all is due to back compat - I believe that if we added enums to the language today, we would not allow importing variants).

I wonder if we should recommend where to import? I prefer imports at module level, and only use them at function-level where a function does something very different from the rest of the module - say it does some io, but that is not the focus of the module. I would find it hard to formalise this rule though.

solson commented 7 years ago

I far prefer use std::fmt; // and use fmt::Write over use std::fmt::Write as FmtWrite; since the former doesn't invent any new names, and I can immediately be pretty sure of what fmt::Write is.

strega-nil commented 7 years ago

@nrc I would allow importing enum variants before a match in a block that was mostly that match.

use EnumName::*;
match enumname {
  Foo => ...
  Bar => ...
}

// as opposed to
match enumname {
  EnumName::Foo => ...
  EnumName::Bar => ...
}
luser commented 7 years ago

@nrc I have found function-level imports convenient when I have had a cfg-specific function, so that I don't have to cfg all the use statements as well.

nrc commented 7 years ago

@ubsan I would not forbid that, although personally I prefer the second. I think that whatever we do in this space, it will be guidelines rather than rules, so I think the question is - how many exceptions do we give, and/or how much detail do we go into?

ben0x539 commented 7 years ago

@nrc you'd require Result::Ok(...) if Result was invented today?

nrc commented 7 years ago

@ben0x539 every rule has exceptions, and Some, Ok, etc. are the exceptions here. I don't think that exception is more generally applicable.

skade commented 7 years ago

I just found that there is an uncovered topic. What is the stance on local imports?

fn main() {
    use std::mem::size_of;
    println!("{}", size_of::<String>());
}

I tend to prefer them for things that I use just once, especially things like the memory operations. Would it make sense to lint for imports that are only used once in a long file? Finding Rules would be a bit hard, though...

nrc commented 7 years ago

To summarise what needs to be decided, the big issue I think is how to format an import when it gets too long for a single line. The options are:

// Single import, visual indent.
use foo::{bar, baz, qux,
          dssdfasf, dsfsdfs};

// Single import, block indent.
use foo::{bar, baz, qux,
    dssdfasf, dfsdfs};

// Multiple imports, maximum names per line.
use foo::{bar, baz, qux};
use foo::{dssdfasf};

// One import, one name per line.
use foo::{
    bar,
    baz,
    qux,
    dssdfas,
};

// Multiple imports, one name per line.
use foo::bar;
use foo::baz;
use foo::qux;

My preference is for "Single import, visual indent." though I don't mind the block indented variation also. I like it because it is easy to identify the set of imports from a single module and minimises vertical space for what is, mostly, boilerplate. My preference for visual over block indent is purely subjective, I think.

A smaller related issue is if we are importing multiple modules (c.f., types, other values), should each module get its own line or not?

joshtriplett commented 7 years ago

Good summary, @nrc. I agree that those are the options on the table.

Ranking those from my most preferred to least preferred:

strega-nil commented 7 years ago

@joshtriplett @nrc There's also the "full block indent" option:

use foo::{
    bar, baz, qux,
    dssdfasf, dsfsdfs,
};

which I prefer. After that, I prefer them in approximately @joshtriplett order, except swap multiple imports and one import, one name per line.

joshtriplett commented 7 years ago

@ubsan Ah, good point; I've seen that style in several places. I'd put that second on my list, behind "multiple imports, maximum names per line", but ahead of everything else. And I wouldn't be disappointed if that were the preferred style; the only downside it has, in my opinion, is that the resulting lines don't match a search for foo.*baz if you want to find foo::baz.

nrc commented 7 years ago

I'm not really a fan of the "full block indent" because of the extra two lines of vertical space. I wouldn't mind this in 'real code' so much, but to me imports are mostly just boilerplate, and I would rather they took up less space than more.

For reference, the ordering in https://github.com/rust-lang-nursery/fmt-rfcs/issues/24#issuecomment-256200090 actually reflects my preferences, although I didn't intend it to (it is copy and pasted from an etherpad).

softprops commented 7 years ago

to me imports are mostly just boilerplate

To me imports are a source of documentation for a module's dependencies. That's one reason I'm also not a fan of glob imports. anything we can do to make that dependency documentation more readable is a plus for me

joshtriplett commented 7 years ago

@solson I'd prefer to use fmt::Write and io::Write as well, but as far as I can tell, unless I import a trait directly (renamed or otherwise), I can't call its methods (including implicitly via the various standard macros that use them). So if I want to use both in the same module, I have to rename one or both.

solson commented 7 years ago

@joshtriplett Ohh, interesting problem.

My current stance is "only use renaming when necessary to avoid name collisions" and I suppose fmt::Write vs io::Write fits that bill even when you only want to import them for method resolution.

joshtriplett commented 7 years ago

See https://internals.rust-lang.org/t/pre-rfc-allow-conflicting-names-on-import-only-complain-if-used-ambiguously/4291 for a pre-RFC that could address that issue.

nrc commented 7 years ago

I think this is ready for FCP. Could someone write up a summary please?

malbarbo commented 7 years ago

Should glob imports be sorted first?

use somecrate::prelude::*;
use somecrate::a::A;
use somecrate::x::X;

vs

use somecrate::a::A;
use somecrate::prelude::*;
use somecrate::x::X;
solson commented 7 years ago

Calling special attention to glob imports one way or another seems like a good idea.

nrc commented 7 years ago

Should glob imports be sorted first?

I would not, I think it is more important to see stuff from similar modules closer together, than to isolate one kind of import.

Calling special attention to glob imports one way or another seems like a good idea.

Why? The import is no different, it just uses a different mechanism to name the items imported.