Merge pull request #2535 from nrc/import-ord

Import ordering

Author: nrc

Configurations.md

@@ -1297,28 +1297,28 @@ Reorder import statements in group
 
 **Note:** This option takes effect only when [`reorder_imports`](#reorder_imports) is set to `true`.
 
-#### `false` (default):
+#### `true` (default):
 
 ```rust
-use std::mem;
 use std::io;
+use std::mem;
 
-use lorem;
-use ipsum;
 use dolor;
+use ipsum;
+use lorem;
 use sit;
 ```
 
-#### `true`:
+#### `false`:
 
-```rust
-use std::io;
-use std::mem;
 
+```rust
 use dolor;
 use ipsum;
 use lorem;
 use sit;
+use std::io;
+use std::mem;
 ```
 
 See also [`reorder_imports`](#reorder_imports).
@@ -1360,7 +1360,11 @@ Reorder `extern crate` statements in group
 - **Possible values**: `true`, `false`
 - **Stable**: No
 
-#### `true` (default):
+#### `false` (default):
+
+This value has no influence beyond the effect of the [`reorder_extern_crates`](#reorder_extern_crates) option. Set [`reorder_extern_crates`](#reorder_extern_crates) to `false` if you do not want `extern crate` groups to be collapsed and ordered.
+
+#### `true`:
 
 **Note:** This only takes effect when [`reorder_extern_crates`](#reorder_extern_crates) is set to `true`.
 
@@ -1374,10 +1378,6 @@ extern crate lorem;
 extern crate sit;
 ```
 
-#### `false`:
-
-This value has no influence beyond the effect of the [`reorder_extern_crates`](#reorder_extern_crates) option. Set [`reorder_extern_crates`](#reorder_extern_crates) to `false` if you do not want `extern crate` groups to be collapsed and ordered.
-
 ## `reorder_modules`
 
 Reorder `mod` declarations alphabetically in group.
@@ -1386,7 +1386,7 @@ Reorder `mod` declarations alphabetically in group.
 - **Possible values**: `true`, `false`
 - **Stable**: No
 
-#### `true`
+#### `true` (default)
 
 ```rust
 mod a;

bootstrap.sh

@@ -7,9 +7,8 @@
 cargo build --release
 
 target/release/rustfmt --write-mode=overwrite src/lib.rs
-target/release/rustfmt --write-mode=overwrite src/bin/rustfmt.rs
-target/release/rustfmt --write-mode=overwrite src/bin/cargo-fmt.rs
-target/release/rustfmt --write-mode=overwrite tests/system.rs
+target/release/rustfmt --write-mode=overwrite src/bin/main.rs
+target/release/rustfmt --write-mode=overwrite src/cargo-fmt/main.rs
 
 for filename in tests/target/*.rs; do
     if ! grep -q "rustfmt-" "$filename"; then

src/bin/main.rs

@@ -14,16 +14,16 @@ extern crate env_logger;
 extern crate getopts;
 extern crate rustfmt_nightly as rustfmt;
 
-use std::{env, error};
 use std::fs::File;
 use std::io::{self, Read, Write};
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
+use std::{env, error};
 
 use getopts::{Matches, Options};
 
-use rustfmt::config::{get_toml_path, Color, Config, WriteMode};
 use rustfmt::config::file_lines::FileLines;
+use rustfmt::config::{get_toml_path, Color, Config, WriteMode};
 use rustfmt::{run, FileName, Input, Summary};
 
 type FmtError = Box<error::Error + Send + Sync>;

src/chains.rs

@@ -77,8 +77,8 @@ use std::borrow::Cow;
 use std::cmp::min;
 use std::iter;
 
-use syntax::{ast, ptr};
 use syntax::codemap::Span;
+use syntax::{ast, ptr};
 
 pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -> Option<String> {
     debug!("rewrite_chain {:?}", shape);

src/closures.rs

@@ -9,9 +9,9 @@
 // except according to those terms.
 
 use config::lists::*;
-use syntax::{ast, ptr};
 use syntax::codemap::Span;
 use syntax::parse::classify;
+use syntax::{ast, ptr};
 
 use codemap::SpanUtils;
 use expr::{block_contains_comment, is_simple_block, is_unsafe_block, rewrite_cond, ToExpr};

src/codemap.rs

@@ -12,8 +12,8 @@
 //! This includes extension traits and methods for looking up spans and line ranges for AST nodes.
 
 use config::file_lines::LineRange;
-use visitor::SnippetProvider;
 use syntax::codemap::{BytePos, CodeMap, Span};
+use visitor::SnippetProvider;
 
 use comment::FindUncommented;
 

src/config/file_lines.rs

@@ -10,9 +10,9 @@
 
 //! This module contains types and functions to support formatting specific line ranges.
 
-use std::{cmp, iter, str};
 use std::collections::HashMap;
 use std::rc::Rc;
+use std::{cmp, iter, str};
 
 use serde::de::{Deserialize, Deserializer};
 use serde_json as json;

src/config/license.rs

@@ -1,6 +1,6 @@
-use std::io;
 use std::fmt;
 use std::fs::File;
+use std::io;
 use std::io::Read;
 
 use regex;

src/config/mod.rs

@@ -8,12 +8,12 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use std::{env, fs};
 use std::cell::Cell;
 use std::default::Default;
 use std::fs::File;
 use std::io::{Error, ErrorKind, Read};
 use std::path::{Path, PathBuf};
+use std::{env, fs};
 
 use regex::Regex;
 
@@ -23,9 +23,9 @@ mod config_type;
 mod options;
 
 pub mod file_lines;
+pub mod license;
 pub mod lists;
 pub mod summary;
-pub mod license;
 
 use config::config_type::ConfigType;
 use config::file_lines::FileLines;
@@ -70,11 +70,11 @@ create_config! {
     // Ordering
     reorder_extern_crates: bool, true, false, "Reorder extern crate statements alphabetically";
     reorder_extern_crates_in_group: bool, true, false, "Reorder extern crate statements in group";
-    reorder_imports: bool, false, false, "Reorder import statements alphabetically";
-    reorder_imports_in_group: bool, false, false, "Reorder import statements in group";
+    reorder_imports: bool, true, false, "Reorder import statements alphabetically";
+    reorder_imports_in_group: bool, true, false, "Reorder import statements in group";
     reorder_imported_names: bool, true, false,
         "Reorder lists of names in import statements alphabetically";
-    reorder_modules: bool, false, false, "Reorder module statemtents alphabetically in group";
+    reorder_modules: bool, true, false, "Reorder module statemtents alphabetically in group";
 
     // Spaces around punctuation
     binop_separator: SeparatorPlace, SeparatorPlace::Front, false,

src/config/summary.rs

@@ -8,8 +8,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use std::time::{Duration, Instant};
 use std::default::Default;
+use std::time::{Duration, Instant};
 
 #[must_use]
 #[derive(Debug, Default, Clone, Copy)]

src/expr.rs

@@ -13,8 +13,8 @@ use std::cmp::min;
 use std::iter::repeat;
 
 use config::lists::*;
-use syntax::{ast, ptr};
 use syntax::codemap::{BytePos, CodeMap, Span};
+use syntax::{ast, ptr};
 
 use chains::rewrite_chain;
 use closures;

src/format-diff/main.rs

@@ -23,10 +23,10 @@ extern crate regex;
 extern crate serde_derive;
 extern crate serde_json as json;
 
-use std::{env, fmt, process};
 use std::collections::HashSet;
 use std::error::Error;
 use std::io::{self, BufRead};
+use std::{env, fmt, process};
 
 use regex::Regex;
 

src/items.rs

@@ -15,9 +15,9 @@ use std::cmp::min;
 
 use config::lists::*;
 use regex::Regex;
-use syntax::{abi, ast, ptr, symbol};
 use syntax::codemap::{self, BytePos, Span};
 use syntax::visit;
+use syntax::{abi, ast, ptr, symbol};
 
 use codemap::{LineRangeUtils, SpanUtils};
 use comment::{combine_strs_with_missing_comments, contains_comment, recover_comment_removed,
@@ -26,8 +26,8 @@ use config::{BraceStyle, Config, Density, IndentStyle};
 use expr::{format_expr, is_empty_block, is_simple_block_stmt, rewrite_assign_rhs,
            rewrite_assign_rhs_with, ExprType, RhsTactics};
 use lists::{definitive_tactic, itemize_list, write_list, ListFormatting, ListItem, Separator};
-use rewrite::{Rewrite, RewriteContext};
 use overflow;
+use rewrite::{Rewrite, RewriteContext};
 use shape::{Indent, Shape};
 use spanned::Spanned;
 use types::TraitTyParamBounds;

src/lib.rs

@@ -38,11 +38,11 @@ use std::path::PathBuf;
 use std::rc::Rc;
 use std::time::Duration;
 
-use syntax::errors::{DiagnosticBuilder, Handler};
-use syntax::errors::emitter::{ColorConfig, EmitterWriter};
 use syntax::ast;
-use syntax::codemap::{CodeMap, FilePathMapping};
 pub use syntax::codemap::FileName;
+use syntax::codemap::{CodeMap, FilePathMapping};
+use syntax::errors::emitter::{ColorConfig, EmitterWriter};
+use syntax::errors::{DiagnosticBuilder, Handler};
 use syntax::parse::{self, ParseSess};
 
 use checkstyle::{output_footer, output_header};

src/lists.rs

@@ -56,7 +56,7 @@ impl AsRef<ListItem> for ListItem {
     }
 }
 
-#[derive(PartialEq, Eq)]
+#[derive(PartialEq, Eq, Debug)]
 pub enum ListItemCommentStyle {
     // Try to keep the comment on the same line with the item.
     SameLine,
@@ -66,6 +66,7 @@ pub enum ListItemCommentStyle {
     None,
 }
 
+#[derive(Debug)]
 pub struct ListItem {
     // None for comments mean that they are not present.
     pub pre_comment: Option<String>,
@@ -118,6 +119,18 @@ impl ListItem {
             new_lines: false,
         }
     }
+
+    // true if the item causes something to be written.
+    fn is_substantial(&self) -> bool {
+        fn empty(s: &Option<String>) -> bool {
+            match *s {
+                Some(ref s) if !s.is_empty() => false,
+                _ => true,
+            }
+        }
+
+        !(empty(&self.pre_comment) && empty(&self.item) && empty(&self.post_comment))
+    }
 }
 
 /// The type of separator for lists.
@@ -220,6 +233,10 @@ where
             item_last_line_width -= indent_str.len();
         }
 
+        if !item.is_substantial() {
+            continue;
+        }
+
         match tactic {
             DefinitiveListTactic::Horizontal if !first => {
                 result.push(' ');
@@ -276,26 +293,28 @@ where
                 rewrite_comment(comment, block_mode, formatting.shape, formatting.config)?;
             result.push_str(&comment);
 
-            if tactic == DefinitiveListTactic::Vertical {
-                // We cannot keep pre-comments on the same line if the comment if normalized.
-                let keep_comment = if formatting.config.normalize_comments()
-                    || item.pre_comment_style == ListItemCommentStyle::DifferentLine
-                {
-                    false
+            if !inner_item.is_empty() {
+                if tactic == DefinitiveListTactic::Vertical {
+                    // We cannot keep pre-comments on the same line if the comment if normalized.
+                    let keep_comment = if formatting.config.normalize_comments()
+                        || item.pre_comment_style == ListItemCommentStyle::DifferentLine
+                    {
+                        false
+                    } else {
+                        // We will try to keep the comment on the same line with the item here.
+                        // 1 = ` `
+                        let total_width = total_item_width(item) + item_sep_len + 1;
+                        total_width <= formatting.shape.width
+                    };
+                    if keep_comment {
+                        result.push(' ');
+                    } else {
+                        result.push('\n');
+                        result.push_str(indent_str);
+                    }
                 } else {
-                    // We will try to keep the comment on the same line with the item here.
-                    // 1 = ` `
-                    let total_width = total_item_width(item) + item_sep_len + 1;
-                    total_width <= formatting.shape.width
-                };
-                if keep_comment {
                     result.push(' ');
-                } else {
-                    result.push('\n');
-                    result.push_str(indent_str);
                 }
-            } else {
-                result.push(' ');
             }
             item_max_width = None;
         }
@@ -304,7 +323,7 @@ where
             result.push_str(formatting.separator.trim());
             result.push(' ');
         }
-        result.push_str(&inner_item[..]);
+        result.push_str(inner_item);
 
         // Post-comments
         if tactic != DefinitiveListTactic::Vertical && item.post_comment.is_some() {

src/macros.rs

@@ -22,7 +22,6 @@
 use std::collections::HashMap;
 
 use config::lists::*;
-use syntax::{ast, ptr};
 use syntax::codemap::{BytePos, Span};
 use syntax::parse::new_parser_from_tts;
 use syntax::parse::parser::Parser;
@@ -31,6 +30,7 @@ use syntax::print::pprust;
 use syntax::symbol;
 use syntax::tokenstream::{Cursor, ThinTokenStream, TokenStream, TokenTree};
 use syntax::util::ThinVec;
+use syntax::{ast, ptr};
 
 use codemap::SpanUtils;
 use comment::{contains_comment, remove_trailing_white_spaces, CharClasses, FindUncommented,