From a9a4520f2905782c20e35dcf47533044ad32c0be Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 15 Nov 2023 21:23:21 -0500 Subject: [PATCH] Avoid missing namespace violations in scripts with shebangs --- .../example.py | 0 crates/ruff_linter/src/checkers/filesystem.rs | 15 ++++-- crates/ruff_linter/src/checkers/tokens.rs | 2 +- crates/ruff_linter/src/linter.rs | 2 +- .../src/rules/flake8_executable/rules/mod.rs | 47 +++++++++---------- .../src/rules/flake8_no_pep420/mod.rs | 2 +- .../rules/implicit_namespace_package.rs | 13 ++++- ...8_no_pep420__tests__test_fail_shebang.snap | 11 ----- ...8_no_pep420__tests__test_pass_shebang.snap | 4 ++ 9 files changed, 52 insertions(+), 44 deletions(-) rename crates/ruff_linter/resources/test/fixtures/flake8_no_pep420/{test_fail_shebang => test_pass_shebang}/example.py (100%) delete mode 100644 crates/ruff_linter/src/rules/flake8_no_pep420/snapshots/ruff_linter__rules__flake8_no_pep420__tests__test_fail_shebang.snap create mode 100644 crates/ruff_linter/src/rules/flake8_no_pep420/snapshots/ruff_linter__rules__flake8_no_pep420__tests__test_pass_shebang.snap diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_no_pep420/test_fail_shebang/example.py b/crates/ruff_linter/resources/test/fixtures/flake8_no_pep420/test_pass_shebang/example.py similarity index 100% rename from crates/ruff_linter/resources/test/fixtures/flake8_no_pep420/test_fail_shebang/example.py rename to crates/ruff_linter/resources/test/fixtures/flake8_no_pep420/test_pass_shebang/example.py diff --git a/crates/ruff_linter/src/checkers/filesystem.rs b/crates/ruff_linter/src/checkers/filesystem.rs index d4de20078c1b4..2d9a3431e6fb5 100644 --- a/crates/ruff_linter/src/checkers/filesystem.rs +++ b/crates/ruff_linter/src/checkers/filesystem.rs @@ -1,6 +1,8 @@ use std::path::Path; use ruff_diagnostics::Diagnostic; +use ruff_python_index::Indexer; +use ruff_source_file::Locator; use crate::registry::Rule; use crate::rules::flake8_no_pep420::rules::implicit_namespace_package; @@ -10,15 +12,22 @@ use crate::settings::LinterSettings; pub(crate) fn check_file_path( path: &Path, package: Option<&Path>, + locator: &Locator, + indexer: &Indexer, settings: &LinterSettings, ) -> Vec { let mut diagnostics: Vec = vec![]; // flake8-no-pep420 if settings.rules.enabled(Rule::ImplicitNamespacePackage) { - if let Some(diagnostic) = - implicit_namespace_package(path, package, &settings.project_root, &settings.src) - { + if let Some(diagnostic) = implicit_namespace_package( + path, + package, + locator, + indexer, + &settings.project_root, + &settings.src, + ) { diagnostics.push(diagnostic); } } diff --git a/crates/ruff_linter/src/checkers/tokens.rs b/crates/ruff_linter/src/checkers/tokens.rs index 9f3f0866f7e63..f04233e24db9b 100644 --- a/crates/ruff_linter/src/checkers/tokens.rs +++ b/crates/ruff_linter/src/checkers/tokens.rs @@ -163,7 +163,7 @@ pub(crate) fn check_tokens( Rule::ShebangNotFirstLine, Rule::ShebangMissingPython, ]) { - flake8_executable::rules::from_tokens(tokens, path, locator, &mut diagnostics); + flake8_executable::rules::from_tokens(&mut diagnostics, path, locator, indexer); } if settings.rules.any_enabled(&[ diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 6b6b26f5048ca..328c6142aeb40 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -117,7 +117,7 @@ pub fn check_path( .iter_enabled() .any(|rule_code| rule_code.lint_source().is_filesystem()) { - diagnostics.extend(check_file_path(path, package, settings)); + diagnostics.extend(check_file_path(path, package, locator, indexer, settings)); } // Run the logical line-based rules. diff --git a/crates/ruff_linter/src/rules/flake8_executable/rules/mod.rs b/crates/ruff_linter/src/rules/flake8_executable/rules/mod.rs index e47e7a4d2cdc2..4feb54de314bf 100644 --- a/crates/ruff_linter/src/rules/flake8_executable/rules/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_executable/rules/mod.rs @@ -1,9 +1,7 @@ use std::path::Path; -use ruff_python_parser::lexer::LexResult; -use ruff_python_parser::Tok; - use ruff_diagnostics::Diagnostic; +use ruff_python_index::Indexer; use ruff_source_file::Locator; pub(crate) use shebang_leading_whitespace::*; pub(crate) use shebang_missing_executable_file::*; @@ -20,32 +18,31 @@ mod shebang_not_executable; mod shebang_not_first_line; pub(crate) fn from_tokens( - tokens: &[LexResult], + diagnostics: &mut Vec, path: &Path, locator: &Locator, - diagnostics: &mut Vec, + indexer: &Indexer, ) { let mut has_any_shebang = false; - for (tok, range) in tokens.iter().flatten() { - if let Tok::Comment(comment) = tok { - if let Some(shebang) = ShebangDirective::try_extract(comment) { - has_any_shebang = true; - - if let Some(diagnostic) = shebang_missing_python(*range, &shebang) { - diagnostics.push(diagnostic); - } - - if let Some(diagnostic) = shebang_not_executable(path, *range) { - diagnostics.push(diagnostic); - } - - if let Some(diagnostic) = shebang_leading_whitespace(*range, locator) { - diagnostics.push(diagnostic); - } - - if let Some(diagnostic) = shebang_not_first_line(*range, locator) { - diagnostics.push(diagnostic); - } + for range in indexer.comment_ranges() { + let comment = locator.slice(*range); + if let Some(shebang) = ShebangDirective::try_extract(comment) { + has_any_shebang = true; + + if let Some(diagnostic) = shebang_missing_python(*range, &shebang) { + diagnostics.push(diagnostic); + } + + if let Some(diagnostic) = shebang_not_executable(path, *range) { + diagnostics.push(diagnostic); + } + + if let Some(diagnostic) = shebang_leading_whitespace(*range, locator) { + diagnostics.push(diagnostic); + } + + if let Some(diagnostic) = shebang_not_first_line(*range, locator) { + diagnostics.push(diagnostic); } } } diff --git a/crates/ruff_linter/src/rules/flake8_no_pep420/mod.rs b/crates/ruff_linter/src/rules/flake8_no_pep420/mod.rs index 092ca12d6c1bd..34e0c3d4f24b2 100644 --- a/crates/ruff_linter/src/rules/flake8_no_pep420/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_no_pep420/mod.rs @@ -16,7 +16,7 @@ mod tests { #[test_case(Path::new("test_pass_init"), Path::new("example.py"))] #[test_case(Path::new("test_fail_empty"), Path::new("example.py"))] #[test_case(Path::new("test_fail_nonempty"), Path::new("example.py"))] - #[test_case(Path::new("test_fail_shebang"), Path::new("example.py"))] + #[test_case(Path::new("test_pass_shebang"), Path::new("example.py"))] #[test_case(Path::new("test_ignored"), Path::new("example.py"))] #[test_case(Path::new("test_pass_namespace_package"), Path::new("example.py"))] #[test_case(Path::new("test_pass_pyi"), Path::new("example.pyi"))] diff --git a/crates/ruff_linter/src/rules/flake8_no_pep420/rules/implicit_namespace_package.rs b/crates/ruff_linter/src/rules/flake8_no_pep420/rules/implicit_namespace_package.rs index a8e0508f2a96d..26cdea8dcb7b6 100644 --- a/crates/ruff_linter/src/rules/flake8_no_pep420/rules/implicit_namespace_package.rs +++ b/crates/ruff_linter/src/rules/flake8_no_pep420/rules/implicit_namespace_package.rs @@ -1,10 +1,12 @@ use std::path::{Path, PathBuf}; -use ruff_text_size::TextRange; - use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_index::Indexer; +use ruff_source_file::Locator; +use ruff_text_size::{TextRange, TextSize}; +use crate::comments::shebang::ShebangDirective; use crate::fs; /// ## What it does @@ -42,6 +44,8 @@ impl Violation for ImplicitNamespacePackage { pub(crate) fn implicit_namespace_package( path: &Path, package: Option<&Path>, + locator: &Locator, + indexer: &Indexer, project_root: &Path, src: &[PathBuf], ) -> Option { @@ -56,6 +60,11 @@ pub(crate) fn implicit_namespace_package( && !path .parent() .is_some_and( |parent| src.iter().any(|src| src == parent)) + // Ignore files that contain a shebang. + && !indexer + .comment_ranges() + .first().filter(|range| range.start() == TextSize::from(0)) + .is_some_and(|range| ShebangDirective::try_extract(locator.slice(*range)).is_some()) { #[cfg(all(test, windows))] let path = path diff --git a/crates/ruff_linter/src/rules/flake8_no_pep420/snapshots/ruff_linter__rules__flake8_no_pep420__tests__test_fail_shebang.snap b/crates/ruff_linter/src/rules/flake8_no_pep420/snapshots/ruff_linter__rules__flake8_no_pep420__tests__test_fail_shebang.snap deleted file mode 100644 index 8d8d47554f86e..0000000000000 --- a/crates/ruff_linter/src/rules/flake8_no_pep420/snapshots/ruff_linter__rules__flake8_no_pep420__tests__test_fail_shebang.snap +++ /dev/null @@ -1,11 +0,0 @@ ---- -source: crates/ruff_linter/src/rules/flake8_no_pep420/mod.rs ---- -example.py:1:1: INP001 File `./resources/test/fixtures/flake8_no_pep420/test_fail_shebang/example.py` is part of an implicit namespace package. Add an `__init__.py`. - | -1 | #!/bin/env/python - | INP001 -2 | print('hi') - | - - diff --git a/crates/ruff_linter/src/rules/flake8_no_pep420/snapshots/ruff_linter__rules__flake8_no_pep420__tests__test_pass_shebang.snap b/crates/ruff_linter/src/rules/flake8_no_pep420/snapshots/ruff_linter__rules__flake8_no_pep420__tests__test_pass_shebang.snap new file mode 100644 index 0000000000000..624dce32257e8 --- /dev/null +++ b/crates/ruff_linter/src/rules/flake8_no_pep420/snapshots/ruff_linter__rules__flake8_no_pep420__tests__test_pass_shebang.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/flake8_no_pep420/mod.rs +--- +