diff --git a/tooling/nargo_fmt/src/chunks.rs b/tooling/nargo_fmt/src/chunks.rs index d4fb540a221..673cf020aed 100644 --- a/tooling/nargo_fmt/src/chunks.rs +++ b/tooling/nargo_fmt/src/chunks.rs @@ -849,10 +849,10 @@ impl<'a> Formatter<'a> { } Chunk::TrailingComment(text_chunk) | Chunk::LeadingComment(text_chunk) => { self.write(&text_chunk.string); - self.write(" "); + self.write_space_without_skipping_whitespace_and_comments(); } Chunk::Group(chunks) => self.format_chunk_group_impl(chunks), - Chunk::SpaceOrLine => self.write(" "), + Chunk::SpaceOrLine => self.write_space_without_skipping_whitespace_and_comments(), Chunk::IncreaseIndentation => self.increase_indentation(), Chunk::DecreaseIndentation => self.decrease_indentation(), Chunk::PushIndentation => self.push_indentation(), @@ -915,9 +915,16 @@ impl<'a> Formatter<'a> { self.write_indentation(); } Chunk::LeadingComment(text_chunk) => { + let ends_with_newline = text_chunk.string.ends_with('\n'); self.write_chunk_lines(text_chunk.string.trim()); - self.write_line_without_skipping_whitespace_and_comments(); - self.write_indentation(); + + // Respect whether the leading comment had a newline before what comes next or not + if ends_with_newline { + self.write_line_without_skipping_whitespace_and_comments(); + self.write_indentation(); + } else { + self.write_space_without_skipping_whitespace_and_comments(); + } } Chunk::Group(mut group) => { if chunks.force_multiline_on_children_with_same_tag_if_multiline diff --git a/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs b/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs index e5f15bc397e..4aba0481e24 100644 --- a/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs +++ b/tooling/nargo_fmt/src/formatter/comments_and_whitespace.rs @@ -301,8 +301,8 @@ mod tests { let src = "global x = [ /* hello */ 1 , 2 ] ;"; let expected = "global x = [ - /* hello */ - 1, 2, + /* hello */ 1, + 2, ]; "; assert_format_with_max_width(src, expected, 20); diff --git a/tooling/nargo_fmt/src/formatter/expression.rs b/tooling/nargo_fmt/src/formatter/expression.rs index 80b9886c047..ebfa3ae78fb 100644 --- a/tooling/nargo_fmt/src/formatter/expression.rs +++ b/tooling/nargo_fmt/src/formatter/expression.rs @@ -446,23 +446,22 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { { let mut comments_chunk = self.skip_comments_and_whitespace_chunk(); - // If the comment is not empty but doesn't have newlines, it's surely `/* comment */`. - // We format that with spaces surrounding it so it looks, for example, like `Foo { /* comment */ field ..`. - if !comments_chunk.string.trim().is_empty() && !comments_chunk.has_newlines { - // Note: there's no space after `{}` because space will be produced by format_items_separated_by_comma - comments_chunk.string = if surround_with_spaces { - format!(" {}", comments_chunk.string.trim()) - } else { - format!(" {} ", comments_chunk.string.trim()) - }; - group.text(comments_chunk); - + // Handle leading block vs. line comments a bit differently. + if comments_chunk.string.trim().starts_with("/*") { group.increase_indentation(); if surround_with_spaces { group.space_or_line(); } else { group.line(); } + + // Note: there's no space before `{}` because it was just produced + comments_chunk.string = if surround_with_spaces { + comments_chunk.string.trim().to_string() + } else { + format!("{} ", comments_chunk.string.trim()) + }; + group.leading_comment(comments_chunk); } else { group.increase_indentation(); if surround_with_spaces { @@ -479,7 +478,24 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { group.text_attached_to_last_group(self.chunk(|formatter| { formatter.write_comma(); })); - group.trailing_comment(self.skip_comments_and_whitespace_chunk()); + let newlines_count_before_comment = self.following_newlines_count(); + group.text(self.chunk(|formatter| { + formatter.skip_whitespace(); + })); + if let Token::BlockComment(..) = &self.token { + // We let block comments be part of the item that's going to be formatted + } else { + // Line comments can be trailing or leading, depending on whether there are newlines before them + let comments_and_whitespace_chunk = self.skip_comments_and_whitespace_chunk(); + if !comments_and_whitespace_chunk.string.trim().is_empty() { + if newlines_count_before_comment > 0 { + group.line(); + group.leading_comment(comments_and_whitespace_chunk); + } else { + group.trailing_comment(comments_and_whitespace_chunk); + } + } + } group.space_or_line(); } format_item(self, expr, group); @@ -1326,6 +1342,56 @@ global y = 1; assert_format_with_config(src, expected, config); } + #[test] + fn format_short_array_with_block_comment_before_elements() { + let src = "global x = [ /* one */ 1, /* two */ 2 ] ;"; + let expected = "global x = [/* one */ 1, /* two */ 2];\n"; + + assert_format(src, expected); + } + + #[test] + fn format_long_array_with_block_comment_before_elements() { + let src = "global x = [ /* one */ 1, /* two */ 123456789012345, 3, 4 ] ;"; + let expected = "global x = [ + /* one */ 1, + /* two */ 123456789012345, + 3, + 4, +]; +"; + + let config = + Config { short_array_element_width_threshold: 5, max_width: 30, ..Config::default() }; + assert_format_with_config(src, expected, config); + } + + #[test] + fn format_long_array_with_line_comment_before_elements() { + let src = "global x = [ + // one + 1, + // two + 123456789012345, + 3, + 4, +]; +"; + let expected = "global x = [ + // one + 1, + // two + 123456789012345, + 3, + 4, +]; +"; + + let config = + Config { short_array_element_width_threshold: 5, max_width: 30, ..Config::default() }; + assert_format_with_config(src, expected, config); + } + #[test] fn format_cast() { let src = "global x = 1 as u8 ;"; diff --git a/tooling/nargo_fmt/src/formatter/use_tree.rs b/tooling/nargo_fmt/src/formatter/use_tree.rs index bc8dc3fcabb..98d63ef6611 100644 --- a/tooling/nargo_fmt/src/formatter/use_tree.rs +++ b/tooling/nargo_fmt/src/formatter/use_tree.rs @@ -211,7 +211,7 @@ mod tests { #[test] fn format_use_list_one_item_with_comments() { let src = " use foo::{ /* do not remove me */ bar, };"; - let expected = "use foo::{ /* do not remove me */ bar};\n"; + let expected = "use foo::{/* do not remove me */ bar};\n"; assert_format(src, expected); } diff --git a/tooling/nargo_fmt/src/formatter/use_tree_merge.rs b/tooling/nargo_fmt/src/formatter/use_tree_merge.rs index e24b7b8cbf6..834280ddba3 100644 --- a/tooling/nargo_fmt/src/formatter/use_tree_merge.rs +++ b/tooling/nargo_fmt/src/formatter/use_tree_merge.rs @@ -397,7 +397,7 @@ mod tests { #[test] fn format_use_list_one_item_with_comments() { let src = " use foo::{ /* do not remove me */ bar, };"; - let expected = "use foo::{ /* do not remove me */ bar};\n"; + let expected = "use foo::{/* do not remove me */ bar};\n"; assert_format(src, expected); } diff --git a/tooling/nargo_fmt/tests/expected/tuple.nr b/tooling/nargo_fmt/tests/expected/tuple.nr index d4b8b239815..29f32f83e55 100644 --- a/tooling/nargo_fmt/tests/expected/tuple.nr +++ b/tooling/nargo_fmt/tests/expected/tuple.nr @@ -4,13 +4,13 @@ fn main() { // hello 1, ); - ( /*hello*/ 1,); + (/*hello*/ 1,); (1/*hello*/,); (1,); - ( /*test*/ 1,); - ( /*a*/ 1/*b*/,); - ( /*a*/ 1/*b*/, /*c*/ 2/*d*/, /*c*/ 2/*d*/); - ( /*a*/ 1/*b*/, /*c*/ 2/*d*/, /*c*/ 2/*d*/, /*e*/ 3/*f*/); + (/*test*/ 1,); + (/*a*/ 1/*b*/,); + (/*a*/ 1/*b*/, /*c*/ 2/*d*/, /*c*/ 2/*d*/); + (/*a*/ 1/*b*/, /*c*/ 2/*d*/, /*c*/ 2/*d*/, /*e*/ 3/*f*/); (1 /*1*/, 2 /* 2*/); @@ -28,7 +28,7 @@ fn main() { 2, ); - ( /*1*/ 1, /*2*/ 2); + (/*1*/ 1, /*2*/ 2); ( ( @@ -39,14 +39,8 @@ fn main() { ), ); ( - /*a*/ - 1 - /*b*/, - /*c*/ - 2/*d*/, - /*c*/ - 2/*d*/, - /*e*/ - 3,/*f*/ + /*a*/ 1 + /*b*/, /*c*/ + 2/*d*/, /*c*/ 2/*d*/, /*e*/ 3,/*f*/ ); }