From a4948738613f9fd78e9023b95d0999b39e927400 Mon Sep 17 00:00:00 2001 From: f01dab1e Date: Tue, 17 Oct 2023 11:23:56 +0000 Subject: [PATCH 01/10] chore: custom configuration for formatter fests --- tooling/nargo_fmt/build.rs | 10 +++++++++- tooling/nargo_fmt/src/config.rs | 15 +++++++++----- tooling/nargo_fmt/src/visitor/expr.rs | 20 +++++++++---------- .../nargo_fmt/tests/expected/nested_parens.nr | 6 ++++++ tooling/nargo_fmt/tests/expected/parens.nr | 1 + .../nargo_fmt/tests/input/nested_parens.nr | 6 ++++++ tooling/nargo_fmt/tests/input/parens.nr | 1 + 7 files changed, 43 insertions(+), 16 deletions(-) create mode 100644 tooling/nargo_fmt/tests/expected/nested_parens.nr create mode 100644 tooling/nargo_fmt/tests/input/nested_parens.nr diff --git a/tooling/nargo_fmt/build.rs b/tooling/nargo_fmt/build.rs index f2e23f9b8c1..3ad7db19e83 100644 --- a/tooling/nargo_fmt/build.rs +++ b/tooling/nargo_fmt/build.rs @@ -40,6 +40,14 @@ fn generate_formatter_tests(test_file: &mut File, test_data_dir: &Path) { let input_source_path = file.path(); let input_source = std::fs::read_to_string(input_source_path).unwrap(); + let config = input_source + .lines() + .by_ref() + .take_while(|line| line.starts_with("//@")) + .map(|line| &line[3..]) + .collect::>() + .join("\n"); + let output_source_path = outputs_dir.join(file_name); let output_source = std::fs::read_to_string(output_source_path).unwrap(); @@ -55,7 +63,7 @@ fn format_{test_name}() {{ let (parsed_module, errors) = noirc_frontend::parse_program(&input); assert!(errors.is_empty()); - let config = nargo_fmt::Config::default(); + let config = nargo_fmt::Config::from_str("{config}").unwrap(); let fmt_text = nargo_fmt::format(&input, parsed_module, &config); diff --git a/tooling/nargo_fmt/src/config.rs b/tooling/nargo_fmt/src/config.rs index 7cb80a9f04d..d51fe1cd169 100644 --- a/tooling/nargo_fmt/src/config.rs +++ b/tooling/nargo_fmt/src/config.rs @@ -44,20 +44,25 @@ macro_rules! config { config! { tab_spaces: usize, 4, "Number of spaces per tab"; remove_nested_parens: bool, true, "Remove nested parens"; + error_on_unformatted: bool, false, "Error if unable to get comments"; } impl Config { pub fn read(path: &Path) -> Result { - let mut config = Self::default(); let config_path = path.join("noirfmt.toml"); - let raw_toml = match std::fs::read_to_string(&config_path) { - Ok(t) => t, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => String::new(), + let input = match std::fs::read_to_string(&config_path) { + Ok(input) => input, + Err(cause) if cause.kind() == std::io::ErrorKind::NotFound => String::new(), Err(cause) => return Err(ConfigError::ReadFailed(config_path, cause)), }; - let toml = toml::from_str(&raw_toml).map_err(ConfigError::MalformedFile)?; + Self::from_str(&input) + } + + pub fn from_str(s: &str) -> Result { + let mut config = Self::default(); + let toml = toml::from_str(s).map_err(ConfigError::MalformedFile)?; config.fill_from_toml(toml); Ok(config) } diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index c2d3b4f6632..f1d47ab2fc3 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -10,9 +10,17 @@ impl FmtVisitor<'_> { let span = expr.span; let rewrite = self.format_expr(expr); - let rewrite = recover_comment_removed(slice!(self, span.start(), span.end()), rewrite); - self.push_rewrite(rewrite, span); + let original = slice!(self, span.start(), span.end()); + let changed_comment_content = changed_comment_content(original, &rewrite); + if changed_comment_content && self.config.error_on_unformatted { + panic!("{original:?} vs {rewrite:?}"); + } + + self.push_rewrite( + if changed_comment_content { original.to_string() } else { rewrite }, + span, + ); self.last_position = span.end(); } @@ -229,14 +237,6 @@ impl FmtVisitor<'_> { } } -fn recover_comment_removed(original: &str, new: String) -> String { - if changed_comment_content(original, &new) { - original.to_string() - } else { - new - } -} - fn changed_comment_content(original: &str, new: &str) -> bool { comments(original) != comments(new) } diff --git a/tooling/nargo_fmt/tests/expected/nested_parens.nr b/tooling/nargo_fmt/tests/expected/nested_parens.nr new file mode 100644 index 00000000000..1bfa7fec62c --- /dev/null +++ b/tooling/nargo_fmt/tests/expected/nested_parens.nr @@ -0,0 +1,6 @@ +//@error_on_unformatted=true +//@remove_nested_parens=false +fn main() { + ((())); + ((((((((())))))))); +} diff --git a/tooling/nargo_fmt/tests/expected/parens.nr b/tooling/nargo_fmt/tests/expected/parens.nr index aeba660f898..41a943d1be0 100644 --- a/tooling/nargo_fmt/tests/expected/parens.nr +++ b/tooling/nargo_fmt/tests/expected/parens.nr @@ -1,3 +1,4 @@ +//@error_on_unformatted=true fn main(x : u64, y : pub u64) { ( // diff --git a/tooling/nargo_fmt/tests/input/nested_parens.nr b/tooling/nargo_fmt/tests/input/nested_parens.nr new file mode 100644 index 00000000000..1bfa7fec62c --- /dev/null +++ b/tooling/nargo_fmt/tests/input/nested_parens.nr @@ -0,0 +1,6 @@ +//@error_on_unformatted=true +//@remove_nested_parens=false +fn main() { + ((())); + ((((((((())))))))); +} diff --git a/tooling/nargo_fmt/tests/input/parens.nr b/tooling/nargo_fmt/tests/input/parens.nr index 769b477a34d..2f0cdbd3ed4 100644 --- a/tooling/nargo_fmt/tests/input/parens.nr +++ b/tooling/nargo_fmt/tests/input/parens.nr @@ -1,3 +1,4 @@ +//@error_on_unformatted=true fn main(x : u64, y : pub u64) { ( // From df5c9caea51227a1362d220ac55b4399aa79038d Mon Sep 17 00:00:00 2001 From: f01dab1e Date: Tue, 17 Oct 2023 11:30:21 +0000 Subject: [PATCH 02/10] `Config::from_str` -> `Config::of` --- tooling/nargo_fmt/build.rs | 2 +- tooling/nargo_fmt/src/config.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tooling/nargo_fmt/build.rs b/tooling/nargo_fmt/build.rs index 3ad7db19e83..04a5c56fd7a 100644 --- a/tooling/nargo_fmt/build.rs +++ b/tooling/nargo_fmt/build.rs @@ -63,7 +63,7 @@ fn format_{test_name}() {{ let (parsed_module, errors) = noirc_frontend::parse_program(&input); assert!(errors.is_empty()); - let config = nargo_fmt::Config::from_str("{config}").unwrap(); + let config = nargo_fmt::Config::of("{config}").unwrap(); let fmt_text = nargo_fmt::format(&input, parsed_module, &config); diff --git a/tooling/nargo_fmt/src/config.rs b/tooling/nargo_fmt/src/config.rs index d51fe1cd169..d886c680e28 100644 --- a/tooling/nargo_fmt/src/config.rs +++ b/tooling/nargo_fmt/src/config.rs @@ -57,10 +57,10 @@ impl Config { Err(cause) => return Err(ConfigError::ReadFailed(config_path, cause)), }; - Self::from_str(&input) + Self::of(&input) } - pub fn from_str(s: &str) -> Result { + pub fn of(s: &str) -> Result { let mut config = Self::default(); let toml = toml::from_str(s).map_err(ConfigError::MalformedFile)?; config.fill_from_toml(toml); From 55987ab407c470e45a1b3f364422ee26cb0c5605 Mon Sep 17 00:00:00 2001 From: f01dab1e Date: Tue, 17 Oct 2023 12:44:49 +0000 Subject: [PATCH 03/10] simplify --- tooling/nargo_fmt/build.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tooling/nargo_fmt/build.rs b/tooling/nargo_fmt/build.rs index 04a5c56fd7a..3da07715e4e 100644 --- a/tooling/nargo_fmt/build.rs +++ b/tooling/nargo_fmt/build.rs @@ -42,9 +42,7 @@ fn generate_formatter_tests(test_file: &mut File, test_data_dir: &Path) { let config = input_source .lines() - .by_ref() - .take_while(|line| line.starts_with("//@")) - .map(|line| &line[3..]) + .flat_map(|line| line.strip_prefix("//@")) .collect::>() .join("\n"); From 263fd29f17f3d58888a4d35a94093921663a099b Mon Sep 17 00:00:00 2001 From: f01dab1e Date: Tue, 17 Oct 2023 13:07:40 +0000 Subject: [PATCH 04/10] improve message --- tooling/nargo_fmt/src/visitor/expr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index f1d47ab2fc3..8526791464c 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -14,7 +14,7 @@ impl FmtVisitor<'_> { let changed_comment_content = changed_comment_content(original, &rewrite); if changed_comment_content && self.config.error_on_unformatted { - panic!("{original:?} vs {rewrite:?}"); + panic!("not formatted because a comment would be lost: {rewrite:?}"); } self.push_rewrite( From 250eef8a9576f6a5ed5b3685b19be227449e5c4c Mon Sep 17 00:00:00 2001 From: f01dab1e Date: Tue, 17 Oct 2023 20:55:36 +0000 Subject: [PATCH 05/10] `error_on_unformatted` -> `error_on_lost_comment` --- tooling/nargo_fmt/src/config.rs | 2 +- tooling/nargo_fmt/src/visitor/expr.rs | 2 +- tooling/nargo_fmt/tests/expected/infix.nr | 1 + tooling/nargo_fmt/tests/expected/nested_parens.nr | 1 - tooling/nargo_fmt/tests/expected/parens.nr | 1 - tooling/nargo_fmt/tests/expected/unary_operators.nr | 1 + tooling/nargo_fmt/tests/input/infix.nr | 1 + tooling/nargo_fmt/tests/input/nested_parens.nr | 1 - tooling/nargo_fmt/tests/input/parens.nr | 1 - tooling/nargo_fmt/tests/input/unary_operators.nr | 1 + 10 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tooling/nargo_fmt/src/config.rs b/tooling/nargo_fmt/src/config.rs index d886c680e28..093fdaf3c69 100644 --- a/tooling/nargo_fmt/src/config.rs +++ b/tooling/nargo_fmt/src/config.rs @@ -44,7 +44,7 @@ macro_rules! config { config! { tab_spaces: usize, 4, "Number of spaces per tab"; remove_nested_parens: bool, true, "Remove nested parens"; - error_on_unformatted: bool, false, "Error if unable to get comments"; + error_on_lost_comment: bool, true, "Error if unable to get comments"; } impl Config { diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index 8526791464c..b31e3c492f5 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -13,7 +13,7 @@ impl FmtVisitor<'_> { let original = slice!(self, span.start(), span.end()); let changed_comment_content = changed_comment_content(original, &rewrite); - if changed_comment_content && self.config.error_on_unformatted { + if changed_comment_content && self.config.error_on_lost_comment { panic!("not formatted because a comment would be lost: {rewrite:?}"); } diff --git a/tooling/nargo_fmt/tests/expected/infix.nr b/tooling/nargo_fmt/tests/expected/infix.nr index f930f79ebcb..0688781ba48 100644 --- a/tooling/nargo_fmt/tests/expected/infix.nr +++ b/tooling/nargo_fmt/tests/expected/infix.nr @@ -1,3 +1,4 @@ +//@error_on_lost_comment=false fn foo() { 40 + 2; !40 + 2; diff --git a/tooling/nargo_fmt/tests/expected/nested_parens.nr b/tooling/nargo_fmt/tests/expected/nested_parens.nr index 1bfa7fec62c..53eaa63c279 100644 --- a/tooling/nargo_fmt/tests/expected/nested_parens.nr +++ b/tooling/nargo_fmt/tests/expected/nested_parens.nr @@ -1,4 +1,3 @@ -//@error_on_unformatted=true //@remove_nested_parens=false fn main() { ((())); diff --git a/tooling/nargo_fmt/tests/expected/parens.nr b/tooling/nargo_fmt/tests/expected/parens.nr index 41a943d1be0..aeba660f898 100644 --- a/tooling/nargo_fmt/tests/expected/parens.nr +++ b/tooling/nargo_fmt/tests/expected/parens.nr @@ -1,4 +1,3 @@ -//@error_on_unformatted=true fn main(x : u64, y : pub u64) { ( // diff --git a/tooling/nargo_fmt/tests/expected/unary_operators.nr b/tooling/nargo_fmt/tests/expected/unary_operators.nr index 1dd5e4ab945..ffea1713c06 100644 --- a/tooling/nargo_fmt/tests/expected/unary_operators.nr +++ b/tooling/nargo_fmt/tests/expected/unary_operators.nr @@ -1,3 +1,4 @@ +//@error_on_lost_comment=false fn main() { -1; -/*test*/1; diff --git a/tooling/nargo_fmt/tests/input/infix.nr b/tooling/nargo_fmt/tests/input/infix.nr index df57f956097..b16ccd02982 100644 --- a/tooling/nargo_fmt/tests/input/infix.nr +++ b/tooling/nargo_fmt/tests/input/infix.nr @@ -1,3 +1,4 @@ +//@error_on_lost_comment=false fn foo() { 40 + 2; !40+2; diff --git a/tooling/nargo_fmt/tests/input/nested_parens.nr b/tooling/nargo_fmt/tests/input/nested_parens.nr index 1bfa7fec62c..53eaa63c279 100644 --- a/tooling/nargo_fmt/tests/input/nested_parens.nr +++ b/tooling/nargo_fmt/tests/input/nested_parens.nr @@ -1,4 +1,3 @@ -//@error_on_unformatted=true //@remove_nested_parens=false fn main() { ((())); diff --git a/tooling/nargo_fmt/tests/input/parens.nr b/tooling/nargo_fmt/tests/input/parens.nr index 2f0cdbd3ed4..769b477a34d 100644 --- a/tooling/nargo_fmt/tests/input/parens.nr +++ b/tooling/nargo_fmt/tests/input/parens.nr @@ -1,4 +1,3 @@ -//@error_on_unformatted=true fn main(x : u64, y : pub u64) { ( // diff --git a/tooling/nargo_fmt/tests/input/unary_operators.nr b/tooling/nargo_fmt/tests/input/unary_operators.nr index e6d42456ef2..4324b8045cc 100644 --- a/tooling/nargo_fmt/tests/input/unary_operators.nr +++ b/tooling/nargo_fmt/tests/input/unary_operators.nr @@ -1,3 +1,4 @@ +//@error_on_lost_comment=false fn main() { -1; -/*test*/1; From 4566c09b3066148aee96fb98e4a8c0cd69ddc574 Mon Sep 17 00:00:00 2001 From: kek kek kek Date: Thu, 19 Oct 2023 17:45:38 +0000 Subject: [PATCH 06/10] remove dead code --- tooling/nargo_fmt/src/utils.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tooling/nargo_fmt/src/utils.rs b/tooling/nargo_fmt/src/utils.rs index 05a79485093..a84099e1f94 100644 --- a/tooling/nargo_fmt/src/utils.rs +++ b/tooling/nargo_fmt/src/utils.rs @@ -4,14 +4,6 @@ use noirc_frontend::lexer::Lexer; use noirc_frontend::token::Token; use noirc_frontend::Expression; -pub(crate) fn recover_comment_removed(original: &str, new: String) -> String { - if changed_comment_content(original, &new) { - original.to_string() - } else { - new - } -} - pub(crate) fn changed_comment_content(original: &str, new: &str) -> bool { comments(original).ne(comments(new)) } From a82b320aaac9ca7d2a6c8425f6b6ec222dd50215 Mon Sep 17 00:00:00 2001 From: kek kek kek Date: Thu, 19 Oct 2023 18:42:36 +0000 Subject: [PATCH 07/10] fix test --- tooling/nargo_fmt/tests/expected/literals.nr | 1 + tooling/nargo_fmt/tests/input/literals.nr | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tooling/nargo_fmt/tests/expected/literals.nr b/tooling/nargo_fmt/tests/expected/literals.nr index 4dc47bf7e2e..1511392a90b 100644 --- a/tooling/nargo_fmt/tests/expected/literals.nr +++ b/tooling/nargo_fmt/tests/expected/literals.nr @@ -1,3 +1,4 @@ +//@error_on_lost_comment=false fn main() { [1, 2, 3, 4, 5]; diff --git a/tooling/nargo_fmt/tests/input/literals.nr b/tooling/nargo_fmt/tests/input/literals.nr index ec7659ac66f..cf15bb37106 100644 --- a/tooling/nargo_fmt/tests/input/literals.nr +++ b/tooling/nargo_fmt/tests/input/literals.nr @@ -1,4 +1,4 @@ - +//@error_on_lost_comment=false fn main() { [1,2,3,4,5]; From c4ca684818feb4000c590b49d232fccaab7e3636 Mon Sep 17 00:00:00 2001 From: kek kek kek Date: Mon, 23 Oct 2023 12:56:56 +0000 Subject: [PATCH 08/10] fix comp error --- tooling/nargo_fmt/src/visitor/expr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index 93666c8c63b..d88b492bd6f 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -14,7 +14,7 @@ impl FmtVisitor<'_> { let span = expr.span; let rewrite = self.format_expr(expr); - let original = slice!(self, span.start(), span.end()); + let original = self.slice(span); let changed_comment_content = utils::changed_comment_content(original, &rewrite); if changed_comment_content && self.config.error_on_lost_comment { From ca2bdd39cc96e44eb62f2221c2b09ad361565bf7 Mon Sep 17 00:00:00 2001 From: kek kek kek Date: Tue, 24 Oct 2023 00:14:08 +0000 Subject: [PATCH 09/10] fix tests --- tooling/nargo_fmt/tests/expected/if.nr | 1 + tooling/nargo_fmt/tests/input/if.nr | 1 + 2 files changed, 2 insertions(+) diff --git a/tooling/nargo_fmt/tests/expected/if.nr b/tooling/nargo_fmt/tests/expected/if.nr index 9893239750c..6ee3f25f1c9 100644 --- a/tooling/nargo_fmt/tests/expected/if.nr +++ b/tooling/nargo_fmt/tests/expected/if.nr @@ -1,3 +1,4 @@ +//@error_on_lost_comment=false fn main() { if false { (); diff --git a/tooling/nargo_fmt/tests/input/if.nr b/tooling/nargo_fmt/tests/input/if.nr index 0985928396d..f9f2a098bd2 100644 --- a/tooling/nargo_fmt/tests/input/if.nr +++ b/tooling/nargo_fmt/tests/input/if.nr @@ -1,3 +1,4 @@ +//@error_on_lost_comment=false fn main() { if false { From cc1a71d7482ef42366721e5b60f5c3255aa2ddda Mon Sep 17 00:00:00 2001 From: kek kek kek Date: Fri, 27 Oct 2023 04:18:33 +0000 Subject: [PATCH 10/10] fix test --- tooling/nargo_fmt/tests/expected/let.nr | 1 + tooling/nargo_fmt/tests/input/let.nr | 1 + 2 files changed, 2 insertions(+) diff --git a/tooling/nargo_fmt/tests/expected/let.nr b/tooling/nargo_fmt/tests/expected/let.nr index dccdb76328c..1c7afc8df5f 100644 --- a/tooling/nargo_fmt/tests/expected/let.nr +++ b/tooling/nargo_fmt/tests/expected/let.nr @@ -1,3 +1,4 @@ +//@error_on_lost_comment=false fn let_() { let fn_call = my_function(some_function(10, "arg1", another_function()), another_func(20, some_function(), 30)); let array = [[[1, 2, 3], [4, 5, 6]], [[7, 8, 9], [10, 11, 12]], [[13, 14, 15], [16, 17, 18]]]; diff --git a/tooling/nargo_fmt/tests/input/let.nr b/tooling/nargo_fmt/tests/input/let.nr index ae23f0150d9..67c4ab8bd52 100644 --- a/tooling/nargo_fmt/tests/input/let.nr +++ b/tooling/nargo_fmt/tests/input/let.nr @@ -1,3 +1,4 @@ +//@error_on_lost_comment=false fn let_() { let fn_call = my_function(some_function( 10, "arg1", another_function() ),another_func (20, some_function() , 30 )); let array = [[[1, 2, 3], [4, 5, 6]], [[7, 8, 9], [10, 11, 12]], [[13, 14, 15], [16, 17, 18]]];