Skip to content

Commit

Permalink
Fix an issue introduced in the new lambda formatting
Browse files Browse the repository at this point in the history
  • Loading branch information
asterite committed Oct 21, 2024
1 parent f6b0ffa commit e6a1088
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 0 deletions.
10 changes: 10 additions & 0 deletions tooling/nargo_fmt/src/formatter/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,16 @@ impl Buffer {
}
}

/// Trim commas from the end of the buffer. Returns true if a comma was trimmed.
pub(super) fn trim_comma(&mut self) -> bool {
if self.buffer.ends_with(',') {
self.buffer.truncate(self.buffer.len() - 1);
true
} else {
false
}
}

pub(crate) fn contents(self) -> String {
self.buffer
}
Expand Down
33 changes: 33 additions & 0 deletions tooling/nargo_fmt/src/formatter/chunks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -729,10 +729,43 @@ impl<'a> Formatter<'a> {
self.write_line();
self.write_indentation();
self.format_chunk_group_impl(group);

// If this lambda was in an expression list and it was formatted in multiple
// lines, it might be that the trailing comma happened after the lambda body:
//
// foo(
// 1,
// |lambda| body,
// )
//
// Because we attach commas to the last text to avoid splitting it, the body
// in this case is "body,", so if we end up writing it as a block it will
// look like this:
//
// foo(
// 1,
// |lambda| {
// body,
// }
// )
//
// So, if after writing the body we find a comma (there will be at most one)
// we remove it, but place it after the right brace, so it looks like this:
//
// foo(
// 1,
// |lambda| {
// body
// },
// )
let comma_trimmed = self.trim_comma();
self.decrease_indentation();
self.write_line();
self.write_indentation();
self.write("}");
if comma_trimmed {
self.write(",");
}
return;
}

Expand Down
5 changes: 5 additions & 0 deletions tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,11 @@ impl<'a> Formatter<'a> {
pub(super) fn trim_spaces(&mut self) {
self.buffer.trim_spaces();
}

/// Trim commas from the end of the buffer. Returns true if a comma was trimmed.
pub(super) fn trim_comma(&mut self) -> bool {
self.buffer.trim_comma()
}
}

#[cfg(test)]
Expand Down
31 changes: 31 additions & 0 deletions tooling/nargo_fmt/src/formatter/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2015,6 +2015,37 @@ global y = 1;
assert_format(src, expected);
}

#[test]
fn format_lambda_as_last_method_call_argument_3() {
let src = "mod moo {
fn foo() {
let mut sorted_write_tuples = unsafe {
get_sorted_tuple(
final_public_data_writes.storage,
|(_, leaf_a): (u32, PublicDataTreeLeaf), (_, leaf_b): (u32, PublicDataTreeLeaf)| full_field_less_than(
1, 2,
),
)
};
}
}
";
let expected = "mod moo {
fn foo() {
let mut sorted_write_tuples = unsafe {
get_sorted_tuple(
final_public_data_writes.storage,
|(_, leaf_a): (u32, PublicDataTreeLeaf), (_, leaf_b): (u32, PublicDataTreeLeaf)| {
full_field_less_than(1, 2)
},
)
};
}
}
";
assert_format(src, expected);
}

#[test]
fn format_lambda_as_last_method_call_chain_argument() {
let src = "global x = foo.bar(1).baz(2, |x| { 1; 2 });";
Expand Down

0 comments on commit e6a1088

Please sign in to comment.