Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: update array formatting style #3486

Merged
merged 6 commits into from Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions tooling/nargo_fmt/src/rewrite.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
mod array;
mod infix;

pub(crate) use infix::rewrite as infix;
pub(crate) use array::rewrite as array;
85 changes: 85 additions & 0 deletions tooling/nargo_fmt/src/rewrite/array.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
use noirc_frontend::{hir::resolution::errors::Span, token::Token, Expression};

use crate::{
utils::{Expr, FindToken},
visitor::FmtVisitor,
};

pub(crate) fn rewrite(mut visitor: FmtVisitor, array: Vec<Expression>, array_span: Span) -> String {
let pattern: &[_] = &[' ', '\t'];

visitor.indent.block_indent(visitor.config);
let nested_indent = visitor.shape();

let indent_str = nested_indent.indent.to_string();

let mut last_position = array_span.start() + 1;
let end_position = array_span.end() - 1;

let mut items = array.into_iter().peekable();

let mut result = Vec::new();
while let Some(item) = items.next() {
let item_span = item.span;

let start: u32 = last_position;
let end = item_span.start();

let leading = visitor.slice(start..end).trim_matches(pattern);
let item = visitor.format_sub_expr(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
.find_token(Token::Comma)
.map(|span| span.end() as usize)
.unwrap_or(trailing.len());
let trailing = trailing[..offset].trim_end_matches(',').trim_matches(pattern);
last_position = item_span.end() + offset as u32;

let (leading, _) = visitor.format_comment_in_block(leading, 0, false);
let (trailing, _) = visitor.format_comment_in_block(trailing, 0, false);

result.push(Expr { leading, value: item, trailing, different_line: false });
}

let slice = visitor.slice(last_position..end_position);
let (comment, _) = visitor.format_comment_in_block(slice, 0, false);
result.push(Expr {
leading: "".into(),
value: "".into(),
trailing: comment,
different_line: false,
});

visitor.indent.block_unindent(visitor.config);

let mut items_str = String::new();
let mut items = result.into_iter().peekable();
while let Some(next) = items.next() {
items_str.push_str(&next.leading);
if next.leading.contains('\n') && !next.value.is_empty() {
items_str.push_str(&indent_str);
}
items_str.push_str(&next.value);
items_str.push_str(&next.trailing);

if let Some(item) = items.peek() {
if !item.value.is_empty() {
items_str.push(',');
}

if !item.leading.contains('\n') && !next.value.is_empty() {
items_str.push(' ');
}
}
}

crate::visitor::expr::wrap_exprs(
"[",
"]",
items_str.trim().into(),
nested_indent,
visitor.shape(),
true,
)
}
25 changes: 18 additions & 7 deletions tooling/nargo_fmt/src/visitor.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
mod expr;
pub(crate) mod expr;
mod item;
mod stmt;

Expand Down Expand Up @@ -53,7 +53,7 @@ impl<'me> FmtVisitor<'me> {
(span.start() + offset..span.end()).into()
}

fn shape(&self) -> Shape {
pub(crate) fn shape(&self) -> Shape {
Shape {
width: self.config.max_width.saturating_sub(self.indent.width()),
indent: self.indent,
Expand Down Expand Up @@ -106,7 +106,7 @@ impl<'me> FmtVisitor<'me> {
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 {
if changed_comment_content && false {
panic!("not formatted because a comment would be lost: {rewrite:?}");
This conversation was marked as resolved.
Show resolved Hide resolved
}

Expand Down Expand Up @@ -163,7 +163,7 @@ impl<'me> FmtVisitor<'me> {
self.push_vertical_spaces(slice);
process_last_slice(self, "", slice);
} else {
let (result, last_end) = self.format_comment_in_block(slice, start);
let (result, last_end) = self.format_comment_in_block(slice, start, true);
if result.trim().is_empty() {
process_last_slice(self, slice, slice);
} else {
Expand All @@ -174,7 +174,12 @@ impl<'me> FmtVisitor<'me> {
}
}

fn format_comment_in_block(&mut self, slice: &str, start: u32) -> (String, u32) {
pub(crate) fn format_comment_in_block(
&mut self,
slice: &str,
start: u32,
fix_indent: bool,
) -> (String, u32) {
let mut result = String::new();
let mut last_end = 0;

Expand All @@ -185,12 +190,14 @@ impl<'me> FmtVisitor<'me> {
let diff = start;
let big_snippet = &self.source[..(span.start() + diff) as usize];
let last_char = big_snippet.chars().rev().find(|rev_c| ![' ', '\t'].contains(rev_c));
let fix_indent = last_char.map_or(true, |rev_c| ['{', '\n'].contains(&rev_c));
let fix_indent =
fix_indent && last_char.map_or(true, |rev_c| ['{', '\n'].contains(&rev_c));

if let Token::LineComment(_, _) | Token::BlockComment(_, _) = comment.into_token() {
let starts_with_newline = slice.starts_with('\n');
let comment = &slice[span.start() as usize..span.end() as usize];

dbg!(fix_indent);
if fix_indent {
This conversation was marked as resolved.
Show resolved Hide resolved
if let Some('{') = last_char {
result.push('\n');
Expand All @@ -205,12 +212,16 @@ impl<'me> FmtVisitor<'me> {
} else {
match (starts_with_newline, self.at_start()) {
(false, false) => {
dbg!(());
result.push(' ');
This conversation was marked as resolved.
Show resolved Hide resolved
}
(true, _) => {
dbg!(());
result.push_str(&self.indent.to_string_with_newline());
This conversation was marked as resolved.
Show resolved Hide resolved
}
_ => {}
(false, _) => {
result.push(' ');
}
};
}

Expand Down
12 changes: 8 additions & 4 deletions tooling/nargo_fmt/src/visitor/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
format!("[{repeated}; {length}]")
}
Literal::Array(ArrayLiteral::Standard(exprs)) => {
format_brackets(self.fork(), false, exprs, span)
rewrite::array(self.fork(), exprs, span)
}
Literal::Unit => "()".to_string(),
},
Expand Down Expand Up @@ -300,7 +300,7 @@
if let Some(first_stmt) = block.first() {
let slice = self.slice(self.last_position..first_stmt.span.start());
let len =
slice.chars().take_while(|ch| ch.is_whitespace()).collect::<String>().rfind('\n');

Check warning on line 303 in tooling/nargo_fmt/src/visitor/expr.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (rfind)
self.last_position += len.unwrap_or(0) as u32;
}
}
Expand Down Expand Up @@ -370,7 +370,7 @@

visitor.indent.block_unindent(visitor.config);

wrap_exprs(prefix, suffix, exprs, nested_indent, visitor.shape())
wrap_exprs(prefix, suffix, exprs, nested_indent, visitor.shape(), false)
}

fn format_brackets(
Expand Down Expand Up @@ -483,16 +483,20 @@
result
}

fn wrap_exprs(
pub(crate) fn wrap_exprs(
prefix: &str,
suffix: &str,
exprs: String,
nested_shape: Shape,
shape: Shape,
array: bool,
) -> String {
let first_line_width = first_line_width(&exprs);

if first_line_width <= shape.width {
let force_one_line =
if array { !exprs.contains('\n') } else { first_line_width <= shape.width };

if force_one_line {
let allow_trailing_newline = exprs
.lines()
.last()
Expand Down
54 changes: 25 additions & 29 deletions tooling/nargo_fmt/tests/expected/array.nr
Original file line number Diff line number Diff line change
@@ -1,42 +1,38 @@
fn big_array() {
[1,
10,
100,
1000,
10000,
100000,
1000000,
10000000,
100000000,
1000000000,
10000000000,
100000000000,
1000000000000,
10000000000000,
100000000000000,
1000000000000000,
10000000000000000,
100000000000000000,
1000000000000000000,
[
1, 10, 100, 1000, 10000, 100000, 1000000, 10000000, 100000000, 1000000000, 10000000000, 100000000000, 1000000000000, 10000000000000, 100000000000000, 1000000000000000, 10000000000000000, 100000000000000000, 1000000000000000000,
10000000000000000000,
100000000000000000000,
1000000000000000000000,
10000000000000000000000,
100000000000000000000000,
1000000000000000000000000];
1000000000000000000000000
];

[1, 10];
[
1,
10
];

[// hello!
1, 10];
[
// hello!
1,
10
];

[// hello!
1, // asd
10];
[
// hello!
1,
// asd
10
];

[// hello!
1, // asd
10// asdasd
[
// hello!
1,
// asd
10
// asdasd
];

[[[1, 2, 3], [4, 5, 6]], [[7, 8, 9], [10, 11, 12]], [[13, 14, 15], [16, 17, 18]]];
Expand Down
26 changes: 10 additions & 16 deletions tooling/nargo_fmt/tests/expected/let.nr
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,18 @@ 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]]];

let padded_sha256_hash: [u8; 259] = [// Padded hash
209, 50, 135, 178, 4, 155, 190, 229, 228, 111, 61, 174, 8, 49, 48, 116, 90, 226, 77, 7, 111,
27, 19, 113, 154, 48, 138, 136, 138, 15, 230, 132, 32, 4, 0, 5, 1, 2, 4, 3, 101, 1, 72, 134,
96, 9, 6, 13, 48, 49, 48, 0, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 1, 0,
let padded_sha256_hash: [u8; 259] = [
// Padded hash
209, 50, 135, 178, 4, 155, 190, 229, 228, 111, 61, 174, 8, 49, 48, 116, 90, 226, 77, 7, 111, 27, 19, 113, 154, 48, 138, 136, 138, 15, 230, 132, 32, 4, 0, 5, 1, 2, 4, 3, 101, 1, 72, 134, 96, 9, 6, 13, 48, 49,
48, 0, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 1, 0,
// Rest is padded with 0s until max bytes
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, 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, 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, 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, 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, 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, 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, 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, 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",
Expand Down
Loading