Skip to content

Skip walking all tokens when loading range suppressions #22446

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
amyreese merged 11 commits into from
Jan 15, 2026
Merged

Skip walking all tokens when loading range suppressions #22446

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f4c51fb
Skip walking all tokens when loading range suppressions
amyreese Jan 7, 2026
0ede5fa
clippy
amyreese Jan 7, 2026
3d92382
remove old code
amyreese Jan 8, 2026
d6a19d0
Clear indents in outer loop
amyreese Jan 13, 2026
5b2da1e
Move final match comments out of loop
amyreese Jan 14, 2026
95afb52
Add `split_at` method to `Tokens`
amyreese Jan 13, 2026
0eaa8dc
Use tokens split_at in range suppression loading
amyreese Jan 13, 2026
47587cd
Continue early if no pending comments
amyreese Jan 15, 2026
6ea9fe4
Use raw split_at
amyreese Jan 15, 2026
e656fee
use matches
amyreese Jan 15, 2026
886b3b0
Document difference in behavior of split_at with and add test case
amyreese Jan 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions crates/ruff_linter/src/linter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,8 @@ pub fn add_noqa_to_path(
);

// Parse range suppression comments
let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens());
let suppressions =
Suppressions::from_tokens(settings, locator.contents(), parsed.tokens(), &indexer);

// Generate diagnostics, ignoring any existing `noqa` directives.
let diagnostics = check_path(
Expand Down Expand Up @@ -479,7 +480,8 @@ pub fn lint_only(
);

// Parse range suppression comments
let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens());
let suppressions =
Suppressions::from_tokens(settings, locator.contents(), parsed.tokens(), &indexer);

// Generate diagnostics.
let diagnostics = check_path(
Expand Down Expand Up @@ -596,7 +598,8 @@ pub fn lint_fix<'a>(
);

// Parse range suppression comments
let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens());
let suppressions =
Suppressions::from_tokens(settings, locator.contents(), parsed.tokens(), &indexer);

