mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #1701 from topecongiro/issue-1697

Browse Source 
Forbid overflowing closure if its header goes multi lines
This commit is contained in:
Seiichi Uchida 2017-06-17 16:25:10 +09:00 committed by GitHub
commit 59c0e4c746
10 changed files with 170 additions and 143 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/chains.rs
View File

@ -165,9 +165,7 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
);
let child_shape_iter = Some(first_child_shape).into_iter().chain(
::std::iter::repeat(
other_child_shape,
).take(subexpr_list.len() - 1),
::std::iter::repeat(other_child_shape).take(subexpr_list.len() - 1),
);
let iter = subexpr_list.iter().rev().zip(child_shape_iter);
let mut rewrites = try_opt!(
@ -178,9 +176,10 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) -
// Total of all items excluding the last.
let last_non_try_index = rewrites.len() - (1 + trailing_try_num);
let almost_total = rewrites[..last_non_try_index]
.iter()
.fold(0, |a, b| a + first_line_width(b)) + parent_rewrite.len();
let almost_total = rewrites[..last_non_try_index].iter().fold(
0,
|a, b| a + first_line_width(b),
) + parent_rewrite.len();
let one_line_len = rewrites.iter().fold(0, |a, r| a + first_line_width(r)) +
parent_rewrite.len();

7
src/comment.rs
View File

@ -45,9 +45,10 @@ pub enum CommentStyle<'a> {
fn custom_opener(s: &str) -> &str {
s.lines().next().map_or("", |first_line| {
first_line
.find(' ')
.map_or(first_line, |space_index| &first_line[0..space_index + 1])
first_line.find(' ').map_or(
first_line,
|space_index| &first_line[0..space_index + 1],
)
})
}

147
src/expr.rs
View File

