From 07ab5150857ec6719b132ec91d5f90af0564a046 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Wed, 23 Oct 2024 19:38:17 -0300 Subject: [PATCH] feat: merge and sort imports (#6322) # Description ## Problem Manually arranging imports is time consuming. ## Summary Added two new configurations, `reorder_imports` (true by default) and `imports_granularity` (`Preserve` by default), similar to the ones found in rustfmt. With the default configuration imports are reorganized alphabetically but not merged. While implementing this I bumped into one code in the test programs like this: ```noir use foo; use foo::bar; ``` Merging those in Rust ends up like this: ```noir use foo::{bar, self}; ``` We don't have `self` in imports, so I had two choices: 1. Try to leave the above unmodified. 2. Implement `self` in imports. It turned out that implement `self` in imports is very easy: when we desugar imports, if the last segment is "self" we just remove it. Then `use foo::self` is desugared into `use foo`. ## Additional Context None. ## Documentation Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: Tom French --- compiler/noirc_frontend/src/ast/statement.rs | 4 +- .../src/parser/parser/use_tree.rs | 15 + compiler/noirc_frontend/src/tests.rs | 22 + noir_stdlib/src/array/check_shuffle.nr | 2 +- noir_stdlib/src/bigint.nr | 2 +- noir_stdlib/src/cmp.nr | 2 +- noir_stdlib/src/collections/map.nr | 6 +- noir_stdlib/src/collections/umap.nr | 4 +- noir_stdlib/src/ec/consts/te.nr | 2 +- noir_stdlib/src/ec/montcurve.nr | 10 +- noir_stdlib/src/ec/swcurve.nr | 8 +- noir_stdlib/src/ec/tecurve.nr | 8 +- noir_stdlib/src/eddsa.nr | 2 +- noir_stdlib/src/embedded_curve_ops.nr | 2 +- noir_stdlib/src/field/bn254.nr | 2 +- noir_stdlib/src/field/mod.nr | 2 +- noir_stdlib/src/hash/mimc.nr | 2 +- noir_stdlib/src/hash/mod.nr | 2 +- noir_stdlib/src/hash/poseidon/bn254/consts.nr | 2 +- noir_stdlib/src/hash/poseidon/mod.nr | 2 +- noir_stdlib/src/hash/poseidon2.nr | 2 +- noir_stdlib/src/meta/expr.nr | 4 +- noir_stdlib/src/meta/trait_constraint.nr | 2 +- noir_stdlib/src/meta/trait_def.nr | 2 +- noir_stdlib/src/ops/mod.nr | 4 +- noir_stdlib/src/option.nr | 4 +- noir_stdlib/src/prelude.nr | 12 +- noir_stdlib/src/uint128.nr | 4 +- .../ec_baby_jubjub/src/main.nr | 2 +- .../mod_nr_entrypoint/src/main.nr | 4 +- .../reexports/src/main.nr | 2 +- .../src/main.nr | 2 +- .../binary_operator_overloading/src/main.nr | 2 +- .../execution_success/eddsa/src/main.nr | 2 +- .../regression_3889/src/main.nr | 2 +- tooling/nargo_fmt/src/config.rs | 14 + tooling/nargo_fmt/src/formatter.rs | 23 +- tooling/nargo_fmt/src/formatter/item.rs | 132 +++- tooling/nargo_fmt/src/formatter/use_tree.rs | 67 +- .../nargo_fmt/src/formatter/use_tree_merge.rs | 627 ++++++++++++++++++ tooling/nargo_fmt/tests/expected/contract.nr | 18 +- 41 files changed, 953 insertions(+), 79 deletions(-) create mode 100644 tooling/nargo_fmt/src/formatter/use_tree_merge.rs diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 441eff99d9e..038a7b76bac 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -373,7 +373,9 @@ impl UseTree { match self.kind { UseTreeKind::Path(name, alias) => { - vec![ImportStatement { visibility, path: prefix.join(name), alias }] + // Desugar `use foo::{self}` to `use foo` + let path = if name.0.contents == "self" { prefix } else { prefix.join(name) }; + vec![ImportStatement { visibility, path, alias }] } UseTreeKind::List(trees) => { let trees = trees.into_iter(); diff --git a/compiler/noirc_frontend/src/parser/parser/use_tree.rs b/compiler/noirc_frontend/src/parser/parser/use_tree.rs index 1c43732c94f..bc95c04d46b 100644 --- a/compiler/noirc_frontend/src/parser/parser/use_tree.rs +++ b/compiler/noirc_frontend/src/parser/parser/use_tree.rs @@ -66,6 +66,14 @@ impl<'a> Parser<'a> { fn parse_use_tree_in_list(&mut self) -> Option { let start_span = self.current_token_span; + // Special case: "self" cannot be followed by anything else + if self.eat_self() { + return Some(UseTree { + prefix: Path { segments: Vec::new(), kind: PathKind::Plain, span: start_span }, + kind: UseTreeKind::Path(Ident::new("self".to_string(), start_span), None), + }); + } + let use_tree = self.parse_use_tree_without_kind( start_span, PathKind::Plain, @@ -250,4 +258,11 @@ mod tests { let (_, errors) = parse_program(src); assert!(!errors.is_empty()); } + + #[test] + fn errors_on_double_colon_after_self() { + let src = "use foo::{self::bar};"; + let (_, errors) = parse_program(src); + assert!(!errors.is_empty()); + } } diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index 17acd17dcc9..86a486a0de5 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -3389,3 +3389,25 @@ fn arithmetic_generics_rounding_fail() { let errors = get_program_errors(src); assert_eq!(errors.len(), 1); } + +#[test] +fn uses_self_in_import() { + let src = r#" + mod moo { + pub mod bar { + pub fn foo() -> i32 { + 1 + } + } + } + + use moo::bar::{self}; + + pub fn baz() -> i32 { + bar::foo() + } + + fn main() {} + "#; + assert_no_errors(src); +} diff --git a/noir_stdlib/src/array/check_shuffle.nr b/noir_stdlib/src/array/check_shuffle.nr index 82028d487c7..2e8c227feee 100644 --- a/noir_stdlib/src/array/check_shuffle.nr +++ b/noir_stdlib/src/array/check_shuffle.nr @@ -59,8 +59,8 @@ where } mod test { - use super::check_shuffle; use crate::cmp::Eq; + use super::check_shuffle; struct CompoundStruct { a: bool, diff --git a/noir_stdlib/src/bigint.nr b/noir_stdlib/src/bigint.nr index 203ff90d444..be072257be3 100644 --- a/noir_stdlib/src/bigint.nr +++ b/noir_stdlib/src/bigint.nr @@ -1,5 +1,5 @@ -use crate::ops::{Add, Sub, Mul, Div}; use crate::cmp::Eq; +use crate::ops::{Add, Div, Mul, Sub}; global bn254_fq = &[ 0x47, 0xFD, 0x7C, 0xD8, 0x16, 0x8C, 0x20, 0x3C, 0x8d, 0xca, 0x71, 0x68, 0x91, 0x6a, 0x81, 0x97, diff --git a/noir_stdlib/src/cmp.nr b/noir_stdlib/src/cmp.nr index 10be6e7b867..ae515150a4d 100644 --- a/noir_stdlib/src/cmp.nr +++ b/noir_stdlib/src/cmp.nr @@ -537,7 +537,7 @@ where } mod cmp_tests { - use crate::cmp::{min, max}; + use crate::cmp::{max, min}; #[test] fn sanity_check_min() { diff --git a/noir_stdlib/src/collections/map.nr b/noir_stdlib/src/collections/map.nr index cd203c43ad3..b46bfa837fb 100644 --- a/noir_stdlib/src/collections/map.nr +++ b/noir_stdlib/src/collections/map.nr @@ -1,8 +1,8 @@ use crate::cmp::Eq; -use crate::option::Option; -use crate::default::Default; -use crate::hash::{Hash, Hasher, BuildHasher}; use crate::collections::bounded_vec::BoundedVec; +use crate::default::Default; +use crate::hash::{BuildHasher, Hash, Hasher}; +use crate::option::Option; // We use load factor alpha_max = 0.75. // Upon exceeding it, assert will fail in order to inform the user diff --git a/noir_stdlib/src/collections/umap.nr b/noir_stdlib/src/collections/umap.nr index 9b72b6173ca..3e074551e9d 100644 --- a/noir_stdlib/src/collections/umap.nr +++ b/noir_stdlib/src/collections/umap.nr @@ -1,7 +1,7 @@ use crate::cmp::Eq; -use crate::option::Option; use crate::default::Default; -use crate::hash::{Hash, Hasher, BuildHasher}; +use crate::hash::{BuildHasher, Hash, Hasher}; +use crate::option::Option; // An unconstrained hash table with open addressing and quadratic probing. // Note that "unconstrained" here means that almost all operations on this diff --git a/noir_stdlib/src/ec/consts/te.nr b/noir_stdlib/src/ec/consts/te.nr index 561c16e846a..150eb849947 100644 --- a/noir_stdlib/src/ec/consts/te.nr +++ b/noir_stdlib/src/ec/consts/te.nr @@ -1,5 +1,5 @@ -use crate::ec::tecurve::affine::Point as TEPoint; use crate::ec::tecurve::affine::Curve as TECurve; +use crate::ec::tecurve::affine::Point as TEPoint; pub struct BabyJubjub { pub curve: TECurve, diff --git a/noir_stdlib/src/ec/montcurve.nr b/noir_stdlib/src/ec/montcurve.nr index 6c83feb1607..239585ba13f 100644 --- a/noir_stdlib/src/ec/montcurve.nr +++ b/noir_stdlib/src/ec/montcurve.nr @@ -3,16 +3,16 @@ pub mod affine { // Points are represented by two-dimensional Cartesian coordinates. // All group operations are induced by those of the corresponding Twisted Edwards curve. // See e.g. for details on the correspondences. + use crate::cmp::Eq; + use crate::ec::is_square; use crate::ec::montcurve::curvegroup; + use crate::ec::safe_inverse; + use crate::ec::sqrt; use crate::ec::swcurve::affine::Curve as SWCurve; use crate::ec::swcurve::affine::Point as SWPoint; use crate::ec::tecurve::affine::Curve as TECurve; use crate::ec::tecurve::affine::Point as TEPoint; - use crate::ec::is_square; - use crate::ec::safe_inverse; - use crate::ec::sqrt; use crate::ec::ZETA; - use crate::cmp::Eq; // Curve specification pub struct Curve { // Montgomery Curve configuration (ky^2 = x^3 + j*x^2 + x) @@ -222,12 +222,12 @@ pub mod curvegroup { // Points are represented by three-dimensional projective (homogeneous) coordinates. // All group operations are induced by those of the corresponding Twisted Edwards curve. // See e.g. for details on the correspondences. + use crate::cmp::Eq; use crate::ec::montcurve::affine; use crate::ec::swcurve::curvegroup::Curve as SWCurve; use crate::ec::swcurve::curvegroup::Point as SWPoint; use crate::ec::tecurve::curvegroup::Curve as TECurve; use crate::ec::tecurve::curvegroup::Point as TEPoint; - use crate::cmp::Eq; pub struct Curve { // Montgomery Curve configuration (ky^2 z = x*(x^2 + j*x*z + z*z)) pub j: Field, diff --git a/noir_stdlib/src/ec/swcurve.nr b/noir_stdlib/src/ec/swcurve.nr index 145b2506f73..d9c1cf8c8c7 100644 --- a/noir_stdlib/src/ec/swcurve.nr +++ b/noir_stdlib/src/ec/swcurve.nr @@ -3,11 +3,11 @@ pub mod affine { // Points are represented by two-dimensional Cartesian coordinates. // Group operations are implemented in terms of those in CurveGroup (in this case, extended Twisted Edwards) coordinates // for reasons of efficiency, cf. . - use crate::ec::swcurve::curvegroup; - use crate::ec::safe_inverse; + use crate::cmp::Eq; use crate::ec::is_square; + use crate::ec::safe_inverse; use crate::ec::sqrt; - use crate::cmp::Eq; + use crate::ec::swcurve::curvegroup; // Curve specification pub struct Curve { // Short Weierstrass curve @@ -190,8 +190,8 @@ pub mod curvegroup { // CurveGroup representation of Weierstrass curves // Points are represented by three-dimensional Jacobian coordinates. // See for details. - use crate::ec::swcurve::affine; use crate::cmp::Eq; + use crate::ec::swcurve::affine; // Curve specification pub struct Curve { // Short Weierstrass curve diff --git a/noir_stdlib/src/ec/tecurve.nr b/noir_stdlib/src/ec/tecurve.nr index 0088896015d..8512413c831 100644 --- a/noir_stdlib/src/ec/tecurve.nr +++ b/noir_stdlib/src/ec/tecurve.nr @@ -4,12 +4,12 @@ pub mod affine { // Group operations are implemented in terms of those in CurveGroup (in this case, extended Twisted Edwards) coordinates // for reasons of efficiency. // See for details. - use crate::ec::tecurve::curvegroup; + use crate::cmp::Eq; use crate::ec::montcurve::affine::Curve as MCurve; use crate::ec::montcurve::affine::Point as MPoint; use crate::ec::swcurve::affine::Curve as SWCurve; use crate::ec::swcurve::affine::Point as SWPoint; - use crate::cmp::Eq; + use crate::ec::tecurve::curvegroup; // Curve specification pub struct Curve { // Twisted Edwards curve @@ -197,12 +197,12 @@ pub mod curvegroup { // CurveGroup coordinate representation of Twisted Edwards curves // Points are represented by four-dimensional projective coordinates, viz. extended Twisted Edwards coordinates. // See section 3 of for details. - use crate::ec::tecurve::affine; + use crate::cmp::Eq; use crate::ec::montcurve::curvegroup::Curve as MCurve; use crate::ec::montcurve::curvegroup::Point as MPoint; use crate::ec::swcurve::curvegroup::Curve as SWCurve; use crate::ec::swcurve::curvegroup::Point as SWPoint; - use crate::cmp::Eq; + use crate::ec::tecurve::affine; // Curve specification pub struct Curve { // Twisted Edwards curve diff --git a/noir_stdlib/src/eddsa.nr b/noir_stdlib/src/eddsa.nr index 89a0b05b072..c049b7abbb5 100644 --- a/noir_stdlib/src/eddsa.nr +++ b/noir_stdlib/src/eddsa.nr @@ -1,8 +1,8 @@ +use crate::default::Default; use crate::ec::consts::te::baby_jubjub; use crate::ec::tecurve::affine::Point as TEPoint; use crate::hash::Hasher; use crate::hash::poseidon::PoseidonHasher; -use crate::default::Default; // Returns true if signature is valid pub fn eddsa_poseidon_verify( diff --git a/noir_stdlib/src/embedded_curve_ops.nr b/noir_stdlib/src/embedded_curve_ops.nr index dd5e4285c00..6b225ee18b2 100644 --- a/noir_stdlib/src/embedded_curve_ops.nr +++ b/noir_stdlib/src/embedded_curve_ops.nr @@ -1,5 +1,5 @@ -use crate::ops::arith::{Add, Sub, Neg}; use crate::cmp::Eq; +use crate::ops::arith::{Add, Neg, Sub}; /// A point on the embedded elliptic curve /// By definition, the base field of the embedded curve is the scalar field of the proof system curve, i.e the Noir Field. diff --git a/noir_stdlib/src/field/bn254.nr b/noir_stdlib/src/field/bn254.nr index 356b47e63cf..9642c2aa1b8 100644 --- a/noir_stdlib/src/field/bn254.nr +++ b/noir_stdlib/src/field/bn254.nr @@ -141,7 +141,7 @@ pub fn lt(a: Field, b: Field) -> bool { mod tests { // TODO: Allow imports from "super" use crate::field::bn254::{ - decompose, compute_lt, assert_gt, gt, TWO_POW_128, compute_lte, PLO, PHI, + assert_gt, compute_lt, compute_lte, decompose, gt, PHI, PLO, TWO_POW_128, }; #[test] diff --git a/noir_stdlib/src/field/mod.nr b/noir_stdlib/src/field/mod.nr index 915ea8f939e..b632cf1f7a2 100644 --- a/noir_stdlib/src/field/mod.nr +++ b/noir_stdlib/src/field/mod.nr @@ -1,6 +1,6 @@ pub mod bn254; -use bn254::lt as bn254_lt; use crate::runtime::is_unconstrained; +use bn254::lt as bn254_lt; impl Field { /// Asserts that `self` can be represented in `bit_size` bits. diff --git a/noir_stdlib/src/hash/mimc.nr b/noir_stdlib/src/hash/mimc.nr index 6045ae3dbdb..55c06964122 100644 --- a/noir_stdlib/src/hash/mimc.nr +++ b/noir_stdlib/src/hash/mimc.nr @@ -1,5 +1,5 @@ -use crate::hash::Hasher; use crate::default::Default; +use crate::hash::Hasher; // mimc-p/p implementation // constants are (publicly generated) random numbers, for instance using keccak as a ROM. diff --git a/noir_stdlib/src/hash/mod.nr b/noir_stdlib/src/hash/mod.nr index 609017d70aa..15112757312 100644 --- a/noir_stdlib/src/hash/mod.nr +++ b/noir_stdlib/src/hash/mod.nr @@ -6,11 +6,11 @@ pub mod sha256; pub mod sha512; use crate::default::Default; -use crate::uint128::U128; use crate::embedded_curve_ops::{ EmbeddedCurvePoint, EmbeddedCurveScalar, multi_scalar_mul, multi_scalar_mul_array_return, }; use crate::meta::derive_via; +use crate::uint128::U128; // Kept for backwards compatibility pub use sha256::{digest, sha256, sha256_compression, sha256_var}; diff --git a/noir_stdlib/src/hash/poseidon/bn254/consts.nr b/noir_stdlib/src/hash/poseidon/bn254/consts.nr index 835ed3ea476..42df0ef662f 100644 --- a/noir_stdlib/src/hash/poseidon/bn254/consts.nr +++ b/noir_stdlib/src/hash/poseidon/bn254/consts.nr @@ -2,8 +2,8 @@ // Used like so: sage generate_parameters_grain.sage 1 0 254 2 8 56 0x30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000001 // Constants for various Poseidon instances in the case of the prime field of the same order as BN254. // Consistent with https://github.com/iden3/circomlib/blob/master/circuits/poseidon.circom and https://github.com/iden3/circomlib/blob/master/circuits/poseidon_constants.circom -use crate::hash::poseidon::PoseidonConfig; use crate::hash::poseidon::config; +use crate::hash::poseidon::PoseidonConfig; // S-box power fn alpha() -> Field { 5 diff --git a/noir_stdlib/src/hash/poseidon/mod.nr b/noir_stdlib/src/hash/poseidon/mod.nr index 0af7951b8dc..6f0e461a610 100644 --- a/noir_stdlib/src/hash/poseidon/mod.nr +++ b/noir_stdlib/src/hash/poseidon/mod.nr @@ -1,6 +1,6 @@ pub mod bn254; // Instantiations of Poseidon for prime field of the same order as BN254 -use crate::hash::Hasher; use crate::default::Default; +use crate::hash::Hasher; // A config struct defining the parameters of the Poseidon instance to use. // diff --git a/noir_stdlib/src/hash/poseidon2.nr b/noir_stdlib/src/hash/poseidon2.nr index 517c2cd8f5f..f2167c43c2c 100644 --- a/noir_stdlib/src/hash/poseidon2.nr +++ b/noir_stdlib/src/hash/poseidon2.nr @@ -1,5 +1,5 @@ -use crate::hash::Hasher; use crate::default::Default; +use crate::hash::Hasher; comptime global RATE: u32 = 3; diff --git a/noir_stdlib/src/meta/expr.nr b/noir_stdlib/src/meta/expr.nr index 1b04a97ab15..bb795a520d3 100644 --- a/noir_stdlib/src/meta/expr.nr +++ b/noir_stdlib/src/meta/expr.nr @@ -1,8 +1,8 @@ //! Contains methods on the built-in `Expr` type for quoted, syntactically valid expressions. -use crate::option::Option; -use crate::meta::op::UnaryOp; use crate::meta::op::BinaryOp; +use crate::meta::op::UnaryOp; +use crate::option::Option; impl Expr { /// If this expression is an array literal `[elem1, ..., elemN]`, this returns a slice of each element in the array. diff --git a/noir_stdlib/src/meta/trait_constraint.nr b/noir_stdlib/src/meta/trait_constraint.nr index bf22f454448..2d9c6639dbd 100644 --- a/noir_stdlib/src/meta/trait_constraint.nr +++ b/noir_stdlib/src/meta/trait_constraint.nr @@ -1,5 +1,5 @@ -use crate::hash::{Hash, Hasher}; use crate::cmp::Eq; +use crate::hash::{Hash, Hasher}; impl Eq for TraitConstraint { comptime fn eq(self, other: Self) -> bool { diff --git a/noir_stdlib/src/meta/trait_def.nr b/noir_stdlib/src/meta/trait_def.nr index cc448b2eae5..75f746337d4 100644 --- a/noir_stdlib/src/meta/trait_def.nr +++ b/noir_stdlib/src/meta/trait_def.nr @@ -1,5 +1,5 @@ -use crate::hash::{Hash, Hasher}; use crate::cmp::Eq; +use crate::hash::{Hash, Hasher}; impl TraitDefinition { #[builtin(trait_def_as_trait_constraint)] diff --git a/noir_stdlib/src/ops/mod.nr b/noir_stdlib/src/ops/mod.nr index bf908ea4b27..0e3a2467c60 100644 --- a/noir_stdlib/src/ops/mod.nr +++ b/noir_stdlib/src/ops/mod.nr @@ -1,5 +1,5 @@ pub(crate) mod arith; pub(crate) mod bit; -pub use arith::{Add, Sub, Mul, Div, Rem, Neg}; -pub use bit::{Not, BitOr, BitAnd, BitXor, Shl, Shr}; +pub use arith::{Add, Div, Mul, Neg, Rem, Sub}; +pub use bit::{BitAnd, BitOr, BitXor, Not, Shl, Shr}; diff --git a/noir_stdlib/src/option.nr b/noir_stdlib/src/option.nr index 5db2ce84efd..0a62e412849 100644 --- a/noir_stdlib/src/option.nr +++ b/noir_stdlib/src/option.nr @@ -1,6 +1,6 @@ -use crate::hash::{Hash, Hasher}; -use crate::cmp::{Ordering, Ord, Eq}; +use crate::cmp::{Eq, Ord, Ordering}; use crate::default::Default; +use crate::hash::{Hash, Hasher}; pub struct Option { _is_some: bool, diff --git a/noir_stdlib/src/prelude.nr b/noir_stdlib/src/prelude.nr index b6e54eaae60..a4a6c35b615 100644 --- a/noir_stdlib/src/prelude.nr +++ b/noir_stdlib/src/prelude.nr @@ -1,10 +1,10 @@ -pub use crate::collections::vec::Vec; -pub use crate::collections::bounded_vec::BoundedVec; -pub use crate::option::Option; -pub use crate::{print, println, assert_constant}; -pub use crate::uint128::U128; +pub use crate::{assert_constant, print, println}; pub use crate::cmp::{Eq, Ord}; -pub use crate::default::Default; +pub use crate::collections::bounded_vec::BoundedVec; +pub use crate::collections::vec::Vec; pub use crate::convert::{From, Into}; +pub use crate::default::Default; pub use crate::meta::{derive, derive_via}; +pub use crate::option::Option; pub use crate::panic::panic; +pub use crate::uint128::U128; diff --git a/noir_stdlib/src/uint128.nr b/noir_stdlib/src/uint128.nr index a4e20859604..06febb3ee00 100644 --- a/noir_stdlib/src/uint128.nr +++ b/noir_stdlib/src/uint128.nr @@ -1,5 +1,5 @@ -use crate::ops::{Add, Sub, Mul, Div, Rem, Not, BitOr, BitAnd, BitXor, Shl, Shr}; use crate::cmp::{Eq, Ord, Ordering}; +use crate::ops::{Add, BitAnd, BitOr, BitXor, Div, Mul, Not, Rem, Shl, Shr, Sub}; global pow64: Field = 18446744073709551616; //2^64; global pow63: Field = 9223372036854775808; // 2^63; @@ -313,7 +313,7 @@ impl Shr for U128 { } mod tests { - use crate::uint128::{U128, pow64, pow63}; + use crate::uint128::{pow63, pow64, U128}; #[test] fn test_not(lo: u64, hi: u64) { diff --git a/test_programs/compile_success_empty/ec_baby_jubjub/src/main.nr b/test_programs/compile_success_empty/ec_baby_jubjub/src/main.nr index 935b1c613ad..caaa51d84f0 100644 --- a/test_programs/compile_success_empty/ec_baby_jubjub/src/main.nr +++ b/test_programs/compile_success_empty/ec_baby_jubjub/src/main.nr @@ -6,9 +6,9 @@ use std::ec::tecurve::curvegroup::Point as G; use std::ec::swcurve::affine::Point as SWGaffine; use std::ec::swcurve::curvegroup::Point as SWG; +use std::compat; use std::ec::montcurve::affine::Point as MGaffine; use std::ec::montcurve::curvegroup::Point as MG; -use std::compat; fn main() { // This test only makes sense if Field is the right prime field. diff --git a/test_programs/compile_success_empty/mod_nr_entrypoint/src/main.nr b/test_programs/compile_success_empty/mod_nr_entrypoint/src/main.nr index 620fd99f6ec..972a64bfeb9 100644 --- a/test_programs/compile_success_empty/mod_nr_entrypoint/src/main.nr +++ b/test_programs/compile_success_empty/mod_nr_entrypoint/src/main.nr @@ -1,6 +1,6 @@ -use crate::foo::in_foo_mod; -use crate::foo::bar::in_bar_mod; use crate::baz::in_baz_mod; +use crate::foo::bar::in_bar_mod; +use crate::foo::in_foo_mod; mod foo; mod baz; diff --git a/test_programs/compile_success_empty/reexports/src/main.nr b/test_programs/compile_success_empty/reexports/src/main.nr index 0fd65a33564..6b568bea682 100644 --- a/test_programs/compile_success_empty/reexports/src/main.nr +++ b/test_programs/compile_success_empty/reexports/src/main.nr @@ -1,4 +1,4 @@ -use reexporting_lib::{FooStruct, MyStruct, lib}; +use reexporting_lib::{FooStruct, lib, MyStruct}; fn main() { let x: FooStruct = MyStruct { inner: 0 }; diff --git a/test_programs/compile_success_empty/turbofish_call_func_diff_types/src/main.nr b/test_programs/compile_success_empty/turbofish_call_func_diff_types/src/main.nr index 535d7b18137..e44df34a193 100644 --- a/test_programs/compile_success_empty/turbofish_call_func_diff_types/src/main.nr +++ b/test_programs/compile_success_empty/turbofish_call_func_diff_types/src/main.nr @@ -1,6 +1,6 @@ use std::hash::Hasher; -use std::hash::poseidon2::Poseidon2Hasher; use std::hash::poseidon::PoseidonHasher; +use std::hash::poseidon2::Poseidon2Hasher; fn main(x: Field, y: pub Field) { let mut hasher = PoseidonHasher::default(); diff --git a/test_programs/execution_success/binary_operator_overloading/src/main.nr b/test_programs/execution_success/binary_operator_overloading/src/main.nr index a5e12fd5da9..03a4e1ed22f 100644 --- a/test_programs/execution_success/binary_operator_overloading/src/main.nr +++ b/test_programs/execution_success/binary_operator_overloading/src/main.nr @@ -1,5 +1,5 @@ -use std::ops::{Add, Sub, Mul, Div, Rem, BitAnd, BitOr, BitXor, Shl, Shr}; use std::cmp::Ordering; +use std::ops::{Add, BitAnd, BitOr, BitXor, Div, Mul, Rem, Shl, Shr, Sub}; // x = 3, y = 9 fn main(x: u32, y: u32) { diff --git a/test_programs/execution_success/eddsa/src/main.nr b/test_programs/execution_success/eddsa/src/main.nr index d0ce8e70053..48e1e4af9fb 100644 --- a/test_programs/execution_success/eddsa/src/main.nr +++ b/test_programs/execution_success/eddsa/src/main.nr @@ -1,8 +1,8 @@ use std::compat; use std::ec::consts::te::baby_jubjub; use std::ec::tecurve::affine::Point as TEPoint; +use std::eddsa::{eddsa_poseidon_verify, eddsa_to_pub, eddsa_verify}; use std::hash; -use std::eddsa::{eddsa_to_pub, eddsa_poseidon_verify, eddsa_verify}; use std::hash::poseidon2::Poseidon2Hasher; fn main(msg: pub Field, _priv_key_a: Field, _priv_key_b: Field) { diff --git a/test_programs/execution_success/regression_3889/src/main.nr b/test_programs/execution_success/regression_3889/src/main.nr index 2b54ae54418..dfd9e8c2c85 100644 --- a/test_programs/execution_success/regression_3889/src/main.nr +++ b/test_programs/execution_success/regression_3889/src/main.nr @@ -5,8 +5,8 @@ mod Foo { } mod Bar { - use crate::Foo::NewType as BarStruct; use crate::Foo::NewType; + use crate::Foo::NewType as BarStruct; } mod Baz { diff --git a/tooling/nargo_fmt/src/config.rs b/tooling/nargo_fmt/src/config.rs index 6a1a019f18d..488647c0b39 100644 --- a/tooling/nargo_fmt/src/config.rs +++ b/tooling/nargo_fmt/src/config.rs @@ -1,5 +1,7 @@ use std::path::Path; +use serde::{Deserialize, Serialize}; + use crate::errors::ConfigError; macro_rules! config { @@ -49,6 +51,8 @@ config! { array_width: usize, 100, "Maximum width of an array literal before falling back to vertical formatting"; fn_call_width: usize, 60, "Maximum width of the args of a function call before falling back to vertical formatting"; single_line_if_else_max_width: usize, 50, "Maximum line length for single line if-else expressions"; + imports_granularity: ImportsGranularity, ImportsGranularity::Preserve, "How imports should be grouped into use statements."; + reorder_imports: bool, true, "Reorder imports alphabetically"; } impl Config { @@ -71,3 +75,13 @@ impl Config { Ok(config) } } + +/// How imports should be grouped into use statements. +/// Imports will be merged or split to the configured level of granularity. +#[derive(Serialize, Deserialize, Copy, Clone, PartialEq, Eq)] +pub enum ImportsGranularity { + /// Do not change the granularity of any imports and preserve the original structure written by the developer. + Preserve, + /// Merge imports from the same crate into a single use statement. + Crate, +} diff --git a/tooling/nargo_fmt/src/formatter.rs b/tooling/nargo_fmt/src/formatter.rs index 558dab2829e..4ae5443a2cc 100644 --- a/tooling/nargo_fmt/src/formatter.rs +++ b/tooling/nargo_fmt/src/formatter.rs @@ -31,6 +31,7 @@ mod traits; mod type_expression; mod types; mod use_tree; +mod use_tree_merge; mod visibility; mod where_clause; @@ -107,21 +108,12 @@ impl<'a> Formatter<'a> { self.format_parsed_module(parsed_module, self.ignore_next); } - pub(crate) fn format_parsed_module( - &mut self, - parsed_module: ParsedModule, - mut ignore_next: bool, - ) { + pub(crate) fn format_parsed_module(&mut self, parsed_module: ParsedModule, ignore_next: bool) { if !parsed_module.inner_doc_comments.is_empty() { self.format_inner_doc_comments(); } - for item in parsed_module.items { - self.format_item(item, ignore_next); - self.write_line(); - ignore_next = self.ignore_next; - } - + self.format_items(parsed_module.items, ignore_next); self.write_line(); } @@ -249,7 +241,14 @@ impl<'a> Formatter<'a> { pub(super) fn write_and_skip_span_without_formatting(&mut self, span: Span) { self.write_source_span(span); - while self.token_span.start() < span.end() { + while self.token_span.start() < span.end() && self.token != Token::EOF { + self.bump(); + } + } + + /// Advances the lexer until past the given span end without writing anything to the buffer. + pub(super) fn skip_past_span_end_without_formatting(&mut self, span_end: u32) { + while self.token_span.start() < span_end && self.token != Token::EOF { self.bump(); } } diff --git a/tooling/nargo_fmt/src/formatter/item.rs b/tooling/nargo_fmt/src/formatter/item.rs index 072f45278ee..77f1cd10cbc 100644 --- a/tooling/nargo_fmt/src/formatter/item.rs +++ b/tooling/nargo_fmt/src/formatter/item.rs @@ -1,8 +1,42 @@ -use noirc_frontend::parser::{Item, ItemKind}; +use noirc_frontend::{ + ast::{ItemVisibility, UseTree}, + hir::resolution::errors::Span, + parser::{Item, ItemKind}, +}; + +use crate::config::ImportsGranularity; use super::Formatter; impl<'a> Formatter<'a> { + pub(super) fn format_items(&mut self, mut items: Vec, mut ignore_next: bool) { + // Reverse the items because we'll be processing them one by one, and it's a bit + // more efficient to pop than to shift. + items.reverse(); + + while !items.is_empty() { + // Format the next import group, if there is one. + let import_group = self.next_import_group(&mut items); + if let Some(import_group) = import_group { + self.merge_and_format_imports(import_group.imports, import_group.visibility); + self.skip_past_span_end_without_formatting(import_group.span_end); + self.write_line(); + ignore_next = self.ignore_next; + + // Continue from the top because the next thing that comes might be another import group + continue; + } + + if let Some(item) = items.pop() { + self.format_item(item, ignore_next); + self.write_line(); + ignore_next = self.ignore_next; + } else { + break; + } + } + } + pub(super) fn format_item(&mut self, item: Item, mut ignore_next: bool) { self.skip_comments_and_whitespace(); @@ -42,4 +76,100 @@ impl<'a> Formatter<'a> { ItemKind::InnerAttribute(..) => self.format_inner_attribute(), } } + + /// Returns the next import group, if there's is one. + /// + /// An import group is one or more `use` statements that all have the same visibility, + /// as long as exactly one newline separates them, and as long as there are no comments + /// in the `use` statements or trailing comments in them. + /// + /// Each import group will be sorted and merged, if the configuration is set to do so. + fn next_import_group(&self, items: &mut Vec) -> Option { + if self.config.imports_granularity == ImportsGranularity::Preserve + && !self.config.reorder_imports + { + return None; + } + + let mut imports = Vec::new(); + + let item = items.last()?; + if self.span_has_comments(item.span) { + return None; + } + + let ItemKind::Import(..) = item.kind else { + return None; + }; + + let item = items.pop().unwrap(); + let ItemKind::Import(use_tree, visibility) = item.kind else { + panic!("Expected import, got {:?}", item.kind); + }; + + imports.push(use_tree); + let mut span_end = item.span.end(); + + while let Some(item) = items.last() { + if self.span_is_import_group_separator(Span::from(span_end..item.span.start())) { + break; + } + + let next_item_start = if items.len() > 1 { + if let Some(next_item) = items.get(items.len() - 2) { + next_item.span.start() + } else { + self.source.len() as u32 + } + } else { + self.source.len() as u32 + }; + + if self.span_starts_with_trailing_comment(Span::from(item.span.end()..next_item_start)) + { + break; + } + + let ItemKind::Import(_, next_visibility) = &item.kind else { + break; + }; + + if visibility != *next_visibility { + break; + } + + let item = items.pop().unwrap(); + let ItemKind::Import(use_tree, _) = item.kind else { + panic!("Expected import, got {:?}", item.kind); + }; + imports.push(use_tree); + span_end = item.span.end(); + } + + Some(ImportGroup { imports, visibility, span_end }) + } + + fn span_has_comments(&self, span: Span) -> bool { + let slice = &self.source[span.start() as usize..span.end() as usize]; + slice.contains("/*") || slice.contains("//") + } + + fn span_starts_with_trailing_comment(&self, span: Span) -> bool { + let slice = &self.source[span.start() as usize..span.end() as usize]; + slice.trim_start_matches(' ').starts_with("//") + } + + /// Returns true if there at most one newline in the given span and it contains no comments. + fn span_is_import_group_separator(&self, span: Span) -> bool { + let slice = &self.source[span.start() as usize..span.end() as usize]; + let number_of_newlines = slice.chars().filter(|char| *char == '\n').count(); + number_of_newlines > 1 || slice.contains("//") || slice.contains("/*") + } +} + +#[derive(Debug)] +struct ImportGroup { + imports: Vec, + visibility: ItemVisibility, + span_end: u32, } diff --git a/tooling/nargo_fmt/src/formatter/use_tree.rs b/tooling/nargo_fmt/src/formatter/use_tree.rs index 0c99bc7ee79..bc8dc3fcabb 100644 --- a/tooling/nargo_fmt/src/formatter/use_tree.rs +++ b/tooling/nargo_fmt/src/formatter/use_tree.rs @@ -68,6 +68,11 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { })); } UseTreeKind::List(use_trees) => { + // We check if there are nested lists. If yes, then each item will be on a separate line + // (it reads better, and this is what rustfmt seems to do too) + let has_nested_list = + use_trees.iter().any(|use_tree| matches!(use_tree.kind, UseTreeKind::List(..))); + let use_trees_len = use_trees.len(); let left_brace_chunk = self.chunk(|formatter| { @@ -99,6 +104,10 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { items_chunk.chunks.into_iter().filter_map(Chunk::group).next().unwrap(); group.chunks.extend(single_group.chunks); } else { + if has_nested_list { + group.one_chunk_per_line = true; + } + group.text(left_brace_chunk); group.chunks.extend(items_chunk.chunks); group.text(right_brace_chunk); @@ -112,10 +121,29 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> { #[cfg(test)] mod tests { - use crate::{assert_format, assert_format_with_max_width}; + use crate::{assert_format_with_config, config::ImportsGranularity, Config}; + + fn assert_format(src: &str, expected: &str) { + let config = Config { + imports_granularity: ImportsGranularity::Preserve, + reorder_imports: false, + ..Config::default() + }; + assert_format_with_config(src, expected, config); + } + + fn assert_format_with_max_width(src: &str, expected: &str, max_width: usize) { + let config = Config { + imports_granularity: ImportsGranularity::Preserve, + reorder_imports: false, + max_width, + ..Config::default() + }; + assert_format_with_config(src, expected, config); + } #[test] - fn format_simple_use() { + fn format_simple_use_without_alias() { let src = " mod moo { pub use foo ; }"; let expected = "mod moo { pub use foo; @@ -152,7 +180,9 @@ mod tests { fn format_use_trees_with_max_width() { let src = " use foo::{ bar, baz , qux , one::{two, three} };"; let expected = "use foo::{ - bar, baz, qux, + bar, + baz, + qux, one::{ two, three, }, @@ -175,7 +205,7 @@ mod tests { four, five, }; "; - assert_format_with_max_width(src, expected, 20); + assert_format_with_max_width(src, expected, 25); } #[test] @@ -201,4 +231,33 @@ mod tests { "; assert_format_with_max_width(src, expected, "use crate::hash::{Hash, Hasher}".len()); } + + #[test] + fn do_not_merge_and_sort_imports() { + let src = " +use aztec::{ + context::Context, log::emit_unencrypted_log, note::{ + note_getter_options::NoteGetterOptions, note_header::NoteHeader, + }, state_vars::{ + Map, PrivateSet, PublicMutable, + }, types::{ + address::AztecAddress, type_serialization::field_serialization::{ + FIELD_SERIALIZED_LEN, FieldSerializationMethods, + }, + }, +}; + "; + let expected = "use aztec::{ + context::Context, + log::emit_unencrypted_log, + note::{note_getter_options::NoteGetterOptions, note_header::NoteHeader}, + state_vars::{Map, PrivateSet, PublicMutable}, + types::{ + address::AztecAddress, + type_serialization::field_serialization::{FIELD_SERIALIZED_LEN, FieldSerializationMethods}, + }, +}; +"; + assert_format(src, expected); + } } diff --git a/tooling/nargo_fmt/src/formatter/use_tree_merge.rs b/tooling/nargo_fmt/src/formatter/use_tree_merge.rs new file mode 100644 index 00000000000..e24b7b8cbf6 --- /dev/null +++ b/tooling/nargo_fmt/src/formatter/use_tree_merge.rs @@ -0,0 +1,627 @@ +use std::{ + cmp::Ordering, + collections::BTreeMap, + fmt::{self, Display}, +}; + +use noirc_frontend::ast::{ItemVisibility, PathKind, UseTree, UseTreeKind}; + +use crate::{ + chunks::{ChunkGroup, TextChunk}, + config::ImportsGranularity, +}; + +use super::Formatter; + +impl<'a> Formatter<'a> { + pub(super) fn merge_and_format_imports( + &mut self, + imports: Vec, + visibility: ItemVisibility, + ) { + match self.config.imports_granularity { + ImportsGranularity::Preserve => { + let mut import_trees: Vec = + imports.into_iter().map(|import| merge_imports(vec![import])).collect(); + import_trees.sort(); + + for (index, import_tree) in import_trees.into_iter().enumerate() { + if index > 0 { + self.write_line_without_skipping_whitespace_and_comments(); + } + + self.format_import_tree(import_tree, visibility); + } + } + ImportsGranularity::Crate => { + let import_tree = merge_imports(imports); + self.format_import_tree(import_tree, visibility); + } + } + } + + fn format_import_tree(&mut self, import_tree: ImportTree, visibility: ItemVisibility) { + for (index, (segment, segment_tree)) in import_tree.tree.into_iter().enumerate() { + if index > 0 { + self.write_line_without_skipping_whitespace_and_comments(); + } + + let tree = ImportTree::single(segment, *segment_tree); + let tree = tree.simplify(); + + let group = format_merged_import_with_visibility(tree, visibility); + self.write_indentation(); + self.format_chunk_group(group); + } + } +} + +// The logic here is similar to that of `use_tree.rs`, except that it works with ImportTree +// instead of UseTree and we never check or advance the lexer. + +fn format_merged_import_with_visibility( + mut import_tree: ImportTree, + visibility: ItemVisibility, +) -> ChunkGroup { + let mut group = ChunkGroup::new(); + match visibility { + ItemVisibility::Private => (), + ItemVisibility::PublicCrate => { + group.text(TextChunk::new("pub(crate) ".to_string())); + } + ItemVisibility::Public => { + group.text(TextChunk::new("pub ".to_string())); + } + } + group.text(TextChunk::new("use ".to_string())); + + let (segment, tree) = import_tree.tree.pop_first().unwrap(); + assert!(import_tree.tree.is_empty()); + + group.group(format_merged_import(segment, *tree)); + group.text_attached_to_last_group(TextChunk::new(";".to_string())); + group +} + +fn format_merged_import(segment: Segment, import_tree: ImportTree) -> ChunkGroup { + let mut group = ChunkGroup::new(); + + group.text(TextChunk::new(segment.to_string())); + + if import_tree.tree.is_empty() { + return group; + } + + // We check if there are nested lists. If yes, then each item will be on a separate line + // (it reads better, and this is what rustfmt seems to do too) + if import_tree.tree.values().all(|tree| tree.tree.is_empty()) { + group.one_chunk_per_line = false; + } + + group.text(TextChunk::new("::{".to_string())); + group.increase_indentation(); + group.line(); + + for (index, (segment, import_tree)) in import_tree.tree.into_iter().enumerate() { + if index > 0 { + group.text_attached_to_last_group(TextChunk::new(",".to_string())); + group.space_or_line(); + } + + group.group(format_merged_import(segment, *import_tree)); + } + + group.trailing_comma(); + group.decrease_indentation(); + group.line(); + group.text(TextChunk::new("}".to_string())); + + group +} + +/// We keep Crate, Super and Dep as special segments so that they are ordered in that way +/// (they'll come in that order before any plain segment). +#[derive(Debug, PartialEq, Eq)] +enum Segment { + /// Represents the end of a path. + /// This is needed because we have want to merge "foo" and "foo::bar", + /// we need to know that "foo" is the end of a path, and "foo::bar" is another one. + /// If we don't, merging "foo" and "foo::bar" will result in just "foo::bar", loosing "foo", + /// when we actually want "foo::{self, bar}". + SelfReference, + Crate, + Super, + Dep, + Plain(String), +} + +impl Segment { + /// Combines two segments into a single one, by joining them with "::". + fn combine(self, other: Segment) -> Segment { + if other == Segment::SelfReference { + self + } else { + Segment::Plain(format!("{}::{}", self, other)) + } + } + + fn order_number(&self) -> usize { + match self { + Segment::SelfReference => 0, + Segment::Crate => 1, + Segment::Super => 2, + Segment::Dep => 3, + Segment::Plain(_) => 4, + } + } +} + +impl Display for Segment { + fn fmt(&self, f: &mut std::fmt::Formatter) -> fmt::Result { + match self { + Segment::Crate => write!(f, "crate"), + Segment::Super => write!(f, "super"), + Segment::Dep => write!(f, "dep"), + Segment::Plain(s) => write!(f, "{}", s), + Segment::SelfReference => write!(f, "self"), + } + } +} + +impl PartialOrd for Segment { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for Segment { + fn cmp(&self, other: &Self) -> Ordering { + let order_number_ordering = self.order_number().cmp(&other.order_number()); + if order_number_ordering != Ordering::Equal { + return order_number_ordering; + } + + if let (Segment::Plain(self_string), Segment::Plain(other_string)) = (self, other) { + // Case-insensitive comparison for plain segments + self_string.to_lowercase().cmp(&other_string.to_lowercase()) + } else { + order_number_ordering + } + } +} + +/// An import tree to represent merged imports. +/// For example for the given imports: +/// +/// use foo::bar::{baz, qux}; +/// use foo::another; +/// +/// an ImportTree that represents the merged imports would be: +/// +/// { +/// "foo" => { +/// "another" => {} +/// "bar" => {"baz", "qux"}, +/// } +/// } +#[derive(Debug, Default, PartialEq, Eq, PartialOrd, Ord)] +struct ImportTree { + tree: BTreeMap>, +} + +impl ImportTree { + fn new() -> Self { + Self { tree: BTreeMap::new() } + } + + /// Creates an import tree that has `segment` as the only element with `tree` as its value. + fn single(segment: Segment, tree: ImportTree) -> Self { + let mut tree_map = BTreeMap::new(); + tree_map.insert(segment, Box::new(tree)); + Self { tree: tree_map } + } + + /// Inserts a segment to the tree, creating the necessary empty children if they don't exist yet. + fn insert(&mut self, segment: Segment) -> &mut ImportTree { + self.tree.entry(segment).or_default() + } + + /// Simplifies a tree by combining segments that only have one child. + /// + /// For example, this tree: + /// + /// { + /// "foo" => { + /// "bar" => {"baz", "qux"} + /// } + /// } + /// + /// will be simplified to: + /// + /// { + /// "foo::bar" => {"baz", "qux"} + /// } + fn simplify(self) -> ImportTree { + let mut new_tree = ImportTree::new(); + for (segment, tree) in self.tree.into_iter() { + let mut tree = tree.simplify(); + if tree.tree.len() == 1 { + let (first_segment, first_tree) = tree.tree.pop_first().unwrap(); + let new_segment = segment.combine(first_segment); + new_tree.tree.insert(new_segment, first_tree); + } else { + new_tree.tree.insert(segment, Box::new(tree)); + } + } + new_tree + } +} + +/// Combines all use trees to form a single ImportTree. +fn merge_imports(imports: Vec) -> ImportTree { + let mut tree = ImportTree::new(); + merge_imports_in_tree(imports, &mut tree); + tree +} + +fn merge_imports_in_tree(imports: Vec, mut tree: &mut ImportTree) { + for import in imports { + let mut tree = match import.prefix.kind { + PathKind::Crate => tree.insert(Segment::Crate), + PathKind::Super => tree.insert(Segment::Super), + PathKind::Dep => tree.insert(Segment::Dep), + PathKind::Plain => &mut tree, + }; + + for segment in import.prefix.segments { + tree = tree.insert(Segment::Plain(segment.ident.to_string())); + } + + match import.kind { + UseTreeKind::Path(ident, alias) => { + if let Some(alias) = alias { + tree = tree.insert(Segment::Plain(format!("{} as {}", ident, alias))); + tree.insert(Segment::SelfReference); + } else if ident.0.contents == "self" { + tree.insert(Segment::SelfReference); + } else { + tree = tree.insert(Segment::Plain(ident.to_string())); + tree.insert(Segment::SelfReference); + } + } + UseTreeKind::List(trees) => { + merge_imports_in_tree(trees, tree); + } + } + } +} + +#[cfg(test)] +mod tests { + use crate::{assert_format_with_config, config::ImportsGranularity, Config}; + + fn assert_format(src: &str, expected: &str) { + let config = Config { + imports_granularity: ImportsGranularity::Crate, + reorder_imports: true, + ..Config::default() + }; + assert_format_with_config(src, expected, config); + } + + fn assert_format_with_max_width(src: &str, expected: &str, max_width: usize) { + let config = Config { + imports_granularity: ImportsGranularity::Crate, + reorder_imports: true, + max_width, + ..Config::default() + }; + assert_format_with_config(src, expected, config); + } + + fn assert_format_preserving_granularity(src: &str, expected: &str) { + let config = Config { + imports_granularity: ImportsGranularity::Preserve, + reorder_imports: true, + ..Config::default() + }; + assert_format_with_config(src, expected, config); + } + + #[test] + fn format_simple_use_without_alias() { + let src = " mod moo { pub use foo ; }"; + let expected = "mod moo { + pub use foo; +} +"; + assert_format(src, expected); + } + + #[test] + fn format_simple_use_with_alias() { + let src = " mod moo { use foo :: bar as baz ; }"; + let expected = "mod moo { + use foo::bar as baz; +} +"; + assert_format(src, expected); + } + + #[test] + fn format_simple_use_with_path_kind() { + let src = "use super :: foo ;"; + let expected = "use super::foo;\n"; + assert_format(src, expected); + } + + #[test] + fn format_use_list_two_items() { + let src = " use foo::{ bar, baz };"; + let expected = "use foo::{bar, baz};\n"; + assert_format(src, expected); + } + + #[test] + fn format_use_trees_with_max_width() { + let src = " use foo::{ bar, baz , qux , one::{two, three} };"; + let expected = "use foo::{ + bar, + baz, + one::{ + three, two, + }, + qux, +}; +"; + assert_format_with_max_width(src, expected, 20); + } + + #[test] + fn format_use_list_one_item() { + let src = " use foo::{ bar, };"; + let expected = "use foo::bar;\n"; + assert_format(src, expected); + } + + #[test] + fn format_long_use_list_one_item() { + let src = "use one::two::{three::{four, five}};"; + let expected = "use one::two::three::{ + five, four, +}; +"; + assert_format_with_max_width(src, expected, 25); + } + + #[test] + fn format_use_list_one_item_with_comments() { + let src = " use foo::{ /* do not remove me */ bar, };"; + let expected = "use foo::{ /* do not remove me */ bar};\n"; + assert_format(src, expected); + } + + #[test] + fn format_use_crate_with_list() { + let src = " use crate :: hash :: { Hash, Hasher }; "; + let expected = "use crate::hash::{Hash, Hasher};\n"; + assert_format(src, expected); + } + + #[test] + fn attaches_semicolon_to_last_group() { + let src = " use crate::hash::{Hash, Hasher}; "; + let expected = "use crate::hash::{ + Hash, Hasher, +}; +"; + assert_format_with_max_width(src, expected, "use crate::hash::{Hash, Hasher}".len()); + } + + #[test] + fn does_not_merge_imports_if_they_are_separated_by_two_lines() { + let src = " + use foo::baz; + + use foo::{def, abc}; +"; + let expected = "use foo::baz; + +use foo::{abc, def}; +"; + assert_format(src, expected); + } + + #[test] + fn does_not_merge_imports_if_they_have_different_visibilities() { + let src = " + pub use foo::baz; + use foo::{def, abc}; +"; + let expected = "pub use foo::baz; +use foo::{abc, def}; +"; + assert_format(src, expected); + } + + #[test] + fn does_not_merge_imports_if_they_have_trailing_comments_on_the_first_use() { + let src = " + use foo; // trailing + use bar; + + fn foo() {} +"; + let expected = "use foo; // trailing +use bar; + +fn foo() {} +"; + assert_format(src, expected); + } + + #[test] + fn does_not_merge_imports_if_they_have_trailing_comments_followed_by_item() { + let src = " + use foo; + use bar; // trailing + + fn foo() {} +"; + let expected = "use foo; +use bar; // trailing + +fn foo() {} +"; + assert_format(src, expected); + } + + #[test] + fn does_not_merge_imports_if_they_have_trailing_comments_followed_by_nothing() { + let src = " + use foo; + use bar; // trailing +"; + let expected = "use foo; +use bar; // trailing +"; + assert_format(src, expected); + } + + #[test] + fn merges_and_sorts_imports_just_two() { + let src = " + use foo::baz; + use foo::bar; + "; + let expected = "use foo::{bar, baz};\n"; + assert_format(src, expected); + } + + #[test] + fn sorts_but_not_merges_if_not_told_so() { + let src = " + use foo::baz; + use foo::{qux, bar}; + use bar; + "; + let expected = "use bar; +use foo::{bar, qux}; +use foo::baz; +"; + assert_format_preserving_granularity(src, expected); + } + + #[test] + fn does_not_sort_imports_in_separate_groups() { + let src = " + use foo::baz; + use foo::{qux, bar}; + + use bar; + "; + let expected = "use foo::{bar, qux}; +use foo::baz; + +use bar; +"; + assert_format_preserving_granularity(src, expected); + } + + #[test] + fn merges_and_sorts_imports_2() { + let src = " +use aztec::{ + context::Context, log::emit_unencrypted_log, note::{ + note_getter_options::NoteGetterOptions, note_header::NoteHeader, + }, state_vars::{ + Map, PrivateSet, PublicMutable, + }, types::{ + address::AztecAddress, type_serialization::field_serialization::{ + FIELD_SERIALIZED_LEN, FieldSerializationMethods, + }, + }, +}; + "; + let expected = "use aztec::{ + context::Context, + log::emit_unencrypted_log, + note::{note_getter_options::NoteGetterOptions, note_header::NoteHeader}, + state_vars::{Map, PrivateSet, PublicMutable}, + types::{ + address::AztecAddress, + type_serialization::field_serialization::{FIELD_SERIALIZED_LEN, FieldSerializationMethods}, + }, +}; +"; + assert_format(src, expected); + } + + #[test] + fn merges_and_sorts_imports_with_different_path_kinds() { + let src = " + use bar::baz; + use foo::bar; + use crate::foo; + "; + let expected = "use crate::foo; +use bar::baz; +use foo::bar; +"; + assert_format(src, expected); + } + + #[test] + fn sorts_import() { + let src = " + use value_note::{ + utils::{increment, decrement}, value_note::{VALUE_NOTE_LEN, ValueNote, ValueNoteMethods}, + }; + "; + let expected = "use value_note::{ + utils::{decrement, increment}, + value_note::{VALUE_NOTE_LEN, ValueNote, ValueNoteMethods}, +}; +"; + assert_format(src, expected); + } + + #[test] + fn sorts_import_ignoring_case() { + let src = " + use foo::{def, ZETA, ABC, efg}; + use BAR; + "; + let expected = "use BAR; +use foo::{ABC, def, efg, ZETA}; +"; + assert_format(src, expected); + } + + #[test] + fn merges_nested_import() { + let src = " + use foo::bar; + use foo; + "; + let expected = "use foo::{self, bar};\n"; + assert_format(src, expected); + } + + #[test] + fn idempotent_test_check_next_test() { + let src = " + use std::as_witness; +use std::merkle::compute_merkle_root; + "; + let expected = "use std::{as_witness, merkle::compute_merkle_root};\n"; + assert_format(src, expected); + } + + #[test] + fn idempotent_test_check_previous_test() { + let src = "use std::{as_witness, merkle::compute_merkle_root};"; + let expected = "use std::{as_witness, merkle::compute_merkle_root};\n"; + assert_format(src, expected); + } +} diff --git a/tooling/nargo_fmt/tests/expected/contract.nr b/tooling/nargo_fmt/tests/expected/contract.nr index 28e4ccd8ffe..ad53e61c911 100644 --- a/tooling/nargo_fmt/tests/expected/contract.nr +++ b/tooling/nargo_fmt/tests/expected/contract.nr @@ -6,15 +6,21 @@ contract Benchmarking { use aztec::protocol_types::abis::function_selector::FunctionSelector; use value_note::{ - utils::{increment, decrement}, value_note::{VALUE_NOTE_LEN, ValueNote, ValueNoteMethods}, + utils::{decrement, increment}, + value_note::{VALUE_NOTE_LEN, ValueNote, ValueNoteMethods}, }; use aztec::{ - context::Context, note::{note_getter_options::NoteGetterOptions, note_header::NoteHeader}, - log::emit_unencrypted_log, state_vars::{Map, PublicMutable, PrivateSet}, - types::type_serialization::field_serialization::{ - FieldSerializationMethods, FIELD_SERIALIZED_LEN, - }, types::address::AztecAddress, + context::Context, + log::emit_unencrypted_log, + note::{note_getter_options::NoteGetterOptions, note_header::NoteHeader}, + state_vars::{Map, PrivateSet, PublicMutable}, + types::{ + address::AztecAddress, + type_serialization::field_serialization::{ + FIELD_SERIALIZED_LEN, FieldSerializationMethods, + }, + }, }; struct Storage {