diff --git a/crates/ruff/src/checkers/noqa.rs b/crates/ruff/src/checkers/noqa.rs index 83b90cd2c8..a11592adda 100644 --- a/crates/ruff/src/checkers/noqa.rs +++ b/crates/ruff/src/checkers/noqa.rs @@ -94,32 +94,17 @@ pub(crate) fn check_noqa( if analyze_directives && settings.rules.enabled(Rule::UnusedNOQA) { for line in noqa_directives.lines() { match &line.directive { - Directive::All(All { - leading_space_len, - noqa_range, - trailing_space_len, - }) => { + Directive::All(All { range }) => { if line.matches.is_empty() { - let mut diagnostic = - Diagnostic::new(UnusedNOQA { codes: None }, *noqa_range); + let mut diagnostic = Diagnostic::new(UnusedNOQA { codes: None }, *range); if settings.rules.should_fix(diagnostic.kind.rule()) { #[allow(deprecated)] - diagnostic.set_fix_from_edit(delete_noqa( - *leading_space_len, - *noqa_range, - *trailing_space_len, - locator, - )); + diagnostic.set_fix_from_edit(delete_noqa(*range, locator)); } diagnostics.push(diagnostic); } } - Directive::Codes(Codes { - leading_space_len, - noqa_range, - codes, - trailing_space_len, - }) => { + Directive::Codes(Codes { range, codes }) => { let mut disabled_codes = vec![]; let mut unknown_codes = vec![]; let mut unmatched_codes = vec![]; @@ -174,22 +159,17 @@ pub(crate) fn check_noqa( .collect(), }), }, - *noqa_range, + *range, ); if settings.rules.should_fix(diagnostic.kind.rule()) { if valid_codes.is_empty() { #[allow(deprecated)] - diagnostic.set_fix_from_edit(delete_noqa( - *leading_space_len, - *noqa_range, - *trailing_space_len, - locator, - )); + diagnostic.set_fix_from_edit(delete_noqa(*range, locator)); } else { #[allow(deprecated)] diagnostic.set_fix(Fix::unspecified(Edit::range_replacement( format!("# noqa: {}", valid_codes.join(", ")), - *noqa_range, + *range, ))); } } @@ -205,39 +185,46 @@ pub(crate) fn check_noqa( } /// Generate a [`Edit`] to delete a `noqa` directive. -fn delete_noqa( - leading_space_len: TextSize, - noqa_range: TextRange, - trailing_space_len: TextSize, - locator: &Locator, -) -> Edit { - let line_range = locator.line_range(noqa_range.start()); +fn delete_noqa(range: TextRange, locator: &Locator) -> Edit { + let line_range = locator.line_range(range.start()); + + // Compute the leading space. + let prefix = locator.slice(TextRange::new(line_range.start(), range.start())); + let leading_space = prefix + .rfind(|c: char| !c.is_whitespace()) + .map_or(prefix.len(), |i| prefix.len() - i - 1); + let leading_space_len = TextSize::try_from(leading_space).unwrap(); + + // Compute the trailing space. + let suffix = locator.slice(TextRange::new(range.end(), line_range.end())); + let trailing_space = suffix + .find(|c: char| !c.is_whitespace()) + .map_or(suffix.len(), |i| i); + let trailing_space_len = TextSize::try_from(trailing_space).unwrap(); // Ex) `# noqa` if line_range == TextRange::new( - noqa_range.start() - leading_space_len, - noqa_range.end() + trailing_space_len, + range.start() - leading_space_len, + range.end() + trailing_space_len, ) { let full_line_end = locator.full_line_end(line_range.end()); Edit::deletion(line_range.start(), full_line_end) } // Ex) `x = 1 # noqa` - else if noqa_range.end() + trailing_space_len == line_range.end() { - Edit::deletion(noqa_range.start() - leading_space_len, line_range.end()) + else if range.end() + trailing_space_len == line_range.end() { + Edit::deletion(range.start() - leading_space_len, line_range.end()) } // Ex) `x = 1 # noqa # type: ignore` - else if locator.contents()[usize::from(noqa_range.end() + trailing_space_len)..] - .starts_with('#') - { - Edit::deletion(noqa_range.start(), noqa_range.end() + trailing_space_len) + else if locator.contents()[usize::from(range.end() + trailing_space_len)..].starts_with('#') { + Edit::deletion(range.start(), range.end() + trailing_space_len) } // Ex) `x = 1 # noqa here` else { Edit::deletion( - noqa_range.start() + "# ".text_len(), - noqa_range.end() + trailing_space_len, + range.start() + "# ".text_len(), + range.end() + trailing_space_len, ) } } diff --git a/crates/ruff/src/noqa.rs b/crates/ruff/src/noqa.rs index 83f724348b..d139e00f79 100644 --- a/crates/ruff/src/noqa.rs +++ b/crates/ruff/src/noqa.rs @@ -19,10 +19,8 @@ use crate::registry::{AsRule, Rule, RuleSet}; use crate::rule_redirects::get_redirect_target; static NOQA_LINE_REGEX: Lazy = Lazy::new(|| { - Regex::new( - r"(?P\s*)(?P(?i:# noqa)(?::\s?(?P[A-Z]+[0-9]+(?:[,\s]+[A-Z]+[0-9]+)*))?)(?P\s*)", - ) - .unwrap() + Regex::new(r"(?P(?i:# noqa)(?::\s?(?P[A-Z]+[0-9]+(?:[,\s]+[A-Z]+[0-9]+)*))?)") + .unwrap() }); /// A directive to ignore a set of rules for a given line of Python source code (e.g., @@ -39,76 +37,47 @@ impl<'a> Directive<'a> { /// Extract the noqa `Directive` from a line of Python source code. pub(crate) fn try_extract(range: TextRange, locator: &'a Locator) -> Option { let text = &locator.contents()[range]; - match NOQA_LINE_REGEX.captures(text) { - Some(caps) => match ( - caps.name("leading_spaces"), - caps.name("noqa"), - caps.name("codes"), - caps.name("trailing_spaces"), - ) { - (Some(leading_spaces), Some(noqa), Some(codes), Some(trailing_spaces)) => { - let codes = codes - .as_str() - .split(|c: char| c.is_whitespace() || c == ',') - .map(str::trim) - .filter(|code| !code.is_empty()) - .collect_vec(); - let start = range.start() + TextSize::try_from(noqa.start()).unwrap(); - if codes.is_empty() { - #[allow(deprecated)] - 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(); - - Some(Self::Codes(Codes { - leading_space_len, - noqa_range, - trailing_space_len, - codes, - })) + let caps = NOQA_LINE_REGEX.captures(text)?; + match (caps.name("noqa"), caps.name("codes")) { + (Some(noqa), Some(codes)) => { + let codes = codes + .as_str() + .split(|c: char| c.is_whitespace() || c == ',') + .map(str::trim) + .filter(|code| !code.is_empty()) + .collect_vec(); + let start = range.start() + TextSize::try_from(noqa.start()).unwrap(); + if codes.is_empty() { + #[allow(deprecated)] + let line = locator.compute_line_index(start); + warn!("Expected rule codes on `noqa` directive: \"{line}\""); } - (Some(leading_spaces), Some(noqa), None, Some(trailing_spaces)) => { - let leading_space_len = leading_spaces.as_str().text_len(); - let noqa_range = TextRange::at( - range.start() + TextSize::try_from(noqa.start()).unwrap(), - noqa.as_str().text_len(), - ); - let trailing_space_len = trailing_spaces.as_str().text_len(); - Some(Self::All(All { - leading_space_len, - noqa_range, - trailing_space_len, - })) - } - _ => None, - }, - None => None, + + let range = TextRange::at(start, noqa.as_str().text_len()); + Some(Self::Codes(Codes { range, codes })) + } + (Some(noqa), None) => { + let range = TextRange::at( + range.start() + TextSize::try_from(noqa.start()).unwrap(), + noqa.as_str().text_len(), + ); + Some(Self::All(All { range })) + } + _ => 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, + pub(crate) range: TextRange, } #[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, + pub(crate) range: TextRange, /// The codes that are ignored by the `noqa` directive. pub(crate) codes: Vec<&'a str>, } @@ -132,8 +101,8 @@ pub(crate) fn rule_is_ignored( let offset = noqa_line_for.resolve(offset); let line_range = locator.line_range(offset); match Directive::try_extract(line_range, locator) { - Some(Directive::All(..)) => true, - Some(Directive::Codes(Codes { codes, .. })) => includes(code, &codes), + Some(Directive::All(_)) => true, + Some(Directive::Codes(Codes { codes, range: _ })) => includes(code, &codes), None => false, } } @@ -289,10 +258,10 @@ fn add_noqa_inner( directives.find_line_with_directive(noqa_line_for.resolve(parent)) { match &directive_line.directive { - Directive::All(..) => { + Directive::All(_) => { continue; } - Directive::Codes(Codes { codes, .. }) => { + Directive::Codes(Codes { codes, range: _ }) => { if includes(diagnostic.kind.rule(), codes) { continue; } @@ -306,10 +275,10 @@ fn add_noqa_inner( // Or ignored by the directive itself? if let Some(directive_line) = directives.find_line_with_directive(noqa_offset) { match &directive_line.directive { - Directive::All(..) => { + Directive::All(_) => { continue; } - Directive::Codes(Codes { codes, .. }) => { + Directive::Codes(Codes { codes, range: _ }) => { let rule = diagnostic.kind.rule(); if !includes(rule, codes) { matches_by_line @@ -355,12 +324,10 @@ fn add_noqa_inner( output.push_str(&line_ending); count += 1; } - Some(Directive::All(..)) => { + Some(Directive::All(_)) => { // Does not get inserted into the map. } - Some(Directive::Codes(Codes { - noqa_range, codes, .. - })) => { + Some(Directive::Codes(Codes { range, codes })) => { // Reconstruct the line based on the preserved rule codes. // This enables us to tally the number of edits. let output_start = output.len(); @@ -368,7 +335,7 @@ fn add_noqa_inner( // Add existing content. output.push_str( locator - .slice(TextRange::new(offset, noqa_range.start())) + .slice(TextRange::new(offset, range.start())) .trim_end(), ); @@ -433,12 +400,11 @@ impl<'a> NoqaDirectives<'a> { ) -> Self { let mut directives = Vec::new(); - for comment_range in comment_ranges { - let line_range = locator.line_range(comment_range.start()); - if let Some(directive) = Directive::try_extract(line_range, locator) { + for range in comment_ranges { + if let Some(directive) = Directive::try_extract(*range, locator) { // noqa comments are guaranteed to be single line. directives.push(NoqaDirectiveLine { - range: line_range, + range: locator.line_range(range.start()), directive, matches: Vec::new(), }); diff --git a/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_all.snap b/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_all.snap index 260721baf2..6d5ffa6fd8 100644 --- a/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_all.snap +++ b/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_all.snap @@ -1,13 +1,11 @@ --- source: crates/ruff/src/noqa.rs -expression: "Directive::extract(range, &locator)" +expression: "Directive::try_extract(range, &locator)" --- Some( All( All { - leading_space_len: 0, - noqa_range: 0..6, - trailing_space_len: 0, + range: 0..6, }, ), ) diff --git a/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_all_case_insensitive.snap b/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_all_case_insensitive.snap index 260721baf2..6d5ffa6fd8 100644 --- a/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_all_case_insensitive.snap +++ b/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_all_case_insensitive.snap @@ -1,13 +1,11 @@ --- source: crates/ruff/src/noqa.rs -expression: "Directive::extract(range, &locator)" +expression: "Directive::try_extract(range, &locator)" --- Some( All( All { - leading_space_len: 0, - noqa_range: 0..6, - trailing_space_len: 0, + range: 0..6, }, ), ) diff --git a/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_code.snap b/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_code.snap index bfb5b2f11e..fd0bc5502d 100644 --- a/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_code.snap +++ b/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_code.snap @@ -1,13 +1,11 @@ --- source: crates/ruff/src/noqa.rs -expression: "Directive::extract(range, &locator)" +expression: "Directive::try_extract(range, &locator)" --- Some( Codes( Codes { - leading_space_len: 0, - noqa_range: 0..12, - trailing_space_len: 0, + range: 0..12, codes: [ "F401", ], diff --git a/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_code_case_insensitive.snap b/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_code_case_insensitive.snap index bfb5b2f11e..fd0bc5502d 100644 --- a/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_code_case_insensitive.snap +++ b/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_code_case_insensitive.snap @@ -1,13 +1,11 @@ --- source: crates/ruff/src/noqa.rs -expression: "Directive::extract(range, &locator)" +expression: "Directive::try_extract(range, &locator)" --- Some( Codes( Codes { - leading_space_len: 0, - noqa_range: 0..12, - trailing_space_len: 0, + range: 0..12, codes: [ "F401", ], diff --git a/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_codes.snap b/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_codes.snap index aecdd27050..61c78604a1 100644 --- a/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_codes.snap +++ b/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_codes.snap @@ -1,13 +1,11 @@ --- source: crates/ruff/src/noqa.rs -expression: "Directive::extract(range, &locator)" +expression: "Directive::try_extract(range, &locator)" --- Some( Codes( Codes { - leading_space_len: 0, - noqa_range: 0..18, - trailing_space_len: 0, + range: 0..18, codes: [ "F401", "F841", diff --git a/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_codes_case_insensitive.snap b/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_codes_case_insensitive.snap index aecdd27050..61c78604a1 100644 --- a/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_codes_case_insensitive.snap +++ b/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_codes_case_insensitive.snap @@ -1,13 +1,11 @@ --- source: crates/ruff/src/noqa.rs -expression: "Directive::extract(range, &locator)" +expression: "Directive::try_extract(range, &locator)" --- Some( Codes( Codes { - leading_space_len: 0, - noqa_range: 0..18, - trailing_space_len: 0, + range: 0..18, codes: [ "F401", "F841", diff --git a/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_leading_space.snap b/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_leading_space.snap index 27602af1d0..85081b1a53 100644 --- a/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_leading_space.snap +++ b/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_leading_space.snap @@ -1,13 +1,11 @@ --- source: crates/ruff/src/noqa.rs -expression: "Directive::extract(range, &locator)" +expression: "Directive::try_extract(range, &locator)" --- Some( Codes( Codes { - leading_space_len: 3, - noqa_range: 4..16, - trailing_space_len: 0, + range: 4..16, codes: [ "F401", ], diff --git a/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_trailing_space.snap b/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_trailing_space.snap index bc8c548b14..fd0bc5502d 100644 --- a/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_trailing_space.snap +++ b/crates/ruff/src/snapshots/ruff__noqa__tests__noqa_trailing_space.snap @@ -1,13 +1,11 @@ --- source: crates/ruff/src/noqa.rs -expression: "Directive::extract(range, &locator)" +expression: "Directive::try_extract(range, &locator)" --- Some( Codes( Codes { - leading_space_len: 0, - noqa_range: 0..12, - trailing_space_len: 3, + range: 0..12, codes: [ "F401", ],