From 76fb1a19de316382e197c6342700ceab1f4bab4b Mon Sep 17 00:00:00 2001 From: Karnav Kmaleshkumar Prajapati <134150848+Karnav018@users.noreply.github.com> Date: Sat, 16 May 2026 22:45:03 +0530 Subject: [PATCH 1/2] fix(cli): sanitize newlines in min-text output format --- pyrefly/lib/error/error.rs | 54 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/pyrefly/lib/error/error.rs b/pyrefly/lib/error/error.rs index c6292c1229..2d7061ef41 100644 --- a/pyrefly/lib/error/error.rs +++ b/pyrefly/lib/error/error.rs @@ -96,13 +96,16 @@ impl Error { writeln!(f, "{details}")?; } } else if self.severity.is_enabled() { + // In min-text format, sanitize the message to ensure one error per line + // by replacing newlines with spaces. + let sanitized_msg = self.msg_header.replace('\n', " "); writeln!( f, "{} {}:{}: {} [{}]", self.severity.label(), self.path_string_with_fragment(project_root), self.display_range, - self.msg_header, + sanitized_msg, self.error_kind.to_name(), )?; } @@ -125,12 +128,15 @@ impl Error { anstream::println!("{details}"); } } else if self.severity.is_enabled() { + // In min-text format, sanitize the message to ensure one error per line + // by replacing newlines with spaces. + let sanitized_msg = self.msg_header.replace('\n', " "); anstream::println!( "{} {}:{}: {} {}", self.severity.painted(), Paint::blue(&self.path_string_with_fragment(project_root)), Paint::dim(self.display_range()), - Paint::new(&*self.msg_header), + Paint::new(&*sanitized_msg), Paint::dim(format!("[{}]", self.error_kind().to_name()).as_str()), ); } @@ -606,4 +612,48 @@ def f(x: None) -> None: assert_eq!(&*annotations[0].label, "has type `None`"); assert_eq!(&*annotations[1].label, "has type `Literal[2]`"); } + + #[test] + fn test_mintext_format_sanitizes_newlines() { + let module_info = Module::new( + ModuleName::from_str("test"), + ModulePath::filesystem(PathBuf::from("test.py")), + Arc::new("x.append(y)".to_owned()), + ); + // Create an error message with embedded newlines (like union type errors) + let error_with_newlines = Error::new( + module_info, + TextRange::new(TextSize::new(0), TextSize::new(11)), + "Object of class `NoneType` has no attribute `append`\nObject of class `bool` has no attribute `append`" + .to_owned(), + Vec::new(), + ErrorKind::MissingAttribute, + ); + + let root = PathBuf::new(); + let mut mintext_output = Vec::new(); + error_with_newlines + .write_line(&mut Cursor::new(&mut mintext_output), root.as_path(), false) + .unwrap(); + + let output_str = str::from_utf8(&mintext_output).unwrap(); + // Verify that embedded newlines are replaced with spaces + // The output ends with a single newline from writeln!, which is expected in min-text format + let lines: Vec<&str> = output_str.split('\n').collect(); + assert_eq!(lines.len(), 2, "output should have exactly 2 parts: the error line and a final empty part after the trailing newline"); + let error_line = lines[0]; + + // Verify the message still appears, but on a single line without embedded newlines + assert!( + error_line.contains("Object of class `NoneType` has no attribute `append`"), + "first part of message should be present" + ); + assert!( + error_line.contains("Object of class `bool` has no attribute `append`"), + "second part of message should be present" + ); + // Verify the format is still min-text: "SEVERITY path:range: message [kind]" + assert!(error_line.starts_with("ERROR test.py:1:1-11:"), "output should start with 'ERROR test.py:1:1-11:'"); + assert!(error_line.ends_with("[missing-attribute]"), "output should end with error kind"); + } } From f242ec817561a9793be46d1b03e87891ad57388b Mon Sep 17 00:00:00 2001 From: Karnav Kmaleshkumar Prajapati <134150848+Karnav018@users.noreply.github.com> Date: Mon, 18 May 2026 18:29:18 +0530 Subject: [PATCH 2/2] Enforce single-line error headers Why: min-text output requires one error per line, but some errors built multiline headers. What: assert headers contain no newline/CR and split missing-attribute union failures into header plus details. Why it works: invalid headers are rejected at construction and details carry the extra lines. --- pyrefly/lib/alt/attr.rs | 7 ++-- pyrefly/lib/error/error.rs | 75 ++++++++++++-------------------------- 2 files changed, 26 insertions(+), 56 deletions(-) diff --git a/pyrefly/lib/alt/attr.rs b/pyrefly/lib/alt/attr.rs index bcd88bc272..f63e168a36 100644 --- a/pyrefly/lib/alt/attr.rs +++ b/pyrefly/lib/alt/attr.rs @@ -27,7 +27,6 @@ use ruff_python_ast::name::Name; use ruff_text_size::TextRange; use starlark_map::small_set::SmallSet; use vec1::Vec1; -use vec1::vec1; use crate::alt::answers::LookupAnswer; use crate::alt::answers_solver::AnswersSolver; @@ -614,7 +613,8 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { } else if !error_messages.is_empty() { error_messages.sort(); error_messages.dedup(); - let mut msg = vec1![error_messages.join("\n")]; + let header = error_messages.remove(0); + let mut details = error_messages; // Skip suggestions when we have a partial union failure to avoid suggesting // attributes from the types that have them when the problem is that some types // don't have the attribute at all. @@ -623,9 +623,8 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> { .as_ref() .and_then(|attr_base| self.suggest_attribute_name(attr_name, attr_base)) { - msg.push(format!("Did you mean `{suggestion}`?")); + details.push(format!("Did you mean `{suggestion}`?")); } - let (header, details) = msg.split_off_first(); errors .error_builder(range, ErrorKind::MissingAttribute, header) .with_details(details) diff --git a/pyrefly/lib/error/error.rs b/pyrefly/lib/error/error.rs index 2d7061ef41..f66634a3ea 100644 --- a/pyrefly/lib/error/error.rs +++ b/pyrefly/lib/error/error.rs @@ -96,16 +96,13 @@ impl Error { writeln!(f, "{details}")?; } } else if self.severity.is_enabled() { - // In min-text format, sanitize the message to ensure one error per line - // by replacing newlines with spaces. - let sanitized_msg = self.msg_header.replace('\n', " "); writeln!( f, "{} {}:{}: {} [{}]", self.severity.label(), self.path_string_with_fragment(project_root), self.display_range, - sanitized_msg, + self.msg_header, self.error_kind.to_name(), )?; } @@ -128,15 +125,12 @@ impl Error { anstream::println!("{details}"); } } else if self.severity.is_enabled() { - // In min-text format, sanitize the message to ensure one error per line - // by replacing newlines with spaces. - let sanitized_msg = self.msg_header.replace('\n', " "); anstream::println!( "{} {}:{}: {} {}", self.severity.painted(), Paint::blue(&self.path_string_with_fragment(project_root)), Paint::dim(self.display_range()), - Paint::new(&*sanitized_msg), + Paint::new(&*self.msg_header), Paint::dim(format!("[{}]", self.error_kind().to_name()).as_str()), ); } @@ -354,6 +348,10 @@ impl Error { error_kind: ErrorKind, ) -> Self { let display_range = module.display_range(range); + assert!( + !header.contains(['\n', '\r']), + "error header must not contain newlines" + ); let msg_header = header.into_boxed_str(); let msg_details = if details.is_empty() { None @@ -591,6 +589,23 @@ mod tests { ); } + #[test] + #[should_panic(expected = "error header must not contain newlines")] + fn test_error_header_rejects_newlines() { + let module_info = Module::new( + ModuleName::from_str("test"), + ModulePath::filesystem(PathBuf::from("test.py")), + Arc::new("x = 1".to_owned()), + ); + let _ = Error::new( + module_info, + TextRange::new(TextSize::new(0), TextSize::new(1)), + "first line\r\nsecond line".to_owned(), + Vec::new(), + ErrorKind::BadReturn, + ); + } + /// Integration test: verify that binary operator errors from the type checker /// produce secondary annotations labeling both operands with their types. #[test] @@ -612,48 +627,4 @@ def f(x: None) -> None: assert_eq!(&*annotations[0].label, "has type `None`"); assert_eq!(&*annotations[1].label, "has type `Literal[2]`"); } - - #[test] - fn test_mintext_format_sanitizes_newlines() { - let module_info = Module::new( - ModuleName::from_str("test"), - ModulePath::filesystem(PathBuf::from("test.py")), - Arc::new("x.append(y)".to_owned()), - ); - // Create an error message with embedded newlines (like union type errors) - let error_with_newlines = Error::new( - module_info, - TextRange::new(TextSize::new(0), TextSize::new(11)), - "Object of class `NoneType` has no attribute `append`\nObject of class `bool` has no attribute `append`" - .to_owned(), - Vec::new(), - ErrorKind::MissingAttribute, - ); - - let root = PathBuf::new(); - let mut mintext_output = Vec::new(); - error_with_newlines - .write_line(&mut Cursor::new(&mut mintext_output), root.as_path(), false) - .unwrap(); - - let output_str = str::from_utf8(&mintext_output).unwrap(); - // Verify that embedded newlines are replaced with spaces - // The output ends with a single newline from writeln!, which is expected in min-text format - let lines: Vec<&str> = output_str.split('\n').collect(); - assert_eq!(lines.len(), 2, "output should have exactly 2 parts: the error line and a final empty part after the trailing newline"); - let error_line = lines[0]; - - // Verify the message still appears, but on a single line without embedded newlines - assert!( - error_line.contains("Object of class `NoneType` has no attribute `append`"), - "first part of message should be present" - ); - assert!( - error_line.contains("Object of class `bool` has no attribute `append`"), - "second part of message should be present" - ); - // Verify the format is still min-text: "SEVERITY path:range: message [kind]" - assert!(error_line.starts_with("ERROR test.py:1:1-11:"), "output should start with 'ERROR test.py:1:1-11:'"); - assert!(error_line.ends_with("[missing-attribute]"), "output should end with error kind"); - } }