From 3364ef957de9eb9d721674c401272ac6f5c4decb Mon Sep 17 00:00:00 2001 From: Auguste Lalande Date: Thu, 25 Apr 2024 14:39:38 -0400 Subject: [PATCH] [`pygrep_hooks`] Fix `blanket-noqa` panic when last line has noqa with no newline (`PGH004`) (#11108) ## Summary Resolves #11102 The error stems from these lines https://github.com/astral-sh/ruff/blob/f5c7a62aa65decb9e286bd65ba17f1a3bd1f91e6/crates/ruff_linter/src/noqa.rs#L697-L702 I don't really understand the purpose of incrementing the last index, but it makes the resulting range invalid for indexing into `contents`. For now I just detect if the index is too high in `blanket_noqa` and adjust it if necessary. ## Test Plan Created fixture from issue example. --- .../test/fixtures/pygrep_hooks/PGH004_1.py | 1 + crates/ruff_linter/src/noqa.rs | 23 ++++++++++--------- .../ruff_linter/src/rules/pygrep_hooks/mod.rs | 1 + .../rules/pygrep_hooks/rules/blanket_noqa.rs | 5 ++-- ...grep_hooks__tests__PGH004_PGH004_1.py.snap | 8 +++++++ 5 files changed, 24 insertions(+), 14 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_1.py create mode 100644 crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_1.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_1.py b/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_1.py new file mode 100644 index 0000000000..ea4cde0074 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pygrep_hooks/PGH004_1.py @@ -0,0 +1 @@ +#noqa \ No newline at end of file diff --git a/crates/ruff_linter/src/noqa.rs b/crates/ruff_linter/src/noqa.rs index 3af740e819..60012adb55 100644 --- a/crates/ruff_linter/src/noqa.rs +++ b/crates/ruff_linter/src/noqa.rs @@ -652,6 +652,8 @@ pub(crate) struct NoqaDirectiveLine<'a> { pub(crate) directive: Directive<'a>, /// The codes that are ignored by the directive. pub(crate) matches: Vec, + // Whether the directive applies to range.end + pub(crate) includes_end: bool, } impl Ranged for NoqaDirectiveLine<'_> { @@ -684,23 +686,18 @@ impl<'a> NoqaDirectives<'a> { } Ok(Some(directive)) => { // noqa comments are guaranteed to be single line. + let range = locator.line_range(range.start()); directives.push(NoqaDirectiveLine { - range: locator.line_range(range.start()), + range, directive, matches: Vec::new(), + includes_end: range.end() == locator.contents().text_len(), }); } Ok(None) => {} } } - // Extend a mapping at the end of the file to also include the EOF token. - if let Some(last) = directives.last_mut() { - if last.range.end() == locator.contents().text_len() { - last.range = last.range.add_end(TextSize::from(1)); - } - } - Self { inner: directives } } @@ -724,11 +721,15 @@ impl<'a> NoqaDirectives<'a> { .binary_search_by(|directive| { if directive.range.end() < offset { std::cmp::Ordering::Less - } else if directive.range.contains(offset) { - std::cmp::Ordering::Equal - } else { + } else if directive.range.start() > offset { std::cmp::Ordering::Greater } + // At this point, end >= offset, start <= offset + else if !directive.includes_end && directive.range.end() == offset { + std::cmp::Ordering::Less + } else { + std::cmp::Ordering::Equal + } }) .ok() } diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/mod.rs b/crates/ruff_linter/src/rules/pygrep_hooks/mod.rs index 8cce061b15..f06e8eda13 100644 --- a/crates/ruff_linter/src/rules/pygrep_hooks/mod.rs +++ b/crates/ruff_linter/src/rules/pygrep_hooks/mod.rs @@ -16,6 +16,7 @@ mod tests { #[test_case(Rule::BlanketTypeIgnore, Path::new("PGH003_0.py"))] #[test_case(Rule::BlanketTypeIgnore, Path::new("PGH003_1.py"))] #[test_case(Rule::BlanketNOQA, Path::new("PGH004_0.py"))] + #[test_case(Rule::BlanketNOQA, Path::new("PGH004_1.py"))] #[test_case(Rule::InvalidMockAccess, Path::new("PGH005_0.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs b/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs index e8cbf7da62..62e40938da 100644 --- a/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs +++ b/crates/ruff_linter/src/rules/pygrep_hooks/rules/blanket_noqa.rs @@ -88,9 +88,8 @@ pub(crate) fn blanket_noqa( ) { for directive_line in noqa_directives.lines() { if let Directive::All(all) = &directive_line.directive { - let line = locator.slice(directive_line.range); - let offset = directive_line.range.start(); - let noqa_end = all.end() - offset; + let line = locator.slice(directive_line); + let noqa_end = all.end() - directive_line.start(); // Skip the `# noqa`, plus any trailing whitespace. let mut cursor = Cursor::new(&line[noqa_end.to_usize()..]); diff --git a/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_1.py.snap b/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_1.py.snap new file mode 100644 index 0000000000..34c0242695 --- /dev/null +++ b/crates/ruff_linter/src/rules/pygrep_hooks/snapshots/ruff_linter__rules__pygrep_hooks__tests__PGH004_PGH004_1.py.snap @@ -0,0 +1,8 @@ +--- +source: crates/ruff_linter/src/rules/pygrep_hooks/mod.rs +--- +PGH004_1.py:1:1: PGH004 Use specific rule codes when using `noqa` + | +1 | #noqa + | ^^^^^ PGH004 + |