From 737186b8901a4c843f5b7eed2f3a9a8a3e41e397 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Sun, 3 Sep 2017 08:09:37 +0900 Subject: [PATCH 1/3] Use rewrite() instead of format_expr --- src/expr.rs | 7 +------ src/visitor.rs | 8 +------- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index b65383311de9..228722474e1b 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -876,13 +876,8 @@ impl Rewrite for ast::Stmt { "" }; - let expr_type = match self.node { - ast::StmtKind::Expr(_) => ExprType::SubExpression, - ast::StmtKind::Semi(_) => ExprType::Statement, - _ => unreachable!(), - }; let shape = try_opt!(shape.sub_width(suffix.len())); - format_expr(ex, expr_type, context, shape).map(|s| s + suffix) + format_expr(ex, ExprType::Statement, context, shape).map(|s| s + suffix) } ast::StmtKind::Mac(..) | ast::StmtKind::Item(..) => None, }; diff --git a/src/visitor.rs b/src/visitor.rs index 56c1783476aa..a792148130b4 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -22,7 +22,6 @@ use comment::{contains_comment, recover_missing_comment_in_span, CodeCharKind, C FindUncommented}; use comment::rewrite_comment; use config::{BraceStyle, 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, SeparatorPlace, @@ -77,12 +76,7 @@ impl<'a> FmtVisitor<'a> { let rewrite = stmt.rewrite(&self.get_context(), self.shape()); self.push_rewrite(stmt.span(), rewrite); } - ast::StmtKind::Expr(ref expr) => { - let rewrite = - format_expr(expr, ExprType::Statement, &self.get_context(), self.shape()); - self.push_rewrite(stmt.span(), rewrite) - } - ast::StmtKind::Semi(..) => { + ast::StmtKind::Expr(..) | ast::StmtKind::Semi(..) => { let rewrite = stmt.rewrite(&self.get_context(), self.shape()); self.push_rewrite(stmt.span(), rewrite) } From f8bdcd62e8f4087f602018c97063733590367ffe Mon Sep 17 00:00:00 2001 From: topecongiro Date: Sun, 3 Sep 2017 08:10:12 +0900 Subject: [PATCH 2/3] Do not allow single-lined closure with block body --- src/expr.rs | 14 +------------- tests/target/chains-visual.rs | 4 +++- tests/target/chains.rs | 4 +++- tests/target/hard-tabs.rs | 4 +++- 4 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index 228722474e1b..cc18d8edbacd 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -630,7 +630,7 @@ fn rewrite_closure( false }; if no_return_type && !needs_block { - // lock.stmts.len() == 1 + // block.stmts.len() == 1 if let Some(expr) = stmt_expr(&block.stmts[0]) { if let Some(rw) = rewrite_closure_expr(expr, &prefix, context, body_shape) { return Some(rw); @@ -638,18 +638,6 @@ fn rewrite_closure( } } - if !needs_block { - // We need braces, but we might still prefer a one-liner. - let stmt = &block.stmts[0]; - // 4 = braces and spaces. - if let Some(body_shape) = body_shape.sub_width(4) { - // Checks if rewrite succeeded and fits on a single line. - if let Some(rewrite) = and_one_line(stmt.rewrite(context, body_shape)) { - return Some(format!("{} {{ {} }}", prefix, rewrite)); - } - } - } - // Either we require a block, or tried without and failed. rewrite_closure_block(block, &prefix, context, body_shape) } else { diff --git a/tests/target/chains-visual.rs b/tests/target/chains-visual.rs index 829a27867dd5..ef166a2bb66c 100644 --- a/tests/target/chains-visual.rs +++ b/tests/target/chains-visual.rs @@ -42,7 +42,9 @@ fn main() { }); fffffffffffffffffffffffffffffffffff(a, { - SCRIPT_TASK_ROOT.with(|root| { *root.borrow_mut() = Some(&script_task); }); + SCRIPT_TASK_ROOT.with(|root| { + *root.borrow_mut() = Some(&script_task); + }); }); let suuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuum = diff --git a/tests/target/chains.rs b/tests/target/chains.rs index 4955817da5c9..2720f51d5f30 100644 --- a/tests/target/chains.rs +++ b/tests/target/chains.rs @@ -45,7 +45,9 @@ fn main() { }); fffffffffffffffffffffffffffffffffff(a, { - SCRIPT_TASK_ROOT.with(|root| { *root.borrow_mut() = Some(&script_task); }); + SCRIPT_TASK_ROOT.with(|root| { + *root.borrow_mut() = Some(&script_task); + }); }); let suuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuum = diff --git a/tests/target/hard-tabs.rs b/tests/target/hard-tabs.rs index 4ac6df5def85..ecaffc165e6d 100644 --- a/tests/target/hard-tabs.rs +++ b/tests/target/hard-tabs.rs @@ -80,7 +80,9 @@ fn main() { }); fffffffffffffffffffffffffffffffffff(a, { - SCRIPT_TASK_ROOT.with(|root| { *root.borrow_mut() = Some(&script_task); }); + SCRIPT_TASK_ROOT.with(|root| { + *root.borrow_mut() = Some(&script_task); + }); }); a.b.c.d(); From 47062c8f0a3a197a8e2f83e070c4be1e866514fa Mon Sep 17 00:00:00 2001 From: topecongiro Date: Sun, 3 Sep 2017 08:14:00 +0900 Subject: [PATCH 3/3] Format long derive --- src/visitor.rs | 34 +++++++++++++++++++++++++++++++++- tests/source/attrib.rs | 4 ++++ tests/target/attrib.rs | 5 +++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/visitor.rs b/src/visitor.rs index a792148130b4..1fe8655309f3 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -973,7 +973,7 @@ impl<'a> Rewrite for [ast::Attribute] { Some(&(_, next_attr)) if is_derive(next_attr) => insert_new_line = false, // If not, rewrite the merged derives. _ => { - result.push_str(&format!("#[derive({})]", derive_args.join(", "))); + result.push_str(&try_opt!(format_derive(context, &derive_args, shape))); derive_args.clear(); } } @@ -990,6 +990,38 @@ impl<'a> Rewrite for [ast::Attribute] { } } +// Format `#[derive(..)]`, using visual indent & mixed style when we need to go multiline. +fn format_derive(context: &RewriteContext, derive_args: &[String], shape: Shape) -> Option { + let mut result = String::with_capacity(128); + result.push_str("#[derive("); + // 11 = `#[derive()]` + let initial_budget = try_opt!(shape.width.checked_sub(11)); + let mut budget = initial_budget; + let num = derive_args.len(); + for (i, a) in derive_args.iter().enumerate() { + // 2 = `, ` or `)]` + let width = a.len() + 2; + if width > budget { + if i > 0 { + // Remove trailing whitespace. + result.pop(); + } + result.push('\n'); + // 9 = `#[derive(` + result.push_str(&(shape.indent + 9).to_string(context.config)); + budget = initial_budget; + } else { + budget = budget.checked_sub(width).unwrap_or(0); + } + result.push_str(a); + if i != num - 1 { + result.push_str(", ") + } + } + result.push_str(")]"); + Some(result) +} + fn is_derive(attr: &ast::Attribute) -> bool { match attr.meta() { Some(meta_item) => match meta_item.node { diff --git a/tests/source/attrib.rs b/tests/source/attrib.rs index 4d23d8b79d11..6653dd2daa97 100644 --- a/tests/source/attrib.rs +++ b/tests/source/attrib.rs @@ -146,3 +146,7 @@ fn attributes_on_statements() { # [ attr ( on ( mac ) ) ] foo!(); } + +// Large derive +#[derive(Add, Sub, Mul, Div, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Debug, Hash, Serialize, Deserialize)] +pub struct HP(pub u8); diff --git a/tests/target/attrib.rs b/tests/target/attrib.rs index 467d168ef4f2..e220b48fb389 100644 --- a/tests/target/attrib.rs +++ b/tests/target/attrib.rs @@ -146,3 +146,8 @@ fn attributes_on_statements() { #[attr(on(mac))] foo!(); } + +// Large derive +#[derive(Add, Sub, Mul, Div, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Debug, Hash, Serialize, + Deserialize)] +pub struct HP(pub u8);