feat: provide formatting subcommand (#2640)

Co-authored-by: kevaundray <kevtheappdev@gmail.com>

Cargo.toml

@@ -16,6 +16,7 @@ members = [
     "tooling/bb_abstraction_leaks",
     "tooling/lsp",
     "tooling/nargo",
+    "tooling/nargo_fmt",
     "tooling/nargo_cli",
     "tooling/nargo_toml",
     "tooling/noirc_abi",
@@ -49,6 +50,7 @@ arena = { path = "compiler/utils/arena" }
 fm = { path = "compiler/fm" }
 iter-extended = { path = "compiler/utils/iter-extended" }
 nargo = { path = "tooling/nargo" }
+nargo_fmt = { path = "tooling/nargo_fmt" }
 nargo_cli = { path = "tooling/nargo_cli" }
 nargo_toml = { path = "tooling/nargo_toml" }
 noir_lsp = { path = "tooling/lsp" }

compiler/noirc_frontend/src/ast/expression.rs

@@ -3,8 +3,8 @@ use std::fmt::Display;
 
 use crate::token::{Attributes, Token};
 use crate::{
-    Distinctness, Ident, Path, Pattern, Recoverable, Statement, UnresolvedTraitConstraint,
-    UnresolvedType, UnresolvedTypeData, Visibility,
+    Distinctness, Ident, Path, Pattern, Recoverable, Statement, StatementKind,
+    UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData, Visibility,
 };
 use acvm::FieldElement;
 use iter_extended::vecmap;
@@ -170,8 +170,14 @@ impl Expression {
         // as a sequence of { if, tuple } rather than a function call. This behavior matches rust.
         let kind = if matches!(&lhs.kind, ExpressionKind::If(..)) {
             ExpressionKind::Block(BlockExpression(vec![
-                Statement::Expression(lhs),
-                Statement::Expression(Expression::new(ExpressionKind::Tuple(arguments), span)),
+                Statement { kind: StatementKind::Expression(lhs), span },
+                Statement {
+                    kind: StatementKind::Expression(Expression::new(
+                        ExpressionKind::Tuple(arguments),
+                        span,
+                    )),
+                    span,
+                },
             ]))
         } else {
             ExpressionKind::Call(Box::new(CallExpression { func: Box::new(lhs), arguments }))
@@ -429,8 +435,8 @@ pub struct IndexExpression {
 pub struct BlockExpression(pub Vec<Statement>);
 
 impl BlockExpression {
-    pub fn pop(&mut self) -> Option<Statement> {
-        self.0.pop()
+    pub fn pop(&mut self) -> Option<StatementKind> {
+        self.0.pop().map(|stmt| stmt.kind)
     }
 
     pub fn len(&self) -> usize {
@@ -497,7 +503,7 @@ impl Display for BlockExpression {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         writeln!(f, "{{")?;
         for statement in &self.0 {
-            let statement = statement.to_string();
+            let statement = statement.kind.to_string();
             for line in statement.lines() {
                 writeln!(f, "    {line}")?;
             }

compiler/noirc_frontend/src/ast/statement.rs

@@ -14,11 +14,17 @@ use noirc_errors::{Span, Spanned};
 /// for an identifier that already failed to parse.
 pub const ERROR_IDENT: &str = "$error";
 
+#[derive(Debug, PartialEq, Eq, Clone)]
+pub struct Statement {
+    pub kind: StatementKind,
+    pub span: Span,
+}
+
 /// Ast node for statements in noir. Statements are always within a block { }
 /// of some kind and are terminated via a Semicolon, except if the statement
 /// ends in a block, such as a Statement::Expression containing an if expression.
 #[derive(Debug, PartialEq, Eq, Clone)]
-pub enum Statement {
+pub enum StatementKind {
     Let(LetStatement),
     Constrain(ConstrainStatement),
     Expression(Expression),
@@ -32,62 +38,67 @@ pub enum Statement {
     Error,
 }
 
-impl Recoverable for Statement {
-    fn error(_: Span) -> Self {
-        Statement::Error
-    }
-}
-
 impl Statement {
-    pub fn new_let(
-        ((pattern, r#type), expression): ((Pattern, UnresolvedType), Expression),
-    ) -> Statement {
-        Statement::Let(LetStatement { pattern, r#type, expression })
-    }
-
     pub fn add_semicolon(
         self,
         semi: Option<Token>,
         span: Span,
         last_statement_in_block: bool,
         emit_error: &mut dyn FnMut(ParserError),
-    ) -> Statement {
+    ) -> Self {
         let missing_semicolon =
             ParserError::with_reason(ParserErrorReason::MissingSeparatingSemi, span);
-        match self {
-            Statement::Let(_)
-            | Statement::Constrain(_)
-            | Statement::Assign(_)
-            | Statement::Semi(_)
-            | Statement::Error => {
+
+        let kind = match self.kind {
+            StatementKind::Let(_)
+            | StatementKind::Constrain(_)
+            | StatementKind::Assign(_)
+            | StatementKind::Semi(_)
+            | StatementKind::Error => {
                 // To match rust, statements always require a semicolon, even at the end of a block
                 if semi.is_none() {
                     emit_error(missing_semicolon);
                 }
-                self
+                self.kind
             }
             // A semicolon on a for loop is optional and does nothing
-            Statement::For(_) => self,
+            StatementKind::For(_) => self.kind,
 
-            Statement::Expression(expr) => {
+            StatementKind::Expression(expr) => {
                 match (&expr.kind, semi, last_statement_in_block) {
                     // Semicolons are optional for these expressions
                     (ExpressionKind::Block(_), semi, _) | (ExpressionKind::If(_), semi, _) => {
                         if semi.is_some() {
-                            Statement::Semi(expr)
+                            StatementKind::Semi(expr)
                         } else {
-                            Statement::Expression(expr)
+                            StatementKind::Expression(expr)
                         }
                     }
                     (_, None, false) => {
                         emit_error(missing_semicolon);
-                        Statement::Expression(expr)
+                        StatementKind::Expression(expr)
                     }
-                    (_, Some(_), _) => Statement::Semi(expr),
-                    (_, None, true) => Statement::Expression(expr),
+                    (_, Some(_), _) => StatementKind::Semi(expr),
+                    (_, None, true) => StatementKind::Expression(expr),
                 }
             }
-        }
+        };
+
+        Statement { kind, span: self.span }
+    }
+}
+
+impl Recoverable for StatementKind {
+    fn error(_: Span) -> Self {
+        StatementKind::Error
+    }
+}
+
+impl StatementKind {
+    pub fn new_let(
+        ((pattern, r#type), expression): ((Pattern, UnresolvedType), Expression),
+    ) -> StatementKind {
+        StatementKind::Let(LetStatement { pattern, r#type, expression })
     }
 
     /// Create a Statement::Assign value, desugaring any combined operators like += if needed.
@@ -96,7 +107,7 @@ impl Statement {
         operator: Token,
         mut expression: Expression,
         span: Span,
-    ) -> Statement {
+    ) -> StatementKind {
         // Desugar `a <op>= b` to `a = a <op> b`. This relies on the evaluation of `a` having no side effects,
         // which is currently enforced by the restricted syntax of LValues.
         if operator != Token::Assign {
@@ -111,7 +122,7 @@ impl Statement {
             expression = Expression::new(ExpressionKind::Infix(Box::new(infix)), span);
         }
 
-        Statement::Assign(AssignStatement { lvalue, expression })
+        StatementKind::Assign(AssignStatement { lvalue, expression })
     }
 }
 
@@ -468,16 +479,16 @@ pub struct ForLoopStatement {
     pub block: Expression,
 }
 
-impl Display for Statement {
+impl Display for StatementKind {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            Statement::Let(let_statement) => let_statement.fmt(f),
-            Statement::Constrain(constrain) => constrain.fmt(f),
-            Statement::Expression(expression) => expression.fmt(f),
-            Statement::Assign(assign) => assign.fmt(f),
-            Statement::For(for_loop) => for_loop.fmt(f),
-            Statement::Semi(semi) => write!(f, "{semi};"),
-            Statement::Error => write!(f, "Error"),
+            StatementKind::Let(let_statement) => let_statement.fmt(f),
+            StatementKind::Constrain(constrain) => constrain.fmt(f),
+            StatementKind::Expression(expression) => expression.fmt(f),
+            StatementKind::Assign(assign) => assign.fmt(f),
+            StatementKind::For(for_loop) => for_loop.fmt(f),
+            StatementKind::Semi(semi) => write!(f, "{semi};"),
+            StatementKind::Error => write!(f, "Error"),
         }
     }
 }

compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs

@@ -17,12 +17,11 @@ use crate::node_interner::{
     FuncId, NodeInterner, StmtId, StructId, TraitId, TraitImplKey, TypeAliasId,
 };
 
-use crate::parser::ParserError;
-
+use crate::parser::{ParserError, SortedModule};
 use crate::{
     ExpressionKind, Generics, Ident, LetStatement, Literal, NoirFunction, NoirStruct, NoirTrait,
-    NoirTypeAlias, ParsedModule, Path, Shared, StructType, TraitItem, Type, TypeBinding,
-    TypeVariableKind, UnresolvedGenerics, UnresolvedType,
+    NoirTypeAlias, Path, Shared, StructType, TraitItem, Type, TypeBinding, TypeVariableKind,
+    UnresolvedGenerics, UnresolvedType,
 };
 use fm::FileId;
 use iter_extended::vecmap;
@@ -195,7 +194,7 @@ impl DefCollector {
     pub fn collect(
         mut def_map: CrateDefMap,
         context: &mut Context,
-        ast: ParsedModule,
+        ast: SortedModule,
         root_file_id: FileId,
     ) -> Vec<(CompilationError, FileId)> {
         let mut errors: Vec<(CompilationError, FileId)> = vec![];

compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs

@@ -7,9 +7,9 @@ use crate::{
     graph::CrateId,
     hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait},
     node_interner::TraitId,
-    parser::SubModule,
+    parser::{SortedModule, SubModule},
     FunctionDefinition, Ident, LetStatement, NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl,
-    NoirTypeAlias, ParsedModule, TraitImplItem, TraitItem, TypeImpl,
+    NoirTypeAlias, TraitImplItem, TraitItem, TypeImpl,
 };
 
 use super::{
@@ -35,7 +35,7 @@ struct ModCollector<'a> {
 /// This performs the entirety of the definition collection phase of the name resolution pass.
 pub fn collect_defs(
     def_collector: &mut DefCollector,
-    ast: ParsedModule,
+    ast: SortedModule,
     file_id: FileId,
     module_id: LocalModuleId,
     crate_id: CrateId,
@@ -410,7 +410,7 @@ impl<'a> ModCollector<'a> {
                 Ok(child) => {
                     errors.extend(collect_defs(
                         self.def_collector,
-                        submodule.contents,
+                        submodule.contents.into_sorted(),
                         file_id,
                         child,
                         crate_id,
@@ -467,8 +467,8 @@ impl<'a> ModCollector<'a> {
         context.visited_files.insert(child_file_id, location);
 
         // Parse the AST for the module we just found and then recursively look for it's defs
-        //let ast = parse_file(&context.file_manager, child_file_id, errors);
         let (ast, parsing_errors) = parse_file(&context.file_manager, child_file_id);
+        let ast = ast.into_sorted();
 
         errors.extend(
             parsing_errors.iter().map(|e| (e.clone().into(), child_file_id)).collect::<Vec<_>>(),

compiler/noirc_frontend/src/hir/def_map/mod.rs

@@ -83,6 +83,7 @@ impl CrateDefMap {
         // First parse the root file.
         let root_file_id = context.crate_graph[crate_id].root_file_id;
         let (ast, parsing_errors) = parse_file(&context.file_manager, root_file_id);
+        let ast = ast.into_sorted();
 
         #[cfg(feature = "aztec")]
         let ast = match super::aztec_library::transform(ast, &crate_id, context) {