[RFC] Merge imports by default

[gnzlbg]

Merging imports by default makes imports clearer, e.g.,

use rocket::{
    Request, Response,
    request::{self, Form},
    response::{Flash, Redirect}
};
use rocket_contrib::JsonValue;

use strum::EnumMessage;

is visually easier to parse than

use rocket::Request;
use rocket::Response;
use rocket::request;
use rocket::request::Form;
use rocket::response::Flash;
use rocket::response::Redirect;
use rocket_contrib::JsonValue;

use strum::EnumMessage;

since the { } in grouped imports make scoping clearer, as opposed to having to parse whether there are differences in lines, like in

use rocket::response::Redirect;
use rocket_contrib::JsonValue;

where there is a common prefix.

Tools can optionally offer an option to disable merging imports by default.

This does not change the rules with respect to grouping imports separated by empty lines (this is still not done), that is, this:

use rocket::Request;
use rocket::Response;

use rocket::request;
use rocket::request::Form;

continues to be merged as

use rocket::{Request, Response};

use rocket::request::{self, Form};

Prior art

rustfmt has a merge_imports option that does this. There are some bugs open for this feature that would need to be closed before stabilization.

Prior discussions

This comment in #24 proposed not merging imports by default, without rationale for that decision.

[dekellum]

There are some typos in your last example, but to the extent I understand it, I would just like to comment (2¢):

It's not entirely clear to me that transforming to the self syntax is going to always be an improvement. Also you might want to consider this in combination with:

• Aliasing Type as Other, self as foo, etc.
• Concurrent feature cfg, with/without whitespace delimiting.

Some of the last might already be of applicable concern, without this RFC addition.

[gnzlbg]

There are some typos in your last example

Thanks, hopefully fixed.

Some of the last might already be of applicable concern, without this RFC addition.

I think these combinations need to be handled without grouping imports as well, but I am interested in examples that cause problems with import grouping, and not without.

One of those is this one, where :

use alloc::vec; // imports *both* the macro and the module
use alloc::vec::Vec;

use alloc::vec::{self, Vec}; // only imports the module and `Vec`. `vec!` is left out.

IIUC, what's imported depends only on the last component of a path, so for alloc::vec, both the vec! macro and the vec module are imported, but for alloc::vec::self, the components before ::self can only be modules or types, so vec is a module, and ::self only imports the module.

[dekellum]

hopefully fixed.

Last example still looks odd to me. It went from rocket::Form to rocket::request::From? Please also note "Form" and "From" spellings.

Sorry, I'm a bit of a details guy.

[gnzlbg]

The original example also had this bug, should be fixed now.

[tmpolaczyk]

Let's just make a special case for importing a module:

use alloc::vec; // imports *both* the macro and the module
use alloc::vec::Vec;

use alloc::{vec, vec::Vec};

I guess all the logic is already there because it's replacing vec with vec::self.

[dekellum]

This might be a bit out of scope, but recent work I'm doing with futures 0.3 + 0.1 offers some complex examples of merged import blocks. Here I manually format with a single outer use {}, just to avoid the need to repeat the #[cfg(feature=…)].

#[cfg(feature = "futures03")] use {
    std::future::Future as Future03,
    futures03::{
        compat::Future01CompatExt,
        future::{Either as Either03, FutureExt as _, TryFutureExt},
    }
};

#[cfg(feature = "futures03")] use {
    std::pin::Pin,
    std::task::{Context, Poll as Poll03},
    futures03::stream::Stream as Stream03,
};

Note in the latter, I manually choice not to use std::{} since, its only two sub-modules, with only one
import for one of those modules. I think the below is more complex to read, so not worth it:

#[cfg(feature = "futures03")] use {
    std::{pin::Pin, task::{Context, Poll as Poll03}},
    futures03::stream::Stream as Stream03,
};

I think this relates to rust-lang/rustfmt#2982, but shouldn't it first be addressed in more detail in the style guide?

[codesections]

I'm not sure how common this practice currently is, but I noticed the following statement by Alex Crichton in an older thread about import order (#24)

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

Applied to your example, this rule would produce:

use rocket::{Request, Response};
use rocket::request::{self, Form};
use rocket::response::{Flash, Redirect};
use rocket_contrib::JsonValue;

use strum::EnumMessage;

It's not a style I've personally used – though now that I see it mentioned, I can see certain advantages. And, as I said, I'm not sure how widespread it is. But I thought it worth mentioning because, if it is widespread, it would conflict with this RFC.

[joshtriplett]

On April 11, 2020 5:49:25 AM PDT, Daniel Sockwell ***@***.***> wrote: I'm not sure how common this practice currently is, but I noticed the following statement by Alex Crichton in an older thread about import order (rust-lang/style-team#24)> > > Never import multiple modules one one line (e.g. use std::{f32, f64}), instead always split them on multiple lines.> > Applied to your example, this rule would produce:> > ```rust> use rocket::{Request, Response};> use rocket::request::{self, Form};> use rocket::response::{Flash, Redirect};> use rocket_contrib::JsonValue;> > use strum::EnumMessage;> ```> > It's not a style I've personally used – though now that I see it mentioned, I can see certain advantages. And, as I said, I'm not sure how widespread it is. But I thought it worth mentioning because, if it is widespread, it would conflict with this RFC.

This is my preferred style as well: one `use` per module, braces for using multiple names from that module.

