From 647548b5e7ce4c60b9bb5ab8d256d41254a360ae Mon Sep 17 00:00:00 2001 From: Auguste Lalande Date: Mon, 22 Apr 2024 13:36:25 -0400 Subject: [PATCH] [`pygrep_hooks`] Move `blanket-noqa` to noqa checker (`PGH004`) (#11053) ## Summary Move `blanket-noqa` rule from the token checker to the noqa checker. This allows us to make use of the line directives already computed in the noqa checker. ## Test Plan Verified test results are unchanged. --- crates/ruff_linter/src/checkers/noqa.rs | 5 +++ crates/ruff_linter/src/checkers/tokens.rs | 4 --- crates/ruff_linter/src/registry.rs | 3 +- .../rules/pygrep_hooks/rules/blanket_noqa.rs | 33 +++++++++---------- 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/crates/ruff_linter/src/checkers/noqa.rs b/crates/ruff_linter/src/checkers/noqa.rs index e82645421e8ab..54648fa8292d4 100644 --- a/crates/ruff_linter/src/checkers/noqa.rs +++ b/crates/ruff_linter/src/checkers/noqa.rs @@ -14,6 +14,7 @@ use crate::noqa; use crate::noqa::{Directive, FileExemption, NoqaDirectives, NoqaMapping}; use crate::registry::{AsRule, Rule, RuleSet}; use crate::rule_redirects::get_redirect_target; +use crate::rules::pygrep_hooks; use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA}; use crate::settings::LinterSettings; @@ -203,6 +204,10 @@ pub(crate) fn check_noqa( } } + if settings.rules.enabled(Rule::BlanketNOQA) { + pygrep_hooks::rules::blanket_noqa(diagnostics, &noqa_directives, locator); + } + ignored_diagnostics.sort_unstable(); ignored_diagnostics } diff --git a/crates/ruff_linter/src/checkers/tokens.rs b/crates/ruff_linter/src/checkers/tokens.rs index 704926c005414..ca08b27f5b447 100644 --- a/crates/ruff_linter/src/checkers/tokens.rs +++ b/crates/ruff_linter/src/checkers/tokens.rs @@ -45,10 +45,6 @@ pub(crate) fn check_tokens( .check_lines(tokens, &mut diagnostics); } - if settings.rules.enabled(Rule::BlanketNOQA) { - pygrep_hooks::rules::blanket_noqa(&mut diagnostics, indexer, locator); - } - if settings.rules.enabled(Rule::BlanketTypeIgnore) { pygrep_hooks::rules::blanket_type_ignore(&mut diagnostics, indexer, locator); } diff --git a/crates/ruff_linter/src/registry.rs b/crates/ruff_linter/src/registry.rs index 03a24e452f8c9..e134adf213ddf 100644 --- a/crates/ruff_linter/src/registry.rs +++ b/crates/ruff_linter/src/registry.rs @@ -246,7 +246,7 @@ impl Rule { pub const fn lint_source(&self) -> LintSource { match self { Rule::InvalidPyprojectToml => LintSource::PyprojectToml, - Rule::UnusedNOQA => LintSource::Noqa, + Rule::BlanketNOQA | Rule::UnusedNOQA => LintSource::Noqa, Rule::BidirectionalUnicode | Rule::BlankLineWithWhitespace | Rule::DocLineTooLong @@ -256,7 +256,6 @@ impl Rule { | Rule::MixedSpacesAndTabs | Rule::TrailingWhitespace => LintSource::PhysicalLines, Rule::AmbiguousUnicodeCharacterComment - | Rule::BlanketNOQA | Rule::BlanketTypeIgnore | Rule::BlankLineAfterDecorator | Rule::BlankLineBetweenMethods 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 5d6fc4ff0eeb0..e8cbf7da62b1c 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 @@ -1,11 +1,10 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_index::Indexer; use ruff_python_trivia::Cursor; use ruff_source_file::Locator; -use ruff_text_size::{Ranged, TextRange, TextSize}; +use ruff_text_size::{Ranged, TextRange}; -use crate::noqa::Directive; +use crate::noqa::{Directive, NoqaDirectives}; /// ## What it does /// Check for `noqa` annotations that suppress all diagnostics, as opposed to @@ -84,46 +83,44 @@ impl Violation for BlanketNOQA { /// PGH004 pub(crate) fn blanket_noqa( diagnostics: &mut Vec, - indexer: &Indexer, + noqa_directives: &NoqaDirectives, locator: &Locator, ) { - for range in indexer.comment_ranges() { - let line = locator.slice(*range); - let offset = range.start(); - if let Ok(Some(Directive::All(all))) = Directive::try_extract(line, TextSize::new(0)) { - // The `all` range is that of the `noqa` directive in, e.g., `# noqa` or `# noqa F401`. - let noqa_start = offset + all.start(); - let noqa_end = offset + all.end(); + 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; // Skip the `# noqa`, plus any trailing whitespace. - let mut cursor = Cursor::new(&line[all.end().to_usize()..]); + let mut cursor = Cursor::new(&line[noqa_end.to_usize()..]); cursor.eat_while(char::is_whitespace); // Check for extraneous spaces before the colon. // Ex) `# noqa : F401` if cursor.first() == ':' { - let start = offset + all.end(); + let start = all.end(); let end = start + cursor.token_len(); let mut diagnostic = Diagnostic::new( BlanketNOQA { missing_colon: false, space_before_colon: true, }, - TextRange::new(noqa_start, end), + TextRange::new(all.start(), end), ); diagnostic.set_fix(Fix::unsafe_edit(Edit::deletion(start, end))); diagnostics.push(diagnostic); } else if Directive::lex_code(cursor.chars().as_str()).is_some() { // Check for a missing colon. // Ex) `# noqa F401` - let start = offset + all.end(); - let end = start + TextSize::new(1); + let start = all.end(); + let end = start + cursor.token_len(); let mut diagnostic = Diagnostic::new( BlanketNOQA { missing_colon: true, space_before_colon: false, }, - TextRange::new(noqa_start, end), + TextRange::new(all.start(), end), ); diagnostic.set_fix(Fix::unsafe_edit(Edit::insertion(':'.to_string(), start))); diagnostics.push(diagnostic); @@ -134,7 +131,7 @@ pub(crate) fn blanket_noqa( missing_colon: false, space_before_colon: false, }, - TextRange::new(noqa_start, noqa_end), + all.range(), )); } }