mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-09-26 11:59:35 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Use structs for noqa Directive variants (#5533)

Browse source 
## Summary

No behavioral changes, just clearer (IMO) and with better documentation.
This commit is contained in:
Charlie Marsh 2023-07-05 17:37:32 -04:00 • committed by GitHub
parent 1a2e444799
commit a0c0b74b6d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 202 additions and 137 deletions
Download patch file Download diff file Expand all files Collapse all files

52
crates/ruff/src/checkers/noqa.rs
View file

@ -7,7 +7,7 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix};
use ruff_python_ast::source_code::Locator; use ruff_python_ast::source_code::Locator;
use crate::noqa; use crate::noqa;
use crate::noqa::{Directive, FileExemption, NoqaDirectives, NoqaMapping}; use crate::noqa::{All, Codes, Directive, FileExemption, NoqaDirectives, NoqaMapping};
use crate::registry::{AsRule, Rule}; use crate::registry::{AsRule, Rule};
use crate::rule_redirects::get_redirect_target; use crate::rule_redirects::get_redirect_target;
use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA}; use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA};
@ -22,7 +22,7 @@ pub(crate) fn check_noqa(
settings: &Settings, settings: &Settings,
) -> Vec<usize> { ) -> Vec<usize> {
// Identify any codes that are globally exempted (within the current file). // Identify any codes that are globally exempted (within the current file).
let exemption = noqa::file_exemption(locator.contents(), comment_ranges); let exemption = FileExemption::extract(locator.contents(), comment_ranges);
// Extract all `noqa` directives. // Extract all `noqa` directives.
let mut noqa_directives = NoqaDirectives::from_commented_ranges(comment_ranges, locator); let mut noqa_directives = NoqaDirectives::from_commented_ranges(comment_ranges, locator);
@ -70,7 +70,7 @@ pub(crate) fn check_noqa(
ignored_diagnostics.push(index); ignored_diagnostics.push(index);
true true
} }
Directive::Codes(.., codes, _) => { Directive::Codes(Codes { codes, .. }) => {
if noqa::includes(diagnostic.kind.rule(), codes) { if noqa::includes(diagnostic.kind.rule(), codes) {
directive_line directive_line
.matches .matches
@ -95,23 +95,32 @@ pub(crate) fn check_noqa(
if analyze_directives && settings.rules.enabled(Rule::UnusedNOQA) { if analyze_directives && settings.rules.enabled(Rule::UnusedNOQA) {
for line in noqa_directives.lines() { for line in noqa_directives.lines() {
match &line.directive { match &line.directive {
Directive::All(leading_spaces, noqa_range, trailing_spaces) => { Directive::All(All {
leading_space_len,
noqa_range,
trailing_space_len,
}) => {
if line.matches.is_empty() { if line.matches.is_empty() {
let mut diagnostic = let mut diagnostic =
Diagnostic::new(UnusedNOQA { codes: None }, *noqa_range); Diagnostic::new(UnusedNOQA { codes: None }, *noqa_range);
if settings.rules.should_fix(diagnostic.kind.rule()) { if settings.rules.should_fix(diagnostic.kind.rule()) {
#[allow(deprecated)] #[allow(deprecated)]
diagnostic.set_fix_from_edit(delete_noqa( diagnostic.set_fix_from_edit(delete_noqa(
*leading_spaces, *leading_space_len,
*noqa_range, *noqa_range,
*trailing_spaces, *trailing_space_len,
locator, locator,
)); ));
} }
diagnostics.push(diagnostic); diagnostics.push(diagnostic);
} }
} }
Directive::Codes(leading_spaces, range, codes, trailing_spaces) => { Directive::Codes(Codes {
leading_space_len,
noqa_range,
codes,
trailing_space_len,
}) => {
let mut disabled_codes = vec![]; let mut disabled_codes = vec![];
let mut unknown_codes = vec![]; let mut unknown_codes = vec![];
let mut unmatched_codes = vec![]; let mut unmatched_codes = vec![];
@ -166,22 +175,22 @@ pub(crate) fn check_noqa(
.collect(), .collect(),
}), }),
}, },
*range, *noqa_range,
); );
if settings.rules.should_fix(diagnostic.kind.rule()) { if settings.rules.should_fix(diagnostic.kind.rule()) {
if valid_codes.is_empty() { if valid_codes.is_empty() {
#[allow(deprecated)] #[allow(deprecated)]
diagnostic.set_fix_from_edit(delete_noqa( diagnostic.set_fix_from_edit(delete_noqa(
*leading_spaces, *leading_space_len,
*range, *noqa_range,
*trailing_spaces, *trailing_space_len,
locator, locator,
)); ));
} else { } else {
#[allow(deprecated)] #[allow(deprecated)]
diagnostic.set_fix(Fix::unspecified(Edit::range_replacement( diagnostic.set_fix(Fix::unspecified(Edit::range_replacement(
format!("# noqa: {}", valid_codes.join(", ")), format!("# noqa: {}", valid_codes.join(", ")),
*range, *noqa_range,
))); )));
} }
} }
@ -199,9 +208,9 @@ pub(crate) fn check_noqa(
/// Generate a [`Edit`] to delete a `noqa` directive. /// Generate a [`Edit`] to delete a `noqa` directive.
fn delete_noqa( fn delete_noqa(
leading_spaces: TextSize, leading_space_len: TextSize,
noqa_range: TextRange, noqa_range: TextRange,
trailing_spaces: TextSize, trailing_space_len: TextSize,
locator: &Locator, locator: &Locator,
) -> Edit { ) -> Edit {
let line_range = locator.line_range(noqa_range.start()); let line_range = locator.line_range(noqa_range.start());
@ -209,27 +218,28 @@ fn delete_noqa(
// Ex) `# noqa` // Ex) `# noqa`
if line_range if line_range
== TextRange::new( == TextRange::new(
noqa_range.start() - leading_spaces, noqa_range.start() - leading_space_len,
noqa_range.end() + trailing_spaces, noqa_range.end() + trailing_space_len,
) )
{ {
let full_line_end = locator.full_line_end(line_range.end()); let full_line_end = locator.full_line_end(line_range.end());
Edit::deletion(line_range.start(), full_line_end) Edit::deletion(line_range.start(), full_line_end)
} }
// Ex) `x = 1 # noqa` // Ex) `x = 1 # noqa`
else if noqa_range.end() + trailing_spaces == line_range.end() { else if noqa_range.end() + trailing_space_len == line_range.end() {
Edit::deletion(noqa_range.start() - leading_spaces, line_range.end()) Edit::deletion(noqa_range.start() - leading_space_len, line_range.end())
} }
// Ex) `x = 1 # noqa # type: ignore` // Ex) `x = 1 # noqa # type: ignore`
else if locator.contents()[usize::from(noqa_range.end() + trailing_spaces)..].starts_with('#') else if locator.contents()[usize::from(noqa_range.end() + trailing_space_len)..]
.starts_with('#')
{ {
Edit::deletion(noqa_range.start(), noqa_range.end() + trailing_spaces) Edit::deletion(noqa_range.start(), noqa_range.end() + trailing_space_len)
} }
// Ex) `x = 1 # noqa here` // Ex) `x = 1 # noqa here`
else { else {
Edit::deletion( Edit::deletion(
noqa_range.start() + "# ".text_len(), noqa_range.start() + "# ".text_len(),
noqa_range.end() + trailing_spaces, noqa_range.end() + trailing_space_len,
) )
} }
} }

287
crates/ruff/src/noqa.rs
View file

@ -25,99 +25,94 @@ static NOQA_LINE_REGEX: Lazy<Regex> = Lazy::new(|| {
.unwrap() .unwrap()
}); });
/// A directive to ignore a set of rules for a given line of Python source code (e.g.,
/// `# noqa: F401, F841`).
#[derive(Debug)] #[derive(Debug)]
pub(crate) enum Directive<'a> { pub(crate) enum Directive<'a> {
/// No `noqa` directive was found.
None, None,
// (leading spaces, noqa_range, trailing_spaces) /// The `noqa` directive ignores all rules (e.g., `# noqa`).
All(TextSize, TextRange, TextSize), All(All),
// (leading spaces, start_offset, end_offset, codes, trailing_spaces) /// The `noqa` directive ignores specific rules (e.g., `# noqa: F401, F841`).
Codes(TextSize, TextRange, Vec<&'a str>, TextSize), Codes(Codes<'a>),
} }
/// Extract the noqa `Directive` from a line of Python source code. impl<'a> Directive<'a> {
pub(crate) fn extract_noqa_directive<'a>(range: TextRange, locator: &'a Locator) -> Directive<'a> { /// Extract the noqa `Directive` from a line of Python source code.
let text = &locator.contents()[range]; pub(crate) fn extract(range: TextRange, locator: &'a Locator) -> Self {
match NOQA_LINE_REGEX.captures(text) { let text = &locator.contents()[range];
Some(caps) => match ( match NOQA_LINE_REGEX.captures(text) {
caps.name("leading_spaces"), Some(caps) => match (
caps.name("noqa"), caps.name("leading_spaces"),
caps.name("codes"), caps.name("noqa"),
caps.name("trailing_spaces"), caps.name("codes"),
) { caps.name("trailing_spaces"),
(Some(leading_spaces), Some(noqa), Some(codes), Some(trailing_spaces)) => { ) {
let codes = codes (Some(leading_spaces), Some(noqa), Some(codes), Some(trailing_spaces)) => {
.as_str() let codes = codes
.split(|c: char| c.is_whitespace() || c == ',') .as_str()
.map(str::trim) .split(|c: char| c.is_whitespace() || c == ',')
.filter(|code| !code.is_empty()) .map(str::trim)
.collect_vec(); .filter(|code| !code.is_empty())
let start = range.start() + TextSize::try_from(noqa.start()).unwrap(); .collect_vec();
if codes.is_empty() { let start = range.start() + TextSize::try_from(noqa.start()).unwrap();
#[allow(deprecated)] if codes.is_empty() {
let line = locator.compute_line_index(start); #[allow(deprecated)]
warn!("Expected rule codes on `noqa` directive: \"{line}\""); let line = locator.compute_line_index(start);
warn!("Expected rule codes on `noqa` directive: \"{line}\"");
}
let leading_space_len = leading_spaces.as_str().text_len();
let noqa_range = TextRange::at(start, noqa.as_str().text_len());
let trailing_space_len = trailing_spaces.as_str().text_len();
Self::Codes(Codes {
leading_space_len,
noqa_range,
trailing_space_len,
codes,
})
} }
Directive::Codes( (Some(leading_spaces), Some(noqa), None, Some(trailing_spaces)) => {
leading_spaces.as_str().text_len(), let leading_space_len = leading_spaces.as_str().text_len();
TextRange::at(start, noqa.as_str().text_len()), let noqa_range = TextRange::at(
codes, range.start() + TextSize::try_from(noqa.start()).unwrap(),
trailing_spaces.as_str().text_len(), noqa.as_str().text_len(),
) );
} let trailing_space_len = trailing_spaces.as_str().text_len();
Self::All(All {
(Some(leading_spaces), Some(noqa), None, Some(trailing_spaces)) => Directive::All( leading_space_len,
leading_spaces.as_str().text_len(), noqa_range,
TextRange::at( trailing_space_len,
range.start() + TextSize::try_from(noqa.start()).unwrap(), })
noqa.as_str().text_len(), }
), _ => Self::None,
trailing_spaces.as_str().text_len(), },
), None => Self::None,
_ => Directive::None,
},
None => Directive::None,
}
}
enum ParsedExemption<'a> {
None,
All,
Codes(Vec<&'a str>),
}
/// Return a [`ParsedExemption`] for a given comment line.
fn parse_file_exemption(line: &str) -> ParsedExemption {
let line = line.trim_whitespace_start();
if line.starts_with("# flake8: noqa")
|| line.starts_with("# flake8: NOQA")
|| line.starts_with("# flake8: NoQA")
{
return ParsedExemption::All;
}
if let Some(remainder) = line
.strip_prefix("# ruff: noqa")
.or_else(|| line.strip_prefix("# ruff: NOQA"))
.or_else(|| line.strip_prefix("# ruff: NoQA"))
{
if remainder.is_empty() {
return ParsedExemption::All;
} else if let Some(codes) = remainder.strip_prefix(':') {
let codes = codes
.split(|c: char| c.is_whitespace() || c == ',')
.map(str::trim)
.filter(|code| !code.is_empty())
.collect_vec();
if codes.is_empty() {
warn!("Expected rule codes on `noqa` directive: \"{line}\"");
}
return ParsedExemption::Codes(codes);
} }
warn!("Unexpected suffix on `noqa` directive: \"{line}\"");
} }
}
ParsedExemption::None #[derive(Debug)]
pub(crate) struct All {
/// The length of the leading whitespace before the `noqa` directive.
pub(crate) leading_space_len: TextSize,
/// The range of the `noqa` directive.
pub(crate) noqa_range: TextRange,
/// The length of the trailing whitespace after the `noqa` directive.
pub(crate) trailing_space_len: TextSize,
}
#[derive(Debug)]
pub(crate) struct Codes<'a> {
/// The length of the leading whitespace before the `noqa` directive.
pub(crate) leading_space_len: TextSize,
/// The range of the `noqa` directive.
pub(crate) noqa_range: TextRange,
/// The length of the trailing whitespace after the `noqa` directive.
pub(crate) trailing_space_len: TextSize,
/// The codes that are ignored by the `noqa` directive.
pub(crate) codes: Vec<&'a str>,
} }
/// Returns `true` if the string list of `codes` includes `code` (or an alias /// Returns `true` if the string list of `codes` includes `code` (or an alias
@ -138,47 +133,105 @@ pub(crate) fn rule_is_ignored(
) -> bool { ) -> bool {
let offset = noqa_line_for.resolve(offset); let offset = noqa_line_for.resolve(offset);
let line_range = locator.line_range(offset); let line_range = locator.line_range(offset);
match extract_noqa_directive(line_range, locator) { match Directive::extract(line_range, locator) {
Directive::None => false, Directive::None => false,
Directive::All(..) => true, Directive::All(..) => true,
Directive::Codes(.., codes, _) => includes(code, &codes), Directive::Codes(Codes { codes, .. }) => includes(code, &codes),
} }
} }
/// The file-level exemptions extracted from a given Python file.
#[derive(Debug)]
pub(crate) enum FileExemption { pub(crate) enum FileExemption {
/// No file-level exemption.
None, None,
/// The file is exempt from all rules.
All, All,
/// The file is exempt from the given rules.
Codes(Vec<NoqaCode>), Codes(Vec<NoqaCode>),
} }
/// Extract the [`FileExemption`] for a given Python source file, enumerating any rules that are impl FileExemption {
/// globally ignored within the file. /// Extract the [`FileExemption`] for a given Python source file, enumerating any rules that are
pub(crate) fn file_exemption(contents: &str, comment_ranges: &[TextRange]) -> FileExemption { /// globally ignored within the file.
let mut exempt_codes: Vec<NoqaCode> = vec![]; pub(crate) fn extract(contents: &str, comment_ranges: &[TextRange]) -> Self {
let mut exempt_codes: Vec<NoqaCode> = vec![];
for range in comment_ranges { for range in comment_ranges {
match parse_file_exemption(&contents[*range]) { match ParsedFileExemption::extract(&contents[*range]) {
ParsedExemption::All => { ParsedFileExemption::All => {
return FileExemption::All; return Self::All;
}
ParsedFileExemption::Codes(codes) => {
exempt_codes.extend(codes.into_iter().filter_map(|code| {
if let Ok(rule) = Rule::from_code(get_redirect_target(code).unwrap_or(code))
{
Some(rule.noqa_code())
} else {
warn!("Invalid code provided to `# ruff: noqa`: {}", code);
None
}
}));
}
ParsedFileExemption::None => {}
} }
ParsedExemption::Codes(codes) => { }
exempt_codes.extend(codes.into_iter().filter_map(|code| {
if let Ok(rule) = Rule::from_code(get_redirect_target(code).unwrap_or(code)) { if exempt_codes.is_empty() {
Some(rule.noqa_code()) Self::None
} else { } else {
warn!("Invalid code provided to `# ruff: noqa`: {}", code); Self::Codes(exempt_codes)
None
}
}));
}
ParsedExemption::None => {}
} }
} }
}
if exempt_codes.is_empty() { /// An individual file-level exemption (e.g., `# ruff: noqa` or `# ruff: noqa: F401, F841`). Like
FileExemption::None /// [`FileExemption`], but only for a single line, as opposed to an aggregated set of exemptions
} else { /// across a source file.
FileExemption::Codes(exempt_codes) #[derive(Debug)]
enum ParsedFileExemption<'a> {
/// No file-level exemption was found.
None,
/// The file-level exemption ignores all rules (e.g., `# ruff: noqa`).
All,
/// The file-level exemption ignores specific rules (e.g., `# ruff: noqa: F401, F841`).
Codes(Vec<&'a str>),
}
impl<'a> ParsedFileExemption<'a> {
/// Return a [`ParsedFileExemption`] for a given comment line.
fn extract(line: &'a str) -> Self {
let line = line.trim_whitespace_start();
if line.starts_with("# flake8: noqa")
|| line.starts_with("# flake8: NOQA")
|| line.starts_with("# flake8: NoQA")
{
return Self::All;
}
if let Some(remainder) = line
.strip_prefix("# ruff: noqa")
.or_else(|| line.strip_prefix("# ruff: NOQA"))
.or_else(|| line.strip_prefix("# ruff: NoQA"))
{
if remainder.is_empty() {
return Self::All;
} else if let Some(codes) = remainder.strip_prefix(':') {
let codes = codes
.split(|c: char| c.is_whitespace() || c == ',')
.map(str::trim)
.filter(|code| !code.is_empty())
.collect_vec();
if codes.is_empty() {
warn!("Expected rule codes on `noqa` directive: \"{line}\"");
}
return Self::Codes(codes);
}
warn!("Unexpected suffix on `noqa` directive: \"{line}\"");
}
Self::None
} }
} }
@ -215,7 +268,7 @@ fn add_noqa_inner(
// Whether the file is exempted from all checks. // Whether the file is exempted from all checks.
// Codes that are globally exempted (within the current file). // Codes that are globally exempted (within the current file).
let exemption = file_exemption(locator.contents(), commented_ranges); let exemption = FileExemption::extract(locator.contents(), commented_ranges);
let directives = NoqaDirectives::from_commented_ranges(commented_ranges, locator); let directives = NoqaDirectives::from_commented_ranges(commented_ranges, locator);
// Mark any non-ignored diagnostics. // Mark any non-ignored diagnostics.
@ -243,7 +296,7 @@ fn add_noqa_inner(
Directive::All(..) => { Directive::All(..) => {
continue; continue;
} }
Directive::Codes(.., codes, _) => { Directive::Codes(Codes { codes, .. }) => {
if includes(diagnostic.kind.rule(), codes) { if includes(diagnostic.kind.rule(), codes) {
continue; continue;
} }
@ -261,7 +314,7 @@ fn add_noqa_inner(
Directive::All(..) => { Directive::All(..) => {
continue; continue;
} }
Directive::Codes(.., codes, _) => { Directive::Codes(Codes { codes, .. }) => {
let rule = diagnostic.kind.rule(); let rule = diagnostic.kind.rule();
if !includes(rule, codes) { if !includes(rule, codes) {
matches_by_line matches_by_line
@ -311,7 +364,9 @@ fn add_noqa_inner(
Some(Directive::All(..)) => { Some(Directive::All(..)) => {
// Does not get inserted into the map. // Does not get inserted into the map.
} }
Some(Directive::Codes(_, noqa_range, existing, _)) => { Some(Directive::Codes(Codes {
noqa_range, codes, ..
})) => {
// Reconstruct the line based on the preserved rule codes. // Reconstruct the line based on the preserved rule codes.
// This enables us to tally the number of edits. // This enables us to tally the number of edits.
let output_start = output.len(); let output_start = output.len();
@ -331,8 +386,8 @@ fn add_noqa_inner(
&mut output, &mut output,
rules rules
.iter() .iter()
.map(|r| r.noqa_code().to_string()) .map(|rule| rule.noqa_code().to_string())
.chain(existing.iter().map(ToString::to_string)) .chain(codes.iter().map(ToString::to_string))
.sorted_unstable(), .sorted_unstable(),
); );
@ -386,7 +441,7 @@ impl<'a> NoqaDirectives<'a> {
for comment_range in comment_ranges { for comment_range in comment_ranges {
let line_range = locator.line_range(comment_range.start()); let line_range = locator.line_range(comment_range.start());
let directive = match extract_noqa_directive(line_range, locator) { let directive = match Directive::extract(line_range, locator) {
Directive::None => { Directive::None => {
continue; continue;
} }