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