Skip to content

Commit

Permalink
[pygrep_hooks] Fix blanket-noqa panic when last line has noqa wit…
Browse files Browse the repository at this point in the history
…h 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.
  • Loading branch information
augustelalande committed Apr 25, 2024
1 parent 77c93fd commit 3364ef9
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
#noqa
23 changes: 12 additions & 11 deletions crates/ruff_linter/src/noqa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<NoqaCode>,
// Whether the directive applies to range.end
pub(crate) includes_end: bool,
}

impl Ranged for NoqaDirectiveLine<'_> {
Expand Down Expand Up @@ -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 }
}

Expand All @@ -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()
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/pygrep_hooks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()..]);
Expand Down
Original file line number Diff line number Diff line change
@@ -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
|

0 comments on commit 3364ef9

Please sign in to comment.