import reordering options

[nrc]

I would like to make reorder_imports on by default and merge all five import ordering options into a single one (part of #1974):

reorder_extern_crates,
reorder_extern_crates_in_group,
reorder_imports,
reorder_imports_in_group,
reorder_imported_names,

(currently the third and forth are off by default)

However, we should decide on the precise behaviour and iron out any bugs (current behviour is at least a bit suprising, try changing the defaults and running on chains.rs, for example).

Tasks:

• decide on behaviour around groups
• decide on behaviour around comments
• fix bugs with reording imports, comments, and doc comments.
• turn the default for reorder_imports and reorder_imports_in_group to true, fixup tests and re-format source code (it might be easier to merge these two options before other stuff because it allows to deal with fewer cases).
• merge the five options into reorder_imports
• nested imports

The format RFCs only state that imports should be ordered alphabetically, but don't specify beyond that (I thought we went into some detail about grouping, but I can't find that discussion).

The grouping thing is that people often group imports using newlines:

use foo::B;
use foo::A;
use bar::D;

use alpha::B;
use alpha::A;

Respecting grouping means reordering to:

use bar::D;
use foo::A;
use foo::B;

use alpha::A;
use alpha::B;

ignoring grouping would give:

use alpha::A;
use alpha::B;
use bar::D;
use foo::A;
use foo::B;

There have also been suggestions that we should actively do some grouping - perhaps separating imports from deps from imports from std and/or imports from the current crate.

I think we should probably respect grouping by newlines, remove more that one newline and not actively group stuff.

What if there are imports lower down in the header? Should they be moved? That should probably be a separate option, but ordering should respect that. I guess at first we should not move (until the option exists).

Should comments group imports? Probably not, but maybe if the comment is more than one line? In any case, we should probably move comments as if they were attached to an import directly below, but not if there are blank lines (in which case there is a group, so we would not move imports above or below). I think this solves the comment/import ordering question.

[topecongiro]

In chains.rs, /// is used instead of //!. The parser thinks it is doc comment on the next import (in this case use shape::Shape) instead of module level doc comment. Therefore, the doc comment gets reordered with use shape::Shape when reorder_imports is set to true. Changing /// to //! solves the issue. So in theory, rustfmt is working as intended here.

I cannot think of any circumstances where we want doc comment on imports, though assuming that it's irrelevant and reordering imports without doc comment will change the semantic of the original code. So I am afraid that we cannot 'fix' the issue.

[nrc]

Hmm, that makes sense. I changed the kind of comments in chains.rs, there might be other places we need to that too.

[topecongiro]

Using 0.3.4-nightly (6714a44 2017-12-23) with reorder_imports = true, use self and use super gets reordered before other imports:

+use self::raw::RawConnection;
+use self::stmt::Statement;
+use self::url::ConnectionOptions;
+use super::backend::Mysql;
 use connection::*;
 use query_builder::*;
 use query_builder::bind_collector::RawBytesBindCollector;
 use query_source::{Queryable, QueryableByName};
 use result::*;
-use self::raw::RawConnection;
-use self::stmt::Statement;
-use self::url::ConnectionOptions;
-use super::backend::Mysql;
 use types::HasSqlType;

is this what we want, or should we force alphabetical order?

[jonhoo]

@topecongiro I think it makes a lot of sense for self and super to be ordered specially. I assume this will also apply to the upcoming use crate?

[nrc]

is this what we want, or should we force alphabetical order?

I think self and super first is a good idea.

[nrc]

cc #2475

[nrc]

#2535 addresses a lot of this. Remaining work is to merge the options and to check behaviour around comments (I believe it is correct, but we need to check). We might consider whether ordering in groups is the correct behaviour to have (I think it is, but I'm not 100% convinced).

[nrc]

Remaining: check behaviour around comments

[nrc]

Also need to implement the spec'ed behaviour for nested imports

[nrc]

The implementation now matches the spec for nested imports, so this issue is done.