chore: update code formatting to prevent parameter line wrapping (noi…

…r-lang#3588)
manastech · Nov 28, 2023 · 7c823fa · 7c823fa
1 parent 847ca7c
commit 7c823fa
Show file tree

Hide file tree

Showing 8 changed files with 94 additions and 28 deletions.
diff --git 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<Expression>, array_span: Span) -> String {
@@ -80,6 +80,6 @@ pub(crate) fn rewrite(mut visitor: FmtVisitor, array: Vec<Expression>, 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
@@ -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
@@ -120,19 +120,19 @@ impl<'me, T> Exprs<'me, T> {
 
 pub(crate) trait FindToken {
     fn find_token(&self, token: Token) -> Option<Span>;
-    fn find_token_with(&self, f: impl Fn(&Token) -> bool) -> Option<u32>;
+    fn find_token_with(&self, f: impl Fn(&Token) -> bool) -> Option<Span>;
 }
 
 impl FindToken for str {
     fn find_token(&self, token: Token) -> Option<Span> {
         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<u32> {
+    fn find_token_with(&self, f: impl Fn(&Token) -> bool) -> Option<Span> {
         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
@@ -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<T: Item>(
     shape: Shape,
@@ -196,17 +198,18 @@ pub(crate) fn format_seq<T: Item>(
     exprs: Vec<T>,
     span: Span,
     tactic: Tactic,
-    soft_newline: bool,
+    mode: NewlineMode,
+    reduce: bool,
 ) -> String {
     let mut nested_indent = shape;
     let shape = shape;
 
     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,21 +228,25 @@ 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<usize>,
     visitor: FmtVisitor,
     shape: Shape,
     trailing_comma: bool,
     exprs: Vec<Expression>,
     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<Expr>,
     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
@@ -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
@@ -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<Statement>) {
@@ -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
@@ -37,3 +37,11 @@ fn apply_binary_field_op<N>(
 ) -> 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
@@ -24,3 +24,7 @@ fn main(tape: [Field; TAPE_LEN], initial_registers: [Field; REGISTER_COUNT], ini
 fn apply_binary_field_op<N>(lhs: RegisterIndex, rhs: RegisterIndex, result: RegisterIndex, op: u8, registers: &mut Registers<N>) -> 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]
+) {}