From 7c823fa9aab79d66f80ebd58251767d54f82dfa0 Mon Sep 17 00:00:00 2001 From: kek kek kek Date: Tue, 28 Nov 2023 04:21:17 -0800 Subject: [PATCH] chore: update code formatting to prevent parameter line wrapping (#3588) --- tooling/nargo_fmt/src/rewrite/array.rs | 4 +- tooling/nargo_fmt/src/rewrite/expr.rs | 17 +++++++-- tooling/nargo_fmt/src/utils.rs | 8 ++-- tooling/nargo_fmt/src/visitor/expr.rs | 52 ++++++++++++++++++++------ tooling/nargo_fmt/src/visitor/item.rs | 18 ++++++--- tooling/nargo_fmt/src/visitor/stmt.rs | 11 +++++- tooling/nargo_fmt/tests/expected/fn.nr | 8 ++++ tooling/nargo_fmt/tests/input/fn.nr | 4 ++ 8 files changed, 94 insertions(+), 28 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/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/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..586d9583e32 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); @@ -186,6 +187,7 @@ impl FmtVisitor<'_> { } } +// TODO: fixme #[allow(clippy::too_many_arguments)] pub(crate) fn format_seq( shape: Shape, @@ -196,7 +198,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 +207,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,10 +228,13 @@ pub(crate) fn format_brackets( exprs, span, Tactic::LimitedHorizontalVertical(array_width), + NewlineMode::Normal, false, ) } +// TODO: fixme +#[allow(clippy::too_many_arguments)] pub(crate) fn format_parens( max_width: Option, visitor: FmtVisitor, @@ -236,10 +242,11 @@ pub(crate) fn format_parens( trailing_comma: bool, exprs: Vec, span: Span, - soft_newline: bool, + 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, soft_newline) + format_seq(shape, "(", ")", visitor, trailing_comma, exprs, span, tactic, mode, reduce) } fn format_exprs( @@ -248,11 +255,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 +333,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 +397,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 +432,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, ) }; 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); 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] +) {}