From fd402585a89010162b3d43f56546e29baf33ae72 Mon Sep 17 00:00:00 2001 From: kek kek kek Date: Thu, 23 Nov 2023 14:28:15 -0800 Subject: [PATCH] chore(fmt): refactor the way we handle shapes in the formatter (#3546) --- tooling/nargo_fmt/src/rewrite.rs | 4 + tooling/nargo_fmt/src/rewrite/array.rs | 2 +- tooling/nargo_fmt/src/rewrite/expr.rs | 144 ++++++++++ tooling/nargo_fmt/src/rewrite/infix.rs | 11 +- .../nargo_fmt/src/rewrite/parenthesized.rs | 67 +++++ tooling/nargo_fmt/src/utils.rs | 40 ++- tooling/nargo_fmt/src/visitor.rs | 10 +- tooling/nargo_fmt/src/visitor/expr.rs | 254 +++--------------- tooling/nargo_fmt/src/visitor/item.rs | 69 ++++- tooling/nargo_fmt/src/visitor/stmt.rs | 22 +- tooling/nargo_fmt/tests/expected/contract.nr | 7 +- tooling/nargo_fmt/tests/expected/fn.nr | 8 +- tooling/nargo_fmt/tests/expected/let.nr | 6 +- 13 files changed, 387 insertions(+), 257 deletions(-) create mode 100644 tooling/nargo_fmt/src/rewrite/expr.rs create mode 100644 tooling/nargo_fmt/src/rewrite/parenthesized.rs diff --git a/tooling/nargo_fmt/src/rewrite.rs b/tooling/nargo_fmt/src/rewrite.rs index c1ac585985e..5a9baf0aa05 100644 --- a/tooling/nargo_fmt/src/rewrite.rs +++ b/tooling/nargo_fmt/src/rewrite.rs @@ -1,5 +1,9 @@ mod array; +mod expr; mod infix; +mod parenthesized; pub(crate) use array::rewrite as array; +pub(crate) use expr::{rewrite as expr, rewrite_sub_expr as sub_expr}; pub(crate) use infix::rewrite as infix; +pub(crate) use parenthesized::rewrite as parenthesized; diff --git a/tooling/nargo_fmt/src/rewrite/array.rs b/tooling/nargo_fmt/src/rewrite/array.rs index 9c49d827528..f67ae5e75fe 100644 --- a/tooling/nargo_fmt/src/rewrite/array.rs +++ b/tooling/nargo_fmt/src/rewrite/array.rs @@ -26,7 +26,7 @@ pub(crate) fn rewrite(mut visitor: FmtVisitor, array: Vec, array_spa let end = item_span.start(); let leading = visitor.slice(start..end).trim_matches(pattern); - let item = visitor.format_sub_expr(item); + let item = super::sub_expr(&visitor, visitor.shape(), item); let next_start = items.peek().map_or(end_position, |expr| expr.span.start()); let trailing = visitor.slice(item_span.end()..next_start); let offset = trailing diff --git a/tooling/nargo_fmt/src/rewrite/expr.rs b/tooling/nargo_fmt/src/rewrite/expr.rs new file mode 100644 index 00000000000..4d7279815df --- /dev/null +++ b/tooling/nargo_fmt/src/rewrite/expr.rs @@ -0,0 +1,144 @@ +use noirc_frontend::{token::Token, ArrayLiteral, Expression, ExpressionKind, Literal, UnaryOp}; + +use crate::visitor::{ + expr::{format_brackets, format_parens}, + ExpressionType, FmtVisitor, Indent, Shape, +}; + +pub(crate) fn rewrite_sub_expr( + visitor: &FmtVisitor, + shape: Shape, + expression: Expression, +) -> String { + rewrite(visitor, expression, ExpressionType::SubExpression, shape) +} + +pub(crate) fn rewrite( + visitor: &FmtVisitor, + Expression { kind, span }: Expression, + expr_type: ExpressionType, + shape: Shape, +) -> String { + match kind { + ExpressionKind::Block(block) => { + let mut visitor = visitor.fork(); + visitor.visit_block(block, span); + visitor.finish() + } + ExpressionKind::Prefix(prefix) => { + let op = match prefix.operator { + UnaryOp::Minus => "-", + UnaryOp::Not => "!", + UnaryOp::MutableReference => "&mut ", + UnaryOp::Dereference { implicitly_added } => { + if implicitly_added { + "" + } else { + "*" + } + } + }; + + format!("{op}{}", rewrite_sub_expr(visitor, shape, prefix.rhs)) + } + ExpressionKind::Cast(cast) => { + format!("{} as {}", rewrite_sub_expr(visitor, shape, cast.lhs), cast.r#type) + } + kind @ ExpressionKind::Infix(_) => { + super::infix(visitor.fork(), Expression { kind, span }, shape) + } + ExpressionKind::Call(call_expr) => { + let args_span = + visitor.span_before(call_expr.func.span.end()..span.end(), Token::LeftParen); + + let callee = rewrite_sub_expr(visitor, shape, *call_expr.func); + let args = format_parens( + visitor.config.fn_call_width.into(), + visitor.fork(), + shape, + false, + call_expr.arguments, + args_span, + true, + ); + + format!("{callee}{args}") + } + ExpressionKind::MethodCall(method_call_expr) => { + let args_span = visitor.span_before( + method_call_expr.method_name.span().end()..span.end(), + Token::LeftParen, + ); + + let object = rewrite_sub_expr(visitor, shape, method_call_expr.object); + let method = method_call_expr.method_name.to_string(); + let args = format_parens( + visitor.config.fn_call_width.into(), + visitor.fork(), + shape, + false, + method_call_expr.arguments, + args_span, + true, + ); + + format!("{object}.{method}{args}") + } + ExpressionKind::MemberAccess(member_access_expr) => { + let lhs_str = rewrite_sub_expr(visitor, shape, member_access_expr.lhs); + format!("{}.{}", lhs_str, member_access_expr.rhs) + } + ExpressionKind::Index(index_expr) => { + let index_span = visitor + .span_before(index_expr.collection.span.end()..span.end(), Token::LeftBracket); + + let collection = rewrite_sub_expr(visitor, shape, index_expr.collection); + let index = format_brackets(visitor.fork(), false, vec![index_expr.index], index_span); + + format!("{collection}{index}") + } + ExpressionKind::Tuple(exprs) => { + format_parens(None, visitor.fork(), shape, exprs.len() == 1, exprs, span, false) + } + ExpressionKind::Literal(literal) => match literal { + Literal::Integer(_) | Literal::Bool(_) | Literal::Str(_) | Literal::FmtStr(_) => { + visitor.slice(span).to_string() + } + Literal::Array(ArrayLiteral::Repeated { repeated_element, length }) => { + let repeated = rewrite_sub_expr(visitor, shape, *repeated_element); + let length = rewrite_sub_expr(visitor, shape, *length); + + format!("[{repeated}; {length}]") + } + Literal::Array(ArrayLiteral::Standard(exprs)) => { + super::array(visitor.fork(), exprs, span) + } + Literal::Unit => "()".to_string(), + }, + ExpressionKind::Parenthesized(sub_expr) => { + super::parenthesized(visitor, shape, span, *sub_expr) + } + ExpressionKind::Constructor(constructor) => { + let type_name = visitor.slice(span.start()..constructor.type_name.span().end()); + let fields_span = visitor + .span_before(constructor.type_name.span().end()..span.end(), Token::LeftBrace); + + visitor.format_struct_lit(type_name, fields_span, *constructor) + } + ExpressionKind::If(if_expr) => { + let allow_single_line = expr_type == ExpressionType::SubExpression; + + if allow_single_line { + let mut visitor = visitor.fork(); + visitor.indent = Indent::default(); + if let Some(line) = visitor.format_if_single_line(*if_expr.clone()) { + return line; + } + } + + visitor.format_if(*if_expr) + } + ExpressionKind::Lambda(_) | ExpressionKind::Variable(_) => visitor.slice(span).to_string(), + ExpressionKind::Error => unreachable!(), + } +} diff --git a/tooling/nargo_fmt/src/rewrite/infix.rs b/tooling/nargo_fmt/src/rewrite/infix.rs index 0fbfa07a841..15f5fe23aae 100644 --- a/tooling/nargo_fmt/src/rewrite/infix.rs +++ b/tooling/nargo_fmt/src/rewrite/infix.rs @@ -3,8 +3,9 @@ use std::iter::zip; use noirc_frontend::{Expression, ExpressionKind}; use crate::{ + rewrite, utils::{first_line_width, is_single_line}, - visitor::{ExpressionType, FmtVisitor, Shape}, + visitor::{FmtVisitor, Shape}, }; pub(crate) fn rewrite(visitor: FmtVisitor, expr: Expression, shape: Shape) -> String { @@ -16,9 +17,9 @@ pub(crate) fn rewrite(visitor: FmtVisitor, expr: Expression, shape: Shape) -> St format!( "{} {} {}", - visitor.format_sub_expr(infix.lhs), + rewrite::sub_expr(&visitor, shape, infix.lhs), infix.operator.contents.as_string(), - visitor.format_sub_expr(infix.rhs) + rewrite::sub_expr(&visitor, shape, infix.rhs) ) } } @@ -87,10 +88,10 @@ pub(crate) fn flatten( } _ => { let rewrite = if result.is_empty() { - visitor.format_expr(node.clone(), ExpressionType::SubExpression) + rewrite::sub_expr(&visitor, visitor.shape(), node.clone()) } else { visitor.indent.block_indent(visitor.config); - visitor.format_expr(node.clone(), ExpressionType::SubExpression) + rewrite::sub_expr(&visitor, visitor.shape(), node.clone()) }; result.push(rewrite); diff --git a/tooling/nargo_fmt/src/rewrite/parenthesized.rs b/tooling/nargo_fmt/src/rewrite/parenthesized.rs new file mode 100644 index 00000000000..3926b52cb73 --- /dev/null +++ b/tooling/nargo_fmt/src/rewrite/parenthesized.rs @@ -0,0 +1,67 @@ +use noirc_frontend::{hir::resolution::errors::Span, Expression, ExpressionKind}; + +use crate::visitor::{FmtVisitor, Shape}; + +pub(crate) fn rewrite( + visitor: &FmtVisitor<'_>, + shape: Shape, + mut span: Span, + mut sub_expr: Expression, +) -> String { + let remove_nested_parens = visitor.config.remove_nested_parens; + + let mut leading; + let mut trailing; + + loop { + let leading_span = span.start() + 1..sub_expr.span.start(); + let trailing_span = sub_expr.span.end()..span.end() - 1; + + leading = visitor.format_comment(leading_span.into()); + trailing = visitor.format_comment(trailing_span.into()); + + if let ExpressionKind::Parenthesized(ref sub_sub_expr) = sub_expr.kind { + if remove_nested_parens && leading.is_empty() && trailing.is_empty() { + span = sub_expr.span; + sub_expr = *sub_sub_expr.clone(); + continue; + } + } + + break; + } + + if !leading.contains("//") && !trailing.contains("//") { + let sub_expr = super::sub_expr(visitor, shape, sub_expr); + format!("({leading}{sub_expr}{trailing})") + } else { + let mut visitor = visitor.fork(); + + let indent = visitor.indent.to_string_with_newline(); + visitor.indent.block_indent(visitor.config); + let nested_indent = visitor.indent.to_string_with_newline(); + + let sub_expr = super::sub_expr(&visitor, shape, sub_expr); + + let mut result = String::new(); + result.push('('); + + if !leading.is_empty() { + result.push_str(&nested_indent); + result.push_str(&leading); + } + + result.push_str(&nested_indent); + result.push_str(&sub_expr); + + if !trailing.is_empty() { + result.push_str(&nested_indent); + result.push_str(&trailing); + } + + result.push_str(&indent); + result.push(')'); + + result + } +} diff --git a/tooling/nargo_fmt/src/utils.rs b/tooling/nargo_fmt/src/utils.rs index 13938079e68..626795959a3 100644 --- a/tooling/nargo_fmt/src/utils.rs +++ b/tooling/nargo_fmt/src/utils.rs @@ -1,4 +1,5 @@ -use crate::visitor::FmtVisitor; +use crate::rewrite; +use crate::visitor::{FmtVisitor, Shape}; use noirc_frontend::hir::resolution::errors::Span; use noirc_frontend::lexer::Lexer; use noirc_frontend::token::Token; @@ -40,15 +41,22 @@ impl Expr { pub(crate) struct Exprs<'me, T> { pub(crate) visitor: &'me FmtVisitor<'me>, + shape: Shape, pub(crate) elements: std::iter::Peekable>, pub(crate) last_position: u32, pub(crate) end_position: u32, } impl<'me, T: Item> Exprs<'me, T> { - pub(crate) fn new(visitor: &'me FmtVisitor<'me>, span: Span, elements: Vec) -> Self { + pub(crate) fn new( + visitor: &'me FmtVisitor<'me>, + shape: Shape, + span: Span, + elements: Vec, + ) -> Self { Self { visitor, + shape, last_position: span.start() + 1, /*(*/ end_position: span.end() - 1, /*)*/ elements: elements.into_iter().peekable(), @@ -70,7 +78,7 @@ impl Iterator for Exprs<'_, T> { let next_start = self.elements.peek().map_or(self.end_position, |expr| expr.start()); let (leading, different_line) = self.leading(start, end); - let expr = element.format(self.visitor); + let expr = element.format(self.visitor, self.shape); let trailing = self.trailing(element_span.end(), next_start, is_last); Expr { leading, value: expr, trailing, different_line }.into() @@ -196,7 +204,7 @@ pub(crate) fn count_newlines(slice: &str) -> usize { pub(crate) trait Item { fn span(&self) -> Span; - fn format(self, visitor: &FmtVisitor) -> String; + fn format(self, visitor: &FmtVisitor, shape: Shape) -> String; fn start(&self) -> u32 { self.span().start() @@ -212,8 +220,8 @@ impl Item for Expression { self.span } - fn format(self, visitor: &FmtVisitor) -> String { - visitor.format_sub_expr(self) + fn format(self, visitor: &FmtVisitor, shape: Shape) -> String { + rewrite::sub_expr(visitor, shape, self) } } @@ -223,11 +231,11 @@ impl Item for (Ident, Expression) { (name.span().start()..value.span.end()).into() } - fn format(self, visitor: &FmtVisitor) -> String { + fn format(self, visitor: &FmtVisitor, shape: Shape) -> String { let (name, expr) = self; let name = name.0.contents; - let expr = visitor.format_sub_expr(expr); + let expr = rewrite::sub_expr(visitor, shape, expr); if name == expr { name @@ -242,7 +250,7 @@ impl Item for Param { self.span } - fn format(self, visitor: &FmtVisitor) -> String { + fn format(self, visitor: &FmtVisitor, _shape: Shape) -> String { let visibility = match self.visibility { Visibility::Public => "pub ", Visibility::Private => "", @@ -259,7 +267,7 @@ impl Item for Ident { self.span() } - fn format(self, visitor: &FmtVisitor) -> String { + fn format(self, visitor: &FmtVisitor, _shape: Shape) -> String { visitor.slice(self.span()).into() } } @@ -268,6 +276,10 @@ pub(crate) fn first_line_width(exprs: &str) -> usize { exprs.lines().next().map_or(0, |line: &str| line.chars().count()) } +pub(crate) fn last_line_width(s: &str) -> usize { + s.rsplit('\n').next().unwrap_or("").chars().count() +} + pub(crate) fn is_single_line(s: &str) -> bool { !s.chars().any(|c| c == '\n') } @@ -275,3 +287,11 @@ pub(crate) fn is_single_line(s: &str) -> bool { pub(crate) fn last_line_contains_single_line_comment(s: &str) -> bool { s.lines().last().map_or(false, |line| line.contains("//")) } + +pub(crate) fn last_line_used_width(s: &str, offset: usize) -> usize { + if s.contains('\n') { + last_line_width(s) + } else { + offset + s.chars().count() + } +} diff --git a/tooling/nargo_fmt/src/visitor.rs b/tooling/nargo_fmt/src/visitor.rs index cf3b3a41e8a..85989db79d8 100644 --- a/tooling/nargo_fmt/src/visitor.rs +++ b/tooling/nargo_fmt/src/visitor.rs @@ -30,12 +30,16 @@ impl<'me> FmtVisitor<'me> { } } + pub(crate) fn budget(&self, used_width: usize) -> usize { + self.config.max_width.saturating_sub(used_width) + } + pub(crate) fn slice(&self, span: impl Into) -> &'me str { let span = span.into(); &self.source[span.start() as usize..span.end() as usize] } - fn span_after(&self, span: impl Into, token: Token) -> Span { + pub(crate) fn span_after(&self, span: impl Into, token: Token) -> Span { let span = span.into(); let slice = self.slice(span); @@ -44,7 +48,7 @@ impl<'me> FmtVisitor<'me> { (span.start() + offset..span.end()).into() } - fn span_before(&self, span: impl Into, token: Token) -> Span { + pub(crate) fn span_before(&self, span: impl Into, token: Token) -> Span { let span = span.into(); let slice = self.slice(span); @@ -253,10 +257,12 @@ impl Indent { self.block_indent } + #[track_caller] pub(crate) fn block_indent(&mut self, config: &Config) { self.block_indent += config.tab_spaces; } + #[track_caller] pub(crate) fn block_unindent(&mut self, config: &Config) { self.block_indent -= config.tab_spaces; } diff --git a/tooling/nargo_fmt/src/visitor/expr.rs b/tooling/nargo_fmt/src/visitor/expr.rs index 0eb192569c3..a5e5a1c7846 100644 --- a/tooling/nargo_fmt/src/visitor/expr.rs +++ b/tooling/nargo_fmt/src/visitor/expr.rs @@ -1,10 +1,9 @@ use noirc_frontend::{ - hir::resolution::errors::Span, lexer::Lexer, token::Token, ArrayLiteral, BlockExpression, - ConstructorExpression, Expression, ExpressionKind, IfExpression, Literal, Statement, - StatementKind, UnaryOp, + hir::resolution::errors::Span, lexer::Lexer, token::Token, BlockExpression, + ConstructorExpression, Expression, ExpressionKind, IfExpression, Statement, StatementKind, }; -use super::{ExpressionType, FmtVisitor, Indent, Shape}; +use super::{ExpressionType, FmtVisitor, Shape}; use crate::{ rewrite, utils::{self, first_line_width, Expr, FindToken, Item}, @@ -14,201 +13,14 @@ 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); + let rewrite = rewrite::expr(self, expr, expr_type, self.shape()); self.push_rewrite(rewrite, span); self.last_position = span.end(); } - pub(crate) fn format_sub_expr(&self, expression: Expression) -> String { - self.format_expr(expression, ExpressionType::SubExpression) - } - - pub(crate) fn format_expr( - &self, - Expression { kind, mut span }: Expression, - expr_type: ExpressionType, - ) -> String { - match kind { - ExpressionKind::Block(block) => { - let mut visitor = self.fork(); - visitor.visit_block(block, span); - visitor.buffer - } - ExpressionKind::Prefix(prefix) => { - let op = match prefix.operator { - UnaryOp::Minus => "-", - UnaryOp::Not => "!", - UnaryOp::MutableReference => "&mut ", - UnaryOp::Dereference { implicitly_added } => { - if implicitly_added { - "" - } else { - "*" - } - } - }; - - format!("{op}{}", self.format_sub_expr(prefix.rhs)) - } - ExpressionKind::Cast(cast) => { - format!("{} as {}", self.format_sub_expr(cast.lhs), cast.r#type) - } - kind @ ExpressionKind::Infix(_) => { - let shape = self.shape(); - rewrite::infix(self.fork(), Expression { kind, span }, shape) - } - ExpressionKind::Call(call_expr) => { - let args_span = - self.span_before(call_expr.func.span.end()..span.end(), Token::LeftParen); - - let callee = self.format_sub_expr(*call_expr.func); - let args = format_parens( - self.config.fn_call_width.into(), - self.fork(), - false, - call_expr.arguments, - args_span, - true, - ); - - format!("{callee}{args}") - } - ExpressionKind::MethodCall(method_call_expr) => { - let args_span = self.span_before( - method_call_expr.method_name.span().end()..span.end(), - Token::LeftParen, - ); - - let object = self.format_sub_expr(method_call_expr.object); - let method = method_call_expr.method_name.to_string(); - let args = format_parens( - self.config.fn_call_width.into(), - self.fork(), - false, - method_call_expr.arguments, - args_span, - true, - ); - - format!("{object}.{method}{args}") - } - ExpressionKind::MemberAccess(member_access_expr) => { - let lhs_str = self.format_sub_expr(member_access_expr.lhs); - format!("{}.{}", lhs_str, member_access_expr.rhs) - } - ExpressionKind::Index(index_expr) => { - let index_span = self - .span_before(index_expr.collection.span.end()..span.end(), Token::LeftBracket); - - let collection = self.format_sub_expr(index_expr.collection); - let index = format_brackets(self.fork(), false, vec![index_expr.index], index_span); - - format!("{collection}{index}") - } - ExpressionKind::Tuple(exprs) => { - format_parens(None, self.fork(), exprs.len() == 1, exprs, span, false) - } - ExpressionKind::Literal(literal) => match literal { - Literal::Integer(_) | Literal::Bool(_) | Literal::Str(_) | Literal::FmtStr(_) => { - self.slice(span).to_string() - } - Literal::Array(ArrayLiteral::Repeated { repeated_element, length }) => { - let repeated = self.format_sub_expr(*repeated_element); - let length = self.format_sub_expr(*length); - - format!("[{repeated}; {length}]") - } - Literal::Array(ArrayLiteral::Standard(exprs)) => { - rewrite::array(self.fork(), exprs, span) - } - Literal::Unit => "()".to_string(), - }, - ExpressionKind::Parenthesized(mut sub_expr) => { - let remove_nested_parens = self.config.remove_nested_parens; - - let mut leading; - let mut trailing; - - loop { - let leading_span = span.start() + 1..sub_expr.span.start(); - let trailing_span = sub_expr.span.end()..span.end() - 1; - - leading = self.format_comment(leading_span.into()); - trailing = self.format_comment(trailing_span.into()); - - if let ExpressionKind::Parenthesized(ref sub_sub_expr) = sub_expr.kind { - if remove_nested_parens && leading.is_empty() && trailing.is_empty() { - span = sub_expr.span; - sub_expr = sub_sub_expr.clone(); - continue; - } - } - - break; - } - - if !leading.contains("//") && !trailing.contains("//") { - let sub_expr = self.format_sub_expr(*sub_expr); - format!("({leading}{sub_expr}{trailing})") - } else { - let mut visitor = self.fork(); - - let indent = visitor.indent.to_string_with_newline(); - visitor.indent.block_indent(self.config); - let nested_indent = visitor.indent.to_string_with_newline(); - - let sub_expr = visitor.format_sub_expr(*sub_expr); - - let mut result = String::new(); - result.push('('); - - if !leading.is_empty() { - result.push_str(&nested_indent); - result.push_str(&leading); - } - - result.push_str(&nested_indent); - result.push_str(&sub_expr); - - if !trailing.is_empty() { - result.push_str(&nested_indent); - result.push_str(&trailing); - } - - result.push_str(&indent); - result.push(')'); - - result - } - } - ExpressionKind::Constructor(constructor) => { - let type_name = self.slice(span.start()..constructor.type_name.span().end()); - let fields_span = self - .span_before(constructor.type_name.span().end()..span.end(), Token::LeftBrace); - - self.format_struct_lit(type_name, fields_span, *constructor) - } - ExpressionKind::If(if_expr) => { - let allow_single_line = expr_type == ExpressionType::SubExpression; - - if allow_single_line { - let mut visitor = self.fork(); - visitor.indent = Indent::default(); - if let Some(line) = visitor.format_if_single_line(*if_expr.clone()) { - return line; - } - } - - self.format_if(*if_expr) - } - ExpressionKind::Lambda(_) | ExpressionKind::Variable(_) => self.slice(span).to_string(), - ExpressionKind::Error => unreachable!(), - } - } - - fn format_if(&self, if_expr: IfExpression) -> String { - let condition_str = self.format_sub_expr(if_expr.condition); - let consequence_str = self.format_sub_expr(if_expr.consequence); + pub(crate) fn format_if(&self, if_expr: IfExpression) -> String { + let condition_str = rewrite::sub_expr(self, self.shape(), if_expr.condition); + let consequence_str = rewrite::sub_expr(self, self.shape(), if_expr.consequence); let mut result = format!("if {condition_str} {consequence_str}"); @@ -218,7 +30,7 @@ impl FmtVisitor<'_> { { self.format_if(*if_expr) } else { - self.format_expr(alternative, ExpressionType::Statement) + rewrite::expr(self, alternative, ExpressionType::Statement, self.shape()) }; result.push_str(" else "); @@ -228,9 +40,10 @@ impl FmtVisitor<'_> { result } - fn format_if_single_line(&self, if_expr: IfExpression) -> Option { - let condition_str = self.format_sub_expr(if_expr.condition); - let consequence_str = self.format_sub_expr(extract_simple_expr(if_expr.consequence)?); + pub(crate) fn format_if_single_line(&self, if_expr: IfExpression) -> Option { + let condition_str = rewrite::sub_expr(self, self.shape(), if_expr.condition); + let consequence_str = + rewrite::sub_expr(self, self.shape(), extract_simple_expr(if_expr.consequence)?); let if_str = if let Some(alternative) = if_expr.alternative { let alternative_str = if let Some(ExpressionKind::If(_)) = @@ -238,7 +51,12 @@ impl FmtVisitor<'_> { { return None; } else { - self.format_expr(extract_simple_expr(alternative)?, ExpressionType::Statement) + rewrite::expr( + self, + extract_simple_expr(alternative)?, + ExpressionType::Statement, + self.shape(), + ) }; format!("if {} {{ {} }} else {{ {} }}", condition_str, consequence_str, alternative_str) @@ -249,7 +67,7 @@ impl FmtVisitor<'_> { (if_str.len() <= self.config.single_line_if_else_max_width).then_some(if_str) } - fn format_struct_lit( + pub(crate) fn format_struct_lit( &self, type_name: &str, fields_span: Span, @@ -263,7 +81,8 @@ impl FmtVisitor<'_> { let nested_indent = visitor.shape(); let exprs: Vec<_> = - utils::Exprs::new(&visitor, fields_span, constructor.fields).collect(); + utils::Exprs::new(&visitor, nested_indent, fields_span, constructor.fields) + .collect(); let exprs = format_exprs( visitor.config, Tactic::HorizontalVertical, @@ -369,27 +188,28 @@ impl FmtVisitor<'_> { #[allow(clippy::too_many_arguments)] pub(crate) fn format_seq( + shape: Shape, prefix: &str, suffix: &str, - mut visitor: FmtVisitor, + visitor: FmtVisitor, trailing_comma: bool, exprs: Vec, span: Span, tactic: Tactic, soft_newline: bool, ) -> String { - visitor.indent.block_indent(visitor.config); + let mut nested_indent = shape; + let shape = shape; - let nested_indent = visitor.shape(); - let exprs: Vec<_> = utils::Exprs::new(&visitor, span, exprs).collect(); - let exprs = format_exprs(visitor.config, tactic, trailing_comma, exprs, nested_indent); + nested_indent.indent.block_indent(visitor.config); - visitor.indent.block_unindent(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); - wrap_exprs(prefix, suffix, exprs, nested_indent, visitor.shape(), soft_newline) + wrap_exprs(prefix, suffix, exprs, nested_indent, shape, soft_newline) } -fn format_brackets( +pub(crate) fn format_brackets( visitor: FmtVisitor, trailing_comma: bool, exprs: Vec, @@ -397,6 +217,7 @@ fn format_brackets( ) -> String { let array_width = visitor.config.array_width; format_seq( + visitor.shape(), "[", "]", visitor, @@ -408,16 +229,17 @@ fn format_brackets( ) } -fn format_parens( +pub(crate) fn format_parens( max_width: Option, visitor: FmtVisitor, + shape: Shape, trailing_comma: bool, exprs: Vec, span: Span, soft_newline: bool, ) -> String { let tactic = max_width.map(Tactic::LimitedHorizontalVertical).unwrap_or(Tactic::Horizontal); - format_seq("(", ")", visitor, trailing_comma, exprs, span, tactic, soft_newline) + format_seq(shape, "(", ")", visitor, trailing_comma, exprs, span, tactic, soft_newline) } fn format_exprs( @@ -511,10 +333,10 @@ pub(crate) fn wrap_exprs( shape: Shape, soft_newline: bool, ) -> String { - let first_line_width = first_line_width(&exprs); - - let force_one_line = - if soft_newline { !exprs.contains('\n') } else { first_line_width <= shape.width }; + let mut force_one_line = first_line_width(&exprs) <= shape.width; + if soft_newline && force_one_line { + force_one_line = !exprs.contains('\n'); + } if force_one_line { let allow_trailing_newline = exprs diff --git a/tooling/nargo_fmt/src/visitor/item.rs b/tooling/nargo_fmt/src/visitor/item.rs index 7774618afea..af375515413 100644 --- a/tooling/nargo_fmt/src/visitor/item.rs +++ b/tooling/nargo_fmt/src/visitor/item.rs @@ -1,17 +1,22 @@ use noirc_frontend::{ + hir::resolution::errors::Span, parser::{Item, ItemKind}, - token::Token, + token::{Keyword, Token}, Distinctness, NoirFunction, ParsedModule, Visibility, }; -use crate::{utils::last_line_contains_single_line_comment, visitor::expr::format_seq}; +use crate::{ + utils::{last_line_contains_single_line_comment, last_line_used_width, FindToken}, + visitor::expr::format_seq, +}; -use super::expr::Tactic::LimitedHorizontalVertical; +use super::{ + expr::Tactic::{HorizontalVertical, LimitedHorizontalVertical}, + Shape, +}; impl super::FmtVisitor<'_> { fn format_fn_before_block(&self, func: NoirFunction, start: u32) -> (String, bool) { - let tactic = LimitedHorizontalVertical(self.config.max_width); - let name_span = func.name_ident().span(); let func_span = func.span(); @@ -30,6 +35,7 @@ impl super::FmtVisitor<'_> { let params_span = params_open..params_end; let return_type_span = func.return_type().span; + let return_type = self.format_return_type(return_type_span, &func, func_span, params_end); let parameters = func.def.parameters; if !func.def.generics.is_empty() { @@ -39,7 +45,17 @@ impl super::FmtVisitor<'_> { let generics = func.def.generics; let span = (start..end).into(); - let generics = format_seq("<", ">", self.fork(), false, generics, span, tactic, false); + let generics = format_seq( + self.shape(), + "<", + ">", + self.fork(), + false, + generics, + span, + HorizontalVertical, + false, + ); result.push_str(&generics); } @@ -47,10 +63,45 @@ impl super::FmtVisitor<'_> { let parameters = if parameters.is_empty() { self.slice(params_span).into() } else { - format_seq("(", ")", self.fork(), false, parameters, params_span.into(), tactic, true) + let fn_start = result.find_token(Token::Keyword(Keyword::Fn)).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 shape = Shape { width: one_line_budget, indent }; + + let tactic = LimitedHorizontalVertical(one_line_budget); + + format_seq( + shape, + "(", + ")", + self.fork(), + false, + parameters, + params_span.into(), + tactic, + true, + ) }; result.push_str(¶meters); + result.push_str(&return_type); + + let maybe_comment = self.slice(params_end..func_span.start()); + + (result.trim_end().to_string(), last_line_contains_single_line_comment(maybe_comment)) + } + + fn format_return_type( + &self, + return_type_span: Option, + func: &NoirFunction, + func_span: Span, + params_end: u32, + ) -> String { + let mut result = String::new(); if let Some(span) = return_type_span { result.push_str(" -> "); @@ -73,9 +124,7 @@ impl super::FmtVisitor<'_> { result.push_str(self.slice(params_end..func_span.start())); } - let maybe_comment = self.slice(params_end..func_span.start()); - - (result.trim_end().to_string(), last_line_contains_single_line_comment(maybe_comment)) + result } pub(crate) fn visit_file(&mut self, module: ParsedModule) { diff --git a/tooling/nargo_fmt/src/visitor/stmt.rs b/tooling/nargo_fmt/src/visitor/stmt.rs index b6dd67323fa..c27b7911d03 100644 --- a/tooling/nargo_fmt/src/visitor/stmt.rs +++ b/tooling/nargo_fmt/src/visitor/stmt.rs @@ -4,6 +4,8 @@ use noirc_frontend::{ ConstrainKind, ConstrainStatement, ExpressionKind, ForRange, Statement, StatementKind, }; +use crate::rewrite; + use super::ExpressionType; impl super::FmtVisitor<'_> { @@ -25,8 +27,8 @@ impl super::FmtVisitor<'_> { StatementKind::Let(let_stmt) => { let let_str = self.slice(span.start()..let_stmt.expression.span.start()).trim_end(); - let expr_str = - self.format_expr(let_stmt.expression, ExpressionType::SubExpression); + + let expr_str = rewrite::sub_expr(self, self.shape(), let_stmt.expression); self.push_rewrite(format!("{let_str} {expr_str};"), span); } @@ -35,14 +37,14 @@ impl super::FmtVisitor<'_> { message.map_or(String::new(), |message| format!(", \"{message}\"")); let constrain = match kind { ConstrainKind::Assert => { - let assertion = self.format_sub_expr(expr); + let assertion = rewrite::sub_expr(self, self.shape(), expr); format!("assert({assertion}{message});") } ConstrainKind::AssertEq => { if let ExpressionKind::Infix(infix) = expr.kind { - let lhs = self.format_sub_expr(infix.lhs); - let rhs = self.format_sub_expr(infix.rhs); + let lhs = rewrite::sub_expr(self, self.shape(), infix.lhs); + let rhs = rewrite::sub_expr(self, self.shape(), infix.rhs); format!("assert_eq({lhs}, {rhs}{message});") } else { @@ -50,7 +52,7 @@ impl super::FmtVisitor<'_> { } } ConstrainKind::Constrain => { - let expr = self.format_sub_expr(expr); + let expr = rewrite::sub_expr(self, self.shape(), expr); format!("constrain {expr};") } }; @@ -62,12 +64,12 @@ impl super::FmtVisitor<'_> { let range = match for_stmt.range { ForRange::Range(start, end) => format!( "{}..{}", - self.format_sub_expr(start), - self.format_sub_expr(end) + rewrite::sub_expr(self, self.shape(), start), + rewrite::sub_expr(self, self.shape(), end) ), - ForRange::Array(array) => self.format_sub_expr(array), + ForRange::Array(array) => rewrite::sub_expr(self, self.shape(), array), }; - let block = self.format_sub_expr(for_stmt.block); + let block = rewrite::sub_expr(self, self.shape(), for_stmt.block); let result = format!("for {identifier} in {range} {block}"); self.push_rewrite(result, span); diff --git a/tooling/nargo_fmt/tests/expected/contract.nr b/tooling/nargo_fmt/tests/expected/contract.nr index 8a77d97bbba..d288b1af7eb 100644 --- a/tooling/nargo_fmt/tests/expected/contract.nr +++ b/tooling/nargo_fmt/tests/expected/contract.nr @@ -70,7 +70,12 @@ contract Benchmarking { emit_unencrypted_log(&mut context, storage.balances.at(owner).read()); } - unconstrained fn compute_note_hash_and_nullifier(contract_address: Field, nonce: Field, storage_slot: Field, preimage: [Field; VALUE_NOTE_LEN]) -> [Field; 4] { + unconstrained fn compute_note_hash_and_nullifier( + contract_address: Field, + nonce: Field, + storage_slot: Field, + preimage: [Field; VALUE_NOTE_LEN] + ) -> [Field; 4] { let note_header = NoteHeader::new(contract_address, nonce, storage_slot); note_utils::compute_note_hash_and_nullifier(ValueNoteMethods, note_header, preimage) } diff --git a/tooling/nargo_fmt/tests/expected/fn.nr b/tooling/nargo_fmt/tests/expected/fn.nr index 5bca2dd8bb1..0e61483398c 100644 --- a/tooling/nargo_fmt/tests/expected/fn.nr +++ b/tooling/nargo_fmt/tests/expected/fn.nr @@ -28,6 +28,12 @@ fn main( initial_call_stack_pointer: u64 ) -> pub ExecutionResult {} -fn apply_binary_field_op(lhs: RegisterIndex, rhs: RegisterIndex, result: RegisterIndex, op: u8, registers: &mut Registers) -> bool {} +fn apply_binary_field_op( + lhs: RegisterIndex, + rhs: RegisterIndex, + result: RegisterIndex, + op: u8, + registers: &mut Registers +) -> bool {} fn main() -> distinct pub [Field;2] {} diff --git a/tooling/nargo_fmt/tests/expected/let.nr b/tooling/nargo_fmt/tests/expected/let.nr index 017a724b099..c57801155a0 100644 --- a/tooling/nargo_fmt/tests/expected/let.nr +++ b/tooling/nargo_fmt/tests/expected/let.nr @@ -17,7 +17,11 @@ fn let_() { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 ]; - let a = BigUint56 { limbs: [1, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0] }; + let a = BigUint56 { + limbs: [ + 1, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 + ] + }; let person = Person { first_name: "John",