Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Avoid removing un-selected codes when applying `--add-noqa` edits (#2465) 

The downside here is that we have to leave blank `# noqa` directives intact. Otherwise, we risk removing necessary `# noqa` coverage for rules that aren't selected.

Closes #2254.
This commit is contained in:
Charlie Marsh 2023-02-01 22:22:31 -05:00 committed by GitHub
parent 30a09ec211
commit f16f3a4a03
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 38 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/linter.rs
View File

@ -222,7 +222,6 @@ pub fn add_noqa_to_path(path: &Path, settings: &Settings) -> Result<usize> {
&contents, &contents,
indexer.commented_lines(), indexer.commented_lines(),
&directives.noqa_line_for, &directives.noqa_line_for,
&settings.external,
stylist.line_ending(), stylist.line_ending(),
) )
} }

70
src/noqa.rs
View File

@ -10,7 +10,6 @@ use rustc_hash::{FxHashMap, FxHashSet};
use crate::registry::{Diagnostic, Rule}; use crate::registry::{Diagnostic, Rule};
use crate::rule_redirects::get_redirect_target; use crate::rule_redirects::get_redirect_target;
use crate::settings::hashable::HashableHashSet;
use crate::source_code::LineEnding; use crate::source_code::LineEnding;
static NOQA_LINE_REGEX: Lazy<Regex> = Lazy::new(|| { static NOQA_LINE_REGEX: Lazy<Regex> = Lazy::new(|| {
@ -83,7 +82,6 @@ pub fn add_noqa(
contents: &str, contents: &str,
commented_lines: &[usize], commented_lines: &[usize],
noqa_line_for: &IntMap<usize, usize>, noqa_line_for: &IntMap<usize, usize>,
external: &HashableHashSet<String>,
line_ending: &LineEnding, line_ending: &LineEnding,
) -> Result<usize> { ) -> Result<usize> {
let (count, output) = add_noqa_inner( let (count, output) = add_noqa_inner(
@ -91,7 +89,6 @@ pub fn add_noqa(
contents, contents,
commented_lines, commented_lines,
noqa_line_for, noqa_line_for,
external,
line_ending, line_ending,
); );
fs::write(path, output)?; fs::write(path, output)?;
@ -103,7 +100,6 @@ fn add_noqa_inner(
contents: &str, contents: &str,
commented_lines: &[usize], commented_lines: &[usize],
noqa_line_for: &IntMap<usize, usize>, noqa_line_for: &IntMap<usize, usize>,
external: &HashableHashSet<String>,
line_ending: &LineEnding, line_ending: &LineEnding,
) -> (usize, String) { ) -> (usize, String) {
let mut matches_by_line: FxHashMap<usize, FxHashSet<&Rule>> = FxHashMap::default(); let mut matches_by_line: FxHashMap<usize, FxHashSet<&Rule>> = FxHashMap::default();
@ -114,8 +110,12 @@ fn add_noqa_inner(
return (0, contents.to_string()); return (0, contents.to_string());
} }
// Grab the noqa (logical) line number for the current (physical) line.
let noqa_lineno = noqa_line_for.get(&(lineno + 1)).unwrap_or(&(lineno + 1)) - 1;
let mut codes: FxHashSet<&Rule> = FxHashSet::default(); let mut codes: FxHashSet<&Rule> = FxHashSet::default();
for diagnostic in diagnostics { for diagnostic in diagnostics {
if diagnostic.location.row() == lineno + 1 {
// Is the violation ignored by a `noqa` directive on the parent line? // Is the violation ignored by a `noqa` directive on the parent line?
if let Some(parent_lineno) = diagnostic.parent.map(|location| location.row()) { if let Some(parent_lineno) = diagnostic.parent.map(|location| location.row()) {
let noqa_lineno = noqa_line_for.get(&parent_lineno).unwrap_or(&parent_lineno); let noqa_lineno = noqa_line_for.get(&parent_lineno).unwrap_or(&parent_lineno);
@ -134,13 +134,29 @@ fn add_noqa_inner(
} }
} }
if diagnostic.location.row() == lineno + 1 { // Is the diagnostic ignored by a `noqa` directive on the same line?
codes.insert(diagnostic.kind.rule()); let diagnostic_lineno = diagnostic.location.row();
let noqa_lineno = noqa_line_for
.get(&diagnostic_lineno)
.unwrap_or(&diagnostic_lineno);
if commented_lines.contains(noqa_lineno) {
match extract_noqa_directive(lines[noqa_lineno - 1]) {
Directive::All(..) => {
continue;
}
Directive::Codes(.., codes) => {
if includes(diagnostic.kind.rule(), &codes) {
continue;
}
}
Directive::None => {}
} }
} }
// Grab the noqa (logical) line number for the current (physical) line. // The diagnostic is not ignored by any `noqa` directive; add it to the list.
let noqa_lineno = noqa_line_for.get(&(lineno + 1)).unwrap_or(&(lineno + 1)) - 1; codes.insert(diagnostic.kind.rule());
}
}
if !codes.is_empty() { if !codes.is_empty() {
matches_by_line matches_by_line
@ -174,22 +190,13 @@ fn add_noqa_inner(
output.push_str(line_ending); output.push_str(line_ending);
count += 1; count += 1;
} }
Directive::All(_, start, _) => { Directive::All(..) => {
// Add existing content. // Leave the line as-is.
output.push_str(line[..start].trim_end()); output.push_str(line);
// Add `noqa` directive.
output.push_str(" # noqa: ");
// Add codes.
let codes: Vec<&str> =
rules.iter().map(|r| r.code()).sorted_unstable().collect();
let suffix = codes.join(", ");
output.push_str(&suffix);
output.push_str(line_ending); output.push_str(line_ending);
count += 1;
} }
Directive::Codes(_, start_byte, _, existing) => { Directive::Codes(_, start_byte, _, existing) => {
println!("existing: {:?}", existing);
// 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 mut formatted = String::with_capacity(line.len()); let mut formatted = String::with_capacity(line.len());
@ -204,7 +211,7 @@ fn add_noqa_inner(
let codes: Vec<&str> = rules let codes: Vec<&str> = rules
.iter() .iter()
.map(|r| r.code()) .map(|r| r.code())
.chain(existing.into_iter().filter(|code| external.contains(*code))) .chain(existing.into_iter())
.sorted_unstable() .sorted_unstable()
.collect(); .collect();
let suffix = codes.join(", "); let suffix = codes.join(", ");
@ -235,7 +242,6 @@ mod tests {
use crate::noqa::{add_noqa_inner, NOQA_LINE_REGEX}; use crate::noqa::{add_noqa_inner, NOQA_LINE_REGEX};
use crate::registry::Diagnostic; use crate::registry::Diagnostic;
use crate::rules::pycodestyle::rules::AmbiguousVariableName; use crate::rules::pycodestyle::rules::AmbiguousVariableName;
use crate::settings::hashable::HashableHashSet;
use crate::source_code::LineEnding; use crate::source_code::LineEnding;
use crate::violations; use crate::violations;
@ -259,13 +265,11 @@ mod tests {
let contents = "x = 1"; let contents = "x = 1";
let commented_lines = vec![]; let commented_lines = vec![];
let noqa_line_for = IntMap::default(); let noqa_line_for = IntMap::default();
let external = HashableHashSet::default();
let (count, output) = add_noqa_inner( let (count, output) = add_noqa_inner(
&diagnostics, &diagnostics,
contents, contents,
&commented_lines, &commented_lines,
&noqa_line_for, &noqa_line_for,
&external,
&LineEnding::Lf, &LineEnding::Lf,
); );
assert_eq!(count, 0); assert_eq!(count, 0);
@ -278,15 +282,13 @@ mod tests {
Range::new(Location::new(1, 0), Location::new(1, 0)), Range::new(Location::new(1, 0), Location::new(1, 0)),
)]; )];
let contents = "x = 1"; let contents = "x = 1";
let commented_lines = vec![]; let commented_lines = vec![1];
let noqa_line_for = IntMap::default(); let noqa_line_for = IntMap::default();
let external = HashableHashSet::default();
let (count, output) = add_noqa_inner( let (count, output) = add_noqa_inner(
&diagnostics, &diagnostics,
contents, contents,
&commented_lines, &commented_lines,
&noqa_line_for, &noqa_line_for,
&external,
&LineEnding::Lf, &LineEnding::Lf,
); );
assert_eq!(count, 1); assert_eq!(count, 1);
@ -305,15 +307,13 @@ mod tests {
), ),
]; ];
let contents = "x = 1 # noqa: E741\n"; let contents = "x = 1 # noqa: E741\n";
let commented_lines = vec![]; let commented_lines = vec![1];
let noqa_line_for = IntMap::default(); let noqa_line_for = IntMap::default();
let external = HashableHashSet::default();
let (count, output) = add_noqa_inner( let (count, output) = add_noqa_inner(
&diagnostics, &diagnostics,
contents, contents,
&commented_lines, &commented_lines,
&noqa_line_for, &noqa_line_for,
&external,
&LineEnding::Lf, &LineEnding::Lf,
); );
assert_eq!(count, 1); assert_eq!(count, 1);
@ -332,18 +332,16 @@ mod tests {
), ),
]; ];
let contents = "x = 1 # noqa"; let contents = "x = 1 # noqa";
let commented_lines = vec![]; let commented_lines = vec![1];
let noqa_line_for = IntMap::default(); let noqa_line_for = IntMap::default();
let external = HashableHashSet::default();
let (count, output) = add_noqa_inner( let (count, output) = add_noqa_inner(
&diagnostics, &diagnostics,
contents, contents,
&commented_lines, &commented_lines,
&noqa_line_for, &noqa_line_for,
&external,
&LineEnding::Lf, &LineEnding::Lf,
); );
assert_eq!(count, 1); assert_eq!(count, 0);
assert_eq!(output, "x = 1 # noqa: E741, F841\n"); assert_eq!(output, "x = 1 # noqa\n");
} }
} }