Don't write justfiles unchanged by formatting (#2479)

src/subcommand.rs

@@ -310,44 +310,44 @@ impl Subcommand {
 
     let formatted = ast.to_string();
 
+    if formatted == src {
+      return Ok(());
+    }
+
     if config.check {
-      return if formatted == src {
-        Ok(())
-      } else {
-        if !config.verbosity.quiet() {
-          use similar::{ChangeTag, TextDiff};
+      if !config.verbosity.quiet() {
+        use similar::{ChangeTag, TextDiff};
 
-          let diff = TextDiff::configure()
-            .algorithm(similar::Algorithm::Patience)
-            .diff_lines(src, &formatted);
+        let diff = TextDiff::configure()
+          .algorithm(similar::Algorithm::Patience)
+          .diff_lines(src, &formatted);
 
-          for op in diff.ops() {
-            for change in diff.iter_changes(op) {
-              let (symbol, color) = match change.tag() {
-                ChangeTag::Delete => ("-", config.color.stdout().diff_deleted()),
-                ChangeTag::Equal => (" ", config.color.stdout()),
-                ChangeTag::Insert => ("+", config.color.stdout().diff_added()),
-              };
+        for op in diff.ops() {
+          for change in diff.iter_changes(op) {
+            let (symbol, color) = match change.tag() {
+              ChangeTag::Delete => ("-", config.color.stdout().diff_deleted()),
+              ChangeTag::Equal => (" ", config.color.stdout()),
+              ChangeTag::Insert => ("+", config.color.stdout().diff_added()),
+            };
 
-              print!("{}{symbol}{change}{}", color.prefix(), color.suffix());
-            }
+            print!("{}{symbol}{change}{}", color.prefix(), color.suffix());
           }
         }
+      }
 
-        Err(Error::FormatCheckFoundDiff)
-      };
+      Err(Error::FormatCheckFoundDiff)
+    } else {
+      fs::write(&search.justfile, formatted).map_err(|io_error| Error::WriteJustfile {
+        justfile: search.justfile.clone(),
+        io_error,
+      })?;
+
+      if config.verbosity.loud() {
+        eprintln!("Wrote justfile to `{}`", search.justfile.display());
+      }
+
+      Ok(())
     }
-
-    fs::write(&search.justfile, formatted).map_err(|io_error| Error::WriteJustfile {
-      justfile: search.justfile.clone(),
-      io_error,
-    })?;
-
-    if config.verbosity.loud() {
-      eprintln!("Wrote justfile to `{}`", search.justfile.display());
-    }
-
-    Ok(())
   }
 
   fn init(config: &Config) -> RunResult<'static> {

tests/format.rs

@@ -1120,3 +1120,21 @@ fn doc_attribute_suppresses_comment() {
     )
     .run();
 }
+
+#[test]
+fn unchanged_justfiles_are_not_written_to_disk() {
+  let tmp = tempdir();
+
+  let justfile = tmp.path().join("justfile");
+
+  fs::write(&justfile, "").unwrap();
+
+  let mut permissions = fs::metadata(&justfile).unwrap().permissions();
+  permissions.set_readonly(true);
+  fs::set_permissions(&justfile, permissions).unwrap();
+
+  Test::with_tempdir(tmp)
+    .no_justfile()
+    .args(["--fmt", "--unstable"])
+    .run();
+}

tests/lib.rs

@@ -59,7 +59,7 @@ mod examples;
 mod explain;
 mod export;
 mod fallback;
-mod fmt;
+mod format;
 mod functions;
 #[cfg(unix)]
 mod global;

tests/unstable.rs

@@ -4,7 +4,7 @@ use super::*;
 fn set_unstable_true_with_env_var() {
   for val in ["true", "some-arbitrary-string"] {
     Test::new()
-      .justfile("")
+      .justfile("# hello")
       .args(["--fmt"])
       .env("JUST_UNSTABLE", val)
       .status(EXIT_SUCCESS)