Merge pull request #1785 from topecongiro/rfc/import

Rfc: use block indent for multi-lined imports

Configurations.md

@@ -979,6 +979,87 @@ match lorem {
 
 See also: [`match_block_trailing_comma`](#match_block_trailing_comma), [`wrap_match_arms`](#wrap_match_arms).
 
+## `imports_indent`
+
+Indent style of imports
+
+- **Default Value**: `"Visual"`
+- **Possible values**: `"Block"`, `"Visual"`
+
+#### `"Block"`
+
+```rust
+use foo::{
+    xxx,
+    yyy,
+    zzz,
+};
+```
+
+#### `"Visual"`
+
+```rust
+use foo::{xxx,
+          yyy,
+          zzz};
+```
+
+See also: [`imports_layout`](#imports_layout).
+
+## `imports_layout`
+
+Item layout inside a imports block
+
+- **Default value**: "Mixed"
+- **Possible values**: "Horizontal", "HorizontalVertical", "Mixed", "Vertical"
+
+#### `"Mixed"`
+
+```rust
+use foo::{xxx, yyy, zzz};
+
+use foo::{aaa, bbb, ccc,
+          ddd, eee, fff};
+```
+
+#### `"Horizontal"`
+
+**Note**: This option forces to put everything on one line and may exceeds `max_width`.
+
+```rust
+use foo::{xxx, yyy, zzz};
+
+use foo::{aaa, bbb, ccc, ddd, eee, fff};
+```
+
+#### `"HorizontalVertical"`
+
+```rust
+use foo::{xxx, yyy, zzz};
+
+use foo::{aaa,
+          bbb,
+          ccc,
+          ddd, 
+          eee, 
+          fff};
+```
+
+#### `"Vertical"`
+
+```rust
+use foo::{xxx,
+          yyy,
+          zzz};
+
+use foo::{aaa,
+          bbb,
+          ccc,
+          ddd,
+          eee,
+          fff};
+```
+
 ## `item_brace_style`
 
 Brace style for structs and enums

src/bin/cargo-fmt.rs

@@ -24,10 +24,9 @@ use std::str;
 use std::collections::HashSet;
 use std::iter::FromIterator;
 
+use getopts::{Matches, Options};
 use json::Value;
 
-use getopts::{Options, Matches};
-
 fn main() {
     let exit_status = execute();
     std::io::stdout().flush().unwrap();

src/bin/rustfmt.rs

@@ -17,10 +17,6 @@ extern crate toml;
 extern crate env_logger;
 extern crate getopts;
 
-use rustfmt::{run, Input, Summary};
-use rustfmt::file_lines::FileLines;
-use rustfmt::config::{Config, WriteMode, get_toml_path};
-
 use std::{env, error};
 use std::fs::File;
 use std::io::{self, Read, Write};
@@ -29,6 +25,10 @@ use std::str::FromStr;
 
 use getopts::{Matches, Options};
 
+use rustfmt::{run, Input, Summary};
+use rustfmt::file_lines::FileLines;
+use rustfmt::config::{get_toml_path, Config, WriteMode};
+
 type FmtError = Box<error::Error + Send + Sync>;
 type FmtResult<T> = std::result::Result<T, FmtError>;
 

src/chains.rs

@@ -77,11 +77,11 @@
 /// ```
 
 use Shape;
-use rewrite::{Rewrite, RewriteContext};
-use utils::{wrap_str, first_line_width, last_line_width, mk_sp, last_line_extendable};
-use expr::rewrite_call;
 use config::IndentStyle;
+use expr::rewrite_call;
 use macros::convert_try_mac;
+use rewrite::{Rewrite, RewriteContext};
+use utils::{first_line_width, last_line_extendable, last_line_width, mk_sp, wrap_str};
 
 use std::cmp::min;
 use std::iter;

src/checkstyle.rs

@@ -7,10 +7,11 @@
 // <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
-use rustfmt_diff::{Mismatch, DiffLine};
-use std::io::{self, Write};
-use config::WriteMode;
 
+use std::io::{self, Write};
+
+use config::WriteMode;
+use rustfmt_diff::{DiffLine, Mismatch};
 
 pub fn output_header<T>(out: &mut T, mode: WriteMode) -> Result<(), io::Error>
 where

src/comment.rs

@@ -17,7 +17,7 @@ use syntax::codemap::Span;
 use {Indent, Shape};
 use config::Config;
 use rewrite::RewriteContext;
-use string::{StringFormat, rewrite_string};
+use string::{rewrite_string, StringFormat};
 use utils::wrap_str;
 
 fn is_custom_comment(comment: &str) -> bool {
@@ -809,8 +809,8 @@ fn remove_comment_header(comment: &str) -> &str {
 
 #[cfg(test)]
 mod test {
-    use super::{CharClasses, CodeCharKind, FullCodeCharKind, contains_comment, rewrite_comment,
-                FindUncommented, CommentCodeSlices};
+    use super::{contains_comment, rewrite_comment, CharClasses, CodeCharKind, CommentCodeSlices,
+                FindUncommented, FullCodeCharKind};
     use {Indent, Shape};
 
     #[test]

src/config.rs

@@ -10,15 +10,14 @@
 
 extern crate toml;
 
+use std::{env, fs};
 use std::cell::Cell;
-use std::fs;
 use std::fs::File;
-use std::env;
 use std::io::{Error, ErrorKind, Read};
 use std::path::{Path, PathBuf};
 
 use file_lines::FileLines;
-use lists::{SeparatorTactic, ListTactic};
+use lists::{ListTactic, SeparatorTactic};
 
 macro_rules! configuration_option_enum{
     ($e:ident: $( $x:ident ),+ $(,)*) => {
@@ -557,9 +556,11 @@ create_config! {
     chain_one_line_max: usize, 60, "Maximum length of a chain to fit on a single line";
     chain_split_single_child: bool, false, "Split a chain with a single child if its length \
                                             exceeds `chain_one_line_max`";
+    imports_indent: IndentStyle, IndentStyle::Visual, "Indent of imports";
+    imports_layout: ListTactic, ListTactic::Mixed, "Item layout inside a import block";
     reorder_imports: bool, false, "Reorder import statements alphabetically";
     reorder_imports_in_group: bool, false, "Reorder import statements in group";
-    reorder_imported_names: bool, false,
+    reorder_imported_names: bool, true,
         "Reorder lists of names in import statements alphabetically";
     single_line_if_else_max_width: usize, 50, "Maximum line length for single line if-else \
                                                 expressions. A value of zero means always break \

src/expr.rs

@@ -8,34 +8,34 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use std::cmp::{Ordering, min};
-use std::iter::ExactSizeIterator;
+use std::cmp::{min, Ordering};
 use std::fmt::Write;
-
-use {Indent, Shape, Spanned};
-use codemap::SpanUtils;
-use rewrite::{Rewrite, RewriteContext};
-use lists::{write_list, itemize_list, ListFormatting, SeparatorTactic, ListTactic,
-            DefinitiveListTactic, definitive_tactic, ListItem, struct_lit_shape,
-            struct_lit_tactic, shape_for_tactic, struct_lit_formatting};
-use string::{StringFormat, rewrite_string};
-use utils::{extra_offset, last_line_width, wrap_str, binary_search, first_line_width,
-            semicolon_for_stmt, trimmed_last_line_width, left_most_sub_expr, stmt_expr,
-            colon_spaces, contains_skip, mk_sp, last_line_extendable, paren_overhead};
-use visitor::FmtVisitor;
-use config::{Config, IndentStyle, MultilineStyle, ControlBraceStyle, Style};
-use comment::{FindUncommented, rewrite_comment, contains_comment, recover_comment_removed};
-use types::{rewrite_path, PathContext, can_be_overflowed_type};
-use items::{span_lo_for_arg, span_hi_for_arg};
-use chains::rewrite_chain;
-use macros::{rewrite_macro, MacroPosition};
-use patterns::{TuplePatField, can_be_overflowed_pat};
-use vertical::rewrite_with_alignment;
+use std::iter::ExactSizeIterator;
 
 use syntax::{ast, ptr};
-use syntax::codemap::{CodeMap, Span, BytePos};
+use syntax::codemap::{BytePos, CodeMap, Span};
 use syntax::parse::classify;
 
+use {Indent, Shape, Spanned};
+use chains::rewrite_chain;
+use codemap::SpanUtils;
+use comment::{contains_comment, recover_comment_removed, rewrite_comment, FindUncommented};
+use config::{Config, ControlBraceStyle, IndentStyle, MultilineStyle, Style};
+use items::{span_hi_for_arg, span_lo_for_arg};
+use lists::{definitive_tactic, itemize_list, shape_for_tactic, struct_lit_formatting,
+            struct_lit_shape, struct_lit_tactic, write_list, DefinitiveListTactic, ListFormatting,
+            ListItem, ListTactic, SeparatorTactic};
+use macros::{rewrite_macro, MacroPosition};
+use patterns::{can_be_overflowed_pat, TuplePatField};
+use rewrite::{Rewrite, RewriteContext};
+use string::{rewrite_string, StringFormat};
+use types::{can_be_overflowed_type, rewrite_path, PathContext};
+use utils::{binary_search, colon_spaces, contains_skip, extra_offset, first_line_width,
+            last_line_extendable, last_line_width, left_most_sub_expr, mk_sp, paren_overhead,
+            semicolon_for_stmt, stmt_expr, trimmed_last_line_width, wrap_str};
+use vertical::rewrite_with_alignment;
+use visitor::FmtVisitor;
+
 impl Rewrite for ast::Expr {
     fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
         format_expr(self, ExprType::SubExpression, context, shape)
@@ -501,10 +501,12 @@ where
             DefinitiveListTactic::Mixed
         },
     };
+    let mut ends_with_newline = tactic.ends_with_newline(context.config.array_layout());
     if context.config.array_horizontal_layout_threshold() > 0 &&
         items.len() > context.config.array_horizontal_layout_threshold()
     {
         tactic = DefinitiveListTactic::Mixed;
+        ends_with_newline = false;
         if context.config.array_layout() == IndentStyle::Block {
             nested_shape = try_opt!(
                 shape
@@ -525,7 +527,7 @@ where
             SeparatorTactic::Vertical
         },
         shape: nested_shape,
-        ends_with_newline: false,
+        ends_with_newline: ends_with_newline,
         config: context.config,
     };
     let list_str = try_opt!(write_list(&items, &fmt));

src/filemap.rs

@@ -11,14 +11,14 @@
 
 // TODO: add tests
 
+use std::fs::{self, File};
+use std::io::{self, BufWriter, Read, Write};
+
 use strings::string_buffer::StringBuffer;
 
-use std::fs::{self, File};
-use std::io::{self, Write, Read, BufWriter};
-
-use config::{NewlineStyle, Config, WriteMode};
+use checkstyle::{output_checkstyle_file, output_footer, output_header};
+use config::{Config, NewlineStyle, WriteMode};
 use rustfmt_diff::{make_diff, print_diff, Mismatch};
-use checkstyle::{output_header, output_footer, output_checkstyle_file};
 
 // A map of the files of a crate, with their new content
 pub type FileMap = Vec<FileRecord>;

src/imports.rs

@@ -8,17 +8,20 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use Shape;
-use utils;
-use syntax::codemap::{BytePos, Span};
-use codemap::SpanUtils;
-use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic, definitive_tactic};
-use types::{rewrite_path, PathContext};
-use rewrite::{Rewrite, RewriteContext};
-use visitor::FmtVisitor;
 use std::cmp::{self, Ordering};
 
 use syntax::{ast, ptr};
+use syntax::codemap::{BytePos, Span};
+
+use Shape;
+use codemap::SpanUtils;
+use config::IndentStyle;
+use lists::{definitive_tactic, itemize_list, write_list, DefinitiveListTactic, ListFormatting,
+            ListItem, SeparatorTactic};
+use rewrite::{Rewrite, RewriteContext};
+use types::{rewrite_path, PathContext};
+use utils;
+use visitor::FmtVisitor;
 
 fn path_of(a: &ast::ViewPath_) -> &ast::Path {
     match *a {
@@ -249,13 +252,12 @@ impl<'a> FmtVisitor<'a> {
 
     pub fn format_import(&mut self, vis: &ast::Visibility, vp: &ast::ViewPath, span: Span) {
         let vis = utils::format_visibility(vis);
-        let mut offset = self.block_indent;
-        offset.alignment += vis.len() + "use ".len();
-        // 1 = ";"
-        match vp.rewrite(
-            &self.get_context(),
-            Shape::legacy(self.config.max_width() - offset.width() - 1, offset),
-        ) {
+        // 4 = `use `, 1 = `;`
+        let rw = Shape::indented(self.block_indent, self.config)
+            .offset_left(vis.len() + 4)
+            .and_then(|shape| shape.sub_width(1))
+            .and_then(|shape| vp.rewrite(&self.get_context(), shape));
+        match rw {
             Some(ref s) if s.is_empty() => {
                 // Format up to last newline
                 let prev_span = utils::mk_sp(self.last_pos, source!(self, span).lo);
@@ -310,9 +312,81 @@ fn append_alias(path_item_str: String, vpi: &ast::PathListItem) -> String {
     }
 }
 
+#[derive(Eq, PartialEq)]
+enum ImportItem<'a> {
+    // `self` or `self as a`
+    SelfImport(&'a str),
+    // name_one, name_two, ...
+    SnakeCase(&'a str),
+    // NameOne, NameTwo, ...
+    CamelCase(&'a str),
+    // NAME_ONE, NAME_TWO, ...
+    AllCaps(&'a str),
+    // Failed to format the import item
+    Invalid,
+}
+
+impl<'a> ImportItem<'a> {
+    fn from_str(s: &str) -> ImportItem {
+        if s == "self" || s.starts_with("self as") {
+            ImportItem::SelfImport(s)
+        } else if s.chars().all(|c| c.is_lowercase() || c == '_' || c == ' ') {
+            ImportItem::SnakeCase(s)
+        } else if s.chars().all(|c| c.is_uppercase() || c == '_' || c == ' ') {
+            ImportItem::AllCaps(s)
+        } else {
+            ImportItem::CamelCase(s)
+        }
+    }
+
+    fn from_opt_str(s: Option<&String>) -> ImportItem {
+        s.map_or(ImportItem::Invalid, |s| ImportItem::from_str(s))
+    }
+
+    fn to_str(&self) -> Option<&str> {
+        match *self {
+            ImportItem::SelfImport(s) |
+            ImportItem::SnakeCase(s) |
+            ImportItem::CamelCase(s) |
+            ImportItem::AllCaps(s) => Some(s),
+            ImportItem::Invalid => None,
+        }
+    }
+
+    fn to_u32(&self) -> u32 {
+        match *self {
+            ImportItem::SelfImport(..) => 0,
+            ImportItem::SnakeCase(..) => 1,
+            ImportItem::CamelCase(..) => 2,
+            ImportItem::AllCaps(..) => 3,
+            ImportItem::Invalid => 4,
+        }
+    }
+}
+
+impl<'a> PartialOrd for ImportItem<'a> {
+    fn partial_cmp(&self, other: &ImportItem<'a>) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl<'a> Ord for ImportItem<'a> {
+    fn cmp(&self, other: &ImportItem<'a>) -> Ordering {
+        let res = self.to_u32().cmp(&other.to_u32());
+        if res != Ordering::Equal {
+            return res;
+        }
+        self.to_str().map_or(Ordering::Greater, |self_str| {
+            other
+                .to_str()
+                .map_or(Ordering::Less, |other_str| self_str.cmp(other_str))
+        })
+    }
+}
+
 // Pretty prints a multi-item import.
 // Assumes that path_list.len() > 0.
-pub fn rewrite_use_list(
+fn rewrite_use_list(
     shape: Shape,
     path: &ast::Path,
     path_list: &[ast::PathListItem],
@@ -334,13 +408,14 @@ pub fn rewrite_use_list(
         _ => (),
     }
 
-    let colons_offset = if path_str.is_empty() { 0 } else { 2 };
+    let path_str = if path_str.is_empty() {
+        path_str
+    } else {
+        format!("{}::", path_str)
+    };
 
     // 2 = "{}"
-    let remaining_width = shape
-        .width
-        .checked_sub(path_str.len() + 2 + colons_offset)
-        .unwrap_or(0);
+    let remaining_width = shape.width.checked_sub(path_str.len() + 2).unwrap_or(0);
 
     let mut items = {
         // Dummy value, see explanation below.
@@ -366,35 +441,60 @@ pub fn rewrite_use_list(
     let first_index = if has_self { 0 } else { 1 };
 
     if context.config.reorder_imported_names() {
-        items[1..].sort_by(|a, b| a.item.cmp(&b.item));
+        items[1..].sort_by(|a, b| {
+            let a = ImportItem::from_opt_str(a.item.as_ref());
+            let b = ImportItem::from_opt_str(b.item.as_ref());
+            a.cmp(&b)
+        });
     }
 
-
     let tactic = definitive_tactic(
         &items[first_index..],
-        ::lists::ListTactic::Mixed,
+        context.config.imports_layout(),
         remaining_width,
     );
 
+    let nested_indent = match context.config.imports_indent() {
+        IndentStyle::Block => shape.indent.block_indent(context.config),
+        // 1 = `{`
+        IndentStyle::Visual => shape.visual_indent(path_str.len() + 1).indent,
+    };
+
+    let nested_shape = match context.config.imports_indent() {
+        IndentStyle::Block => Shape::indented(nested_indent, context.config),
+        IndentStyle::Visual => Shape::legacy(remaining_width, nested_indent),
+    };
+
+    let ends_with_newline = context.config.imports_indent() == IndentStyle::Block &&
+        tactic != DefinitiveListTactic::Horizontal;
+
     let fmt = ListFormatting {
         tactic: tactic,
         separator: ",",
-        trailing_separator: SeparatorTactic::Never,
-        // Add one to the indent to account for "{"
-        shape: Shape::legacy(
-            remaining_width,
-            shape.indent + path_str.len() + colons_offset + 1,
-        ),
-        ends_with_newline: false,
+        trailing_separator: if ends_with_newline {
+            context.config.trailing_comma()
+        } else {
+            SeparatorTactic::Never
+        },
+        shape: nested_shape,
+        ends_with_newline: ends_with_newline,
         config: context.config,
     };
     let list_str = try_opt!(write_list(&items[first_index..], &fmt));
 
-    Some(if path_str.is_empty() {
-        format!("{{{}}}", list_str)
-    } else {
-        format!("{}::{{{}}}", path_str, list_str)
-    })
+    let result =
+        if list_str.contains('\n') && context.config.imports_indent() == IndentStyle::Block {
+            format!(
+                "{}{{\n{}{}\n{}}}",
+                path_str,
+                nested_shape.indent.to_string(context.config),
+                list_str,
+                shape.indent.to_string(context.config)
+            )
+        } else {
+            format!("{}{{{}}}", path_str, list_str)
+        };
+    Some(result)
 }
 
 // Returns true when self item was found.

src/issues.rs

@@ -13,6 +13,7 @@
 // associated issue number.
 
 use std::fmt;
+
 pub use config::ReportTactic;
 
 const TO_DO_CHARS: &'static [char] = &['T', 'O', 'D', 'O'];

src/items.rs

@@ -10,25 +10,25 @@
 
 // Formatting top-level items - functions, structs, enums, traits, impls.
 
+use syntax::{abi, ast, ptr, symbol};
+use syntax::ast::ImplItem;
+use syntax::codemap::{BytePos, Span};
+
 use {Indent, Shape, Spanned};
 use codemap::SpanUtils;
-use utils::{format_mutability, format_visibility, contains_skip, end_typaram, wrap_str,
-            last_line_width, format_unsafety, trim_newlines, stmt_expr, semicolon_for_expr,
-            trimmed_last_line_width, colon_spaces, mk_sp};
-use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic,
-            DefinitiveListTactic, ListTactic, definitive_tactic};
+use comment::{contains_comment, recover_comment_removed, rewrite_comment, FindUncommented};
+use config::{BraceStyle, Config, Density, IndentStyle, ReturnIndent, Style};
 use expr::{format_expr, is_empty_block, is_simple_block_stmt, rewrite_assign_rhs,
            rewrite_call_inner, ExprType};
-use comment::{FindUncommented, contains_comment, rewrite_comment, recover_comment_removed};
-use visitor::FmtVisitor;
+use lists::{definitive_tactic, itemize_list, write_list, DefinitiveListTactic, ListFormatting,
+            ListItem, ListTactic, SeparatorTactic};
 use rewrite::{Rewrite, RewriteContext};
-use config::{Config, IndentStyle, Density, ReturnIndent, BraceStyle, Style};
 use types::join_bounds;
+use utils::{colon_spaces, contains_skip, end_typaram, format_mutability, format_unsafety,
+            format_visibility, last_line_width, mk_sp, semicolon_for_expr, stmt_expr,
+            trim_newlines, trimmed_last_line_width, wrap_str};
 use vertical::rewrite_with_alignment;
-
-use syntax::{ast, abi, ptr, symbol};
-use syntax::codemap::{Span, BytePos};
-use syntax::ast::ImplItem;
+use visitor::FmtVisitor;
 
 fn type_annotation_separator(config: &Config) -> &str {
     colon_spaces(
@@ -2204,21 +2204,18 @@ fn rewrite_args(
         .and_then(|item| item.post_comment.as_ref())
         .map_or(false, |s| s.trim().starts_with("//"));
 
-    let (indent, trailing_comma, end_with_newline) = match context.config.fn_args_layout() {
-        IndentStyle::Block if fits_in_one_line => (
-            indent.block_indent(context.config),
-            SeparatorTactic::Never,
-            true,
-        ),
+    let (indent, trailing_comma) = match context.config.fn_args_layout() {
+        IndentStyle::Block if fits_in_one_line => {
+            (indent.block_indent(context.config), SeparatorTactic::Never)
+        }
         IndentStyle::Block => (
             indent.block_indent(context.config),
             context.config.trailing_comma(),
-            true,
         ),
         IndentStyle::Visual if last_line_ends_with_comment => {
-            (arg_indent, context.config.trailing_comma(), true)
+            (arg_indent, context.config.trailing_comma())
         }
-        IndentStyle::Visual => (arg_indent, SeparatorTactic::Never, false),
+        IndentStyle::Visual => (arg_indent, SeparatorTactic::Never),
     };
 
     let tactic = definitive_tactic(
@@ -2242,7 +2239,7 @@ fn rewrite_args(
             trailing_comma
         },
         shape: Shape::legacy(budget, indent),
-        ends_with_newline: end_with_newline,
+        ends_with_newline: tactic.ends_with_newline(context.config.fn_args_layout()),
         config: context.config,
     };
 
@@ -2406,8 +2403,6 @@ where
     let item_vec = items.collect::<Vec<_>>();
 
     let tactic = definitive_tactic(&item_vec, ListTactic::HorizontalVertical, one_line_budget);
-    let ends_with_newline = context.config.generics_indent() == IndentStyle::Block &&
-        tactic == DefinitiveListTactic::Vertical;
     let fmt = ListFormatting {
         tactic: tactic,
         separator: ",",
@@ -2417,7 +2412,7 @@ where
             context.config.trailing_comma()
         },
         shape: shape,
-        ends_with_newline: ends_with_newline,
+        ends_with_newline: tactic.ends_with_newline(context.config.generics_indent()),
         config: context.config,
     };
 
@@ -2631,7 +2626,7 @@ fn rewrite_where_clause(
         separator: ",",
         trailing_separator: comma_tactic,
         shape: Shape::legacy(budget, offset),
-        ends_with_newline: true,
+        ends_with_newline: tactic.ends_with_newline(context.config.where_pred_indent()),
         config: context.config,
     };
     let preds_str = try_opt!(write_list(&item_vec, &fmt));

src/lib.rs

@@ -28,27 +28,26 @@ extern crate regex;
 extern crate diff;
 extern crate term;
 
-use errors::{Handler, DiagnosticBuilder};
-use errors::emitter::{ColorConfig, EmitterWriter};
-use syntax::ast;
-use syntax::codemap::{CodeMap, Span, FilePathMapping};
-use syntax::parse::{self, ParseSess};
-
-use strings::string_buffer::StringBuffer;
-
+use std::collections::HashMap;
+use std::fmt;
 use std::io::{self, stdout, Write};
 use std::ops::{Add, Sub};
 use std::path::{Path, PathBuf};
 use std::rc::Rc;
-use std::collections::HashMap;
-use std::fmt;
 
-use issues::{BadIssueSeeker, Issue};
-use filemap::FileMap;
-use visitor::FmtVisitor;
+use errors::{DiagnosticBuilder, Handler};
+use errors::emitter::{ColorConfig, EmitterWriter};
+use strings::string_buffer::StringBuffer;
+use syntax::ast;
+use syntax::codemap::{CodeMap, FilePathMapping, Span};
+use syntax::parse::{self, ParseSess};
+
+use checkstyle::{output_footer, output_header};
 use config::Config;
-use checkstyle::{output_header, output_footer};
+use filemap::FileMap;
+use issues::{BadIssueSeeker, Issue};
 use utils::mk_sp;
+use visitor::FmtVisitor;
 
 pub use self::summary::Summary;
 
@@ -78,10 +77,6 @@ mod patterns;
 mod summary;
 mod vertical;
 
-const MIN_STRING: usize = 10;
-// When we get scoped annotations, we should have rustfmt::skip.
-const SKIP_ANNOTATION: &'static str = "rustfmt_skip";
-
 pub trait Spanned {
     fn span(&self) -> Span;
 }

src/lists.rs

@@ -11,7 +11,7 @@
 use std::cmp;
 use std::iter::Peekable;
 
-use syntax::codemap::{CodeMap, BytePos};
+use syntax::codemap::{BytePos, CodeMap};
 
 use {Indent, Shape};
 use comment::{find_comment_end, rewrite_comment, FindUncommented};
@@ -124,6 +124,15 @@ pub enum DefinitiveListTactic {
     Mixed,
 }
 
+impl DefinitiveListTactic {
+    pub fn ends_with_newline(&self, indent_style: IndentStyle) -> bool {
+        match indent_style {
+            IndentStyle::Block => *self != DefinitiveListTactic::Horizontal,
+            IndentStyle::Visual => false,
+        }
+    }
+}
+
 pub fn definitive_tactic<I, T>(items: I, tactic: ListTactic, width: usize) -> DefinitiveListTactic
 where
     I: IntoIterator<Item = T> + Clone,
@@ -169,7 +178,7 @@ where
 
     // Now that we know how we will layout, we can decide for sure if there
     // will be a trailing separator.
-    let trailing_separator = needs_trailing_separator(formatting.trailing_separator, tactic);
+    let mut trailing_separator = needs_trailing_separator(formatting.trailing_separator, tactic);
     let mut result = String::new();
     let cloned_items = items.clone();
     let mut iter = items.into_iter().enumerate().peekable();
@@ -182,7 +191,7 @@ where
         let inner_item = try_opt!(item.item.as_ref());
         let first = i == 0;
         let last = iter.peek().is_none();
-        let separate = !last || trailing_separator;
+        let mut separate = !last || trailing_separator;
         let item_sep_len = if separate { sep_len } else { 0 };
 
         // Item string may be multi-line. Its length (used for block comment alignment)
@@ -213,6 +222,13 @@ where
                     result.push('\n');
                     result.push_str(indent_str);
                     line_len = 0;
+                    if tactic == DefinitiveListTactic::Mixed && formatting.ends_with_newline {
+                        if last {
+                            separate = true;
+                        } else {
+                            trailing_separator = true;
+                        }
+                    }
                 }
 
                 if line_len > 0 {

src/macros.rs

@@ -21,17 +21,17 @@
 
 use syntax::ast;
 use syntax::codemap::BytePos;
-use syntax::parse::token::Token;
 use syntax::parse::new_parser_from_tts;
-use syntax::tokenstream::TokenStream;
+use syntax::parse::token::Token;
 use syntax::symbol;
+use syntax::tokenstream::TokenStream;
 use syntax::util::ThinVec;
 
 use Shape;
 use codemap::SpanUtils;
+use comment::{contains_comment, FindUncommented};
+use expr::{rewrite_array, rewrite_call_inner};
 use rewrite::{Rewrite, RewriteContext};
-use expr::{rewrite_call_inner, rewrite_array};
-use comment::{FindUncommented, contains_comment};
 use utils::mk_sp;
 
 const FORCED_BRACKET_MACROS: &'static [&'static str] = &["vec!"];

src/missed_spans.rs

@@ -8,12 +8,12 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use config::WriteMode;
-use visitor::FmtVisitor;
-use syntax::codemap::{BytePos, Span, Pos};
-use comment::{CodeCharKind, CommentCodeSlices, rewrite_comment};
 use Shape;
+use comment::{rewrite_comment, CodeCharKind, CommentCodeSlices};
+use config::WriteMode;
+use syntax::codemap::{BytePos, Pos, Span};
 use utils::mk_sp;
+use visitor::FmtVisitor;
 
 impl<'a> FmtVisitor<'a> {
     fn output_at_start(&self) -> bool {

src/modules.rs

@@ -8,15 +8,15 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use utils;
-
-use std::path::{Path, PathBuf};
 use std::collections::BTreeMap;
+use std::path::{Path, PathBuf};
 
 use syntax::ast;
 use syntax::codemap;
 use syntax::parse::parser;
 
+use utils::contains_skip;
+
 
 /// List all the files containing modules of a crate.
 /// If a file is used twice in a crate, it appears only once.
@@ -46,7 +46,7 @@ fn list_submodules<'a>(
     debug!("list_submodules: search_dir: {:?}", search_dir);
     for item in &module.items {
         if let ast::ItemKind::Mod(ref sub_mod) = item.node {
-            if !utils::contains_skip(&item.attrs) {
+            if !contains_skip(&item.attrs) {
                 let is_internal =
                     codemap.span_to_filename(item.span) == codemap.span_to_filename(sub_mod.inner);
                 let dir_path = if is_internal {

src/patterns.rs

@@ -8,21 +8,20 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use Shape;
-use codemap::SpanUtils;
-use rewrite::{Rewrite, RewriteContext};
-use utils::{wrap_str, format_mutability, mk_sp};
-use lists::{DefinitiveListTactic, SeparatorTactic, itemize_list, struct_lit_shape,
-            struct_lit_tactic, shape_for_tactic, struct_lit_formatting, write_list};
-use expr::{rewrite_call_inner, rewrite_unary_prefix, rewrite_pair, can_be_overflowed_expr,
-           wrap_struct_field};
-use types::{rewrite_path, PathContext};
-use super::Spanned;
-use comment::FindUncommented;
-
-use syntax::ast::{self, BindingMode, Pat, PatKind, FieldPat, RangeEnd};
-use syntax::ptr;
+use syntax::ast::{self, BindingMode, FieldPat, Pat, PatKind, RangeEnd};
 use syntax::codemap::{self, BytePos, Span};
+use syntax::ptr;
+
+use {Shape, Spanned};
+use codemap::SpanUtils;
+use comment::FindUncommented;
+use expr::{can_be_overflowed_expr, rewrite_call_inner, rewrite_pair, rewrite_unary_prefix,
+           wrap_struct_field};
+use lists::{itemize_list, shape_for_tactic, struct_lit_formatting, struct_lit_shape,
+            struct_lit_tactic, write_list, DefinitiveListTactic, SeparatorTactic};
+use rewrite::{Rewrite, RewriteContext};
+use types::{rewrite_path, PathContext};
+use utils::{format_mutability, mk_sp, wrap_str};
 
 impl Rewrite for Pat {
     fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {

src/rustfmt_diff.rs

@@ -1,7 +1,17 @@
-use std::collections::VecDeque;
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
 use diff;
-use term;
+use std::collections::VecDeque;
 use std::io;
+use term;
 
 #[derive(Debug, PartialEq)]
 pub enum DiffLine {

src/string.rs

@@ -10,14 +10,14 @@
 
 // Format string literals.
 
-use unicode_segmentation::UnicodeSegmentation;
 use regex::Regex;
+use unicode_segmentation::UnicodeSegmentation;
 
 use Shape;
 use config::Config;
 use utils::wrap_str;
 
-use MIN_STRING;
+const MIN_STRING: usize = 10;
 
 pub struct StringFormat<'a> {
     pub opener: &'a str,
@@ -127,7 +127,7 @@ pub fn rewrite_string<'a>(orig: &str, fmt: &StringFormat<'a>) -> Option<String>
 
 #[cfg(test)]
 mod test {
-    use super::{StringFormat, rewrite_string};
+    use super::{rewrite_string, StringFormat};
 
     #[test]
     fn issue343() {

src/types.rs

@@ -8,24 +8,24 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use std::ops::Deref;
 use std::iter::ExactSizeIterator;
+use std::ops::Deref;
 
 use syntax::abi;
-use syntax::ast::{self, Mutability, FunctionRetTy};
-use syntax::codemap::{self, Span, BytePos};
+use syntax::ast::{self, FunctionRetTy, Mutability};
+use syntax::codemap::{self, BytePos, Span};
 use syntax::print::pprust;
 use syntax::symbol::keywords;
 
 use {Shape, Spanned};
 use codemap::SpanUtils;
-use items::{format_generics_item_list, generics_shape_from_config};
-use lists::{write_list, itemize_list, ListFormatting, SeparatorTactic, ListTactic,
-            definitive_tactic};
-use rewrite::{Rewrite, RewriteContext};
-use utils::{extra_offset, format_mutability, colon_spaces, wrap_str, mk_sp, last_line_width};
-use expr::{rewrite_unary_prefix, rewrite_pair, rewrite_tuple, wrap_args_with_parens};
 use config::{IndentStyle, Style, TypeDensity};
+use expr::{rewrite_pair, rewrite_tuple, rewrite_unary_prefix, wrap_args_with_parens};
+use items::{format_generics_item_list, generics_shape_from_config};
+use lists::{definitive_tactic, itemize_list, write_list, ListFormatting, ListTactic,
+            SeparatorTactic};
+use rewrite::{Rewrite, RewriteContext};
+use utils::{colon_spaces, extra_offset, format_mutability, last_line_width, mk_sp, wrap_str};
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub enum PathContext {
@@ -360,7 +360,7 @@ where
             context.config.trailing_comma()
         },
         shape: list_shape,
-        ends_with_newline: false,
+        ends_with_newline: tactic.ends_with_newline(context.config.fn_call_style()),
         config: context.config,
     };
 

src/utils.rs

@@ -11,15 +11,16 @@
 use std::borrow::Cow;
 use std::cmp::Ordering;
 
-use syntax::ast::{self, Visibility, Attribute, MetaItem, MetaItemKind, NestedMetaItem,
-                  NestedMetaItemKind, Path};
-use syntax::codemap::{BytePos, Span, NO_EXPANSION};
 use syntax::abi;
+use syntax::ast::{self, Attribute, MetaItem, MetaItemKind, NestedMetaItem, NestedMetaItemKind,
+                  Path, Visibility};
+use syntax::codemap::{BytePos, Span, NO_EXPANSION};
 
 use Shape;
 use rewrite::{Rewrite, RewriteContext};
 
-use SKIP_ANNOTATION;
+// When we get scoped annotations, we should have rustfmt::skip.
+const SKIP_ANNOTATION: &'static str = "rustfmt_skip";
 
 // Computes the length of a string's last line, minus offset.
 pub fn extra_offset(text: &str, shape: Shape) -> usize {

src/vertical.rs

@@ -12,6 +12,9 @@
 
 use std::cmp;
 
+use syntax::ast;
+use syntax::codemap::{BytePos, Span};
+
 use {Indent, Shape, Spanned};
 use codemap::SpanUtils;
 use comment::contains_comment;
@@ -21,9 +24,6 @@ use lists::{definitive_tactic, itemize_list, write_list, ListFormatting, ListTac
 use rewrite::{Rewrite, RewriteContext};
 use utils::{contains_skip, mk_sp};
 
-use syntax::ast;
-use syntax::codemap::{Span, BytePos};
-
 pub trait AlignedItem {
     fn skip(&self) -> bool;
     fn get_span(&self) -> Span;

src/visitor.rs

@@ -10,24 +10,23 @@
 
 use std::cmp;
 
+use strings::string_buffer::StringBuffer;
 use syntax::{ast, ptr, visit};
-use syntax::codemap::{CodeMap, Span, BytePos};
+use syntax::codemap::{BytePos, CodeMap, Span};
 use syntax::parse::ParseSess;
 
-use strings::string_buffer::StringBuffer;
-
 use {Indent, Shape};
-use expr::{format_expr, ExprType};
-use utils::{self, mk_sp};
 use codemap::{LineRangeUtils, SpanUtils};
 use comment::{contains_comment, FindUncommented};
-use config::Config;
-use rewrite::{Rewrite, RewriteContext};
 use comment::rewrite_comment;
-use macros::{rewrite_macro, MacroPosition};
-use items::{rewrite_static, rewrite_associated_type, rewrite_associated_impl_type,
-            rewrite_type_alias, format_impl, format_trait};
+use config::Config;
+use expr::{format_expr, ExprType};
+use items::{format_impl, format_trait, rewrite_associated_impl_type, rewrite_associated_type,
+            rewrite_static, rewrite_type_alias};
 use lists::{itemize_list, write_list, DefinitiveListTactic, ListFormatting, SeparatorTactic};
+use macros::{rewrite_macro, MacroPosition};
+use rewrite::{Rewrite, RewriteContext};
+use utils::{self, mk_sp};
 
 fn is_use_item(item: &ast::Item) -> bool {
     match item.node {

tests/system.rs

@@ -15,7 +15,7 @@ extern crate term;
 
 use std::collections::HashMap;
 use std::fs;
-use std::io::{self, Read, BufRead, BufReader};
+use std::io::{self, BufRead, BufReader, Read};
 use std::path::{Path, PathBuf};
 
 use rustfmt::*;

tests/target/configs-fn_args_density-compressed.rs

@@ -10,12 +10,12 @@ trait Lorem {
 
     fn lorem(
         ipsum: Ipsum, dolor: Dolor, sit: Sit, amet: Amet, consectetur: onsectetur,
-        adipiscing: Adipiscing, elit: Elit
+        adipiscing: Adipiscing, elit: Elit,
     );
 
     fn lorem(
         ipsum: Ipsum, dolor: Dolor, sit: Sit, amet: Amet, consectetur: onsectetur,
-        adipiscing: Adipiscing, elit: Elit
+        adipiscing: Adipiscing, elit: Elit,
     ) {
         // body
     }

tests/target/configs-imports_indent-block.rs

@@ -0,0 +1,7 @@
+// rustfmt-imports_indent: Block
+
+use lists::{
+    definitive_tactic, itemize_list, shape_for_tactic, struct_lit_formatting, struct_lit_shape,
+    struct_lit_tactic, write_list, DefinitiveListTactic, ListFormatting, ListItem, ListTactic,
+    SeparatorTactic,
+};

tests/target/configs-imports_layout-horizontal_vertical.rs

@@ -0,0 +1,18 @@
+// rustfmt-imports_indent: Block
+// rustfmt-imports_layout: HorizontalVertical
+
+use comment::{contains_comment, recover_comment_removed, rewrite_comment, FindUncommented};
+use lists::{
+    definitive_tactic,
+    itemize_list,
+    shape_for_tactic,
+    struct_lit_formatting,
+    struct_lit_shape,
+    struct_lit_tactic,
+    write_list,
+    DefinitiveListTactic,
+    ListFormatting,
+    ListItem,
+    ListTactic,
+    SeparatorTactic,
+};

tests/target/configs-imports_layout-mixed.rs

@@ -0,0 +1,9 @@
+// rustfmt-imports_indent: Block
+// rustfmt-imports_layout: Mixed
+
+use comment::{contains_comment, recover_comment_removed, rewrite_comment, FindUncommented};
+use lists::{
+    definitive_tactic, itemize_list, shape_for_tactic, struct_lit_formatting, struct_lit_shape,
+    struct_lit_tactic, write_list, DefinitiveListTactic, ListFormatting, ListItem, ListTactic,
+    SeparatorTactic,
+};

tests/target/fn-custom.rs

@@ -4,7 +4,7 @@
 // Test compressed layout of args.
 fn foo(
     a: Aaaaaaaaaaaaaaa, b: Bbbbbbbbbbbbbbbb, c: Ccccccccccccccccc, d: Ddddddddddddddddddddddddd,
-    e: Eeeeeeeeeeeeeeeeeee
+    e: Eeeeeeeeeeeeeeeeeee,
 ) {
     foo();
 }
@@ -12,7 +12,7 @@ fn foo(
 impl Foo {
     fn foo(
         self, a: Aaaaaaaaaaaaaaa, b: Bbbbbbbbbbbbbbbb, c: Ccccccccccccccccc,
-        d: Ddddddddddddddddddddddddd, e: Eeeeeeeeeeeeeeeeeee
+        d: Ddddddddddddddddddddddddd, e: Eeeeeeeeeeeeeeeeeee,
     ) {
         foo();
     }

tests/target/fn-simple.rs

@@ -42,7 +42,7 @@ where
         C,
         D,
         // pre comment
-        E, /* last comment */
+        E, // last comment
     ) -> &SomeType,
 {
     arg(a, b, c, d, e)

tests/target/hard-tabs.rs

@@ -67,7 +67,7 @@ fn main() {
 			C,
 			D,
 			// pre comment
-			E, /* last comment */
+			E, // last comment
 		) -> &SomeType,
 	{
 		arg(a, b, c, d, e)

tests/target/imports-reorder-lines.rs

@@ -1,7 +1,7 @@
 // rustfmt-reorder_imports: true
 
-use std::cmp::{d, c, b, a};
-use std::cmp::{b, e, g, f};
+use std::cmp::{a, b, c, d};
+use std::cmp::{b, e, f, g};
 // This comment should stay with `use std::ddd;`
 use std::ddd;
 use std::ddd::aaa;

tests/target/imports-reorder.rs

@@ -3,4 +3,4 @@
 
 use path::{self /* self */, /* A */ A, B /* B */, C};
 
-use {Z, aa, ab, ac, b};
+use {aa, ab, ac, b, Z};

tests/target/imports.rs

@@ -4,36 +4,36 @@
 // Imports.
 
 // Long import.
-use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic, ItemDefaultImpl};
+use syntax::ast::{ItemDefaultImpl, ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic};
 use exceedingly::looooooooooooooooooooooooooooooooooooooooooooooooooooooooooong::import::path::{ItemA,
                                                                                                 ItemB};
 use exceedingly::loooooooooooooooooooooooooooooooooooooooooooooooooooooooong::import::path::{ItemA,
                                                                                              ItemB};
 
-use list::{// Some item
-           SomeItem, // Comment
-           // Another item
+use list::{// Another item
            AnotherItem, // Another Comment
            // Last Item
-           LastItem};
+           LastItem,
+           // Some item
+           SomeItem /* Comment */};
 
 use test::{/* A */ self /* B */, Other /* C */};
 
 use syntax;
-use {/* Pre-comment! */ Foo, Bar /* comment */};
+use {Bar /* comment */, /* Pre-comment! */ Foo};
 use Foo::{Bar, Baz};
-pub use syntax::ast::{Expr_, Expr, ExprAssign, ExprCall, ExprMethodCall, ExprPath};
+pub use syntax::ast::{Expr, ExprAssign, ExprCall, ExprMethodCall, ExprPath, Expr_};
 
 use self;
 use std::io;
 use std::io;
 
 mod Foo {
-    pub use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic, ItemDefaultImpl};
+    pub use syntax::ast::{ItemDefaultImpl, ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic};
 
     mod Foo2 {
-        pub use syntax::ast::{self, ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic,
-                              ItemDefaultImpl};
+        pub use syntax::ast::{self, ItemDefaultImpl, ItemForeignMod, ItemImpl, ItemMac, ItemMod,
+                              ItemStatic};
     }
 }
 

tests/target/multiple.rs

@@ -17,9 +17,9 @@ extern crate foo;
 extern crate foo;
 
 use std::cell::*;
-use std::{self, any, ascii, borrow, boxed, char, borrow, boxed, char, borrow, borrow, boxed, char,
-          borrow, boxed, char, borrow, boxed, char, borrow, boxed, char, borrow, boxed, char,
-          borrow, boxed, char, borrow, boxed, char, borrow, boxed, char};
+use std::{self, any, ascii, borrow, borrow, borrow, borrow, borrow, borrow, borrow, borrow,
+          borrow, borrow, borrow, boxed, boxed, boxed, boxed, boxed, boxed, boxed, boxed, boxed,
+          boxed, char, char, char, char, char, char, char, char, char, char};
 
 mod doc;
 mod other;

tests/target/nestedmod/mod.rs

@@ -3,7 +3,7 @@ mod mod2a;
 mod mod2b;
 
 mod mymod1 {
-    use mod2a::{Foo, Bar};
+    use mod2a::{Bar, Foo};
     mod mod3a;
 }
 

tests/target/type.rs

@@ -25,7 +25,7 @@ struct F {
         y: String, // comment 3
         z: Foo,
         // comment
-        ... /* comment 2 */
+        ... // comment 2
     ),
 }