mirror of
https://github.com/astral-sh/ruff.git
synced 2025-10-03 07:04:37 +00:00
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.
7cc3f1ebe9/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
This commit is contained in:
parent
5b6d0d17f1
commit
c433865801
2 changed files with 26 additions and 1 deletions
|
@ -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,
|
||||
|
|
|
@ -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.
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue