Skip to content

Commit

Permalink
Avoid missing namespace violations in scripts with shebangs (#8710)
Browse files Browse the repository at this point in the history
## Summary

I think it's reasonable to avoid raising `INP001` for scripts, and
shebangs are one sufficient way to detect scripts.

Closes #8690.
  • Loading branch information
charliermarsh authored Nov 16, 2023
1 parent d1e88dc commit 6d5d079
Show file tree
Hide file tree
Showing 9 changed files with 52 additions and 44 deletions.
15 changes: 12 additions & 3 deletions crates/ruff_linter/src/checkers/filesystem.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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<Diagnostic> {
let mut diagnostics: Vec<Diagnostic> = 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);
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/checkers/tokens.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,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(&[
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
47 changes: 22 additions & 25 deletions crates/ruff_linter/src/rules/flake8_executable/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -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::*;
Expand All @@ -20,32 +18,31 @@ mod shebang_not_executable;
mod shebang_not_first_line;

pub(crate) fn from_tokens(
tokens: &[LexResult],
diagnostics: &mut Vec<Diagnostic>,
path: &Path,
locator: &Locator,
diagnostics: &mut Vec<Diagnostic>,
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);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/flake8_no_pep420/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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<Diagnostic> {
Expand All @@ -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
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
source: crates/ruff_linter/src/rules/flake8_no_pep420/mod.rs
---

0 comments on commit 6d5d079

Please sign in to comment.