From 191f8365a7045f9633834bb1b71aa08e0a296950 Mon Sep 17 00:00:00 2001 From: kek kek kek Date: Mon, 30 Oct 2023 04:01:43 -0700 Subject: [PATCH] chore: custom configuration for formatter tests (#3202) Co-authored-by: kevaundray --- tooling/nargo_fmt/build.rs | 8 +++++++- tooling/nargo_fmt/src/config.rs | 15 ++++++++++----- tooling/nargo_fmt/src/utils.rs | 8 -------- tooling/nargo_fmt/src/visitor.rs | 10 +++++++++- tooling/nargo_fmt/src/visitor/expr.rs | 2 -- tooling/nargo_fmt/tests/expected/if.nr | 1 + tooling/nargo_fmt/tests/expected/infix.nr | 1 + tooling/nargo_fmt/tests/expected/let.nr | 1 + tooling/nargo_fmt/tests/expected/literals.nr | 1 + tooling/nargo_fmt/tests/expected/nested_parens.nr | 5 +++++ .../nargo_fmt/tests/expected/unary_operators.nr | 1 + tooling/nargo_fmt/tests/input/if.nr | 1 + tooling/nargo_fmt/tests/input/infix.nr | 1 + tooling/nargo_fmt/tests/input/let.nr | 1 + tooling/nargo_fmt/tests/input/literals.nr | 2 +- tooling/nargo_fmt/tests/input/nested_parens.nr | 5 +++++ tooling/nargo_fmt/tests/input/unary_operators.nr | 1 + 17 files changed, 46 insertions(+), 18 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..3da07715e4e 100644 --- a/tooling/nargo_fmt/build.rs +++ b/tooling/nargo_fmt/build.rs @@ -40,6 +40,12 @@ 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() + .flat_map(|line| line.strip_prefix("//@")) + .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 +61,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::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 b81fc47cf09..c026b1576c6 100644 --- a/tooling/nargo_fmt/src/config.rs +++ b/tooling/nargo_fmt/src/config.rs @@ -45,6 +45,7 @@ config! { max_width: usize, 100, "Maximum width of each line"; tab_spaces: usize, 4, "Number of spaces per tab"; remove_nested_parens: bool, true, "Remove nested parens"; + error_on_lost_comment: bool, true, "Error if unable to get comments"; short_array_element_width_threshold: usize, 10, "Width threshold for an array element to be considered short"; array_width: usize, 100, "Maximum width of an array literal before falling back to vertical formatting"; single_line_if_else_max_width: usize, 50, "Maximum line length for single line if-else expressions"; @@ -52,16 +53,20 @@ config! { 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::of(&input) + } + + 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); Ok(config) } diff --git a/tooling/nargo_fmt/src/utils.rs b/tooling/nargo_fmt/src/utils.rs index a13c0298ed7..44ffda976f9 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, Ident}; -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)) } diff --git a/tooling/nargo_fmt/src/visitor.rs b/tooling/nargo_fmt/src/visitor.rs index 20eb332f9f3..fd156074ec3 100644 --- a/tooling/nargo_fmt/src/visitor.rs +++ b/tooling/nargo_fmt/src/visitor.rs @@ -89,7 +89,15 @@ impl<'me> FmtVisitor<'me> { #[track_caller] fn push_rewrite(&mut self, rewrite: String, span: Span) { - let rewrite = utils::recover_comment_removed(self.slice(span), rewrite); + 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 { + panic!("not formatted because a comment would be lost: {rewrite:?}"); + } + + let rewrite = if changed_comment_content { original.to_string() } else { rewrite }; + self.format_missing_indent(span.start(), true); self.push_str(&rewrite); } diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index 604d5ca9caf..b7f59526701 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -13,10 +13,8 @@ use crate::{ impl FmtVisitor<'_> { pub(crate) fn visit_expr(&mut self, expr: Expression, expr_type: ExpressionType) { let span = expr.span; - let rewrite = self.format_expr(expr, expr_type); self.push_rewrite(rewrite, span); - self.last_position = span.end(); } diff --git a/tooling/nargo_fmt/tests/expected/if.nr b/tooling/nargo_fmt/tests/expected/if.nr index 30a60e50577..39ad7d18cdd 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() { let (x,y) = if is_square(gx1) { (x1, sqrt(gx1)) 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/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/expected/literals.nr b/tooling/nargo_fmt/tests/expected/literals.nr index abe14c14965..5a9a735337f 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/expected/nested_parens.nr b/tooling/nargo_fmt/tests/expected/nested_parens.nr new file mode 100644 index 00000000000..53eaa63c279 --- /dev/null +++ b/tooling/nargo_fmt/tests/expected/nested_parens.nr @@ -0,0 +1,5 @@ +//@remove_nested_parens=false +fn main() { + ((())); + ((((((((())))))))); +} 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/if.nr b/tooling/nargo_fmt/tests/input/if.nr index ab39df006ea..be72eb79a18 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() { let (x,y) = if is_square(gx1) {(x1, sqrt(gx1))} else {(x2, sqrt(gx2))}; 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/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]]]; diff --git a/tooling/nargo_fmt/tests/input/literals.nr b/tooling/nargo_fmt/tests/input/literals.nr index 3490c1e7d0d..fbdc7676845 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]; 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..53eaa63c279 --- /dev/null +++ b/tooling/nargo_fmt/tests/input/nested_parens.nr @@ -0,0 +1,5 @@ +//@remove_nested_parens=false +fn main() { + ((())); + ((((((((())))))))); +} 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;