// Generate diagnostics.
let diagnostics = check_path(
Expand Down Expand Up @@ -978,7 +981,8 @@ mod tests {
&locator,
&indexer,
);
let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens());
let suppressions =
Suppressions::from_tokens(settings, locator.contents(), parsed.tokens(), &indexer);
let mut diagnostics = check_path(
path,
None,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/pyflakes/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -957,7 +957,7 @@ mod tests {
&indexer,
);
let suppressions =
Suppressions::from_tokens(&settings, locator.contents(), parsed.tokens());
Suppressions::from_tokens(&settings, locator.contents(), parsed.tokens(), &indexer);
let mut messages = check_path(
Path::new("<filename>"),
None,
Expand Down
162 changes: 102 additions & 60 deletions crates/ruff_linter/src/suppression.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ use core::fmt;
use ruff_db::diagnostic::Diagnostic;
use ruff_diagnostics::{Edit, Fix};
use ruff_python_ast::token::{TokenKind, Tokens};
use ruff_python_ast::whitespace::indentation;
use ruff_python_index::Indexer;
use rustc_hash::FxHashSet;
use std::cell::Cell;
use std::{error::Error, fmt::Formatter};
use thiserror::Error;

use ruff_python_trivia::Cursor;
use ruff_python_trivia::{Cursor, indentation_at_offset};
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize, TextSlice};
use smallvec::{SmallVec, smallvec};

Expand Down Expand Up @@ -153,10 +153,15 @@ pub struct Suppressions {
}

impl Suppressions {
pub fn from_tokens(settings: &LinterSettings, source: &str, tokens: &Tokens) -> Suppressions {
pub fn from_tokens(
settings: &LinterSettings,
source: &str,
tokens: &Tokens,
indexer: &Indexer,
) -> Suppressions {
if is_range_suppressions_enabled(settings) {
let builder = SuppressionsBuilder::new(source);
builder.load_from_tokens(tokens)
builder.load_from_tokens(tokens, indexer)
} else {
Suppressions::default()
}
Expand Down Expand Up @@ -355,7 +360,6 @@ pub(crate) struct SuppressionsBuilder<'a> {

valid: Vec<Suppression>,
invalid: Vec<InvalidSuppression>,
errors: Vec<ParseError>,

pending: Vec<PendingSuppressionComment<'a>>,
}
Expand All @@ -368,74 +372,109 @@ impl<'a> SuppressionsBuilder<'a> {
}
}

pub(crate) fn load_from_tokens(mut self, tokens: &Tokens) -> Suppressions {
let default_indent = "";
pub(crate) fn load_from_tokens(mut self, tokens: &Tokens, indexer: &Indexer) -> Suppressions {
let mut indents: Vec<&str> = vec![];

// Iterate through tokens, tracking indentation, filtering trailing comments, and then
// looking for matching comments from the previous block when reaching a dedent token.
for (token_index, token) in tokens.iter().enumerate() {
match token.kind() {
TokenKind::Indent => {
indents.push(self.source.slice(token));
}
TokenKind::Dedent => {
self.match_comments(indents.last().copied().unwrap_or_default(), token.range());
indents.pop();
let mut errors = Vec::new();

let mut suppressions = indexer
.comment_ranges()
.iter()
.copied()
.filter_map(|comment_range| {
let mut parser = SuppressionParser::new(self.source, comment_range);
match parser.parse_comment() {
Ok(comment) => Some(comment),
Err(ParseError {
kind: ParseErrorKind::NotASuppression,
..
}) => None,
Err(error) => {
errors.push(error);
None
}
}
TokenKind::Comment => {
let mut parser = SuppressionParser::new(self.source, token.range());
match parser.parse_comment() {
Ok(comment) => {
let indent = indentation(self.source, &comment.range);

let Some(indent) = indent else {
// trailing suppressions are not supported
self.invalid.push(InvalidSuppression {
kind: InvalidSuppressionKind::Trailing,
comment,
});
continue;
};

// comment matches current block's indentation, or precedes an indent/dedent token
if indent == indents.last().copied().unwrap_or_default()
|| tokens[token_index..]
.iter()
.find(|t| !t.kind().is_trivia())
.is_some_and(|t| {
matches!(t.kind(), TokenKind::Dedent | TokenKind::Indent)
})
{
self.pending
.push(PendingSuppressionComment { indent, comment });
} else {
// weirdly indented? ¯\_(ツ)_/¯
self.invalid.push(InvalidSuppression {
kind: InvalidSuppressionKind::Indentation,
comment,
});
}
})
.peekable();

'comments: while let Some(suppression) = suppressions.peek() {
indents.clear();

let (before, after) = tokens.split_at(suppression.range.start());
let last_indent = before
.iter()
.rfind(|token| token.kind() == TokenKind::Indent)
.map(|token| self.source.slice(token))
.unwrap_or_default();

indents.push(last_indent);

// Iterate through tokens, tracking indentation, filtering trailing comments, and then
// looking for matching comments from the previous block when reaching a dedent token.
for (token_index, token) in after.iter().enumerate() {
let current_indent = indents.last().copied().unwrap_or_default();
match token.kind() {
TokenKind::Indent => {
indents.push(self.source.slice(token));
}
TokenKind::Dedent => {
self.match_comments(current_indent, token.range());

indents.pop();

if indents.is_empty() || self.pending.is_empty() {
continue 'comments;
}
Err(ParseError {
kind: ParseErrorKind::NotASuppression,
..
}) => {}
Err(error) => {
self.errors.push(error);
}
TokenKind::Comment => {
let Some(suppression) =
suppressions.next_if(|suppression| suppression.range == token.range())
else {
continue;
};

let Some(indent) =
indentation_at_offset(suppression.range.start(), self.source)
else {
// trailing suppressions are not supported
self.invalid.push(InvalidSuppression {
kind: InvalidSuppressionKind::Trailing,
comment: suppression,
});
continue;
};

// comment matches current block's indentation, or precedes an indent/dedent token
if indent == current_indent
|| after[token_index..]
.iter()
.find(|t| !t.kind().is_trivia())
.is_some_and(|t| {
matches!(t.kind(), TokenKind::Dedent | TokenKind::Indent)
})
{
self.pending.push(PendingSuppressionComment {
indent,
comment: suppression,
});
} else {
// weirdly indented? ¯\_(ツ)_/¯
self.invalid.push(InvalidSuppression {
kind: InvalidSuppressionKind::Indentation,
comment: suppression,
});
}
}
_ => {}
}
_ => {}
}
}

self.match_comments(default_indent, TextRange::up_to(self.source.text_len()));
self.match_comments("", TextRange::up_to(self.source.text_len()));

Suppressions {
valid: self.valid,
invalid: self.invalid,
errors: self.errors,
errors,
}
}

Expand Down Expand Up @@ -691,6 +730,7 @@ mod tests {

use insta::assert_debug_snapshot;
use itertools::Itertools;
use ruff_python_index::Indexer;
use ruff_python_parser::{Mode, ParseOptions, parse};
use ruff_text_size::{TextLen, TextRange, TextSize};
use similar::DiffableStr;
Expand Down Expand Up @@ -1585,10 +1625,12 @@ def bar():
/// Parse all suppressions and errors in a module for testing
fn debug(source: &'_ str) -> DebugSuppressions<'_> {
let parsed = parse(source, ParseOptions::from(Mode::Module)).unwrap();
let indexer = Indexer::from_tokens(parsed.tokens(), source);
let suppressions = Suppressions::from_tokens(
&LinterSettings::default().with_preview_mode(),
source,
parsed.tokens(),
&indexer,
);
DebugSuppressions {
source,
Expand Down
5 changes: 3 additions & 2 deletions crates/ruff_linter/src/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,8 @@ pub(crate) fn test_contents<'a>(
&locator,
&indexer,
);
let suppressions = Suppressions::from_tokens(settings, locator.contents(), parsed.tokens());
let suppressions =
Suppressions::from_tokens(settings, locator.contents(), parsed.tokens(), &indexer);
let messages = check_path(
path,
path.parent()
Expand Down Expand Up @@ -303,7 +304,7 @@ pub(crate) fn test_contents<'a>(
);

let suppressions =
Suppressions::from_tokens(settings, locator.contents(), parsed.tokens());
Suppressions::from_tokens(settings, locator.contents(), parsed.tokens(), &indexer);
let fixed_messages = check_path(
path,
None,
Expand Down
Loading