Skip to content

Commit

Permalink
fix: let formatter respect newlines between comments (noir-lang#6458)
Browse files Browse the repository at this point in the history
  • Loading branch information
asterite authored Nov 5, 2024
1 parent 35dedb5 commit fb1a8ca
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 16 deletions.
11 changes: 9 additions & 2 deletions tooling/nargo_fmt/src/chunks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -915,11 +915,18 @@ impl<'a> Formatter<'a> {
self.write_indentation();
}
Chunk::LeadingComment(text_chunk) => {
let ends_with_newline = text_chunk.string.ends_with('\n');
let ends_with_multiple_newlines = text_chunk.string.ends_with("\n\n");
let ends_with_newline =
ends_with_multiple_newlines || text_chunk.string.ends_with('\n');
self.write_chunk_lines(text_chunk.string.trim());

// Respect whether the leading comment had a newline before what comes next or not
if ends_with_newline {
if ends_with_multiple_newlines {
// Remove any indentation that could exist (we'll add it later)
self.buffer.trim_spaces();
self.write_multiple_lines_without_skipping_whitespace_and_comments();
self.write_indentation();
} else if ends_with_newline {
self.write_line_without_skipping_whitespace_and_comments();
self.write_indentation();
} else {
Expand Down
4 changes: 3 additions & 1 deletion tooling/nargo_fmt/src/formatter/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,18 @@ impl Buffer {
}

/// Trim spaces from the end of the buffer.
pub(super) fn trim_spaces(&mut self) {
pub(crate) fn trim_spaces(&mut self) {
while self.buffer.ends_with(' ') {
self.buffer.truncate(self.buffer.len() - 1);
self.current_line_width -= 1;
}
}

/// 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);
self.current_line_width -= 1;
true
} else {
false
Expand Down
22 changes: 20 additions & 2 deletions tooling/nargo_fmt/src/formatter/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1133,11 +1133,15 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> {
if count > 0 {
// If newlines follow, we first add a line, then add the comment chunk
group.lines(count > 1);
group.leading_comment(self.skip_comments_and_whitespace_chunk());
group.leading_comment(self.chunk(|formatter| {
formatter.skip_comments_and_whitespace_writing_multiple_lines_if_found();
}));
ignore_next = self.ignore_next;
} else {
// Otherwise, add the comment first as it's a trailing comment
group.trailing_comment(self.skip_comments_and_whitespace_chunk());
group.trailing_comment(self.chunk(|formatter| {
formatter.skip_comments_and_whitespace_writing_multiple_lines_if_found();
}));
ignore_next = self.ignore_next;
group.line();
}
Expand All @@ -1146,6 +1150,20 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> {
self.format_statement(statement, group, ignore_next);
}

// See how many newlines follow the last statement
let count = self.following_newlines_count();

group.text(self.chunk(|formatter| {
formatter.skip_whitespace();
}));

// After skipping whitespace we check if there's a comment. If so, we respect
// how many lines were before that comment.
if count > 0 && matches!(self.token, Token::LineComment(..) | Token::BlockComment(..)) {
group.lines(count > 1);
}

// Finally format the comment, if any
group.text(self.chunk(|formatter| {
formatter.skip_comments_and_whitespace();
}));
Expand Down
58 changes: 48 additions & 10 deletions tooling/nargo_fmt/src/formatter/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,16 +280,12 @@ impl<'a> Formatter<'a> {
}

pub(super) fn format_function_body(&mut self, body: BlockExpression) {
if body.is_empty() {
self.format_empty_block_contents();
} else {
let mut group = ChunkGroup::new();
self.chunk_formatter().format_non_empty_block_expression_contents(
body, true, // force multiple lines
&mut group,
);
self.format_chunk_group(group);
}
let mut group = ChunkGroup::new();
self.chunk_formatter().format_block_expression_contents(
body, true, // force multiple newlines
&mut group,
);
self.format_chunk_group(group);
}
}

Expand Down Expand Up @@ -537,4 +533,46 @@ fn baz() { let z = 3 ;
";
assert_format(src, expected);
}

#[test]
fn comment_in_body_respects_newlines() {
let src = "fn foo() {
let x = 1;
// comment
let y = 2;
}
";
let expected = src;
assert_format(src, expected);
}

#[test]
fn final_comment_in_body_respects_newlines() {
let src = "fn foo() {
let x = 1;
let y = 2;
// comment
}
";
let expected = src;
assert_format(src, expected);
}

#[test]
fn initial_comment_in_body_respects_newlines() {
let src = "fn foo() {
// comment
let x = 1;
let y = 2;
}
";
let expected = src;
assert_format(src, expected);
}
}
22 changes: 21 additions & 1 deletion tooling/nargo_fmt/src/formatter/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,15 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> {
group: &mut ChunkGroup,
mut ignore_next: bool,
) {
group.leading_comment(self.skip_comments_and_whitespace_chunk());
// First skip any whitespace to avoid writing multiple lines
group.text(self.chunk(|formatter| {
formatter.skip_whitespace();
}));

// Now write any leading comment respecting multiple newlines after them
group.leading_comment(self.chunk(|formatter| {
formatter.skip_comments_and_whitespace_writing_multiple_lines_if_found();
}));

ignore_next |= self.ignore_next;

Expand Down Expand Up @@ -690,4 +698,16 @@ mod tests {
";
assert_format_with_max_width(src, expected, " a_long_variable = foo(1, 2);".len() - 1);
}

#[test]
fn long_let_preceded_by_two_newlines() {
let src = "fn foo() {
let y = 0;
let x = 123456;
}
";
let expected = src;
assert_format_with_max_width(src, expected, " let x = 123456;".len());
}
}
1 change: 1 addition & 0 deletions tooling/nargo_fmt/tests/expected/parens.nr
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ fn main(x: u64, y: pub u64) {

(
/*a*/

(
// test
1
Expand Down

0 comments on commit fb1a8ca

Please sign in to comment.