From c433865801e2ee06902199d6b0a223cfca52269b Mon Sep 17 00:00:00 2001 From: Brent Westbrook <36778786+ntBre@users.noreply.github.com> Date: Mon, 11 Aug 2025 08:52:27 -0400 Subject: [PATCH] Avoid underflow in default ranges before a BOM (#19839) Summary -- This fixes a regression caused by the BOM handling in #19806. Most diagnostics already account for the BOM in their ranges, but those that use `TextRange::default` to mean the beginning of the file do not, causing an underflow in `RenderableAnnotation::new` when subtracting the BOM-shifted `snippet_start` from the annotation range. I ran into this when trying to run benchmarks on CPython in preparation for caching work. The file `cpython/Lib/test/bad_coding2.py` was causing a crash because it had a default-range `I002` diagnostic, with a BOM. https://github.com/astral-sh/ruff/blob/7cc3f1ebe9386e77e7009bc411fc6480d3851015/crates/ruff_linter/src/rules/isort/rules/add_required_imports.rs#L122-L126 The fix here is just to saturate to zero instead of panicking. I considered adding a `TextRange::saturating_sub` method, but I wasn't sure it was worth it for this one use. I'm happy to do that if preferred, though. Saturating seemed easier than shifting the affected annotations over, but that could be another solution. Test Plan -- A new `ruff_db` test that reproduced the issue and manual testing against the CPython file mentioned above --- crates/ruff_db/src/diagnostic/render.rs | 6 +++++- crates/ruff_db/src/diagnostic/render/full.rs | 21 ++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/crates/ruff_db/src/diagnostic/render.rs b/crates/ruff_db/src/diagnostic/render.rs index 43c676856d..eba3253c96 100644 --- a/crates/ruff_db/src/diagnostic/render.rs +++ b/crates/ruff_db/src/diagnostic/render.rs @@ -710,7 +710,11 @@ impl<'r> RenderableAnnotation<'r> { /// lifetime parameter here refers to the lifetime of the resolver that /// created the given `ResolvedAnnotation`. fn new(snippet_start: TextSize, ann: &'_ ResolvedAnnotation<'r>) -> RenderableAnnotation<'r> { - let range = ann.range - snippet_start; + // This should only ever saturate if a BOM is present _and_ the annotation range points + // before the BOM (i.e. at offset 0). In Ruff this typically results from the use of + // `TextRange::default()` for a diagnostic range instead of a range relative to file + // contents. + let range = ann.range.checked_sub(snippet_start).unwrap_or(ann.range); RenderableAnnotation { range, message: ann.message, diff --git a/crates/ruff_db/src/diagnostic/render/full.rs b/crates/ruff_db/src/diagnostic/render/full.rs index 300e370705..0eee73e543 100644 --- a/crates/ruff_db/src/diagnostic/render/full.rs +++ b/crates/ruff_db/src/diagnostic/render/full.rs @@ -512,6 +512,27 @@ print() "); } + #[test] + fn bom_with_default_range() { + 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::default()); + 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 | import foo + | ^ + | + "); + } + /// 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.