[faern]

The merge_imports (rust-lang/rustfmt#3362) issue has seen a lot of discussion about different styles of use statements and which one should be default. In this comment (rust-lang/rustfmt#3362 (comment)) @dtolnay summarize four distinct import merge styles and give them some names.

The style @codesections talks about, from a citation from Alex Crichton is there named the module merge style. Some others call it the "shallow" import style, but that is a more ambigious and opaque name IMO.

Even if there are four ways of merging (plus off, disabling merging) there has to be a default value. The way I interpret the discussion in that thread, most people stand behind two of the styles:

• module - Because it's easy to read and does not make the use section of a Rust file become very long.
• item "java style" - Because it prevents merge conflicts.

[faern]

My personal opinion is that the module style merge is a great default. It's readable and still somewhat compact. It's IMO the perfect balance between all trade-offs one has to make in the different styles.

module is perfect for small to mid size Rust projects in small teams or teams who don't overlap very much while developing. Meaning probably the vast majority of Rust crates. And almost by definition how all Rust crates start out. Therefore it's a good default. The merge conflict argument for item is only valid for large projects with lots of active developers regularly touching the same modules. So large organizations can easily set the item style in their rustfmt settings and coding style guide. I don't think the defaults should reflect what's best for a large company with massive resources, rather what makes it easy to start out and satisfy the majority of crates.

[quodlibetor]

I like module style imports, one but one question that isn't answered naively by them is how to handle over-long lines?

I recall it being discussed, and I think some of the options were:

No-Overflow:

use foo::{bar, Baz, Quux};

Overflow-wrapped:

use foo::{
    bar, Baz,
    Quux,
};

Overflow-vertical:

use foo::{
    bar,
    Baz,
    Quux,
}

Overflow-module-prefix:

use foo::{bar, Baz};
use foo::Quux;

Overflow-module-prefix-semantic (prefer to break where different types of imports begin, so break between lowercase and titlecase names, instead of at end of line):

use foo::bar;
use foo::{Baz, Quux};

Personally I like any of the overflow-module-prefix styles since they make grep more likely to be useful. Aesthetically I prefer overflow-module-prefix-semantic, but I recognize that the implementation complexity for that might not be worth it.

[timonbimon]

Putting in a vote for the 'item' style here (aka having each 'use' on a separate line). It seems to be the only one of the suggested options that actually has reasons going for it that is not based solely on differing opinions on aesthetics. Those reasons are (as @faern mentioned) fewer merge conflicts and (in addition) more readable git diffs during code reviews. You immediately see which 'use' statements got added and which deleted, whereas in the 'module' style you have to parse all things on the two diff lines to understand what is going on.

-use alib::afeature::{TypeA, TypeC, TypeD};
+use alib::afeature::{TypeA, TypeB, TypeC};

vs

use alib::afeature::TypeA;
+use alib::afeature::TypeB;
use alib::afeature::TypeC;
-use alib::afeature::TypeD;

For an example of how it avoids merge conflicts, see here.

I cannot really follow the reasoning why the 'module' style is supposed to be better for small to mid-size Rust projects. I do agree that merge conflicts and code reviews become more important in larger projects, but using that as an argument that therefore the 'item' style is less useful in smaller projects than the 'module' style feels like specious reasoning to me (logically reversing the above statement only means that the 'item' style is less useful in small projects than in large projects, but that does not imply that it is less useful than the 'module' style).

[faern]

and (in addition) more readable git diffs during code reviews

The git diffs are more readable. I agree with that. But the code itself is way less readable under the item style IMO. So one question is if people read code or if they read diffs. This is shown very well in the examples in the OP #140 (comment). item style just verbosely repeat the exact same thing over and over again. It's very hard to see where rocket::request imports stop and where rocket::response imports start. Because every line is just a massive blob of text with 99% same content.

I cannot really follow the reasoning why the 'module' style is supposed to be better for small to mid-size Rust projects

item style is just very very long. Makes the import section of each module very long. Separates the module documentation and code by too much vertical space. Probably more so in a large project. The only advantage the item style has is the git merge property. Not counting the merge conflict property I think module style is the best style for any sized project. The only reason item might be better for larger projects is that they are usually maintained by larger teams, and as such have more overlapping changes (merge conflict potential). So yeah, saying module is better for mid-size Rust projects is probably not ideal. It would be better to say it's better for small to mid-size Rust teams.

[timonbimon]

I totally believe you that you think the module style is more readable. I just think the reason why is very much due to personal preference. The next person could now easily stand here and put in an emotional speech for why they think the item style is much more readable. Personally I think what someone's personal preference is mostly depends on what they are used to, because in that case it is objectively "easier" to read for them, because their brain is now quicker at parsing the style they are used to.

If one were intent on deciding based on personal preference than the best thing to do would probably to have a poll to ask as many Rust programmers as possible about their preferences and let the people decide. I am a little uncomfortable with the thought of shoving a style on all Rust programmers which justification is: "The 3 people discussing the GitHub issue at the time really liked that style".

Thus if you disregard aesthetics I think the "item" style is the one that is easiest to justify when someone asks later on.

All that being said => I would also be completely fine with the person who ends up implementing the different styles getting to decide which one they prefer. After all they are the ones doing the real work here. 😉 I would in any case be a pretty happy cookie if rustfmt supported different styles for the imports irregardless of which one is the default. 🙃