From 0ce88feaf523b7b029c4d5db82a41e5937e948a5 Mon Sep 17 00:00:00 2001 From: f01dab1e Date: Wed, 22 Nov 2023 12:53:11 +0000 Subject: [PATCH 1/7] feat: add --check option to nargo fmt for dry-run formatting verification --- Cargo.lock | 1 + Cargo.toml | 1 + tooling/nargo_cli/Cargo.toml | 1 + tooling/nargo_cli/src/cli/fmt_cmd.rs | 39 ++++++++++++++++++++++------ tooling/nargo_fmt/Cargo.toml | 2 +- 5 files changed, 35 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5dbbd711e5c..7fb4baec464 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2436,6 +2436,7 @@ dependencies = [ "rustc_version", "serde", "serde_json", + "similar-asserts", "tempfile", "termcolor", "test-binary", diff --git a/Cargo.toml b/Cargo.toml index bc2c4e9f73e..919efca4ccb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -118,6 +118,7 @@ hex = "0.4.2" const_format = "0.2.30" num-bigint = "0.4" num-traits = "0.2" +similar-asserts = "1.5.0" [profile.dev] # This is required to be able to run `cargo test` in acvm_js due to the `locals exceeds maximum` error. diff --git a/tooling/nargo_cli/Cargo.toml b/tooling/nargo_cli/Cargo.toml index 1a08514dc5f..07298ae25d2 100644 --- a/tooling/nargo_cli/Cargo.toml +++ b/tooling/nargo_cli/Cargo.toml @@ -43,6 +43,7 @@ tower.workspace = true async-lsp = { workspace = true, features = ["client-monitor", "stdio", "tracing", "tokio"] } const_format.workspace = true hex.workspace = true +similar-asserts.workspace = true termcolor = "1.1.2" color-eyre = "0.6.2" tokio = { version = "1.0", features = ["io-std"] } diff --git a/tooling/nargo_cli/src/cli/fmt_cmd.rs b/tooling/nargo_cli/src/cli/fmt_cmd.rs index 638eaa64b3e..91490fe2a45 100644 --- a/tooling/nargo_cli/src/cli/fmt_cmd.rs +++ b/tooling/nargo_cli/src/cli/fmt_cmd.rs @@ -13,9 +13,15 @@ use super::NargoConfig; /// Format the Noir files in a workspace #[derive(Debug, Clone, Args)] -pub(crate) struct FormatCommand {} +pub(crate) struct FormatCommand { + /// Run nargofmt in check mode + #[arg(long)] + check: bool, +} + +pub(crate) fn run(args: FormatCommand, config: NargoConfig) -> Result<(), CliError> { + let check_mode = args.check; -pub(crate) fn run(_args: FormatCommand, config: NargoConfig) -> Result<(), CliError> { let toml_path = get_package_manifest(&config.program_dir)?; let workspace = resolve_workspace_from_toml( &toml_path, @@ -26,6 +32,8 @@ pub(crate) fn run(_args: FormatCommand, config: NargoConfig) -> Result<(), CliEr let config = nargo_fmt::Config::read(&config.program_dir) .map_err(|err| CliError::Generic(err.to_string()))?; + let mut exit_code_one = false; + for package in &workspace { let mut file_manager = FileManager::new(&package.root_dir, Box::new(|path| std::fs::read_to_string(path))); @@ -53,16 +61,31 @@ pub(crate) fn run(_args: FormatCommand, config: NargoConfig) -> Result<(), CliEr return Ok(()); } - let source = nargo_fmt::format( - file_manager.fetch_file(file_id).source(), - parsed_module, - &config, - ); + let original = file_manager.fetch_file(file_id).source(); + let formatted = nargo_fmt::format(original, parsed_module, &config); - std::fs::write(entry.path(), source) + if check_mode { + exit_code_one = original != formatted; + + let diff = similar_asserts::SimpleDiff::from_str( + &original, + &formatted, + "original", + "formatted", + ); + println!("{diff}"); + Ok(()) + } else { + std::fs::write(entry.path(), formatted) + } }) .map_err(|error| CliError::Generic(error.to_string()))?; } + + if exit_code_one { + std::process::exit(1); + } + Ok(()) } diff --git a/tooling/nargo_fmt/Cargo.toml b/tooling/nargo_fmt/Cargo.toml index 921c9893ab5..374413ac9f2 100644 --- a/tooling/nargo_fmt/Cargo.toml +++ b/tooling/nargo_fmt/Cargo.toml @@ -13,4 +13,4 @@ toml.workspace = true thiserror.workspace = true [dev-dependencies] -similar-asserts = "1.5.0" +similar-asserts.workspace = true From 23425c78260a3a71c029769f2b3d9023a4cf0432 Mon Sep 17 00:00:00 2001 From: kek kek kek Date: Wed, 22 Nov 2023 04:59:50 -0800 Subject: [PATCH 2/7] nargofmt -> noirfmt --- tooling/nargo_cli/src/cli/fmt_cmd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/fmt_cmd.rs b/tooling/nargo_cli/src/cli/fmt_cmd.rs index 91490fe2a45..b0d34c12072 100644 --- a/tooling/nargo_cli/src/cli/fmt_cmd.rs +++ b/tooling/nargo_cli/src/cli/fmt_cmd.rs @@ -14,7 +14,7 @@ use super::NargoConfig; /// Format the Noir files in a workspace #[derive(Debug, Clone, Args)] pub(crate) struct FormatCommand { - /// Run nargofmt in check mode + /// Run noirfmt in check mode #[arg(long)] check: bool, } From 6dd4329445a2492751ac4941af77c34cba9a4f4c Mon Sep 17 00:00:00 2001 From: kek kek kek Date: Wed, 22 Nov 2023 05:02:50 -0800 Subject: [PATCH 3/7] fix clippy --- tooling/nargo_cli/src/cli/fmt_cmd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/fmt_cmd.rs b/tooling/nargo_cli/src/cli/fmt_cmd.rs index b0d34c12072..6f166cd24a3 100644 --- a/tooling/nargo_cli/src/cli/fmt_cmd.rs +++ b/tooling/nargo_cli/src/cli/fmt_cmd.rs @@ -68,7 +68,7 @@ pub(crate) fn run(args: FormatCommand, config: NargoConfig) -> Result<(), CliErr exit_code_one = original != formatted; let diff = similar_asserts::SimpleDiff::from_str( - &original, + original, &formatted, "original", "formatted", From 8797d7747059247a0b2a432323c76c1c8057f070 Mon Sep 17 00:00:00 2001 From: f01dab1e Date: Wed, 22 Nov 2023 13:34:01 +0000 Subject: [PATCH 4/7] fix multi files --- tooling/nargo_cli/src/cli/fmt_cmd.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/fmt_cmd.rs b/tooling/nargo_cli/src/cli/fmt_cmd.rs index 6f166cd24a3..bdea6f551ca 100644 --- a/tooling/nargo_cli/src/cli/fmt_cmd.rs +++ b/tooling/nargo_cli/src/cli/fmt_cmd.rs @@ -65,7 +65,9 @@ pub(crate) fn run(args: FormatCommand, config: NargoConfig) -> Result<(), CliErr let formatted = nargo_fmt::format(original, parsed_module, &config); if check_mode { - exit_code_one = original != formatted; + if !exit_code_one { + exit_code_one = original != formatted; + } let diff = similar_asserts::SimpleDiff::from_str( original, From b5b95d4f955158e3110d58a099de224e2541c24a Mon Sep 17 00:00:00 2001 From: f01dab1e Date: Wed, 22 Nov 2023 14:28:31 +0000 Subject: [PATCH 5/7] update message --- tooling/nargo_cli/src/cli/fmt_cmd.rs | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/tooling/nargo_cli/src/cli/fmt_cmd.rs b/tooling/nargo_cli/src/cli/fmt_cmd.rs index bdea6f551ca..0d792305095 100644 --- a/tooling/nargo_cli/src/cli/fmt_cmd.rs +++ b/tooling/nargo_cli/src/cli/fmt_cmd.rs @@ -32,7 +32,7 @@ pub(crate) fn run(args: FormatCommand, config: NargoConfig) -> Result<(), CliErr let config = nargo_fmt::Config::read(&config.program_dir) .map_err(|err| CliError::Generic(err.to_string()))?; - let mut exit_code_one = false; + let mut check_exit_code_one = false; for package in &workspace { let mut file_manager = @@ -65,17 +65,22 @@ pub(crate) fn run(args: FormatCommand, config: NargoConfig) -> Result<(), CliErr let formatted = nargo_fmt::format(original, parsed_module, &config); if check_mode { - if !exit_code_one { - exit_code_one = original != formatted; - } - let diff = similar_asserts::SimpleDiff::from_str( original, &formatted, "original", "formatted", - ); - println!("{diff}"); + ) + .to_string(); + + if !diff.contains("Invisible differences") { + if !check_exit_code_one { + check_exit_code_one = true; + } + + println!("{diff}"); + } + Ok(()) } else { std::fs::write(entry.path(), formatted) @@ -84,8 +89,10 @@ pub(crate) fn run(args: FormatCommand, config: NargoConfig) -> Result<(), CliErr .map_err(|error| CliError::Generic(error.to_string()))?; } - if exit_code_one { + if check_exit_code_one { std::process::exit(1); + } else { + println!("No formatting changes were detected"); } Ok(()) From 2903db6ef9852f5e9c6f5ea6537a4984275ac9c0 Mon Sep 17 00:00:00 2001 From: f01dab1e Date: Wed, 22 Nov 2023 14:31:13 +0000 Subject: [PATCH 6/7] maybe better way --- tooling/nargo_cli/src/cli/fmt_cmd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/fmt_cmd.rs b/tooling/nargo_cli/src/cli/fmt_cmd.rs index 0d792305095..7ee39c73cd0 100644 --- a/tooling/nargo_cli/src/cli/fmt_cmd.rs +++ b/tooling/nargo_cli/src/cli/fmt_cmd.rs @@ -73,7 +73,7 @@ pub(crate) fn run(args: FormatCommand, config: NargoConfig) -> Result<(), CliErr ) .to_string(); - if !diff.contains("Invisible differences") { + if !diff.lines().next().is_some_and(|line| line.contains("Invisible differences")) { if !check_exit_code_one { check_exit_code_one = true; } From 7f71c6d3091dd3a0ae3b1ec8b330fe6546c8dcc1 Mon Sep 17 00:00:00 2001 From: kek kek kek Date: Wed, 22 Nov 2023 06:37:59 -0800 Subject: [PATCH 7/7] Update tooling/nargo_cli/src/cli/fmt_cmd.rs --- tooling/nargo_cli/src/cli/fmt_cmd.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/fmt_cmd.rs b/tooling/nargo_cli/src/cli/fmt_cmd.rs index 7ee39c73cd0..ec3d373a483 100644 --- a/tooling/nargo_cli/src/cli/fmt_cmd.rs +++ b/tooling/nargo_cli/src/cli/fmt_cmd.rs @@ -91,7 +91,7 @@ pub(crate) fn run(args: FormatCommand, config: NargoConfig) -> Result<(), CliErr if check_exit_code_one { std::process::exit(1); - } else { + } else if check_mode { println!("No formatting changes were detected"); }