Skip to content

Commit

Permalink
Use correct indent when formatting complex fn type (#3731)
Browse files Browse the repository at this point in the history
  • Loading branch information
topecongiro authored Aug 16, 2019
1 parent dfd2766 commit 1643d72
Show file tree
Hide file tree
Showing 6 changed files with 139 additions and 25 deletions.
66 changes: 47 additions & 19 deletions src/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1822,9 +1822,17 @@ impl Rewrite for ast::FunctionRetTy {
match *self {
ast::FunctionRetTy::Default(_) => Some(String::new()),
ast::FunctionRetTy::Ty(ref ty) => {
let inner_width = shape.width.checked_sub(3)?;
ty.rewrite(context, Shape::legacy(inner_width, shape.indent + 3))
.map(|r| format!("-> {}", r))
if context.config.version() == Version::One
|| context.config.indent_style() == IndentStyle::Visual
{
let inner_width = shape.width.checked_sub(3)?;
return ty
.rewrite(context, Shape::legacy(inner_width, shape.indent + 3))
.map(|r| format!("-> {}", r));
}

ty.rewrite(context, shape.offset_left(3)?)
.map(|s| format!("-> {}", s))
}
}
}
Expand Down Expand Up @@ -2147,20 +2155,39 @@ fn rewrite_fn_base(
sig_length > context.config.max_width()
}
};
let ret_indent = if ret_should_indent {
let indent = if arg_str.is_empty() {
// Aligning with non-existent args looks silly.
force_new_line_for_brace = true;
indent + 4
let ret_shape = if ret_should_indent {
if context.config.version() == Version::One
|| context.config.indent_style() == IndentStyle::Visual
{
let indent = if arg_str.is_empty() {
// Aligning with non-existent args looks silly.
force_new_line_for_brace = true;
indent + 4
} else {
// FIXME: we might want to check that using the arg indent
// doesn't blow our budget, and if it does, then fallback to
// the where-clause indent.
arg_indent
};

result.push_str(&indent.to_string_with_newline(context.config));
Shape::indented(indent, context.config)
} else {
// FIXME: we might want to check that using the arg indent
// doesn't blow our budget, and if it does, then fallback to
// the where-clause indent.
arg_indent
};
let mut ret_shape = Shape::indented(indent, context.config);
if arg_str.is_empty() {
// Aligning with non-existent args looks silly.
force_new_line_for_brace = true;
ret_shape = if context.use_block_indent() {
ret_shape.offset_left(4).unwrap_or(ret_shape)
} else {
ret_shape.indent = ret_shape.indent + 4;
ret_shape
};
}

result.push_str(&indent.to_string_with_newline(context.config));
indent
result.push_str(&ret_shape.indent.to_string_with_newline(context.config));
ret_shape
}
} else {
if context.config.version() == Version::Two {
if !arg_str.is_empty() || !no_args_and_over_max_width {
Expand All @@ -2170,15 +2197,16 @@ fn rewrite_fn_base(
result.push(' ');
}

Indent::new(indent.block_indent, last_line_width(&result))
let ret_shape = Shape::indented(indent, context.config);
ret_shape
.offset_left(last_line_width(&result))
.unwrap_or(ret_shape)
};

if multi_line_ret_str || ret_should_indent {
// Now that we know the proper indent and width, we need to
// re-layout the return type.
let ret_str = fd
.output
.rewrite(context, Shape::indented(ret_indent, context.config))?;
let ret_str = fd.output.rewrite(context, ret_shape)?;
result.push_str(&ret_str);
} else {
result.push_str(&ret_str);
Expand Down
48 changes: 42 additions & 6 deletions src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use syntax::source_map::{self, BytePos, Span};
use syntax::symbol::kw;

use crate::config::lists::*;
use crate::config::{IndentStyle, TypeDensity};
use crate::config::{IndentStyle, TypeDensity, Version};
use crate::expr::{format_expr, rewrite_assign_rhs, rewrite_tuple, rewrite_unary_prefix, ExprType};
use crate::lists::{
definitive_tactic, itemize_list, write_list, ListFormatting, ListItem, Separator,
Expand Down Expand Up @@ -678,9 +678,35 @@ impl Rewrite for ast::Ty {
// FIXME: we drop any comments here, even though it's a silly place to put
// comments.
ast::TyKind::Paren(ref ty) => {
let budget = shape.width.checked_sub(2)?;
ty.rewrite(context, Shape::legacy(budget, shape.indent + 1))
.map(|ty_str| format!("({})", ty_str))
if context.config.version() == Version::One
|| context.config.indent_style() == IndentStyle::Visual
{
let budget = shape.width.checked_sub(2)?;
return ty
.rewrite(context, Shape::legacy(budget, shape.indent + 1))
.map(|ty_str| format!("({})", ty_str));
}

// 2 = ()
if let Some(sh) = shape.sub_width(2) {
if let Some(ref s) = ty.rewrite(context, sh) {
if !s.contains('\n') {
return Some(format!("({})", s));
}
}
}

let indent_str = shape.indent.to_string_with_newline(context.config);
let shape = shape
.block_indent(context.config.tab_spaces())
.with_max_width(context.config);
let rw = ty.rewrite(context, shape)?;
Some(format!(
"({}{}{})",
shape.to_string_with_newline(context.config),
rw,
indent_str
))
}
ast::TyKind::Slice(ref ty) => {
let budget = shape.width.checked_sub(4)?;
Expand Down Expand Up @@ -716,7 +742,15 @@ impl Rewrite for ast::Ty {
ast::TyKind::ImplicitSelf => Some(String::from("")),
ast::TyKind::ImplTrait(_, ref it) => {
// Empty trait is not a parser error.
it.rewrite(context, shape).map(|it_str| {
if it.is_empty() {
return Some("impl".to_owned());
}
let rw = if context.config.version() == Version::One {
it.rewrite(context, shape)
} else {
join_bounds(context, shape, it, false)
};
rw.map(|it_str| {
let space = if it_str.is_empty() { "" } else { " " };
format!("impl{}{}", space, it_str)
})
Expand Down Expand Up @@ -818,7 +852,9 @@ fn join_bounds(
// We need to use multiple lines.
let (type_strs, offset) = if need_indent {
// Rewrite with additional indentation.
let nested_shape = shape.block_indent(context.config.tab_spaces());
let nested_shape = shape
.block_indent(context.config.tab_spaces())
.with_max_width(context.config);
let type_strs = items
.iter()
.map(|item| item.rewrite(context, nested_shape))
Expand Down
12 changes: 12 additions & 0 deletions tests/source/issue-3701/one.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// rustfmt-version: One

fn build_sorted_static_get_entry_names(
mut entries: Vec<(u8, &'static str)>,
) -> (impl Fn(
AlphabeticalTraversal,
Box<dyn dirents_sink::Sink<AlphabeticalTraversal>>,
) -> BoxFuture<'static, Result<Box<dyn dirents_sink::Sealed>, Status>>
+ Send
+ Sync
+ 'static) {
}
12 changes: 12 additions & 0 deletions tests/source/issue-3701/two.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// rustfmt-version: Two

fn build_sorted_static_get_entry_names(
mut entries: Vec<(u8, &'static str)>,
) -> (impl Fn(
AlphabeticalTraversal,
Box<dyn dirents_sink::Sink<AlphabeticalTraversal>>,
) -> BoxFuture<'static, Result<Box<dyn dirents_sink::Sealed>, Status>>
+ Send
+ Sync
+ 'static) {
}
12 changes: 12 additions & 0 deletions tests/target/issue-3701/one.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// rustfmt-version: One

fn build_sorted_static_get_entry_names(
mut entries: Vec<(u8, &'static str)>,
) -> (impl Fn(
AlphabeticalTraversal,
Box<dyn dirents_sink::Sink<AlphabeticalTraversal>>,
) -> BoxFuture<'static, Result<Box<dyn dirents_sink::Sealed>, Status>>
+ Send
+ Sync
+ 'static) {
}
14 changes: 14 additions & 0 deletions tests/target/issue-3701/two.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// rustfmt-version: Two

fn build_sorted_static_get_entry_names(
mut entries: Vec<(u8, &'static str)>,
) -> (
impl Fn(
AlphabeticalTraversal,
Box<dyn dirents_sink::Sink<AlphabeticalTraversal>>,
) -> BoxFuture<'static, Result<Box<dyn dirents_sink::Sealed>, Status>>
+ Send
+ Sync
+ 'static
) {
}

0 comments on commit 1643d72

Please sign in to comment.