From 91aec074836139fa990a1843a846b4e4fbb5da41 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 7 Aug 2025 08:47:41 -0400 Subject: [PATCH 1/9] failing carriage return test --- crates/ruff_db/src/diagnostic/render/full.rs | 30 +++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/crates/ruff_db/src/diagnostic/render/full.rs b/crates/ruff_db/src/diagnostic/render/full.rs index b42f4043f0297..bc99399aec763 100644 --- a/crates/ruff_db/src/diagnostic/render/full.rs +++ b/crates/ruff_db/src/diagnostic/render/full.rs @@ -1,7 +1,7 @@ #[cfg(test)] mod tests { use ruff_diagnostics::Applicability; - use ruff_text_size::TextRange; + use ruff_text_size::{TextRange, TextSize}; use crate::diagnostic::{ Annotation, DiagnosticFormat, Severity, @@ -400,4 +400,32 @@ print() help: Remove `print` statement "); } + + /// Carriage return (`\r`) is a valid line-ending in Python, so we should normalize this to a + /// line feed (`\n`) for rendering. Otherwise we report a single long line for this case. + #[test] + fn normalize_carriage_return() { + let mut env = TestEnvironment::new(); + env.add( + "example.py", + "# Keep parenthesis around preserved CR\rint(-\r 1)\rint(+\r 1)", + ); + env.format(DiagnosticFormat::Full); + + let mut diagnostic = env.err().build(); + let span = env + .path("example.py") + .with_range(TextRange::at(TextSize::new(39), TextSize::new(0))); + let annotation = Annotation::primary(span); + diagnostic.annotate(annotation); + + insta::assert_snapshot!(env.render(&diagnostic), @" + error[test-diagnostic]: main diagnostic message + --> example.py:1:40 + | + 1 | # Keep parenthesis around preserved CR\rint(-\r 1)\rint(+ + | ^ + | + "); + } } From b64928954015fbbfd3570422041ef842a906e7c2 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 7 Aug 2025 08:37:32 -0400 Subject: [PATCH 2/9] normalize carriage return line endings before rendering --- crates/ruff_db/src/diagnostic/render.rs | 7 ++++++- crates/ruff_db/src/diagnostic/render/full.rs | 17 ++++++++++------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index bcdaaad15650e..7ebb1de25420a 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -1000,7 +1000,12 @@ fn replace_unprintable<'r>( let mut last_end = 0; let mut result = String::new(); for (index, c) in source.char_indices() { - if let Some(printable) = unprintable_replacement(c) { + // normalize `\r` line endings but don't double `\r\n` + if c == '\r' && !matches!(source.get(index + 1..index + 2), Some("\n")) { + result.push_str(&source[last_end..index]); + result.push('\n'); + last_end = index + 1; + } else if let Some(printable) = unprintable_replacement(c) { result.push_str(&source[last_end..index]); let len = printable.text_len().to_u32(); diff --git a/crates/ruff_db/src/diagnostic/render/full.rs b/crates/ruff_db/src/diagnostic/render/full.rs index bc99399aec763..3ccfa1c461540 100644 --- a/crates/ruff_db/src/diagnostic/render/full.rs +++ b/crates/ruff_db/src/diagnostic/render/full.rs @@ -419,13 +419,16 @@ print() let annotation = Annotation::primary(span); diagnostic.annotate(annotation); - insta::assert_snapshot!(env.render(&diagnostic), @" - error[test-diagnostic]: main diagnostic message - --> example.py:1:40 - | - 1 | # Keep parenthesis around preserved CR\rint(-\r 1)\rint(+ - | ^ - | + insta::assert_snapshot!(env.render(&diagnostic), @r" + error[test-diagnostic]: main diagnostic message + --> example.py:2:1 + | + 1 | # Keep parenthesis around preserved CR + 2 | int(- + | ^ + 3 | 1) + 4 | int(+ + | "); } } From 6ad20b41fd0b7e5e77fac51aa3b38f1e2e7d10a4 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 7 Aug 2025 08:52:28 -0400 Subject: [PATCH 3/9] failing BOM test --- crates/ruff_db/src/diagnostic/render/full.rs | 24 ++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/crates/ruff_db/src/diagnostic/render/full.rs b/crates/ruff_db/src/diagnostic/render/full.rs index 3ccfa1c461540..cd1b8aaa7fe03 100644 --- a/crates/ruff_db/src/diagnostic/render/full.rs +++ b/crates/ruff_db/src/diagnostic/render/full.rs @@ -431,4 +431,28 @@ print() | "); } + + /// Without stripping the BOM, we report an error in column 2, unlike Ruff. + #[test] + fn strip_bom() { + let mut env = TestEnvironment::new(); + env.add("example.py", "\u{feff}import foo"); + env.format(DiagnosticFormat::Full); + + let mut diagnostic = env.err().build(); + let span = env + .path("example.py") + .with_range(TextRange::at(TextSize::new(3), TextSize::new(0))); + let annotation = Annotation::primary(span); + diagnostic.annotate(annotation); + + insta::assert_snapshot!(env.render(&diagnostic), @r" + error[test-diagnostic]: main diagnostic message + --> example.py:1:2 + | + 1 | import foo + | ^ + | + "); + } } From 0f8c62c77a34a5b7649a361708acde1ce95ec1d1 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 7 Aug 2025 08:37:59 -0400 Subject: [PATCH 4/9] strip the BOM when normalizing snippets --- crates/ruff_db/src/diagnostic/render.rs | 16 ++++++++++++++++ crates/ruff_db/src/diagnostic/render/full.rs | 4 ++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index 7ebb1de25420a..2ce2e551da91c 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -655,6 +655,22 @@ impl<'r> RenderableSnippet<'r> { .as_source_code() .slice(TextRange::new(snippet_start, snippet_end)); + // Strip the BOM from the beginning of the snippet, if present. Doing this here saves us the + // trouble of updating the annotation ranges in `replace_unprintable`, and also allows us to + // check that the BOM is at the very beginning of the file, not just the beginning of the + // snippet. + const BOM: char = '\u{feff}'; + let bom_len = BOM.text_len(); + let (snippet, snippet_start) = + if snippet_start == TextSize::default() && snippet.starts_with(BOM) { + ( + &snippet[bom_len.to_usize()..], + snippet_start + TextSize::new(bom_len.to_u32()), + ) + } else { + (snippet, snippet_start) + }; + let annotations = anns .iter() .map(|ann| RenderableAnnotation::new(snippet_start, ann)) diff --git a/crates/ruff_db/src/diagnostic/render/full.rs b/crates/ruff_db/src/diagnostic/render/full.rs index cd1b8aaa7fe03..ce49de36a000b 100644 --- a/crates/ruff_db/src/diagnostic/render/full.rs +++ b/crates/ruff_db/src/diagnostic/render/full.rs @@ -448,9 +448,9 @@ print() insta::assert_snapshot!(env.render(&diagnostic), @r" error[test-diagnostic]: main diagnostic message - --> example.py:1:2 + --> example.py:1:1 | - 1 | import foo + 1 | import foo | ^ | "); From e4322e7b0e244544352ca00b70f5848a1ccd5b11 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 7 Aug 2025 08:57:16 -0400 Subject: [PATCH 5/9] failing eof test --- crates/ruff_db/src/diagnostic/render/full.rs | 29 +++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/crates/ruff_db/src/diagnostic/render/full.rs b/crates/ruff_db/src/diagnostic/render/full.rs index ce49de36a000b..60f4c1ac4ad98 100644 --- a/crates/ruff_db/src/diagnostic/render/full.rs +++ b/crates/ruff_db/src/diagnostic/render/full.rs @@ -1,7 +1,7 @@ #[cfg(test)] mod tests { use ruff_diagnostics::Applicability; - use ruff_text_size::{TextRange, TextSize}; + use ruff_text_size::{TextLen, TextRange, TextSize}; use crate::diagnostic::{ Annotation, DiagnosticFormat, Severity, @@ -455,4 +455,31 @@ print() | "); } + + /// We previously rendered this correctly, but the header was falling back to 1:1 for ranges + /// pointing to the final newline in a file. Like Ruff, we now use the offset of the first + /// character in the nonexistent final line in the header. + #[test] + fn end_of_file() { + let mut env = TestEnvironment::new(); + let contents = "unexpected eof\n"; + env.add("example.py", contents); + env.format(DiagnosticFormat::Full); + + let mut diagnostic = env.err().build(); + let span = env + .path("example.py") + .with_range(TextRange::at(contents.text_len(), TextSize::new(0))); + let annotation = Annotation::primary(span); + diagnostic.annotate(annotation); + + insta::assert_snapshot!(env.render(&diagnostic), @r" + error[test-diagnostic]: main diagnostic message + --> example.py:1:1 + | + 1 | unexpected eof + | ^ + | + "); + } } From ee971eae82565a041f306e65d48f38b9a61a7b1a Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 7 Aug 2025 08:38:17 -0400 Subject: [PATCH 6/9] allow ranges at the very end of a file --- crates/ruff_annotate_snippets/src/renderer/display_list.rs | 6 +++++- crates/ruff_db/src/diagnostic/render/full.rs | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/ruff_annotate_snippets/src/renderer/display_list.rs b/crates/ruff_annotate_snippets/src/renderer/display_list.rs index 8367c38c5d192..0c9980d507947 100644 --- a/crates/ruff_annotate_snippets/src/renderer/display_list.rs +++ b/crates/ruff_annotate_snippets/src/renderer/display_list.rs @@ -1273,13 +1273,17 @@ fn format_header<'a>( .. } = item { - if main_range >= range.0 && main_range < range.1 + max(*end_line as usize, 1) { + let end_of_range = range.1 + max(*end_line as usize, 1); + if main_range >= range.0 && main_range < end_of_range { let char_column = text[0..(main_range - range.0).min(text.len())] .chars() .count(); col = char_column + 1; line_offset = lineno.unwrap_or(1); break; + } else if main_range == end_of_range { + line_offset = lineno.map_or(1, |line| line + 1); + break; } } } diff --git a/crates/ruff_db/src/diagnostic/render/full.rs b/crates/ruff_db/src/diagnostic/render/full.rs index 60f4c1ac4ad98..1b01c16a3fd41 100644 --- a/crates/ruff_db/src/diagnostic/render/full.rs +++ b/crates/ruff_db/src/diagnostic/render/full.rs @@ -475,7 +475,7 @@ print() insta::assert_snapshot!(env.render(&diagnostic), @r" error[test-diagnostic]: main diagnostic message - --> example.py:1:1 + --> example.py:2:1 | 1 | unexpected eof | ^ From 47815c7ef5d7e6ef5e88026f4cf0ea755b7b62c6 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 7 Aug 2025 09:57:25 -0400 Subject: [PATCH 7/9] simplify newline check --- crates/ruff_db/src/diagnostic/render.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index 2ce2e551da91c..64b81cc367d21 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -1017,7 +1017,7 @@ fn replace_unprintable<'r>( let mut result = String::new(); for (index, c) in source.char_indices() { // normalize `\r` line endings but don't double `\r\n` - if c == '\r' && !matches!(source.get(index + 1..index + 2), Some("\n")) { + if c == '\r' && !source[index + 1..].starts_with("\n") { result.push_str(&source[last_end..index]); result.push('\n'); last_end = index + 1; From e02704a48e1b5976059f1a39bf3b68db346827e2 Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Thu, 7 Aug 2025 10:08:08 -0400 Subject: [PATCH 8/9] add TextSize::ZERO, use it for BOM check --- crates/ruff_db/src/diagnostic/render.rs | 2 +- crates/ruff_text_size/src/size.rs | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index 64b81cc367d21..8b8436cc49210 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -662,7 +662,7 @@ impl<'r> RenderableSnippet<'r> { const BOM: char = '\u{feff}'; let bom_len = BOM.text_len(); let (snippet, snippet_start) = - if snippet_start == TextSize::default() && snippet.starts_with(BOM) { + if snippet_start == TextSize::ZERO && snippet.starts_with(BOM) { ( &snippet[bom_len.to_usize()..], snippet_start + TextSize::new(bom_len.to_u32()), diff --git a/crates/ruff_text_size/src/size.rs b/crates/ruff_text_size/src/size.rs index dda5ded61db12..e4e45be7a8da4 100644 --- a/crates/ruff_text_size/src/size.rs +++ b/crates/ruff_text_size/src/size.rs @@ -33,6 +33,9 @@ impl fmt::Debug for TextSize { } impl TextSize { + /// A `TextSize` of zero. + pub const ZERO: TextSize = TextSize::new(0); + /// Creates a new `TextSize` at the given `offset`. /// /// # Examples From 9bfeaaf8379cbf0479a96dd3b1482330d6854d0b Mon Sep 17 00:00:00 2001 From: Brent Westbrook Date: Fri, 8 Aug 2025 10:19:18 -0400 Subject: [PATCH 9/9] add comment --- crates/ruff_annotate_snippets/src/renderer/display_list.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/ruff_annotate_snippets/src/renderer/display_list.rs b/crates/ruff_annotate_snippets/src/renderer/display_list.rs index 0c9980d507947..03bcf662965d1 100644 --- a/crates/ruff_annotate_snippets/src/renderer/display_list.rs +++ b/crates/ruff_annotate_snippets/src/renderer/display_list.rs @@ -1273,6 +1273,9 @@ fn format_header<'a>( .. } = item { + // At the very end of the `main_range`, report the location as the first character + // in the next line instead of falling back to the default location of `1:1`. This + // is another divergence from upstream. let end_of_range = range.1 + max(*end_line as usize, 1); if main_range >= range.0 && main_range < end_of_range { let char_column = text[0..(main_range - range.0).min(text.len())]