Rename SuppressableError -> SerializedError

Summary: This is re-used for unused ignores, and the name was a bit confusing. Attempt to make it's use more clear.

Reviewed By: yangdanny97

Differential Revision: D91708274

fbshipit-source-id: 35c95915de9566e7bac01bf9f324e4988e260b0b

Author: maggiemoss

pyrefly/lib/commands/check.rs

@@ -62,7 +62,7 @@ use crate::error::legacy::LegacyErrors;
 use crate::error::legacy::severity_to_str;
 use crate::error::summarize::print_error_summary;
 use crate::error::suppress;
-use crate::error::suppress::SuppressableError;
+use crate::error::suppress::SerializedError;
 use crate::module::typeshed::stdlib_search_path;
 use crate::report;
 use crate::state::load::FileContents;
@@ -1011,12 +1011,13 @@ impl CheckArgs {
         }
         if self.behavior.suppress_errors {
             // TODO: Move this into separate command
-            let suppressable_errors: Vec<SuppressableError> = shown_errors
+            let serialized_errors: Vec<SerializedError> = shown_errors
                 .iter()
                 .filter(|e| e.severity() >= Severity::Warn)
-                .filter_map(SuppressableError::from_error)
+                .filter_map(SerializedError::from_error)
+                .filter(|e| !e.is_unused_ignore())
                 .collect();
-            suppress::suppress_errors(suppressable_errors);
+            suppress::suppress_errors(serialized_errors);
         }
         if self.behavior.remove_unused_ignores {
             // TODO: Move this into separate command

pyrefly/lib/commands/suppress.rs

@@ -14,7 +14,7 @@ use pyrefly_config::error_kind::Severity;
 use crate::commands::files::FilesArgs;
 use crate::commands::util::CommandExitStatus;
 use crate::error::suppress;
-use crate::error::suppress::SuppressableError;
+use crate::error::suppress::SerializedError;
 
 /// Suppress type errors by adding ignore comments to source files.
 #[derive(Clone, Debug, Parser)]
@@ -28,17 +28,21 @@ pub struct SuppressArgs {
     config_override: ConfigOverrideArgs,
 
     /// Path to a JSON file containing errors to suppress.
-    /// The JSON should be an array of objects with "path", "line", and "name" fields.
+    /// The JSON should be an array of objects with "path", "line", "name", and "message" fields.
     #[arg(long)]
     json: Option<PathBuf>,
 }
 
 impl SuppressArgs {
     pub fn run(&self) -> anyhow::Result<CommandExitStatus> {
-        let suppressable_errors = if let Some(json_path) = &self.json {
-            // Parse errors from JSON file
+        let suppressable_errors: Vec<SerializedError> = if let Some(json_path) = &self.json {
+            // Parse errors from JSON file, filtering out UnusedIgnore errors
             let json_content = std::fs::read_to_string(json_path)?;
-            serde_json::from_str(&json_content)?
+            let errors: Vec<SerializedError> = serde_json::from_str(&json_content)?;
+            errors
+                .into_iter()
+                .filter(|e| !e.is_unused_ignore())
+                .collect()
         } else {
             // Run type checking to collect errors
             self.config_override.validate()?;
@@ -49,11 +53,12 @@ impl SuppressArgs {
                 super::check::CheckArgs::parse_from(["check", "--output-format", "omit-errors"]);
             let (_, errors) = check_args.run_once(files_to_check, config_finder)?;
 
-            // Convert to SuppressableErrors, filtering by severity
+            // Convert to SerializedErrors, filtering by severity and excluding UnusedIgnore
             errors
                 .into_iter()
                 .filter(|e| e.severity() >= Severity::Warn)
-                .filter_map(|e| SuppressableError::from_error(&e))
+                .filter_map(|e| SerializedError::from_error(&e))
+                .filter(|e| !e.is_unused_ignore())
                 .collect()
         };
 

pyrefly/lib/error/suppress.rs

@@ -18,33 +18,31 @@ use pyrefly_util::fs_anyhow;
 use regex::Regex;
 use ruff_python_ast::PySourceType;
 use serde::Deserialize;
+use serde::Serialize;
 use starlark_map::small_map::SmallMap;
 use starlark_map::small_set::SmallSet;
 use tracing::info;
 
 use crate::error::error::Error;
 
-/// A minimal representation of an error for suppression purposes.
-/// This struct holds only the fields needed to add or remove a suppression comment.
-#[derive(Deserialize)]
-pub struct SuppressableError {
+/// A serializable representation of an error for JSON input/output.
+/// This struct holds the fields needed to add or remove a suppression comment.
+#[derive(Deserialize, Serialize)]
+pub struct SerializedError {
     /// The file path where the error occurs.
     pub path: PathBuf,
     /// The 0-indexed line number where the error occurs.
     pub line: usize,
     /// The kebab-case name of the error kind (e.g., "bad-assignment").
     pub name: String,
+    /// The error message. Used for UnusedIgnore errors to determine what to remove.
+    pub message: String,
 }
 
-impl SuppressableError {
-    /// Creates a SuppressableError from an internal Error.
-    /// Returns None if the error is not from a filesystem path or if the error
-    /// is an UnusedIgnore (which cannot be suppressed).
+impl SerializedError {
+    /// Creates a SerializedError from an internal Error.
+    /// Returns None if the error is not from a filesystem path.
     pub fn from_error(error: &Error) -> Option<Self> {
-        // UnusedIgnore errors cannot be suppressed
-        if error.error_kind() == ErrorKind::UnusedIgnore {
-            return None;
-        }
         if let ModulePathDetails::FileSystem(path) = error.path().details() {
             Some(Self {
                 path: (**path).clone(),
@@ -54,11 +52,17 @@ impl SuppressableError {
                     .line_within_file()
                     .to_zero_indexed() as usize,
                 name: error.error_kind().to_name().to_owned(),
+                message: error.msg().to_owned(),
             })
         } else {
             None
         }
     }
+
+    /// Returns true if this error is an UnusedIgnore error.
+    pub fn is_unused_ignore(&self) -> bool {
+        self.name == ErrorKind::UnusedIgnore.to_name()
+    }
 }
 
 /// Detects the line ending style used in a string.
@@ -73,7 +77,7 @@ fn detect_line_ending(content: &str) -> &'static str {
 
 /// Combines all errors that affect one line into a single entry.
 /// The current format is: `# pyrefly: ignore [error1, error2, ...]`
-fn dedup_errors(errors: &[SuppressableError]) -> SmallMap<usize, String> {
+fn dedup_errors(errors: &[SerializedError]) -> SmallMap<usize, String> {
     let mut deduped_errors: SmallMap<usize, HashSet<String>> = SmallMap::new();
     for error in errors {
         deduped_errors
@@ -199,7 +203,7 @@ fn replace_ignore_comment(line: &str, merged_comment: &str) -> String {
 /// Returns a list of files that failed to be patched, and a list of files that were patched.
 /// The list of failures includes the error that occurred, which may be a read or write error.
 fn add_suppressions(
-    path_errors: &SmallMap<PathBuf, Vec<SuppressableError>>,
+    path_errors: &SmallMap<PathBuf, Vec<SerializedError>>,
 ) -> (Vec<(&PathBuf, anyhow::Error)>, Vec<&PathBuf>) {
     let mut failures = vec![];
     let mut successes = vec![];
@@ -309,9 +313,9 @@ fn extract_error_codes(comment: &str) -> Vec<String> {
 }
 
 /// Suppresses errors by adding ignore comments to source files.
-/// Takes a list of SuppressableErrors
-pub fn suppress_errors(errors: Vec<SuppressableError>) {
-    let mut path_errors: SmallMap<PathBuf, Vec<SuppressableError>> = SmallMap::new();
+/// Takes a list of SerializedErrors
+pub fn suppress_errors(errors: Vec<SerializedError>) {
+    let mut path_errors: SmallMap<PathBuf, Vec<SerializedError>> = SmallMap::new();
     for e in errors {
         path_errors.entry(e.path.clone()).or_default().push(e);
     }
@@ -512,12 +516,12 @@ mod tests {
 
     fn assert_suppress_errors(before: &str, after: &str) {
         let (errors, tdir) = get_errors(before);
-        let suppressable_errors: Vec<SuppressableError> = errors
+        let suppressable_errors: Vec<SerializedError> = errors
             .collect_errors()
             .shown
             .iter()
             .filter(|e| e.severity() >= Severity::Warn)
-            .filter_map(SuppressableError::from_error)
+            .filter_map(SerializedError::from_error)
             .collect();
         suppress::suppress_errors(suppressable_errors);
         let got_file = fs_anyhow::read_to_string(&get_path(&tdir)).unwrap();