From d9484853711132c56528afb91221fe04af529e35 Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Mon, 16 Jan 2017 16:37:58 +1300 Subject: [PATCH] Allow empty blocks on one line in more places In particular if they contain only a single-line comment. Fixes #493 --- src/expr.rs | 32 +++++++++++++++++++++++++------- tests/source/chains-visual.rs | 4 ++-- tests/source/chains.rs | 4 ++-- tests/target/chains-visual.rs | 6 ++++-- tests/target/chains.rs | 6 ++++-- tests/target/expr.rs | 17 +++++------------ tests/target/hard-tabs.rs | 3 +-- tests/target/issue-447.rs | 3 +-- tests/target/match.rs | 4 +--- 9 files changed, 45 insertions(+), 34 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index b0b2315f1fe..9794ff78093 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use std::cmp::Ordering; +use std::cmp::{Ordering, min}; use std::borrow::Borrow; use std::mem::swap; use std::ops::Deref; @@ -595,9 +595,19 @@ impl Rewrite for ast::Block { // width is used only for the single line case: either the empty block `{}`, // or an unsafe expression `unsafe { e }`. + if self.stmts.is_empty() && !block_contains_comment(self, context.codemap) && width >= 2 { + return Some("{}".to_owned()); + } + + // If a block contains only a single-line comment, then leave it on one line. let user_str = context.snippet(self.span); - if user_str == "{}" && width >= 2 { - return Some(user_str); + let user_str = user_str.trim(); + if user_str.starts_with('{') && user_str.ends_with('}') { + let comment_str = user_str[1..user_str.len() - 1].trim(); + if self.stmts.is_empty() && !comment_str.contains('\n') && + !comment_str.starts_with("//") && comment_str.len() + 4 <= width { + return Some(format!("{{ {} }}", comment_str)); + } } let mut visitor = FmtVisitor::from_codemap(context.parse_session, context.config); @@ -870,11 +880,17 @@ impl<'a> Rewrite for ControlFlow<'a> { } } - // This is used only for the empty block case: `{}`. + // This is used only for the empty block case: `{}`. So, we use 1 if we know + // we should avoid the single line case. // 2 = spaces after keyword and condition. let block_width = try_opt!(width.checked_sub(label_string.len() + self.keyword.len() + extra_offset(&pat_expr_string, inner_offset) + 2)); + let block_width = if self.else_block.is_some() || self.nested_if { + min(1, block_width) + } else { + block_width + }; let block_str = try_opt!(self.block.rewrite(context, block_width, offset)); @@ -949,7 +965,9 @@ impl<'a> Rewrite for ControlFlow<'a> { } _ => { last_in_chain = true; - else_block.rewrite(context, width, offset) + // When rewriting a block, the width is only used for single line + // blocks, passing 1 lets us avoid that. + else_block.rewrite(context, min(1, width), offset) } }; @@ -1021,8 +1039,8 @@ fn block_contains_comment(block: &ast::Block, codemap: &CodeMap) -> bool { // FIXME: incorrectly returns false when comment is contained completely within // the expression. pub fn is_simple_block(block: &ast::Block, codemap: &CodeMap) -> bool { - block.stmts.len() == 1 && stmt_is_expr(&block.stmts[0]) && - !block_contains_comment(block, codemap) + (block.stmts.len() == 1 && stmt_is_expr(&block.stmts[0]) && + !block_contains_comment(block, codemap)) } /// Checks whether a block contains at most one statement or expression, and no comments. diff --git a/tests/source/chains-visual.rs b/tests/source/chains-visual.rs index cc9899fcc60..ea7eeb60524 100644 --- a/tests/source/chains-visual.rs +++ b/tests/source/chains-visual.rs @@ -95,11 +95,11 @@ fn floaters() { .bar() .baz(); - Foo { x: val } .baz(|| { /*force multiline */ }) .quux(); + Foo { x: val } .baz(|| { force(); multiline(); }) .quux(); Foo { y: i_am_multi_line, z: ok } .baz(|| { - // force multiline + force(); multiline(); }) .quux(); diff --git a/tests/source/chains.rs b/tests/source/chains.rs index de5c3e6e823..7fcda72d83d 100644 --- a/tests/source/chains.rs +++ b/tests/source/chains.rs @@ -93,11 +93,11 @@ fn floaters() { .bar() .baz(); - Foo { x: val } .baz(|| { /*force multiline */ }) .quux(); + Foo { x: val } .baz(|| { force(); multiline(); }) .quux(); Foo { y: i_am_multi_line, z: ok } .baz(|| { - // force multiline + force(); multiline(); }) .quux(); diff --git a/tests/target/chains-visual.rs b/tests/target/chains-visual.rs index 3ece67291da..aff6f170529 100644 --- a/tests/target/chains-visual.rs +++ b/tests/target/chains-visual.rs @@ -105,7 +105,8 @@ fn floaters() { Foo { x: val } .baz(|| { - // force multiline + force(); + multiline(); }) .quux(); @@ -114,7 +115,8 @@ fn floaters() { z: ok, } .baz(|| { - // force multiline + force(); + multiline(); }) .quux(); diff --git a/tests/target/chains.rs b/tests/target/chains.rs index 488d64afacd..d9e7d3a58fe 100644 --- a/tests/target/chains.rs +++ b/tests/target/chains.rs @@ -102,7 +102,8 @@ fn floaters() { Foo { x: val } .baz(|| { - // force multiline + force(); + multiline(); }) .quux(); @@ -111,7 +112,8 @@ fn floaters() { z: ok, } .baz(|| { - // force multiline + force(); + multiline(); }) .quux(); diff --git a/tests/target/expr.rs b/tests/target/expr.rs index 38007e31f1e..6b1ae7ec555 100644 --- a/tests/target/expr.rs +++ b/tests/target/expr.rs @@ -60,8 +60,7 @@ fn foo() -> bool { 1111 + 2222 {} if let (some_very_large, - tuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuple) = 1 + 2 + 3 { - } + tuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuple) = 1 + 2 + 3 {} let test = if true { 5 } else { 3 }; @@ -136,12 +135,8 @@ fn baz() { fn qux() { {} // FIXME this one could be done better. - { - // a block with a comment - } - { - - } + { /* a block with a comment */ } + {} { // A block with a comment. } @@ -293,12 +288,10 @@ fn complex_if_else() { fn issue1106() { { if let hir::ItemEnum(ref enum_def, ref generics) = - self.ast_map.expect_item(enum_node_id).node { - } + self.ast_map.expect_item(enum_node_id).node {} } for entry in WalkDir::new(path) .into_iter() - .filter_entry(|entry| exclusions.filter_entry(entry)) { - } + .filter_entry(|entry| exclusions.filter_entry(entry)) {} } diff --git a/tests/target/hard-tabs.rs b/tests/target/hard-tabs.rs index c48f43581f9..43b00774a22 100644 --- a/tests/target/hard-tabs.rs +++ b/tests/target/hard-tabs.rs @@ -31,8 +31,7 @@ fn main() { let str = "AAAAAAAAAAAAAAaAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAaAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAaAa"; if let (some_very_large, - tuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuple) = 1 + 2 + 3 { - } + tuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuple) = 1 + 2 + 3 {} if cond() { something(); diff --git a/tests/target/issue-447.rs b/tests/target/issue-447.rs index 7c1d370d0e8..d41cdb65cd0 100644 --- a/tests/target/issue-447.rs +++ b/tests/target/issue-447.rs @@ -36,6 +36,5 @@ fn main() { let Some(x) = y // shouldn't be dropped // shouldn't be dropped - { - } + {} } diff --git a/tests/target/match.rs b/tests/target/match.rs index e6c680f3cb7..7ebffdc2889 100644 --- a/tests/target/match.rs +++ b/tests/target/match.rs @@ -78,9 +78,7 @@ fn main() { }; match x { - y => { - // Block with comment. Preserve me. - } + y => { /*Block with comment. Preserve me.*/ } z => { stmt(); }