make --remove-unused-ignores a no-op for files with no unused ignores (#2217)

self-sasi · meta-codesync[bot] · commit 46a55b24629a · 2026-01-26T08:48:59.000-08:00
Summary: Fixes #2185 This change makes `--remove-unused-ignores` avoid rewriting files when there are no unused ignores to remove. Files that have no ignore comments, or only have used ignore comments, are now left untouched. This prevents unnecessary file writes and avoids introducing formatting-only changes such as added newlines. Pull Request resolved: #2217 Test Plan: Added tests to verify no newline characters are appended to input when there are no unused ignores to remove. Reviewed By: stroxler Differential Revision: D91474177 Pulled By: yangdanny97 fbshipit-source-id: 65651cf15219074f9edd77a086280609205e56a4
diff --git a/crates/pyrefly_python/src/ignore.rs b/crates/pyrefly_python/src/ignore.rs
@@ -401,6 +401,11 @@ impl Ignore {
     pub fn get(&self, line: &LineNumber) -> Option<&Vec<Suppression>> {
         self.ignores.get(line)
     }
+
+    /// Returns true if there are no suppressions.
+    pub fn is_empty(&self) -> bool {
+        self.ignores.is_empty() && self.ignore_all.is_empty()
+    }
 }
 
 #[cfg(test)]
diff --git a/pyrefly/lib/error/suppress.rs b/pyrefly/lib/error/suppress.rs
@@ -373,7 +373,9 @@ pub fn remove_unused_ignores(loads: &Errors, all: bool) -> usize {
     // Collect ignores with full suppression data (including comment_line)
     let mut all_ignores: SmallMap<&PathBuf, &Ignore> = SmallMap::new();
     for (module_path, ignore) in loads.collect_ignores() {
-        if let ModulePathDetails::FileSystem(path) = module_path.details() {
+        if let ModulePathDetails::FileSystem(path) = module_path.details()
+            && !ignore.is_empty()
+        {
             all_ignores.insert(path, ignore);
         }
     }
@@ -490,10 +492,12 @@ pub fn remove_unused_ignores(loads: &Errors, all: bool) -> usize {
                 buf.push_str(line);
                 buf.push_str(line_ending);
             }
-            if let Err(e) = fs_anyhow::write(path, buf) {
-                error!("Failed to remove unused error suppressions in {} files:", e);
-            } else if unused_ignore_count > 0 {
-                removed_ignores.insert(path, unused_ignore_count);
+            if unused_ignore_count > 0 {
+                if let Err(e) = fs_anyhow::write(path, buf) {
+                    error!("Failed to remove unused error suppressions in {} files:", e);
+                } else {
+                    removed_ignores.insert(path, unused_ignore_count);
+                }
             }
         }
     }
@@ -912,10 +916,24 @@ def f() -> int:
     fn test_no_remove_suppression() {
         let input = r#"
 def g() -> int:
-    return "hello" # pyrefly: ignore [bad-return]
-"#;
+    return "hello" # pyrefly: ignore [bad-return]"#;
+        // No trailing newline on purpose.
+        // Ensures files with only used suppressions are not rewritten (no newline added).
+        // https://github.com/facebook/pyrefly/issues/2185
         assert_remove_ignores(input, input, false, 0);
     }
+
+    #[test]
+    fn test_remove_unused_ignores_no_ignores() {
+        let input = r#"
+def f(x: int) -> int:
+    return x + 1"#;
+        // No trailing newline on purpose.
+        // Ensures files without suppressions are not rewritten (no newline added).
+        // https://github.com/facebook/pyrefly/issues/2185
+        assert_remove_ignores(input, input, false, 0);
+    }
+
     #[test]
     fn test_remove_generic_suppression() {
         let before = r#"

Original file line number	Diff line number	Diff line change
`@@ -401,6 +401,11 @@ impl Ignore {`
`401`	`401`	`pub fn get(&self, line: &LineNumber) -> Option<&Vec<Suppression>> {`
`402`	`402`	`self.ignores.get(line)`
`403`	`403`	`}`
	`404`	`+`
	`405`	`+ /// Returns true if there are no suppressions.`
	`406`	`+ pub fn is_empty(&self) -> bool {`
	`407`	`+ self.ignores.is_empty() && self.ignore_all.is_empty()`
	`408`	`+ }`
`404`	`409`	`}`
`405`	`410`
`406`	`411`	`#[cfg(test)]`