Skip to content

Fix overlong function signature #1089

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Aug 1, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions src/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1357,10 +1357,21 @@ fn rewrite_fn_base(context: &RewriteContext,
FnArgLayoutStyle::Block if put_args_in_block => false,
FnArgLayoutStyle::BlockAlways => false,
_ => {
// If we've already gone multi-line, or the return type would push
// over the max width, then put the return type on a new line.
result.contains("\n") || multi_line_ret_str ||
result.len() + indent.width() + ret_str_len > context.config.max_width
// If we've already gone multi-line, or the return type would push over the max
// width, then put the return type on a new line. With the +1 for the signature
// length an additional space between the closing parenthesis of the argument and
// the arrow '->' is considered.
let mut sig_length = result.len() + indent.width() + ret_str_len + 1;

// If there is no where clause, take into account the space after the return type
// and the brace.
if where_clause.predicates.is_empty() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to check if the return type exists? I.e., not do this if there is no return type?

Also, nit, brace = {}, I think you mean parenthesis in this comment and above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right. No return type means no arrow means no additional space.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this checked on ln 1354 already? if !ret_str.is_empty()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I overlooked that... 😅 but to be sure I will add a test for that case later.

sig_length += 2;
}

let overlong_sig = sig_length > context.config.max_width;

result.contains("\n") || multi_line_ret_str || overlong_sig
}
};
let ret_indent = if ret_should_indent {
Expand Down Expand Up @@ -1388,10 +1399,8 @@ fn rewrite_fn_base(context: &RewriteContext,
if multi_line_ret_str {
// Now that we know the proper indent and width, we need to
// re-layout the return type.

let budget = try_opt!(context.config.max_width.checked_sub(ret_indent.width()));
let ret_str = try_opt!(fd.output
.rewrite(context, budget, ret_indent));
let ret_str = try_opt!(fd.output.rewrite(context, budget, ret_indent));
result.push_str(&ret_str);
} else {
result.push_str(&ret_str);
Expand Down
16 changes: 16 additions & 0 deletions tests/source/issue-1049.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Test overlong function signature
pub unsafe fn reborrow_mut(&mut X: Abcde) -> Handle<NodeRef<marker::Mut, K, V, NodeType>, HandleType> {
}

pub fn merge(mut X: Abcdef) -> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>, marker::Edge> {
}

impl Handle {
pub fn merge(a: Abcd) -> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>, marker::Edge> {
}
}

// Long function without return type that should not be reformated.
fn veeeeeeeeeeeeeeeeeeeeery_long_name(a: FirstTypeeeeeeeeee, b: SecondTypeeeeeeeeeeeeeeeeeeeeeee) {}

fn veeeeeeeeeeeeeeeeeeeeeery_long_name(a: FirstTypeeeeeeeeee, b: SecondTypeeeeeeeeeeeeeeeeeeeeeee) {}
21 changes: 21 additions & 0 deletions tests/target/issue-1049.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Test overlong function signature
pub unsafe fn reborrow_mut(&mut X: Abcde)
-> Handle<NodeRef<marker::Mut, K, V, NodeType>, HandleType> {
}

pub fn merge(mut X: Abcdef)
-> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>, marker::Edge> {
}

impl Handle {
pub fn merge(a: Abcd)
-> Handle<NodeRef<marker::Mut<'a>, K, V, marker::Internal>, marker::Edge> {
}
}

// Long function without return type that should not be reformated.
fn veeeeeeeeeeeeeeeeeeeeery_long_name(a: FirstTypeeeeeeeeee, b: SecondTypeeeeeeeeeeeeeeeeeeeeeee) {}

fn veeeeeeeeeeeeeeeeeeeeeery_long_name(a: FirstTypeeeeeeeeee,
b: SecondTypeeeeeeeeeeeeeeeeeeeeeee) {
}