Preserve existing noqa codes in --add-noqa (#913)

This commit is contained in:
Charlie Marsh 2022-11-26 14:42:19 -05:00 committed by GitHub
parent d19a8aa54d
commit d28e026525
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 90 additions and 68 deletions

View File

@ -38,6 +38,7 @@ pub fn check_lines(
noqa_line_for: &IntMap<usize, usize>,
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
};

View File

@ -115,6 +115,7 @@ pub fn check(path: &Path, contents: &str, autofix: bool) -> Result<Vec<Check>> {
&directives,
&settings,
autofix,
false,
)?;
Ok(checks)

View File

@ -54,6 +54,7 @@ pub(crate) fn check_path(
directives: &Directives,
settings: &Settings,
autofix: bool,
ignore_noqa: bool,
) -> Result<Vec<Check>> {
// Aggregate all checks.
let mut checks: Vec<Check> = 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<usize> {
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<Vec<
&directives,
settings,
autofix,
false,
)
}

View File

@ -99,18 +99,42 @@ fn add_noqa_inner(
Some(codes) => {
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;
}
}
}