From 2a23898ff60a94da92aa2304a7805509702f719d Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Mon, 24 Jul 2017 18:02:17 +0900 Subject: [PATCH 1/6] Avoid early return from format_expr --- src/expr.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index c1d5e8ccc39..715413ff0e6 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -185,10 +185,11 @@ pub fn format_expr( } else { // Rewrite block without trying to put it in a single line. if let rw @ Some(_) = rewrite_empty_block(context, block, shape) { - return rw; + rw + } else { + let prefix = try_opt!(block_prefix(context, block, shape)); + rewrite_block_with_visitor(context, &prefix, block, shape) } - let prefix = try_opt!(block_prefix(context, block, shape)); - rewrite_block_with_visitor(context, &prefix, block, shape) } } ExprType::SubExpression => block.rewrite(context, shape), @@ -337,15 +338,16 @@ pub fn format_expr( if let rewrite @ Some(_) = rewrite_single_line_block(context, "do catch ", block, shape) { - return rewrite; + rewrite + } else { + // 9 = `do catch ` + let budget = shape.width.checked_sub(9).unwrap_or(0); + let shape = Shape::legacy(budget, shape.indent).add_offset(9); + Some(format!( + "do catch {}", + try_opt!(block.rewrite(&context, shape)) + )) } - // 9 = `do catch ` - let budget = shape.width.checked_sub(9).unwrap_or(0); - Some(format!( - "{}{}", - "do catch ", - try_opt!(block.rewrite(&context, Shape::legacy(budget, shape.indent))) - )) } }; From 221480b28ca8e3c289a3ad00424b7a575b8c1c6a Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Mon, 24 Jul 2017 18:02:32 +0900 Subject: [PATCH 2/6] Cover attributes in Spanned trait implementation of ast::Stmt --- src/lib.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index bd611202fb7..0d0ddc3cba3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -93,12 +93,11 @@ impl Spanned for ast::Expr { impl Spanned for ast::Stmt { fn span(&self) -> Span { - match self.node { - // Cover attributes - ast::StmtKind::Expr(ref expr) | ast::StmtKind::Semi(ref expr) => { - mk_sp(expr.span().lo, self.span.hi) - } - _ => self.span, + let attrs = stmt_attrs(self); + if attrs.is_empty() { + self.span + } else { + mk_sp(attrs[0].span.lo, self.span.hi) } } } @@ -205,6 +204,15 @@ impl Spanned for ast::TyParamBound { } } +fn stmt_attrs(stmt: &ast::Stmt) -> &[ast::Attribute] { + match stmt.node { + ast::StmtKind::Local(ref local) => &local.attrs, + ast::StmtKind::Item(ref item) => &item.attrs, + ast::StmtKind::Expr(ref expr) | ast::StmtKind::Semi(ref expr) => &expr.attrs, + ast::StmtKind::Mac(ref mac) => &mac.2, + } +} + #[derive(Copy, Clone, Debug)] pub struct Indent { // Width of the block indent, in characters. Must be a multiple of From 9d902a4656d1048f185054d54600c6d065e924d3 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Mon, 24 Jul 2017 18:03:56 +0900 Subject: [PATCH 3/6] Format attributes on ast::StmtKind::Local and ast::StmtKind::Mac --- src/items.rs | 8 +++++++- src/visitor.rs | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/items.rs b/src/items.rs index 087b5615f02..52b6c2d96f7 100644 --- a/src/items.rs +++ b/src/items.rs @@ -50,7 +50,13 @@ impl Rewrite for ast::Local { shape.width, shape.indent ); - let mut result = "let ".to_owned(); + let mut result = if self.attrs.is_empty() { + "let ".to_owned() + } else { + let attrs_str = try_opt!(self.attrs.rewrite(context, shape)); + let indent_str = shape.indent.to_string(context.config); + format!("{}\n{}let ", attrs_str, indent_str) + }; // 4 = "let ".len() let pat_shape = try_opt!(shape.offset_left(4)); diff --git a/src/visitor.rs b/src/visitor.rs index 24802133593..2cc44b71747 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -88,7 +88,7 @@ impl<'a> FmtVisitor<'a> { Shape::indented(self.block_indent, self.config), ) }; - self.push_rewrite(stmt.span, rewrite); + self.push_rewrite(stmt.span(), rewrite); } ast::StmtKind::Expr(ref expr) => { let rewrite = format_expr( @@ -121,6 +121,12 @@ impl<'a> FmtVisitor<'a> { if contains_skip(attrs) { self.push_rewrite(mac.span, None); } else { + if !attrs.is_empty() { + let shape = Shape::indented(self.block_indent, self.config); + let attrs_rw = attrs.rewrite(&self.get_context(), shape); + let span = mk_sp(attrs[0].span.lo, attrs.last().unwrap().span.hi); + self.push_rewrite(span, attrs_rw); + } self.visit_mac(mac, None, MacroPosition::Statement); } self.format_missing(stmt.span.hi); From 51aeb52c0471c2b755f7ae30e64b74de596756b1 Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Mon, 24 Jul 2017 18:06:53 +0900 Subject: [PATCH 4/6] Use stmt.rewrite() instead of format_expr() --- src/expr.rs | 6 +----- src/visitor.rs | 26 +++----------------------- 2 files changed, 4 insertions(+), 28 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index 715413ff0e6..3509f519f92 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -950,11 +950,7 @@ impl Rewrite for ast::Stmt { format_expr( ex, - match self.node { - ast::StmtKind::Expr(_) => ExprType::SubExpression, - ast::StmtKind::Semi(_) => ExprType::Statement, - _ => unreachable!(), - }, + ExprType::Statement, context, try_opt!(shape.sub_width(suffix.len())), ).map(|s| s + suffix) diff --git a/src/visitor.rs b/src/visitor.rs index 2cc44b71747..1cc05538426 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -15,12 +15,11 @@ use syntax::{ast, ptr, visit}; use syntax::codemap::{BytePos, CodeMap, Span}; use syntax::parse::ParseSess; -use {Indent, Shape}; +use {Indent, Shape, Spanned}; use codemap::{LineRangeUtils, SpanUtils}; use comment::{contains_comment, 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, SeparatorTactic}; @@ -90,31 +89,12 @@ impl<'a> FmtVisitor<'a> { }; self.push_rewrite(stmt.span(), rewrite); } - ast::StmtKind::Expr(ref expr) => { - let rewrite = format_expr( - expr, - ExprType::Statement, - &self.get_context(), - Shape::indented(self.block_indent, self.config), - ); - let span = if expr.attrs.is_empty() { - stmt.span - } else { - mk_sp(expr.attrs[0].span.lo, stmt.span.hi) - }; - self.push_rewrite(span, rewrite) - } - ast::StmtKind::Semi(ref expr) => { + ast::StmtKind::Expr(..) | ast::StmtKind::Semi(..) => { let rewrite = stmt.rewrite( &self.get_context(), Shape::indented(self.block_indent, self.config), ); - let span = if expr.attrs.is_empty() { - stmt.span - } else { - mk_sp(expr.attrs[0].span.lo, stmt.span.hi) - }; - self.push_rewrite(span, rewrite) + self.push_rewrite(stmt.span(), rewrite) } ast::StmtKind::Mac(ref mac) => { let (ref mac, _macro_style, ref attrs) = **mac; From 40ec94e9f38aa9354d3ab2b70506e2ea5ba15cdd Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Mon, 24 Jul 2017 18:08:28 +0900 Subject: [PATCH 5/6] Construct span for catch to recover missing do --- src/expr.rs | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index 3509f519f92..f8098599126 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -59,10 +59,16 @@ fn combine_attr_and_expr( String::new() } else { // Try to recover comments between the attributes and the expression if available. - let missing_snippet = context.snippet(mk_sp( - expr.attrs[expr.attrs.len() - 1].span.hi, - expr.span.lo, - )); + // Some dirty hack involved here to recover missing `do` from rust parser. + let hi = if let ast::ExprKind::Catch(..) = expr.node { + let missing_span = mk_sp(expr.attrs[expr.attrs.len() - 1].span.hi, expr.span.lo); + let snippet = context.snippet(missing_span); + let do_pos = snippet.find_uncommented("do").unwrap(); + missing_span.lo + BytePos(do_pos as u32) + } else { + expr.span.lo + }; + let missing_snippet = context.snippet(mk_sp(expr.attrs[expr.attrs.len() - 1].span.hi, hi)); let comment_opening_pos = missing_snippet.chars().position(|c| c == '/'); let prefer_same_line = if let Some(pos) = comment_opening_pos { !missing_snippet[..pos].contains('\n') @@ -353,7 +359,16 @@ pub fn format_expr( expr_rw .and_then(|expr_str| { - recover_comment_removed(expr_str, expr.span, context, shape) + // Unfortunately, we are missing `do` in span of catch from parser. We must add `do` + // manually when recovering missing comments. + recover_comment_removed(expr_str, expr.span, context, shape).map(|recoverd_str| { + if let ast::ExprKind::Catch(..) = expr.node { + if !recoverd_str.starts_with("do") { + return String::from("do ") + &recoverd_str; + } + } + recoverd_str + }) }) .and_then(|expr_str| { combine_attr_and_expr(context, shape, expr, &expr_str) From 430a0b09b4391f5dd0cb197e0800ebc0cddc03af Mon Sep 17 00:00:00 2001 From: Seiichi Uchida Date: Mon, 24 Jul 2017 18:08:44 +0900 Subject: [PATCH 6/6] Update tests --- tests/source/attrib.rs | 23 +++++++++++++++++++++++ tests/source/catch.rs | 3 +++ tests/system.rs | 4 ---- tests/target/attrib.rs | 23 +++++++++++++++++++++++ tests/target/catch.rs | 3 +++ 5 files changed, 52 insertions(+), 4 deletions(-) diff --git a/tests/source/attrib.rs b/tests/source/attrib.rs index 361d9fb784c..cdb7d0eb81d 100644 --- a/tests/source/attrib.rs +++ b/tests/source/attrib.rs @@ -91,3 +91,26 @@ fn issue_1799() { // https://github.com/rust-lang/rust/issues/43336 Some( Err(error) ) ; } + +// #1813 +fn attributes_on_statemetns() { + // Semi + # [ an_attribute ( rustfmt ) ] + foo(1); + + // Local + # [ an_attribute ( rustfmt ) ] + let x = foo(a, b, c); + + // Item + # [ an_attribute ( rustfmt ) ] + use foobar; + + // Mac + # [ an_attribute ( rustfmt ) ] + vec![1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 1, 1, 2, 1]; + + // Expr + # [ an_attribute ( rustfmt ) ] + {} +} diff --git a/tests/source/catch.rs b/tests/source/catch.rs index 64cc9e7a207..1765853110b 100644 --- a/tests/source/catch.rs +++ b/tests/source/catch.rs @@ -24,4 +24,7 @@ fn main() { do catch { // Regular do catch block }; + + #[ an_attribute(rustfmt) ] + do catch { foo()? }; } diff --git a/tests/system.rs b/tests/system.rs index d915e11d534..83785fa99ac 100644 --- a/tests/system.rs +++ b/tests/system.rs @@ -346,10 +346,6 @@ fn handle_result( if fmt_text != text { let diff = make_diff(&text, &fmt_text, DIFF_CONTEXT_SIZE); - assert!( - !diff.is_empty(), - "Empty diff? Maybe due to a missing a newline at the end of a file?" - ); failures.insert(file_name, diff); } } diff --git a/tests/target/attrib.rs b/tests/target/attrib.rs index 7ff79b16411..2be31bb3129 100644 --- a/tests/target/attrib.rs +++ b/tests/target/attrib.rs @@ -91,3 +91,26 @@ fn issue_1799() { // https://github.com/rust-lang/rust/issues/43336 Some(Err(error)); } + +// #1813 +fn attributes_on_statemetns() { + // Semi + #[an_attribute(rustfmt)] + foo(1); + + // Local + #[an_attribute(rustfmt)] + let x = foo(a, b, c); + + // Item + #[an_attribute(rustfmt)] + use foobar; + + // Mac + #[an_attribute(rustfmt)] + vec![1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 1, 1, 2, 1]; + + // Expr + #[an_attribute(rustfmt)] + {} +} diff --git a/tests/target/catch.rs b/tests/target/catch.rs index 640f9bade49..3a7a5090825 100644 --- a/tests/target/catch.rs +++ b/tests/target/catch.rs @@ -18,4 +18,7 @@ fn main() { do catch { // Regular do catch block }; + + #[an_attribute(rustfmt)] + do catch { foo()? }; }