From a24e2f501cfad0ab0757b68247d6a39fd2114517 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Tue, 18 Jul 2023 09:45:27 +0300 Subject: [PATCH 1/5] Add filename to `noqa` warnings --- crates/ruff/src/checkers/noqa.rs | 7 +++++-- crates/ruff/src/linter.rs | 1 + crates/ruff/src/noqa.rs | 14 ++++++++++---- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/crates/ruff/src/checkers/noqa.rs b/crates/ruff/src/checkers/noqa.rs index 16cea1d7cb82f..bcf22e2c4b295 100644 --- a/crates/ruff/src/checkers/noqa.rs +++ b/crates/ruff/src/checkers/noqa.rs @@ -1,5 +1,7 @@ //! `NoQA` enforcement and validation. +use std::path::Path; + use itertools::Itertools; use ruff_text_size::{TextLen, TextRange, TextSize}; use rustpython_parser::ast::Ranged; @@ -15,6 +17,7 @@ use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA}; use crate::settings::Settings; pub(crate) fn check_noqa( + path: &Path, diagnostics: &mut Vec, locator: &Locator, comment_ranges: &[TextRange], @@ -23,10 +26,10 @@ pub(crate) fn check_noqa( settings: &Settings, ) -> Vec { // Identify any codes that are globally exempted (within the current file). - let exemption = FileExemption::try_extract(locator.contents(), comment_ranges, locator); + let exemption = FileExemption::try_extract(path, locator.contents(), comment_ranges, locator); // Extract all `noqa` directives. - let mut noqa_directives = NoqaDirectives::from_commented_ranges(comment_ranges, locator); + let mut noqa_directives = NoqaDirectives::from_commented_ranges(path, comment_ranges, locator); // Indices of diagnostics that were ignored by a `noqa` directive. let mut ignored_diagnostics = vec![]; diff --git a/crates/ruff/src/linter.rs b/crates/ruff/src/linter.rs index ebdfded081327..725ae204a5d77 100644 --- a/crates/ruff/src/linter.rs +++ b/crates/ruff/src/linter.rs @@ -213,6 +213,7 @@ pub fn check_path( .any(|rule_code| rule_code.lint_source().is_noqa()) { let ignored = check_noqa( + path, &mut diagnostics, locator, indexer.comment_ranges(), diff --git a/crates/ruff/src/noqa.rs b/crates/ruff/src/noqa.rs index 0660b36702efa..8972585067d29 100644 --- a/crates/ruff/src/noqa.rs +++ b/crates/ruff/src/noqa.rs @@ -223,6 +223,7 @@ impl FileExemption { /// Extract the [`FileExemption`] for a given Python source file, enumerating any rules that are /// globally ignored within the file. pub(crate) fn try_extract( + path: &Path, contents: &str, comment_ranges: &[TextRange], locator: &Locator, @@ -234,7 +235,8 @@ impl FileExemption { Err(err) => { #[allow(deprecated)] let line = locator.compute_line_index(range.start()); - warn!("Invalid `# noqa` directive on line {line}: {err}"); + let path_display = path.display(); + warn!("Invalid `# noqa` directive on {path_display}:{line}: {err}"); } Ok(Some(ParsedFileExemption::All)) => { return Some(Self::All); @@ -437,6 +439,7 @@ pub(crate) fn add_noqa( line_ending: LineEnding, ) -> Result { let (count, output) = add_noqa_inner( + path, diagnostics, locator, commented_lines, @@ -448,6 +451,7 @@ pub(crate) fn add_noqa( } fn add_noqa_inner( + path: &Path, diagnostics: &[Diagnostic], locator: &Locator, commented_ranges: &[TextRange], @@ -460,8 +464,8 @@ fn add_noqa_inner( // Whether the file is exempted from all checks. // Codes that are globally exempted (within the current file). - let exemption = FileExemption::try_extract(locator.contents(), commented_ranges, locator); - let directives = NoqaDirectives::from_commented_ranges(commented_ranges, locator); + let exemption = FileExemption::try_extract(path, locator.contents(), commented_ranges, locator); + let directives = NoqaDirectives::from_commented_ranges(path, commented_ranges, locator); // Mark any non-ignored diagnostics. for diagnostic in diagnostics { @@ -624,6 +628,7 @@ pub(crate) struct NoqaDirectives<'a> { impl<'a> NoqaDirectives<'a> { pub(crate) fn from_commented_ranges( + path: &Path, comment_ranges: &[TextRange], locator: &'a Locator<'a>, ) -> Self { @@ -634,7 +639,8 @@ impl<'a> NoqaDirectives<'a> { Err(err) => { #[allow(deprecated)] let line = locator.compute_line_index(range.start()); - warn!("Invalid `# noqa` directive on line {line}: {err}"); + let path_display = path.display(); + warn!("Invalid `# noqa` directive on {path_display}:{line}: {err}"); } Ok(Some(directive)) => { // noqa comments are guaranteed to be single line. From a7f60d0786418bfb0c9c82f8f4b5c72bea488581 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Tue, 18 Jul 2023 09:51:42 +0300 Subject: [PATCH 2/5] Fix CI --- crates/ruff/src/noqa.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/ruff/src/noqa.rs b/crates/ruff/src/noqa.rs index 8972585067d29..556cf73254914 100644 --- a/crates/ruff/src/noqa.rs +++ b/crates/ruff/src/noqa.rs @@ -952,9 +952,12 @@ mod tests { #[test] fn modification() { + let path = Path::new("/tmp/foo.txt"); + let contents = "x = 1"; let noqa_line_for = NoqaMapping::default(); let (count, output) = add_noqa_inner( + &path, &[], &Locator::new(contents), &[], @@ -974,6 +977,7 @@ mod tests { let contents = "x = 1"; let noqa_line_for = NoqaMapping::default(); let (count, output) = add_noqa_inner( + &path, &diagnostics, &Locator::new(contents), &[], @@ -998,6 +1002,7 @@ mod tests { let contents = "x = 1 # noqa: E741\n"; let noqa_line_for = NoqaMapping::default(); let (count, output) = add_noqa_inner( + &path, &diagnostics, &Locator::new(contents), &[TextRange::new(TextSize::from(7), TextSize::from(19))], @@ -1022,6 +1027,7 @@ mod tests { let contents = "x = 1 # noqa"; let noqa_line_for = NoqaMapping::default(); let (count, output) = add_noqa_inner( + &path, &diagnostics, &Locator::new(contents), &[TextRange::new(TextSize::from(7), TextSize::from(13))], From fd0633a6ef76ef4d27bdb27b1551f7260993ebf9 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Tue, 18 Jul 2023 09:55:52 +0300 Subject: [PATCH 3/5] Fix CI --- crates/ruff/src/noqa.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/ruff/src/noqa.rs b/crates/ruff/src/noqa.rs index 556cf73254914..2f3c49ea64b7f 100644 --- a/crates/ruff/src/noqa.rs +++ b/crates/ruff/src/noqa.rs @@ -764,6 +764,8 @@ impl FromIterator for NoqaMapping { #[cfg(test)] mod tests { + use std::path::Path; + use insta::assert_debug_snapshot; use ruff_text_size::{TextRange, TextSize}; From 8a18ef05819f515532f6977035b6ab2fd51ac8a5 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Tue, 18 Jul 2023 10:04:22 +0300 Subject: [PATCH 4/5] Fix clippy --- crates/ruff/src/noqa.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/ruff/src/noqa.rs b/crates/ruff/src/noqa.rs index 2f3c49ea64b7f..eb1bdbe1c838a 100644 --- a/crates/ruff/src/noqa.rs +++ b/crates/ruff/src/noqa.rs @@ -959,7 +959,7 @@ mod tests { let contents = "x = 1"; let noqa_line_for = NoqaMapping::default(); let (count, output) = add_noqa_inner( - &path, + path, &[], &Locator::new(contents), &[], @@ -979,7 +979,7 @@ mod tests { let contents = "x = 1"; let noqa_line_for = NoqaMapping::default(); let (count, output) = add_noqa_inner( - &path, + path, &diagnostics, &Locator::new(contents), &[], @@ -1004,7 +1004,7 @@ mod tests { let contents = "x = 1 # noqa: E741\n"; let noqa_line_for = NoqaMapping::default(); let (count, output) = add_noqa_inner( - &path, + path, &diagnostics, &Locator::new(contents), &[TextRange::new(TextSize::from(7), TextSize::from(19))], @@ -1029,7 +1029,7 @@ mod tests { let contents = "x = 1 # noqa"; let noqa_line_for = NoqaMapping::default(); let (count, output) = add_noqa_inner( - &path, + path, &diagnostics, &Locator::new(contents), &[TextRange::new(TextSize::from(7), TextSize::from(13))], From 63431ec55b0f5a96f9740a5a69e39e1eb282bf37 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 18 Jul 2023 09:57:57 -0400 Subject: [PATCH 5/5] Switch order of arguments --- crates/ruff/src/checkers/noqa.rs | 6 +++--- crates/ruff/src/linter.rs | 2 +- crates/ruff/src/noqa.rs | 11 ++++++----- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/ruff/src/checkers/noqa.rs b/crates/ruff/src/checkers/noqa.rs index bcf22e2c4b295..56dac2c8f1d74 100644 --- a/crates/ruff/src/checkers/noqa.rs +++ b/crates/ruff/src/checkers/noqa.rs @@ -17,8 +17,8 @@ use crate::rules::ruff::rules::{UnusedCodes, UnusedNOQA}; use crate::settings::Settings; pub(crate) fn check_noqa( - path: &Path, diagnostics: &mut Vec, + path: &Path, locator: &Locator, comment_ranges: &[TextRange], noqa_line_for: &NoqaMapping, @@ -26,10 +26,10 @@ pub(crate) fn check_noqa( settings: &Settings, ) -> Vec { // Identify any codes that are globally exempted (within the current file). - let exemption = FileExemption::try_extract(path, locator.contents(), comment_ranges, locator); + let exemption = FileExemption::try_extract(locator.contents(), comment_ranges, path, locator); // Extract all `noqa` directives. - let mut noqa_directives = NoqaDirectives::from_commented_ranges(path, comment_ranges, locator); + let mut noqa_directives = NoqaDirectives::from_commented_ranges(comment_ranges, path, locator); // Indices of diagnostics that were ignored by a `noqa` directive. let mut ignored_diagnostics = vec![]; diff --git a/crates/ruff/src/linter.rs b/crates/ruff/src/linter.rs index 725ae204a5d77..bc13cc67b209b 100644 --- a/crates/ruff/src/linter.rs +++ b/crates/ruff/src/linter.rs @@ -213,8 +213,8 @@ pub fn check_path( .any(|rule_code| rule_code.lint_source().is_noqa()) { let ignored = check_noqa( - path, &mut diagnostics, + path, locator, indexer.comment_ranges(), &directives.noqa_line_for, diff --git a/crates/ruff/src/noqa.rs b/crates/ruff/src/noqa.rs index eb1bdbe1c838a..63b48c3e23fc7 100644 --- a/crates/ruff/src/noqa.rs +++ b/crates/ruff/src/noqa.rs @@ -16,6 +16,7 @@ use ruff_python_ast::source_code::Locator; use ruff_python_whitespace::LineEnding; use crate::codes::NoqaCode; +use crate::fs::relativize_path; use crate::registry::{AsRule, Rule, RuleSet}; use crate::rule_redirects::get_redirect_target; @@ -223,9 +224,9 @@ impl FileExemption { /// Extract the [`FileExemption`] for a given Python source file, enumerating any rules that are /// globally ignored within the file. pub(crate) fn try_extract( - path: &Path, contents: &str, comment_ranges: &[TextRange], + path: &Path, locator: &Locator, ) -> Option { let mut exempt_codes: Vec = vec![]; @@ -464,8 +465,8 @@ fn add_noqa_inner( // Whether the file is exempted from all checks. // Codes that are globally exempted (within the current file). - let exemption = FileExemption::try_extract(path, locator.contents(), commented_ranges, locator); - let directives = NoqaDirectives::from_commented_ranges(path, commented_ranges, locator); + let exemption = FileExemption::try_extract(locator.contents(), commented_ranges, path, locator); + let directives = NoqaDirectives::from_commented_ranges(commented_ranges, path, locator); // Mark any non-ignored diagnostics. for diagnostic in diagnostics { @@ -628,8 +629,8 @@ pub(crate) struct NoqaDirectives<'a> { impl<'a> NoqaDirectives<'a> { pub(crate) fn from_commented_ranges( - path: &Path, comment_ranges: &[TextRange], + path: &Path, locator: &'a Locator<'a>, ) -> Self { let mut directives = Vec::new(); @@ -639,7 +640,7 @@ impl<'a> NoqaDirectives<'a> { Err(err) => { #[allow(deprecated)] let line = locator.compute_line_index(range.start()); - let path_display = path.display(); + let path_display = relativize_path(path); warn!("Invalid `# noqa` directive on {path_display}:{line}: {err}"); } Ok(Some(directive)) => {