diff --git a/src/check_lines.rs b/src/check_lines.rs index 07795ed8af..5bd64c75f8 100644 --- a/src/check_lines.rs +++ b/src/check_lines.rs @@ -38,6 +38,7 @@ pub fn check_lines( noqa_line_for: &IntMap, settings: &Settings, autofix: bool, + ignore_noqa: bool, ) { let enforce_unnecessary_coding_comment = settings.enabled.contains(&CheckCode::U009); let enforce_line_too_long = settings.enabled.contains(&CheckCode::E501); @@ -53,6 +54,30 @@ pub fn check_lines( assert!(check.location.row() >= 1); } + macro_rules! add_if { + ($check:expr, $noqa:expr) => {{ + match $noqa { + (Directive::All(..), matches) => { + matches.push($check.kind.code().as_ref()); + if ignore_noqa { + line_checks.push($check); + } + } + (Directive::Codes(.., codes), matches) => { + if codes.contains(&$check.kind.code().as_ref()) { + matches.push($check.kind.code().as_ref()); + if ignore_noqa { + line_checks.push($check); + } + } else { + line_checks.push($check); + } + } + (Directive::None, ..) => line_checks.push($check), + } + }}; + } + let lines: Vec<&str> = contents.lines().collect(); for (lineno, line) in lines.iter().enumerate() { // Grab the noqa (logical) line number for the current (physical) line. @@ -65,10 +90,6 @@ pub fn check_lines( if lineno < 2 { // PEP3120 makes utf-8 the default encoding. if CODING_COMMENT_REGEX.is_match(line) { - let noqa = noqa_directives.entry(noqa_lineno).or_insert_with(|| { - (noqa::extract_noqa_directive(lines[noqa_lineno]), vec![]) - }); - let mut check = Check::new( CheckKind::PEP3120UnnecessaryCodingComment, Range { @@ -83,19 +104,10 @@ pub fn check_lines( )); } - match noqa { - (Directive::All(..), matches) => { - matches.push(check.kind.code().as_ref()); - } - (Directive::Codes(.., codes), matches) => { - if codes.contains(&check.kind.code().as_ref()) { - matches.push(check.kind.code().as_ref()); - } else { - line_checks.push(check); - } - } - (Directive::None, ..) => line_checks.push(check), - } + let noqa = noqa_directives.entry(noqa_lineno).or_insert_with(|| { + (noqa::extract_noqa_directive(lines[noqa_lineno]), vec![]) + }); + add_if!(check, noqa); } } } @@ -133,10 +145,6 @@ pub fn check_lines( if enforce_line_too_long { let line_length = line.chars().count(); if should_enforce_line_length(line, line_length, settings.line_length) { - let noqa = noqa_directives - .entry(noqa_lineno) - .or_insert_with(|| (noqa::extract_noqa_directive(lines[noqa_lineno]), vec![])); - let check = Check::new( CheckKind::LineTooLong(line_length, settings.line_length), Range { @@ -145,35 +153,19 @@ pub fn check_lines( }, ); - match noqa { - (Directive::All(..), matches) => { - matches.push(check.kind.code().as_ref()); - } - (Directive::Codes(.., codes), matches) => { - if codes.contains(&check.kind.code().as_ref()) { - matches.push(check.kind.code().as_ref()); - } else { - line_checks.push(check); - } - } - (Directive::None, ..) => line_checks.push(check), - } + let noqa = noqa_directives + .entry(noqa_lineno) + .or_insert_with(|| (noqa::extract_noqa_directive(lines[noqa_lineno]), vec![])); + add_if!(check, noqa); } } } - // Enforce newlines at end of files. + // Enforce newlines at end of files (W292). if settings.enabled.contains(&CheckCode::W292) && !contents.ends_with('\n') { // Note: if `lines.last()` is `None`, then `contents` is empty (and so we don't // want to raise W292 anyway). if let Some(line) = lines.last() { - let lineno = lines.len() - 1; - let noqa_lineno = noqa_line_for.get(&(lineno + 1)).unwrap_or(&(lineno + 1)) - 1; - - let noqa = noqa_directives - .entry(noqa_lineno) - .or_insert_with(|| (noqa::extract_noqa_directive(lines[noqa_lineno]), vec![])); - let check = Check::new( CheckKind::NoNewLineAtEndOfFile, Range { @@ -182,23 +174,16 @@ pub fn check_lines( }, ); - match noqa { - (Directive::All(..), matches) => { - matches.push(check.kind.code().as_ref()); - } - (Directive::Codes(.., codes), matches) => { - if codes.contains(&check.kind.code().as_ref()) { - matches.push(check.kind.code().as_ref()); - } else { - line_checks.push(check); - } - } - (Directive::None, ..) => line_checks.push(check), - } + let lineno = lines.len() - 1; + let noqa_lineno = noqa_line_for.get(&(lineno + 1)).unwrap_or(&(lineno + 1)) - 1; + let noqa = noqa_directives + .entry(noqa_lineno) + .or_insert_with(|| (noqa::extract_noqa_directive(lines[noqa_lineno]), vec![])); + add_if!(check, noqa); } } - // Enforce that the noqa directive was actually used. + // Enforce that the noqa directive was actually used (M001). if enforce_noqa { for (row, (directive, matches)) in noqa_directives { match directive { @@ -261,9 +246,11 @@ pub fn check_lines( } } - ignored.sort_unstable(); - for index in ignored.iter().rev() { - checks.swap_remove(*index); + if !ignore_noqa { + ignored.sort_unstable(); + for index in ignored.iter().rev() { + checks.swap_remove(*index); + } } checks.extend(line_checks); } @@ -291,6 +278,7 @@ mod tests { ..Settings::for_rule(CheckCode::E501) }, true, + false, ); checks }; diff --git a/src/lib.rs b/src/lib.rs index 1cbb700c19..b34883e946 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -115,6 +115,7 @@ pub fn check(path: &Path, contents: &str, autofix: bool) -> Result> { &directives, &settings, autofix, + false, )?; Ok(checks) diff --git a/src/linter.rs b/src/linter.rs index b29d92476a..1bcc156509 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -54,6 +54,7 @@ pub(crate) fn check_path( directives: &Directives, settings: &Settings, autofix: bool, + ignore_noqa: bool, ) -> Result> { // Aggregate all checks. let mut checks: Vec = vec![]; @@ -113,6 +114,7 @@ pub(crate) fn check_path( &directives.noqa_line_for, settings, autofix, + ignore_noqa, ); // Create path ignores. @@ -179,6 +181,7 @@ pub fn lint_path( &directives, settings, autofix.into(), + false, )?; // Apply autofix. @@ -242,15 +245,19 @@ pub fn add_noqa_to_path(path: &Path, settings: &Settings) -> Result { directives::Flags::from_settings(settings), ); - // Generate checks. + // Generate checks, ignoring any existing `noqa` directives. let checks = check_path( path, &contents, tokens, &locator, - &directives, + &Directives { + noqa_line_for: Default::default(), + isort_exclusions: directives.isort_exclusions, + }, settings, false, + true, )?; add_noqa(&checks, &contents, &directives.noqa_line_for, path) @@ -313,6 +320,7 @@ pub fn lint_stdin( &directives, settings, autofix.into(), + false, )?; // Apply autofix. @@ -373,6 +381,7 @@ pub fn test_path(path: &Path, settings: &Settings, autofix: bool) -> Result { match extract_noqa_directive(line) { Directive::None => { - output.push_str(line); + // Add existing content. + output.push_str(line.trim_end()); + + // Add `noqa` directive. output.push_str(" # noqa: "); + + // Add codes. + let codes: Vec<&str> = codes.iter().map(AsRef::as_ref).collect(); + let suffix = codes.join(", "); + output.push_str(&suffix); + output.push('\n'); + count += 1; } Directive::All(_, start, _) | Directive::Codes(_, start, ..) => { - output.push_str(&line[..start]); - output.push_str("# noqa: "); + let mut new_line = String::new(); + + // Add existing content. + new_line.push_str(&line[..start].trim_end()); + + // Add `noqa` directive. + new_line.push_str(" # noqa: "); + + // Add codes. + let codes: Vec<&str> = codes.iter().map(AsRef::as_ref).collect(); + let suffix = codes.join(", "); + new_line.push_str(&suffix); + + output.push_str(&new_line); + output.push('\n'); + + // Only count if the new line is an actual edit. + if &new_line != line { + count += 1; + } } }; - let codes: Vec<&str> = codes.iter().map(AsRef::as_ref).collect(); - output.push_str(&codes.join(", ")); - output.push('\n'); - count += 1; } } }