@ -298,9 +298,7 @@ fn format_expr(
Some(format!(
"{}{}",
"do catch ",
try_opt!(
block.rewrite(&context, Shape::legacy(budget, shape.indent))
)
try_opt!(block.rewrite(&context, Shape::legacy(budget, shape.indent)))
))
}
};
@ -563,7 +561,7 @@ fn rewrite_closure_fn_decl(
fn_decl.inputs.iter(),
"|",
|arg| span_lo_for_arg(arg),
|arg| span_hi_for_arg(arg),
|arg| span_hi_for_arg(context, arg),
|arg| arg.rewrite(context, arg_shape),
context.codemap.span_after(span, "|"),
body.span.lo,
@ -941,13 +939,9 @@ fn to_control_flow<'a>(expr: &'a ast::Expr, expr_type: ExprType) -> Option<Contr
ast::ExprKind::Loop(ref block, label) => Some(
ControlFlow::new_loop(block, label, expr.span),
),
ast::ExprKind::While(ref cond, ref block, label) => Some(ControlFlow::new_while(
None,
cond,
block,
label,
expr.span,
)),
ast::ExprKind::While(ref cond, ref block, label) => Some(
ControlFlow::new_while(None, cond, block, label, expr.span),
),
ast::ExprKind::WhileLet(ref pat, ref cond, ref block, label) => {
Some(ControlFlow::new_while(
Some(pat),
@ -1302,14 +1296,13 @@ fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
}
};
let between_kwd_else_block =
mk_sp(
self.block.span.hi,
context.codemap.span_before(
mk_sp(self.block.span.hi, else_block.span.lo),
"else",
),
);
let between_kwd_else_block = mk_sp(
self.block.span.hi,
context.codemap.span_before(
mk_sp(self.block.span.hi, else_block.span.lo),
"else",
),
);
let between_kwd_else_block_comment =
extract_comment(between_kwd_else_block, context, shape);
@ -1434,10 +1427,9 @@ fn rewrite_match_arm_comment(
result.push_str(&missed_str[..first_brk]);
let missed_str = &missed_str[first_brk..]; // If missed_str had one newline, it starts with it
let first = missed_str.find(|c: char| !c.is_whitespace()).unwrap_or(
missed_str
.len(),
);
let first = missed_str
.find(|c: char| !c.is_whitespace())
.unwrap_or(missed_str.len());
if missed_str[..first].chars().filter(|c| c == &'\n').count() >= 2 {
// Excessive vertical whitespace before comment should be preserved
// FIXME handle vertical whitespace better
@ -2053,20 +2045,16 @@ pub fn rewrite_call_inner<'a, T>(
Ok(format!(
"{}{}",
callee_str,
wrap_args_with_parens(
context,
&list_str,
extendable,
args_shape,
nested_shape,
)
wrap_args_with_parens(context, &list_str, extendable, args_shape, nested_shape)
))
}
fn need_block_indent(s: &str, shape: Shape) -> bool {
s.lines().skip(1).any(|s| {
s.find(|c| !char::is_whitespace(c))
.map_or(false, |w| w + 1 < shape.indent.width())
s.find(|c| !char::is_whitespace(c)).map_or(
false,
|w| w + 1 < shape.indent.width(),
)
})
}
@ -2207,6 +2195,48 @@ fn last_arg_shape(
})
}
// Rewriting closure which is placed at the end of the function call's arg.
// Returns `None` if the reformatted closure 'looks bad'.
fn rewrite_last_closure(
context: &RewriteContext,
expr: &ast::Expr,
shape: Shape,
) -> Option<String> {
if let ast::ExprKind::Closure(capture, ref fn_decl, ref body, _) = expr.node {
let body = match body.node {
ast::ExprKind::Block(ref block) if block.stmts.len() == 1 => {
stmt_expr(&block.stmts[0]).unwrap_or(body)
}
_ => body,
};
let (prefix, extra_offset) = try_opt!(rewrite_closure_fn_decl(
capture,
fn_decl,
body,
expr.span,
context,
shape,
));
// If the closure goes multi line before its body, do not overflow the closure.
if prefix.contains('\n') {
return None;
}
let body_shape = try_opt!(shape.offset_left(extra_offset));
// When overflowing the closure which consists of a single control flow expression,
// force to use block if its condition uses multi line.
if rewrite_cond(context, body, body_shape)
.map(|cond| cond.contains('\n'))
.unwrap_or(false)
{
return rewrite_closure_with_block(context, body_shape, &prefix, body);
}
// Seems fine, just format the closure in usual manner.
return expr.rewrite(context, shape);
}
None
}
fn rewrite_last_arg_with_overflow<'a, T>(
context: &RewriteContext,
last_arg: &T,
@ -2220,31 +2250,7 @@ fn rewrite_last_arg_with_overflow<'a, T>(
match expr.node {
// When overflowing the closure which consists of a single control flow expression,
// force to use block if its condition uses multi line.
ast::ExprKind::Closure(capture, ref fn_decl, ref body, _) => {
let try_closure_with_block = || {
let body = match body.node {
ast::ExprKind::Block(ref block) if block.stmts.len() == 1 => {
try_opt!(stmt_expr(&block.stmts[0]))
}
_ => body,
};
let (prefix, extra_offset) = try_opt!(rewrite_closure_fn_decl(
capture,
fn_decl,
body,
expr.span,
context,
shape,
));
let shape = try_opt!(shape.offset_left(extra_offset));
rewrite_cond(context, body, shape).map_or(None, |cond| if cond.contains('\n') {
rewrite_closure_with_block(context, shape, &prefix, body)
} else {
None
})
};
try_closure_with_block().or_else(|| expr.rewrite(context, shape))
}
ast::ExprKind::Closure(..) => rewrite_last_closure(context, expr, shape),
_ => expr.rewrite(context, shape),
}
} else {
@ -2390,10 +2396,12 @@ fn rewrite_index(
let indent = indent.to_string(&context.config);
// FIXME this is not right, since we don't take into account that shape.width
// might be reduced from max_width by something on the right.
let budget = try_opt!(context.config.max_width().checked_sub(
indent.len() + lbr.len() +
rbr.len(),
));
let budget = try_opt!(
context
.config
.max_width()
.checked_sub(indent.len() + lbr.len() + rbr.len())
);
let index_str = try_opt!(index.rewrite(context, Shape::legacy(budget, shape.indent)));
Some(format!(
"{}\n{}{}{}{}",
@ -2558,7 +2566,12 @@ fn shape_from_fn_call_style(
offset: usize,
) -> Option<Shape> {
if context.use_block_indent() {
Some(shape.block().block_indent(context.config.tab_spaces()))
// 1 = ","
shape
.block()
.block_indent(context.config.tab_spaces())
.with_max_width(context.config)
.sub_width(1)
} else {
shape.visual_indent(offset).sub_width(overhead)
}
@ -2580,12 +2593,10 @@ fn rewrite_tuple_in_visual_indent_style<'a, T>(
// 3 = "(" + ",)"
let nested_shape = try_opt!(shape.sub_width(3)).visual_indent(1);
return items.next().unwrap().rewrite(context, nested_shape).map(
|s| {
if context.config.spaces_within_parens() {
format!("( {}, )", s)
} else {
format!("({},)", s)
}
|s| if context.config.spaces_within_parens() {
format!("( {}, )", s)
} else {
format!("({},)", s)
},
);
}

67
src/items.rs
View File

@ -803,10 +803,9 @@ fn format_impl_ref_and_type(
Style::Legacy => new_line_offset + trait_ref_overhead,
Style::Rfc => new_line_offset,
};
result.push_str(&*try_opt!(self_ty.rewrite(
context,
Shape::legacy(budget, type_offset),
)));
result.push_str(&*try_opt!(
self_ty.rewrite(context, Shape::legacy(budget, type_offset))
));
Some(result)
} else {
unreachable!();
@ -967,8 +966,7 @@ pub fn format_trait(context: &RewriteContext, item: &ast::Item, offset: Indent)
where_density,
"{",
!has_body,
trait_bound_str.is_empty() &&
last_line_width(&generics_str) == 1,
trait_bound_str.is_empty() && last_line_width(&generics_str) == 1,
None,
));
// If the where clause cannot fit on the same line,
@ -1260,10 +1258,12 @@ fn format_tuple_struct(
context.codemap.span_after(span, "("),
span.hi,
);
let body_budget = try_opt!(context.config.max_width().checked_sub(
offset.block_only().width() +
result.len() + 3,
));
let body_budget = try_opt!(
context
.config
.max_width()
.checked_sub(offset.block_only().width() + result.len() + 3)
);
let body = try_opt!(list_helper(
items,
// TODO budget is wrong in block case
@ -1548,8 +1548,7 @@ pub fn rewrite_static(
let ty_str = try_opt!(ty.rewrite(
context,
Shape::legacy(
context.config.max_width() - offset.block_indent -
prefix.len() - 2,
context.config.max_width() - offset.block_indent - prefix.len() - 2,
offset.block_only(),
),
));
@ -1613,8 +1612,7 @@ pub fn rewrite_associated_type(
let ty_str = try_opt!(ty.rewrite(
context,
Shape::legacy(
context.config.max_width() - indent.block_indent -
prefix.len() - 2,
context.config.max_width() - indent.block_indent - prefix.len() - 2,
indent.block_only(),
),
));
@ -1659,6 +1657,16 @@ fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
}
}
fn is_empty_infer(context: &RewriteContext, ty: &ast::Ty) -> bool {
match ty.node {
ast::TyKind::Infer => {
let original = context.snippet(ty.span);
original != "_"
}
_ => false,
}
}
impl Rewrite for ast::Arg {
fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
if is_named_arg(self) {
@ -1667,7 +1675,7 @@ fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
Shape::legacy(shape.width, shape.indent),
));
if self.ty.node != ast::TyKind::Infer {
if !is_empty_infer(context, &*self.ty) {
if context.config.space_before_type_annotation() {
result.push_str(" ");
}
@ -1752,8 +1760,9 @@ pub fn span_lo_for_arg(arg: &ast::Arg) -> BytePos {
}
}
pub fn span_hi_for_arg(arg: &ast::Arg) -> BytePos {
pub fn span_hi_for_arg(context: &RewriteContext, arg: &ast::Arg) -> BytePos {
match arg.ty.node {
ast::TyKind::Infer if context.snippet(arg.ty.span) == "_" => arg.ty.span.hi,
ast::TyKind::Infer if is_named_arg(arg) => arg.pat.span.hi,
_ => arg.ty.span.hi,
}
@ -1857,8 +1866,7 @@ fn rewrite_fn_base(
2
};
let shape = try_opt!(
Shape::indented(indent + last_line_width(&result), context.config)
.sub_width(overhead)
Shape::indented(indent + last_line_width(&result), context.config).sub_width(overhead)
);
let g_span = mk_sp(span.lo, span_for_return(&fd.output).lo);
let generics_str = try_opt!(rewrite_generics(context, generics, shape, g_span));
@ -1979,10 +1987,10 @@ fn rewrite_fn_base(
}
// If the last line of args contains comment, we cannot put the closing paren
// on the same line.
if arg_str
.lines()
.last()
.map_or(false, |last_line| last_line.contains("//"))
if arg_str.lines().last().map_or(
false,
|last_line| last_line.contains("//"),
)
{
args_last_line_contains_comment = true;
result.push('\n');
@ -2055,13 +2063,14 @@ fn rewrite_fn_base(
let snippet_hi = span.hi;
let snippet = context.snippet(mk_sp(snippet_lo, snippet_hi));
// Try to preserve the layout of the original snippet.
let original_starts_with_newline =
snippet
.find(|c| c != ' ')
.map_or(false, |i| snippet[i..].starts_with('\n'));
let original_ends_with_newline = snippet
.rfind(|c| c != ' ')
.map_or(false, |i| snippet[i..].ends_with('\n'));
let original_starts_with_newline = snippet.find(|c| c != ' ').map_or(
false,
|i| snippet[i..].starts_with('\n'),
);
let original_ends_with_newline = snippet.rfind(|c| c != ' ').map_or(
false,
|i| snippet[i..].ends_with('\n'),
);
let snippet = snippet.trim();
if !snippet.is_empty() {
result.push(if original_starts_with_newline {

21
src/lists.rs
View File

@ -132,9 +132,10 @@ pub fn is_multiline(&self) -> bool {
}
pub fn has_line_pre_comment(&self) -> bool {
self.pre_comment
.as_ref()
.map_or(false, |comment| comment.starts_with("//"))
self.pre_comment.as_ref().map_or(
false,
|comment| comment.starts_with("//"),
)
}
pub fn from_str<S: Into<String>>(s: S) -> ListItem {
@ -419,10 +420,9 @@ fn next(&mut self) -> Option<Self::Item> {
}
}
None => {
post_snippet.find_uncommented(self.terminator).unwrap_or(
post_snippet
.len(),
)
post_snippet
.find_uncommented(self.terminator)
.unwrap_or(post_snippet.len())
}
};
@ -435,10 +435,9 @@ fn next(&mut self) -> Option<Self::Item> {
let first_newline = test_snippet.find('\n').unwrap_or(test_snippet.len());
// From the end of the first line of comments.
let test_snippet = &test_snippet[first_newline..];
let first = test_snippet.find(|c: char| !c.is_whitespace()).unwrap_or(
test_snippet
.len(),
);
let first = test_snippet
.find(|c: char| !c.is_whitespace())
.unwrap_or(test_snippet.len());
// From the end of the first line of comments to the next non-whitespace char.
let test_snippet = &test_snippet[..first];

10
src/types.rs
View File

@ -590,10 +590,7 @@ fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
let max_path_width = try_opt!(shape.width.checked_sub(extra_offset));
let path_str = try_opt!(self.trait_ref.rewrite(
context,
Shape::legacy(
max_path_width,
shape.indent + extra_offset,
),
Shape::legacy(max_path_width, shape.indent + extra_offset),
));
Some(if context.config.spaces_within_angle_brackets() &&
@ -645,10 +642,7 @@ fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
mut_str,
try_opt!(mt.ty.rewrite(
context,
Shape::legacy(
budget,
shape.indent + 2 + mut_len + lt_len,
),
Shape::legacy(budget, shape.indent + 2 + mut_len + lt_len),
))
)
}

8
src/visitor.rs
View File

@ -782,8 +782,8 @@ fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
impl Rewrite for ast::Attribute {
fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
try_opt!(self.meta()).rewrite(context, shape).map(|rw| {
if rw.starts_with("///") {
try_opt!(self.meta()).rewrite(context, shape).map(
|rw| if rw.starts_with("///") {
rw
} else {
let original = context.snippet(self.span);
@ -792,8 +792,8 @@ fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option<String> {
} else {
format!("#[{}]", rw)
}
}
})
},
)
}
}

8
tests/source/closure.rs
View File

@ -143,3 +143,11 @@ fn issue1329() {
fn issue325() {
let f = || unsafe { xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx };
}
fn issue1697() {
Test.func_a(A_VERY_LONG_CONST_VARIABLE_NAME, move |arg1, arg2, arg3, arg4| arg1 + arg2 + arg3 + arg4)
}
fn issue1694() {
foooooo(|_referencefffffffff: _, _target_reference: _, _oid: _, _target_oid: _| format!("refs/pull/{}/merge", pr_id))
}

25
tests/target/closure.rs
View File

@ -128,11 +128,8 @@ fn issue470() {
{
{
{
let explicit_arg_decls =
explicit_arguments.into_iter().enumerate().map(|(
index,
(ty, pattern),
)| {
let explicit_arg_decls = explicit_arguments.into_iter().enumerate().map(
|(index, (ty, pattern))| {
let lvalue = Lvalue::Arg(index as u32);
block = this.pattern(
block,
@ -141,7 +138,8 @@ fn issue470() {
&lvalue,
);
ArgDecl { ty: ty }
});
},
);
}
}
}
@ -169,3 +167,18 @@ fn issue325() {
let f =
|| unsafe { xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx };
}
fn issue1697() {
Test.func_a(
A_VERY_LONG_CONST_VARIABLE_NAME,
move |arg1, arg2, arg3, arg4| arg1 + arg2 + arg3 + arg4,
)
}
fn issue1694() {
foooooo(
|_referencefffffffff: _, _target_reference: _, _oid: _, _target_oid: _| {
format!("refs/pull/{}/merge", pr_id)
},
)
}

9
tests/target/match.rs
View File

@ -79,14 +79,7 @@ fn main() {
),
Variantttttttttttttttttttttttt => (
"variant",
vec![
"id",
"name",
"qualname",
"type",
"value",
"scopeid",
],
vec!["id", "name", "qualname", "type", "value", "scopeid"],
true,
true,
),