From 914e02a919bb2b70b2d1c1c0f524488314a57123 Mon Sep 17 00:00:00 2001 From: Nikita Sobolev Date: Tue, 18 Jul 2023 17:08:22 +0300 Subject: [PATCH] Add filename to `noqa` warnings (#5856) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Before: ``` » ruff litestar tests --fix warning: Invalid `# noqa` directive on line 19: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`). warning: Invalid `# noqa` directive on line 65: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`). warning: Invalid `# noqa` directive on line 74: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`). warning: Invalid `# noqa` directive on line 22: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`). warning: Invalid `# noqa` directive on line 66: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`). warning: Invalid `# noqa` directive on line 75: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`). ``` After: ``` » cargo run --bin ruff ../litestar/litestar ../litestar/tests Finished dev [unoptimized + debuginfo] target(s) in 0.15s Running `target/debug/ruff ../litestar/litestar ../litestar/tests` warning: Detected debug build without --no-cache. warning: Invalid `# noqa` directive on /Users/sobolev/Desktop/litestar/tests/unit/test_contrib/test_sqlalchemy/models_bigint.py:19: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`). warning: Invalid `# noqa` directive on /Users/sobolev/Desktop/litestar/tests/unit/test_contrib/test_sqlalchemy/models_bigint.py:65: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`). warning: Invalid `# noqa` directive on /Users/sobolev/Desktop/litestar/tests/unit/test_contrib/test_sqlalchemy/models_bigint.py:74: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`). warning: Invalid `# noqa` directive on /Users/sobolev/Desktop/litestar/tests/unit/test_contrib/test_sqlalchemy/models_uuid.py:22: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`). warning: Invalid `# noqa` directive on /Users/sobolev/Desktop/litestar/tests/unit/test_contrib/test_sqlalchemy/models_uuid.py:66: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`). warning: Invalid `# noqa` directive on /Users/sobolev/Desktop/litestar/tests/unit/test_contrib/test_sqlalchemy/models_uuid.py:75: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`). ``` ## Test Plan I didn't find any existing tests with this warning. Closes https://github.com/astral-sh/ruff/issues/5855 --- crates/ruff/src/checkers/noqa.rs | 7 +++++-- crates/ruff/src/linter.rs | 1 + crates/ruff/src/noqa.rs | 23 +++++++++++++++++++---- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/crates/ruff/src/checkers/noqa.rs b/crates/ruff/src/checkers/noqa.rs index 16cea1d7cb82fa..56dac2c8f1d74a 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; @@ -16,6 +18,7 @@ use crate::settings::Settings; pub(crate) fn check_noqa( diagnostics: &mut Vec, + path: &Path, locator: &Locator, comment_ranges: &[TextRange], noqa_line_for: &NoqaMapping, @@ -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(locator.contents(), comment_ranges, path, locator); // Extract all `noqa` directives. - let mut noqa_directives = NoqaDirectives::from_commented_ranges(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 ebdfded0813276..bc13cc67b209b8 100644 --- a/crates/ruff/src/linter.rs +++ b/crates/ruff/src/linter.rs @@ -214,6 +214,7 @@ pub fn check_path( { let ignored = check_noqa( &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 0660b36702efa3..63b48c3e23fc7b 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; @@ -225,6 +226,7 @@ impl FileExemption { pub(crate) fn try_extract( contents: &str, comment_ranges: &[TextRange], + path: &Path, locator: &Locator, ) -> Option { let mut exempt_codes: Vec = vec![]; @@ -234,7 +236,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 +440,7 @@ pub(crate) fn add_noqa( line_ending: LineEnding, ) -> Result { let (count, output) = add_noqa_inner( + path, diagnostics, locator, commented_lines, @@ -448,6 +452,7 @@ pub(crate) fn add_noqa( } fn add_noqa_inner( + path: &Path, diagnostics: &[Diagnostic], locator: &Locator, commented_ranges: &[TextRange], @@ -460,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(locator.contents(), commented_ranges, locator); - let directives = NoqaDirectives::from_commented_ranges(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 { @@ -625,6 +630,7 @@ pub(crate) struct NoqaDirectives<'a> { impl<'a> NoqaDirectives<'a> { pub(crate) fn from_commented_ranges( comment_ranges: &[TextRange], + path: &Path, locator: &'a Locator<'a>, ) -> Self { let mut directives = Vec::new(); @@ -634,7 +640,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 = relativize_path(path); + warn!("Invalid `# noqa` directive on {path_display}:{line}: {err}"); } Ok(Some(directive)) => { // noqa comments are guaranteed to be single line. @@ -758,6 +765,8 @@ impl FromIterator for NoqaMapping { #[cfg(test)] mod tests { + use std::path::Path; + use insta::assert_debug_snapshot; use ruff_text_size::{TextRange, TextSize}; @@ -946,9 +955,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), &[], @@ -968,6 +980,7 @@ mod tests { let contents = "x = 1"; let noqa_line_for = NoqaMapping::default(); let (count, output) = add_noqa_inner( + path, &diagnostics, &Locator::new(contents), &[], @@ -992,6 +1005,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))], @@ -1016,6 +1030,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))],