fix tests, clippy happy, use absolute path when filtering package root

software-mansion · Oct 3, 2024 · dd47515 · dd47515
1 parent c19b6f3
commit dd47515
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 12 deletions.
diff --git a/scarb/src/ops/fmt.rs b/scarb/src/ops/fmt.rs
@@ -14,6 +14,7 @@ use tracing::{info, warn};
 
 use crate::core::workspace::Workspace;
 use crate::core::{Package, PackageId};
+use crate::internal::fsx::canonicalize;
 use crate::internal::serdex::toml_merge;
 
 #[derive(Debug, Clone)]
@@ -50,8 +51,9 @@ pub fn format(opts: FmtOptions, ws: &Workspace<'_>) -> Result<bool> {
             format_single_file(path, &opts, ws, &all_correct)?;
             return Ok(all_correct.load(Ordering::Acquire));
         }
-        let pkg = ws.members().find(|member| path.starts_with(member.root()));
 
+        let absolute_path = canonicalize(path)?;
+        let pkg = ws.members().find(|member| absolute_path.starts_with(member.root()));
         if let Some(pkg) = pkg {
             format_package(&pkg, &opts, ws, &all_correct)?;
             return Ok(all_correct.load(Ordering::Acquire));
@@ -98,16 +100,16 @@ fn format_package(
 }
 
 fn format_single_file(
-    target_file: &Utf8PathBuf,
+    file: &Utf8PathBuf,
     opts: &FmtOptions,
     ws: &Workspace<'_>,
     all_correct: &AtomicBool,
 ) -> Result<bool> {
-    let target_file = target_file.canonicalize_utf8()?;
+    let absolute_file_path = canonicalize(file)?;
     let pkg = ws
         .members()
-        .find(|member| target_file.starts_with(member.root()))
-        .ok_or_else(|| anyhow::anyhow!("file {:?} is not part of the workspace.", target_file))?;
+        .find(|member| absolute_file_path.starts_with(member.root()))
+        .ok_or_else(|| anyhow::anyhow!("file {:?} is not part of the workspace.", file))?;
 
     let mut config = FormatterConfig::default();
     if let Some(overrides) = pkg.tool_metadata("fmt") {
@@ -116,13 +118,13 @@ fn format_single_file(
     let fmt = CairoFormatter::new(config);
 
     let success = match &opts.action {
-        FmtAction::Fix => format_file_in_place(&fmt, opts, ws, target_file.as_std_path()),
-        FmtAction::Check => check_file_formatting(&fmt, opts, ws, target_file.as_std_path()),
+        FmtAction::Fix => format_file_in_place(&fmt, opts, ws, absolute_file_path.as_path()),
+        FmtAction::Check => check_file_formatting(&fmt, opts, ws, absolute_file_path.as_path()),
         FmtAction::Emit(target) => emit_formatted_file(
             &fmt,
             target,
             ws,
-            target_file.as_std_path(),
+            absolute_file_path.as_path(),
             EmitMode::WithoutPath,
         ),
     };
@@ -251,7 +253,7 @@ impl Emittable for FmtEmitTarget {
             (Self::Stdout, EmitMode::WithoutPath) => ws
                 .config()
                 .ui()
-                .print(TextWithNewline(format!("{}", formatted))),
+                .print(TextWithNewline(formatted.to_string())),
         }
     }
 }

diff --git a/scarb/tests/fmt.rs b/scarb/tests/fmt.rs
@@ -70,7 +70,7 @@ fn simple_emit_invalid() {
         .assert()
         .failure()
         .stdout_eq(format!(
-            "{}:\n{}\n",
+            "{}:\n\n{}",
             fsx::canonicalize(t.child("src/lib.cairo"))
                 .unwrap()
                 .display(),
@@ -356,7 +356,7 @@ fn workspace_emit_with_root() {
         .assert()
         .failure()
         .stdout_eq(format!(
-            "{}:\n{}\n",
+            "{}:\n\n{}",
             fsx::canonicalize(t.child("src/lib.cairo"))
                 .unwrap()
                 .display(),
@@ -376,7 +376,7 @@ fn workspace_emit_with_root() {
         .assert()
         .failure()
         .stdout_eq(format!(
-            "{}:\n{}\n{}:\n{}\n{}:\n{}\n",
+            "{}:\n\n{}{}:\n\n{}{}:\n\n{}",
             fsx::canonicalize(t.child("first/src/lib.cairo"))
                 .unwrap()
                 .display(),
@@ -425,3 +425,48 @@ fn format_specific_file() {
     let other_content = t.child("src/other.cairo").read_to_string();
     assert_eq!(other_content, SIMPLE_ORIGINAL);
 }
+
+#[test]
+fn format_all_files_in_path() {
+    let t = TempDir::new().unwrap();
+
+    // Create a Scarb.toml file
+    t.child("Scarb.toml")
+        .write_str(
+            r#"
+            [package]
+            name = "test_package"
+            version = "0.1.0"
+            "#,
+        )
+        .unwrap();
+
+    // Create multiple Cairo files with unformatted content
+    for i in 1..=3 {
+        t.child(format!("src/fmt/file{}.cairo", i))
+            .write_str(SIMPLE_ORIGINAL)
+            .unwrap();
+    }
+
+    t.child("src/no_fmt/file.cairo")
+        .write_str(SIMPLE_ORIGINAL)
+        .unwrap();
+
+    // Run the formatter on the src directory
+    Scarb::quick_snapbox()
+        .arg("fmt")
+        .arg("src/fmt")
+        .current_dir(&t)
+        .assert()
+        .success();
+
+    // Check that all files in the src directory were formatted
+    for i in 1..=3 {
+        let content = t.child(format!("src/fmt/file{}.cairo", i)).read_to_string();
+        assert_eq!(content, SIMPLE_FORMATTED);
+    }
+
+    // Check that the file in the no_fmt directory was not formatted
+    let content = t.child("src/no_fmt/file.cairo").read_to_string();
+    assert_eq!(content, SIMPLE_ORIGINAL);
+}