Repository: dprint/dprint (language: rust)

Issue:
Error when a plugin returns an empty string for non-whitespace text with char length over certain amount
If a formatter is provided a string length greater than say 100 and the output is an empty text (then double check for non-whitespace char length of greater than 100), then dprint should error because this is most likely a mistake.

I thought I already had this built into dprint, but I can't find the code atm. Maybe it was just in a branch (Edit: Oh, it was in the exec plugin https://github.com/dprint/dprint-plugin-exec/blob/5637a11b28260264a331b66d516a9f5432e2bdb4/src/handler.rs#L243C9-L255)

Related: https://github.com/dprint/dprint-plugin-typescript/issues/563


Discussion / hints:

Fix patch:
diff --git a/crates/dprint/src/commands/formatting.rs b/crates/dprint/src/commands/formatting.rs
index 1f842d114..0b4275e72 100644
--- a/crates/dprint/src/commands/formatting.rs
+++ b/crates/dprint/src/commands/formatting.rs
@@ -378,7 +378,7 @@ mod test {
       environment.take_stderr_messages(),
       vec![String::from("Error formatting /file.txt. Message: Did error.")]
     );
-    assert_eq!(error_message.to_string(), "Had 1 error(s) formatting.");
+    assert_eq!(error_message.to_string(), "Had 1 error formatting.");
   }
 
   #[test]
@@ -391,7 +391,7 @@ mod test {
       environment.take_stderr_messages(),
       vec![String::from("Error formatting /file.txt_ps. Message: Did error.")]
     );
-    assert_eq!(error_message.to_string(), "Had 1 error(s) formatting.");
+    assert_eq!(error_message.to_string(), "Had 1 error formatting.");
   }
 
   #[test]
@@ -409,7 +409,7 @@ mod test {
       "This may be a bug in the plugin, the dprint cli is out of date, or the plugin is out of date.",
     );
     assert_eq!(&logged_errors[0][..expected_start_text.len()], expected_start_text);
-    assert_eq!(error_message.to_string(), "Had 1 error(s) formatting.");
+    assert_eq!(error_message.to_string(), "Had 1 error formatting.");
     // should still format with the other plugin
     assert_eq!(environment.read_file("/file2.txt_ps").unwrap(), "test_formatted_process");
   }
