From 423ba5eb321f5b476fb588141bfae2daabdb68ef Mon Sep 17 00:00:00 2001 From: f01dab1e Date: Mon, 27 Nov 2023 07:59:31 +0000 Subject: [PATCH 1/5] chore: update code formatting to prevent parameter line wrapping --- tooling/nargo_fmt/src/rewrite/array.rs | 4 +- tooling/nargo_fmt/src/utils.rs | 8 ++-- tooling/nargo_fmt/src/visitor/expr.rs | 59 +++++++++++++++++++++----- tooling/nargo_fmt/src/visitor/item.rs | 18 +++++--- 4 files changed, 67 insertions(+), 22 deletions(-) diff --git a/tooling/nargo_fmt/src/rewrite/array.rs b/tooling/nargo_fmt/src/rewrite/array.rs index f67ae5e75fe..fc5b240f83e 100644 --- a/tooling/nargo_fmt/src/rewrite/array.rs +++ b/tooling/nargo_fmt/src/rewrite/array.rs @@ -2,7 +2,7 @@ use noirc_frontend::{hir::resolution::errors::Span, token::Token, Expression}; use crate::{ utils::{Expr, FindToken}, - visitor::FmtVisitor, + visitor::{expr::NewlineMode, FmtVisitor}, }; pub(crate) fn rewrite(mut visitor: FmtVisitor, array: Vec, array_span: Span) -> String { @@ -80,6 +80,6 @@ pub(crate) fn rewrite(mut visitor: FmtVisitor, array: Vec, array_spa items_str.trim().into(), nested_indent, visitor.shape(), - true, + NewlineMode::IfContainsNewLineAndWidth, ) } diff --git a/tooling/nargo_fmt/src/utils.rs b/tooling/nargo_fmt/src/utils.rs index 626795959a3..0d422e57de1 100644 --- a/tooling/nargo_fmt/src/utils.rs +++ b/tooling/nargo_fmt/src/utils.rs @@ -120,7 +120,7 @@ impl<'me, T> Exprs<'me, T> { pub(crate) trait FindToken { fn find_token(&self, token: Token) -> Option; - fn find_token_with(&self, f: impl Fn(&Token) -> bool) -> Option; + fn find_token_with(&self, f: impl Fn(&Token) -> bool) -> Option; } impl FindToken for str { @@ -128,11 +128,11 @@ impl FindToken for str { Lexer::new(self).flatten().find_map(|it| (it.token() == &token).then(|| it.to_span())) } - fn find_token_with(&self, f: impl Fn(&Token) -> bool) -> Option { + fn find_token_with(&self, f: impl Fn(&Token) -> bool) -> Option { Lexer::new(self) .skip_comments(false) .flatten() - .find_map(|spanned| f(spanned.token()).then(|| spanned.to_span().end())) + .find_map(|spanned| f(spanned.token()).then(|| spanned.to_span())) } } @@ -142,7 +142,7 @@ pub(crate) fn find_comment_end(slice: &str, is_last: bool) -> usize { .find_token_with(|token| { matches!(token, Token::LineComment(_, _) | Token::BlockComment(_, _)) }) - .map(|index| index as usize) + .map(|index| index.end() as usize) .unwrap_or(slice.len()) } diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index a5e5a1c7846..2ba2c0a91c3 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -89,6 +89,7 @@ impl FmtVisitor<'_> { false, exprs, nested_indent, + true, ); visitor.indent.block_unindent(visitor.config); @@ -196,7 +197,8 @@ pub(crate) fn format_seq( exprs: Vec, span: Span, tactic: Tactic, - soft_newline: bool, + mode: NewlineMode, + reduce: bool, ) -> String { let mut nested_indent = shape; let shape = shape; @@ -204,9 +206,9 @@ pub(crate) fn format_seq( nested_indent.indent.block_indent(visitor.config); let exprs: Vec<_> = utils::Exprs::new(&visitor, nested_indent, span, exprs).collect(); - let exprs = format_exprs(visitor.config, tactic, trailing_comma, exprs, nested_indent); + let exprs = format_exprs(visitor.config, tactic, trailing_comma, exprs, nested_indent, reduce); - wrap_exprs(prefix, suffix, exprs, nested_indent, shape, soft_newline) + wrap_exprs(prefix, suffix, exprs, nested_indent, shape, mode) } pub(crate) fn format_brackets( @@ -225,6 +227,7 @@ pub(crate) fn format_brackets( exprs, span, Tactic::LimitedHorizontalVertical(array_width), + NewlineMode::Normal, false, ) } @@ -236,10 +239,21 @@ pub(crate) fn format_parens( trailing_comma: bool, exprs: Vec, span: Span, - soft_newline: bool, + reduce: bool, ) -> String { let tactic = max_width.map(Tactic::LimitedHorizontalVertical).unwrap_or(Tactic::Horizontal); - format_seq(shape, "(", ")", visitor, trailing_comma, exprs, span, tactic, soft_newline) + format_seq( + shape, + "(", + ")", + visitor, + trailing_comma, + exprs, + span, + tactic, + NewlineMode::Normal, + reduce, + ) } fn format_exprs( @@ -248,11 +262,12 @@ fn format_exprs( trailing_comma: bool, exprs: Vec, shape: Shape, + reduce: bool, ) -> String { let mut result = String::new(); let indent_str = shape.indent.to_string(); - let tactic = tactic.definitive(&exprs, config.short_array_element_width_threshold); + let tactic = tactic.definitive(&exprs, config.short_array_element_width_threshold, reduce); let mut exprs = exprs.into_iter().enumerate().peekable(); let mut line_len = 0; let mut prev_expr_trailing_comment = false; @@ -325,16 +340,32 @@ fn format_exprs( result } +#[derive(PartialEq, Eq)] +pub(crate) enum NewlineMode { + IfContainsNewLine, + IfContainsNewLineAndWidth, + Normal, +} + pub(crate) fn wrap_exprs( prefix: &str, suffix: &str, exprs: String, nested_shape: Shape, shape: Shape, - soft_newline: bool, + newline_mode: NewlineMode, ) -> String { - let mut force_one_line = first_line_width(&exprs) <= shape.width; - if soft_newline && force_one_line { + let mut force_one_line = if newline_mode == NewlineMode::IfContainsNewLine { + true + } else { + first_line_width(&exprs) <= shape.width + }; + + if matches!( + newline_mode, + NewlineMode::IfContainsNewLine | NewlineMode::IfContainsNewLineAndWidth + ) && force_one_line + { force_one_line = !exprs.contains('\n'); } @@ -373,7 +404,8 @@ impl Tactic { fn definitive( self, exprs: &[Expr], - short_array_element_width_threshold: usize, + short_width_threshold: usize, + reduce: bool, ) -> DefinitiveTactic { let tactic = || { let has_single_line_comment = exprs.iter().any(|item| { @@ -407,7 +439,12 @@ impl Tactic { } }; - tactic().reduce(exprs, short_array_element_width_threshold) + let definitive_tactic = tactic(); + if reduce { + definitive_tactic.reduce(exprs, short_width_threshold) + } else { + definitive_tactic + } } } diff --git a/tooling/nargo_fmt/src/visitor/item.rs b/tooling/nargo_fmt/src/visitor/item.rs index af375515413..c0a255b7ef6 100644 --- a/tooling/nargo_fmt/src/visitor/item.rs +++ b/tooling/nargo_fmt/src/visitor/item.rs @@ -7,7 +7,7 @@ use noirc_frontend::{ use crate::{ utils::{last_line_contains_single_line_comment, last_line_used_width, FindToken}, - visitor::expr::format_seq, + visitor::expr::{format_seq, NewlineMode}, }; use super::{ @@ -54,6 +54,7 @@ impl super::FmtVisitor<'_> { generics, span, HorizontalVertical, + NewlineMode::IfContainsNewLine, false, ); @@ -63,12 +64,18 @@ impl super::FmtVisitor<'_> { let parameters = if parameters.is_empty() { self.slice(params_span).into() } else { - let fn_start = result.find_token(Token::Keyword(Keyword::Fn)).unwrap().start(); - let slice = self.slice(fn_start..result.len() as u32); + let fn_start = result + .find_token_with(|token| { + matches!(token, Token::Keyword(Keyword::Fn | Keyword::Unconstrained)) + }) + .unwrap() + .start(); + let slice = self.slice(fn_start..result.len() as u32); let indent = self.indent; let used_width = last_line_used_width(slice, indent.width()); - let one_line_budget = self.budget(used_width + return_type.len()); + let overhead = if return_type.is_empty() { 2 } else { 3 }; // 2 = `()`, 3 = `() ` + let one_line_budget = self.budget(used_width + return_type.len() + overhead); let shape = Shape { width: one_line_budget, indent }; let tactic = LimitedHorizontalVertical(one_line_budget); @@ -82,7 +89,8 @@ impl super::FmtVisitor<'_> { parameters, params_span.into(), tactic, - true, + NewlineMode::IfContainsNewLine, + false, ) }; From b2491ec47ea5f747abfd3a808e21eec8c0b0ad67 Mon Sep 17 00:00:00 2001 From: f01dab1e Date: Mon, 27 Nov 2023 08:12:07 +0000 Subject: [PATCH 2/5] chore: fix typo --- 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 2ba2c0a91c3..380bb876701 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -251,7 +251,7 @@ pub(crate) fn format_parens( exprs, span, tactic, - NewlineMode::Normal, + NewlineMode::IfContainsNewLineAndWidth, reduce, ) } From 89e8dd07eb908636c5cdf571202cf28ae2926041 Mon Sep 17 00:00:00 2001 From: f01dab1e Date: Mon, 27 Nov 2023 08:47:32 +0000 Subject: [PATCH 3/5] chore: fix bug :) --- tooling/nargo_fmt/src/rewrite/expr.rs | 17 +++++++++++++---- tooling/nargo_fmt/src/visitor/expr.rs | 17 +++++------------ 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/tooling/nargo_fmt/src/rewrite/expr.rs b/tooling/nargo_fmt/src/rewrite/expr.rs index 4d7279815df..e026d515333 100644 --- a/tooling/nargo_fmt/src/rewrite/expr.rs +++ b/tooling/nargo_fmt/src/rewrite/expr.rs @@ -1,7 +1,7 @@ use noirc_frontend::{token::Token, ArrayLiteral, Expression, ExpressionKind, Literal, UnaryOp}; use crate::visitor::{ - expr::{format_brackets, format_parens}, + expr::{format_brackets, format_parens, NewlineMode}, ExpressionType, FmtVisitor, Indent, Shape, }; @@ -60,6 +60,7 @@ pub(crate) fn rewrite( call_expr.arguments, args_span, true, + NewlineMode::IfContainsNewLineAndWidth, ); format!("{callee}{args}") @@ -80,6 +81,7 @@ pub(crate) fn rewrite( method_call_expr.arguments, args_span, true, + NewlineMode::IfContainsNewLineAndWidth, ); format!("{object}.{method}{args}") @@ -97,9 +99,16 @@ pub(crate) fn rewrite( format!("{collection}{index}") } - ExpressionKind::Tuple(exprs) => { - format_parens(None, visitor.fork(), shape, exprs.len() == 1, exprs, span, false) - } + ExpressionKind::Tuple(exprs) => format_parens( + None, + visitor.fork(), + shape, + exprs.len() == 1, + exprs, + span, + true, + NewlineMode::Normal, + ), ExpressionKind::Literal(literal) => match literal { Literal::Integer(_) | Literal::Bool(_) | Literal::Str(_) | Literal::FmtStr(_) => { visitor.slice(span).to_string() diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index 380bb876701..586d9583e32 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -187,6 +187,7 @@ impl FmtVisitor<'_> { } } +// TODO: fixme #[allow(clippy::too_many_arguments)] pub(crate) fn format_seq( shape: Shape, @@ -232,6 +233,8 @@ pub(crate) fn format_brackets( ) } +// TODO: fixme +#[allow(clippy::too_many_arguments)] pub(crate) fn format_parens( max_width: Option, visitor: FmtVisitor, @@ -240,20 +243,10 @@ pub(crate) fn format_parens( exprs: Vec, span: Span, reduce: bool, + mode: NewlineMode, ) -> String { let tactic = max_width.map(Tactic::LimitedHorizontalVertical).unwrap_or(Tactic::Horizontal); - format_seq( - shape, - "(", - ")", - visitor, - trailing_comma, - exprs, - span, - tactic, - NewlineMode::IfContainsNewLineAndWidth, - reduce, - ) + format_seq(shape, "(", ")", visitor, trailing_comma, exprs, span, tactic, mode, reduce) } fn format_exprs( From e133704acbf2e4208934954029cc51e3aede5acc Mon Sep 17 00:00:00 2001 From: f01dab1e Date: Mon, 27 Nov 2023 10:35:46 +0000 Subject: [PATCH 4/5] chore: add test --- tooling/nargo_fmt/tests/expected/fn.nr | 8 ++++++++ tooling/nargo_fmt/tests/input/fn.nr | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/tooling/nargo_fmt/tests/expected/fn.nr b/tooling/nargo_fmt/tests/expected/fn.nr index 0e61483398c..7fd45648c67 100644 --- a/tooling/nargo_fmt/tests/expected/fn.nr +++ b/tooling/nargo_fmt/tests/expected/fn.nr @@ -37,3 +37,11 @@ fn apply_binary_field_op( ) -> bool {} fn main() -> distinct pub [Field;2] {} + +fn main( + message: [u8; 10], + message_field: Field, + pub_key_x: Field, + pub_key_y: Field, + signature: [u8; 64] +) {} diff --git a/tooling/nargo_fmt/tests/input/fn.nr b/tooling/nargo_fmt/tests/input/fn.nr index f503db99853..45dc3370f14 100644 --- a/tooling/nargo_fmt/tests/input/fn.nr +++ b/tooling/nargo_fmt/tests/input/fn.nr @@ -24,3 +24,7 @@ fn main(tape: [Field; TAPE_LEN], initial_registers: [Field; REGISTER_COUNT], ini fn apply_binary_field_op(lhs: RegisterIndex, rhs: RegisterIndex, result: RegisterIndex, op: u8, registers: &mut Registers) -> bool {} fn main() -> distinct pub [Field;2] {} + +fn main( + message: [u8; 10], message_field: Field, pub_key_x: Field, pub_key_y: Field, signature: [u8; 64] +) {} From b9c9956f1f0a0fc093707bb3fbfce82af6982c16 Mon Sep 17 00:00:00 2001 From: f01dab1e Date: Tue, 28 Nov 2023 09:50:23 +0000 Subject: [PATCH 5/5] chore: fix compile error --- tooling/nargo_fmt/src/visitor/stmt.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tooling/nargo_fmt/src/visitor/stmt.rs b/tooling/nargo_fmt/src/visitor/stmt.rs index eabdb1a150a..800a8656ef3 100644 --- a/tooling/nargo_fmt/src/visitor/stmt.rs +++ b/tooling/nargo_fmt/src/visitor/stmt.rs @@ -6,7 +6,7 @@ use noirc_frontend::{ use crate::{rewrite, visitor::expr::wrap_exprs}; -use super::ExpressionType; +use super::{expr::NewlineMode, ExpressionType}; impl super::FmtVisitor<'_> { pub(crate) fn visit_stmts(&mut self, stmts: Vec) { @@ -68,7 +68,14 @@ impl super::FmtVisitor<'_> { } }; - let args = wrap_exprs("(", ")", args, nested_shape, shape, true); + let args = wrap_exprs( + "(", + ")", + args, + nested_shape, + shape, + NewlineMode::IfContainsNewLineAndWidth, + ); let constrain = format!("{callee}{args};"); self.push_rewrite(constrain, span);