diff --git a/crates/ruff/resources/test/fixtures/ruff/RUF100_3.py b/crates/ruff/resources/test/fixtures/ruff/RUF100_3.py new file mode 100644 index 0000000000..138994c1a8 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/ruff/RUF100_3.py @@ -0,0 +1,13 @@ +# noqa +# noqa # comment +print() # noqa +print() # noqa # comment +print(a) # noqa +print(a) # noqa # comment + +# noqa: E501, F821 +# noqa: E501, F821 # comment +print() # noqa: E501, F821 +print() # noqa: E501, F821 # comment +print(a) # noqa: E501, F821 +print(a) # noqa: E501, F821 # comment diff --git a/crates/ruff/src/checkers/noqa.rs b/crates/ruff/src/checkers/noqa.rs index 011cf39e08..f6dfcd52b9 100644 --- a/crates/ruff/src/checkers/noqa.rs +++ b/crates/ruff/src/checkers/noqa.rs @@ -97,7 +97,7 @@ pub fn check_noqa( ignored_diagnostics.push(index); continue; } - (Directive::Codes(.., codes), matches) => { + (Directive::Codes(.., codes, _), matches) => { if noqa::includes(diagnostic.kind.rule(), codes) { matches.push(diagnostic.kind.rule().noqa_code()); ignored_diagnostics.push(index); @@ -124,7 +124,7 @@ pub fn check_noqa( ignored_diagnostics.push(index); continue; } - (Directive::Codes(.., codes), matches) => { + (Directive::Codes(.., codes, _), matches) => { if noqa::includes(diagnostic.kind.rule(), codes) { matches.push(diagnostic.kind.rule().noqa_code()); ignored_diagnostics.push(index); @@ -140,7 +140,7 @@ pub fn check_noqa( if enforce_noqa { for (row, (directive, matches)) in noqa_directives { match directive { - Directive::All(spaces, start_byte, end_byte) => { + Directive::All(leading_spaces, start_byte, end_byte, trailing_spaces) => { if matches.is_empty() { let start = lines[row][..start_byte].chars().count(); let end = start + lines[row][start_byte..end_byte].chars().count(); @@ -150,15 +150,27 @@ pub fn check_noqa( Range::new(Location::new(row + 1, start), Location::new(row + 1, end)), ); if autofix.into() && settings.rules.should_fix(diagnostic.kind.rule()) { - diagnostic.amend(Fix::deletion( - Location::new(row + 1, start - spaces), - Location::new(row + 1, lines[row].chars().count()), - )); + if start - leading_spaces == 0 && end == lines[row].chars().count() { + diagnostic.amend(Fix::deletion( + Location::new(row + 1, 0), + Location::new(row + 2, 0), + )); + } else if end == lines[row].chars().count() { + diagnostic.amend(Fix::deletion( + Location::new(row + 1, start - leading_spaces), + Location::new(row + 1, end + trailing_spaces), + )); + } else { + diagnostic.amend(Fix::deletion( + Location::new(row + 1, start), + Location::new(row + 1, end + trailing_spaces), + )); + } } diagnostics.push(diagnostic); } } - Directive::Codes(spaces, start_byte, end_byte, codes) => { + Directive::Codes(leading_spaces, start_byte, end_byte, codes, trailing_spaces) => { let mut disabled_codes = vec![]; let mut unknown_codes = vec![]; let mut unmatched_codes = vec![]; @@ -218,15 +230,28 @@ pub fn check_noqa( ); if autofix.into() && settings.rules.should_fix(diagnostic.kind.rule()) { if valid_codes.is_empty() { - diagnostic.amend(Fix::deletion( - Location::new(row + 1, start - spaces), - Location::new(row + 1, lines[row].chars().count()), - )); + if start - leading_spaces == 0 && end == lines[row].chars().count() + { + diagnostic.amend(Fix::deletion( + Location::new(row + 1, 0), + Location::new(row + 2, 0), + )); + } else if end == lines[row].chars().count() { + diagnostic.amend(Fix::deletion( + Location::new(row + 1, start - leading_spaces), + Location::new(row + 1, end + trailing_spaces), + )); + } else { + diagnostic.amend(Fix::deletion( + Location::new(row + 1, start), + Location::new(row + 1, end + trailing_spaces), + )); + } } else { diagnostic.amend(Fix::replacement( format!("# noqa: {}", valid_codes.join(", ")), Location::new(row + 1, start), - Location::new(row + 1, lines[row].chars().count()), + Location::new(row + 1, end), )); } } diff --git a/crates/ruff/src/noqa.rs b/crates/ruff/src/noqa.rs index 148965efb6..ad97360211 100644 --- a/crates/ruff/src/noqa.rs +++ b/crates/ruff/src/noqa.rs @@ -21,7 +21,7 @@ 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]+)?)+))?)", + r"(?P\s*)(?P(?i:# noqa)(?::\s?(?P(?:[A-Z]+[0-9]+)(?:[,\s]+[A-Z]+[0-9]+)*))?)(?P\s*)", ) .unwrap() }); @@ -73,35 +73,42 @@ pub fn extract_file_exemption(line: &str) -> Exemption { #[derive(Debug)] pub enum Directive<'a> { None, - All(usize, usize, usize), - Codes(usize, usize, usize, Vec<&'a str>), + All(usize, usize, usize, usize), + Codes(usize, usize, usize, Vec<&'a str>, usize), } /// Extract the noqa `Directive` from a line of Python source code. pub fn extract_noqa_directive(line: &str) -> Directive { match NOQA_LINE_REGEX.captures(line) { - Some(caps) => match caps.name("spaces") { - Some(spaces) => match caps.name("noqa") { - Some(noqa) => match caps.name("codes") { - Some(codes) => { - let codes: Vec<&str> = SPLIT_COMMA_REGEX - .split(codes.as_str().trim()) - .map(str::trim) - .filter(|code| !code.is_empty()) - .collect(); - if codes.is_empty() { - warn!("Expected rule codes on `noqa` directive: \"{line}\""); + Some(caps) => match caps.name("leading_spaces") { + Some(leading_spaces) => match caps.name("trailing_spaces") { + Some(trailing_spaces) => match caps.name("noqa") { + Some(noqa) => match caps.name("codes") { + Some(codes) => { + let codes: Vec<&str> = SPLIT_COMMA_REGEX + .split(codes.as_str().trim()) + .map(str::trim) + .filter(|code| !code.is_empty()) + .collect(); + if codes.is_empty() { + warn!("Expected rule codes on `noqa` directive: \"{line}\""); + } + Directive::Codes( + leading_spaces.as_str().chars().count(), + noqa.start(), + noqa.end(), + codes, + trailing_spaces.as_str().chars().count(), + ) } - Directive::Codes( - spaces.as_str().chars().count(), + None => Directive::All( + leading_spaces.as_str().chars().count(), noqa.start(), noqa.end(), - codes, - ) - } - None => { - Directive::All(spaces.as_str().chars().count(), noqa.start(), noqa.end()) - } + trailing_spaces.as_str().chars().count(), + ), + }, + None => Directive::None, }, None => Directive::None, }, @@ -135,7 +142,7 @@ pub fn rule_is_ignored( match extract_noqa_directive(line) { Directive::None => false, Directive::All(..) => true, - Directive::Codes(.., codes) => includes(code, &codes), + Directive::Codes(.., codes, _) => includes(code, &codes), } } @@ -216,7 +223,7 @@ fn add_noqa_inner( Directive::All(..) => { continue; } - Directive::Codes(.., codes) => { + Directive::Codes(.., codes, _) => { if includes(diagnostic.kind.rule(), &codes) { continue; } @@ -236,7 +243,7 @@ fn add_noqa_inner( Directive::All(..) => { continue; } - Directive::Codes(.., codes) => { + Directive::Codes(.., codes, _) => { if includes(diagnostic.kind.rule(), &codes) { continue; } @@ -281,7 +288,7 @@ fn add_noqa_inner( output.push_str(line); output.push_str(line_ending); } - Directive::Codes(_, start_byte, _, existing) => { + Directive::Codes(_, start_byte, _, existing, _) => { // Reconstruct the line based on the preserved rule codes. // This enables us to tally the number of edits. let mut formatted = String::with_capacity(line.len()); diff --git a/crates/ruff/src/rules/ruff/mod.rs b/crates/ruff/src/rules/ruff/mod.rs index 0f5d48bde4..4fa134da43 100644 --- a/crates/ruff/src/rules/ruff/mod.rs +++ b/crates/ruff/src/rules/ruff/mod.rs @@ -89,6 +89,20 @@ mod tests { Ok(()) } + #[test] + fn ruf100_3() -> Result<()> { + let diagnostics = test_path( + Path::new("ruff/RUF100_3.py"), + &settings::Settings::for_rules(vec![ + Rule::UnusedNOQA, + Rule::LineTooLong, + Rule::UndefinedName, + ]), + )?; + assert_yaml_snapshot!(diagnostics); + Ok(()) + } + #[test] fn flake8_noqa() -> Result<()> { let diagnostics = test_path( diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruf100_3.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruf100_3.snap new file mode 100644 index 0000000000..9ed8cd078e --- /dev/null +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__ruf100_3.snap @@ -0,0 +1,205 @@ +--- +source: crates/ruff/src/rules/ruff/mod.rs +expression: diagnostics +--- +- kind: + name: UnusedNOQA + body: "Unused blanket `noqa` directive" + suggestion: "Remove unused `noqa` directive" + fixable: true + location: + row: 1 + column: 0 + end_location: + row: 1 + column: 6 + fix: + content: "" + location: + row: 1 + column: 0 + end_location: + row: 2 + column: 0 + parent: ~ +- kind: + name: UnusedNOQA + body: "Unused blanket `noqa` directive" + suggestion: "Remove unused `noqa` directive" + fixable: true + location: + row: 2 + column: 0 + end_location: + row: 2 + column: 6 + fix: + content: "" + location: + row: 2 + column: 0 + end_location: + row: 2 + column: 7 + parent: ~ +- kind: + name: UnusedNOQA + body: "Unused blanket `noqa` directive" + suggestion: "Remove unused `noqa` directive" + fixable: true + location: + row: 3 + column: 9 + end_location: + row: 3 + column: 15 + fix: + content: "" + location: + row: 3 + column: 7 + end_location: + row: 3 + column: 15 + parent: ~ +- kind: + name: UnusedNOQA + body: "Unused blanket `noqa` directive" + suggestion: "Remove unused `noqa` directive" + fixable: true + location: + row: 4 + column: 9 + end_location: + row: 4 + column: 15 + fix: + content: "" + location: + row: 4 + column: 9 + end_location: + row: 4 + column: 16 + parent: ~ +- kind: + name: UnusedNOQA + body: "Unused `noqa` directive (unused: `E501`, `F821`)" + suggestion: "Remove unused `noqa` directive" + fixable: true + location: + row: 8 + column: 0 + end_location: + row: 8 + column: 18 + fix: + content: "" + location: + row: 8 + column: 0 + end_location: + row: 9 + column: 0 + parent: ~ +- kind: + name: UnusedNOQA + body: "Unused `noqa` directive (unused: `E501`, `F821`)" + suggestion: "Remove unused `noqa` directive" + fixable: true + location: + row: 9 + column: 0 + end_location: + row: 9 + column: 18 + fix: + content: "" + location: + row: 9 + column: 0 + end_location: + row: 9 + column: 19 + parent: ~ +- kind: + name: UnusedNOQA + body: "Unused `noqa` directive (unused: `E501`, `F821`)" + suggestion: "Remove unused `noqa` directive" + fixable: true + location: + row: 10 + column: 9 + end_location: + row: 10 + column: 27 + fix: + content: "" + location: + row: 10 + column: 7 + end_location: + row: 10 + column: 27 + parent: ~ +- kind: + name: UnusedNOQA + body: "Unused `noqa` directive (unused: `E501`, `F821`)" + suggestion: "Remove unused `noqa` directive" + fixable: true + location: + row: 11 + column: 9 + end_location: + row: 11 + column: 27 + fix: + content: "" + location: + row: 11 + column: 9 + end_location: + row: 11 + column: 28 + parent: ~ +- kind: + name: UnusedNOQA + body: "Unused `noqa` directive (unused: `E501`)" + suggestion: "Remove unused `noqa` directive" + fixable: true + location: + row: 12 + column: 10 + end_location: + row: 12 + column: 28 + fix: + content: "# noqa: F821" + location: + row: 12 + column: 10 + end_location: + row: 12 + column: 28 + parent: ~ +- kind: + name: UnusedNOQA + body: "Unused `noqa` directive (unused: `E501`)" + suggestion: "Remove unused `noqa` directive" + fixable: true + location: + row: 13 + column: 10 + end_location: + row: 13 + column: 28 + fix: + content: "# noqa: F821" + location: + row: 13 + column: 10 + end_location: + row: 13 + column: 28 + parent: ~ +