@@ -465,7 +465,7 @@ mod test {
       .write_file("/file.txt", "plugin: should_error")
       .build();
     let error_message = run_test_cli(vec!["fmt", "/file.txt"], &environment).err().unwrap();
-    assert_eq!(error_message.to_string(), "Had 1 error(s) formatting.");
+    assert_eq!(error_message.to_string(), "Had 1 error formatting.");
     assert_eq!(
       environment.take_stderr_messages(),
       vec![String::from("Error formatting /file.txt. Message: Did error.")]
@@ -523,7 +523,7 @@ mod test {
       .write_file("/file.txt_ps", "plugin: should_error")
       .build();
     let error_message = run_test_cli(vec!["fmt", "/file.txt_ps"], &environment).err().unwrap();
-    assert_eq!(error_message.to_string(), "Had 1 error(s) formatting.");
+    assert_eq!(error_message.to_string(), "Had 1 error formatting.");
     assert_eq!(
       environment.take_stderr_messages(),
       vec![String::from("Error formatting /file.txt_ps. Message: Did error.")]
@@ -865,7 +865,7 @@ mod test {
 
     let error_message = run_test_cli(vec!["fmt", "**/*.txt"], &environment).err().unwrap();
 
-    assert_eq!(error_message.to_string(), "Had 1 error(s) formatting.");
+    assert_eq!(error_message.to_string(), "Had 1 error formatting.");
     assert_eq!(
       environment.take_stderr_messages(),
       vec![
@@ -896,7 +896,7 @@ mod test {
 
     let error_message = run_test_cli(vec!["fmt", "**/*.txt_ps"], &environment).err().unwrap();
 
-    assert_eq!(error_message.to_string(), "Had 1 error(s) formatting.");
+    assert_eq!(error_message.to_string(), "Had 1 error formatting.");
     assert_eq!(
       environment.take_stderr_messages(),
       vec![
@@ -937,7 +937,7 @@ mod test {
 
     let error_message = run_test_cli(vec!["fmt", "--config", "/config.json"], &environment).err().unwrap();
 
-    assert_eq!(error_message.to_string(), "Had 1 error(s) formatting.");
+    assert_eq!(error_message.to_string(), "Had 1 error formatting.");
     assert_eq!(
       environment.take_stderr_messages(),
       vec![
@@ -1584,7 +1584,7 @@ mod test {
 
     let result = run_test_cli(vec!["fmt", "--incremental"], &environment).err().unwrap();
 
-    assert_eq!(result.to_string(), "Had 1 error(s) formatting.");
+    assert_eq!(result.to_string(), "Had 1 error formatting.");
     assert_eq!(
       environment.take_stderr_messages(),
       vec![concat!(
@@ -1604,7 +1604,7 @@ mod test {
 
     let result = run_test_cli(vec!["fmt", "--incremental"], &environment).err().unwrap();
 
-    assert_eq!(result.to_string(), "Had 1 error(s) formatting.");
+    assert_eq!(result.to_string(), "Had 1 error formatting.");
     assert_eq!(
       environment.take_stderr_messages(),
       vec![concat!(
@@ -2102,7 +2102,7 @@ mod test {
 
     let error_message = run_test_cli(vec!["fmt", "**/*.txt"], &environment).err().unwrap();
 
-    assert_eq!(error_message.to_string(), "Had 1 error(s) formatting.");
+    assert_eq!(error_message.to_string(), "Had 1 error formatting.");
     assert_eq!(
       environment.take_stderr_messages(),
       vec![
@@ -2195,4 +2195,28 @@ mod test {
     );
     err.assert_exit_code(14);
   }
+
+  #[test]
+  fn should_error_format_to_empty() {
+    // this is to prevent a plugin accidentally formatting a file to an empty file
+    let file_path1 = "/file.txt";
+    let environment = TestEnvironmentBuilder::with_initialized_remote_wasm_plugin()
+      .with_local_config("/dprint.json", |config| {
+        config.add_remote_wasm_plugin();
+      })
+      .write_file(&file_path1, &format!("format_to_empty {}", "a".repeat(400)))
+      .build();
+    let err = run_test_cli(vec!["fmt"], &environment).err().unwrap();
+    assert_eq!(err.to_string(), "Had 1 error formatting.");
+    err.assert_exit_code(1);
+    assert_eq!(
+      environment.take_stderr_messages(),
+      vec![concat!(
+        "Error formatting /file.txt. Message: The original file text was greater than 300 characters, ",
+        "but the formatted text was empty. Most likely this is a bug in the plugin and the dprint CLI has ",
+        "prevented the plugin from formatting the file to an empty file. Please report this scenario."
+      )
+      .to_string()]
+    );
+  }
 }
diff --git a/crates/dprint/src/format.rs b/crates/dprint/src/format.rs
index bd733df31..ecd12f445 100644
--- a/crates/dprint/src/format.rs
+++ b/crates/dprint/src/format.rs
@@ -175,7 +175,7 @@ where
   return if error_count == 0 {
     Ok(())
   } else {
-    bail!("Had {0} error(s) formatting.", error_count)
+    bail!("Had {} error{} formatting.", error_count, if error_count == 1 { "" } else { "s" })
   };
 
   #[inline]
@@ -275,6 +275,7 @@ where
     file_text: &[u8],
   ) -> Result<(Instant, Vec<u8>)> {
     let start_instant = Instant::now();
+    let original_text = file_text;
     let mut file_text = Cow::Borrowed(file_text);
     let plugins_len = plugins.len();
     for (i, plugin) in plugins.iter().enumerate() {
@@ -304,6 +305,24 @@ where
         file_text = Cow::Owned(text)
       }
     }
+
+    // some heuristic to stop plugins accidentally formatting a file to empty
+    const MIN_CHARS_TO_EMPTY: usize = 300;
+    if file_text.len() < 100 && original_text.len() > MIN_CHARS_TO_EMPTY {
+      let original_text = String::from_utf8_lossy(original_text);
+      let new_text = String::from_utf8_lossy(&file_text);
+      if original_text.trim().len() > MIN_CHARS_TO_EMPTY && new_text.trim().is_empty() {
+        bail!(
+          concat!(
+            "The original file text was greater than {} characters, but the formatted text was empty. ",
+            "Most likely this is a bug in the plugin and the dprint CLI has prevented the plugin from ",
+            "formatting the file to an empty file. Please report this scenario.",
+          ),
+          MIN_CHARS_TO_EMPTY
+        )
+      }
+    }
+
     Ok((start_instant, file_text.into_owned()))
   }
 }


Test patch:
diff --git a/crates/test-plugin/src/lib.rs b/crates/test-plugin/src/lib.rs
index 4ffa2c2f6..94ef27090 100644
--- a/crates/test-plugin/src/lib.rs
+++ b/crates/test-plugin/src/lib.rs
@@ -122,6 +122,8 @@ impl SyncPluginHandler<Configuration> for TestWasmPlugin {
       "unstable_fmt_false".to_string()
     } else if file_text == "unstable_fmt_false" {
       "unstable_fmt_true".to_string()
+    } else if file_text.starts_with("format_to_empty") {
+      return Ok(Some(String::new().into_bytes()));
     } else {
       file_text.to_string()
     };
diff --git a/crates/test-plugin/test_plugin.wasm b/crates/test-plugin/test_plugin.wasm
index 97b537a10..2ab4b7740 100644
Binary files a/crates/test-plugin/test_plugin.wasm and b/crates/test-plugin/test_plugin.wasm differ