diff --git a/crates/ruff_linter/__init__.py b/bar/baz.py similarity index 100% rename from crates/ruff_linter/__init__.py rename to bar/baz.py diff --git a/crates/ruff/src/cache.rs b/crates/ruff/src/cache.rs index 814f6e0348..40b495189a 100644 --- a/crates/ruff/src/cache.rs +++ b/crates/ruff/src/cache.rs @@ -20,6 +20,7 @@ use tempfile::NamedTempFile; use ruff_cache::{CacheKey, CacheKeyHasher}; use ruff_diagnostics::{DiagnosticKind, Fix}; use ruff_linter::message::{DiagnosticMessage, Message}; +use ruff_linter::package::PackageRoot; use ruff_linter::{warn_user, VERSION}; use ruff_macros::CacheKey; use ruff_notebook::NotebookIndex; @@ -497,7 +498,7 @@ pub(crate) struct PackageCacheMap<'a>(FxHashMap<&'a Path, Cache>); impl<'a> PackageCacheMap<'a> { pub(crate) fn init( - package_roots: &FxHashMap<&'a Path, Option<&'a Path>>, + package_roots: &FxHashMap<&'a Path, Option>>, resolver: &Resolver, ) -> Self { fn init_cache(path: &Path) { @@ -513,7 +514,9 @@ impl<'a> PackageCacheMap<'a> { Self( package_roots .iter() - .map(|(package, package_root)| package_root.unwrap_or(package)) + .map(|(package, package_root)| { + package_root.map(PackageRoot::path).unwrap_or(package) + }) .unique() .par_bridge() .map(|cache_root| { @@ -587,6 +590,7 @@ mod tests { use ruff_cache::CACHE_DIR_NAME; use ruff_linter::message::Message; + use ruff_linter::package::PackageRoot; use ruff_linter::settings::flags; use ruff_linter::settings::types::UnsafeFixes; use ruff_python_ast::PySourceType; @@ -641,7 +645,7 @@ mod tests { let diagnostics = lint_path( &path, - Some(&package_root), + Some(PackageRoot::root(&package_root)), &settings.linter, Some(&cache), flags::Noqa::Enabled, @@ -683,7 +687,7 @@ mod tests { for path in paths { got_diagnostics += lint_path( &path, - Some(&package_root), + Some(PackageRoot::root(&package_root)), &settings.linter, Some(&cache), flags::Noqa::Enabled, @@ -1056,7 +1060,7 @@ mod tests { ) -> Result { lint_path( &self.package_root.join(path), - Some(&self.package_root), + Some(PackageRoot::root(&self.package_root)), &self.settings.linter, Some(cache), flags::Noqa::Enabled, diff --git a/crates/ruff/src/commands/analyze_graph.rs b/crates/ruff/src/commands/analyze_graph.rs index f95592623e..f8a0684d42 100644 --- a/crates/ruff/src/commands/analyze_graph.rs +++ b/crates/ruff/src/commands/analyze_graph.rs @@ -6,6 +6,7 @@ use log::{debug, warn}; use path_absolutize::CWD; use ruff_db::system::{SystemPath, SystemPathBuf}; use ruff_graph::{Direction, ImportMap, ModuleDb, ModuleImports}; +use ruff_linter::package::PackageRoot; use ruff_linter::{warn_user, warn_user_once}; use ruff_python_ast::{PySourceType, SourceType}; use ruff_workspace::resolver::{match_exclusion, python_files_in_path, ResolvedFile}; @@ -49,7 +50,12 @@ pub(crate) fn analyze_graph( .collect::>(), ) .into_iter() - .map(|(path, package)| (path.to_path_buf(), package.map(Path::to_path_buf))) + .map(|(path, package)| { + ( + path.to_path_buf(), + package.map(PackageRoot::path).map(Path::to_path_buf), + ) + }) .collect::>(); // Create a database from the source roots. diff --git a/crates/ruff/src/commands/check.rs b/crates/ruff/src/commands/check.rs index 24bc3d94d9..0a547b3551 100644 --- a/crates/ruff/src/commands/check.rs +++ b/crates/ruff/src/commands/check.rs @@ -13,6 +13,7 @@ use rustc_hash::FxHashMap; use ruff_diagnostics::Diagnostic; use ruff_linter::message::Message; +use ruff_linter::package::PackageRoot; use ruff_linter::registry::Rule; use ruff_linter::settings::types::UnsafeFixes; use ruff_linter::settings::{flags, LinterSettings}; @@ -87,7 +88,9 @@ pub(crate) fn check( return None; } - let cache_root = package.unwrap_or_else(|| path.parent().unwrap_or(path)); + let cache_root = package + .map(PackageRoot::path) + .unwrap_or_else(|| path.parent().unwrap_or(path)); let cache = caches.get(cache_root); lint_path( @@ -181,7 +184,7 @@ pub(crate) fn check( #[allow(clippy::too_many_arguments)] fn lint_path( path: &Path, - package: Option<&Path>, + package: Option>, settings: &LinterSettings, cache: Option<&Cache>, noqa: flags::Noqa, diff --git a/crates/ruff/src/commands/check_stdin.rs b/crates/ruff/src/commands/check_stdin.rs index d300dd4c2a..4e7be0b540 100644 --- a/crates/ruff/src/commands/check_stdin.rs +++ b/crates/ruff/src/commands/check_stdin.rs @@ -1,7 +1,7 @@ use std::path::Path; use anyhow::Result; - +use ruff_linter::package::PackageRoot; use ruff_linter::packaging; use ruff_linter::settings::flags; use ruff_workspace::resolver::{match_exclusion, python_file_at_path, PyprojectConfig, Resolver}; @@ -42,6 +42,7 @@ pub(crate) fn check_stdin( let stdin = read_from_stdin()?; let package_root = filename.and_then(Path::parent).and_then(|path| { packaging::detect_package_root(path, &resolver.base_settings().linter.namespace_packages) + .map(PackageRoot::root) }); let mut diagnostics = lint_stdin( filename, diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 4ad789d139..d346a323f9 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -18,6 +18,7 @@ use tracing::debug; use ruff_diagnostics::SourceMap; use ruff_linter::fs; use ruff_linter::logging::{DisplayParseError, LogLevel}; +use ruff_linter::package::PackageRoot; use ruff_linter::registry::Rule; use ruff_linter::rules::flake8_quotes::settings::Quote; use ruff_linter::source_kind::{SourceError, SourceKind}; @@ -136,7 +137,9 @@ pub(crate) fn format( .parent() .and_then(|parent| package_roots.get(parent).copied()) .flatten(); - let cache_root = package.unwrap_or_else(|| path.parent().unwrap_or(path)); + let cache_root = package + .map(PackageRoot::path) + .unwrap_or_else(|| path.parent().unwrap_or(path)); let cache = caches.get(cache_root); Some( diff --git a/crates/ruff/src/diagnostics.rs b/crates/ruff/src/diagnostics.rs index 99fab94051..620be66454 100644 --- a/crates/ruff/src/diagnostics.rs +++ b/crates/ruff/src/diagnostics.rs @@ -16,6 +16,7 @@ use ruff_diagnostics::Diagnostic; use ruff_linter::codes::Rule; use ruff_linter::linter::{lint_fix, lint_only, FixTable, FixerResult, LinterResult, ParseSource}; use ruff_linter::message::{Message, SyntaxErrorMessage}; +use ruff_linter::package::PackageRoot; use ruff_linter::pyproject_toml::lint_pyproject_toml; use ruff_linter::settings::types::UnsafeFixes; use ruff_linter::settings::{flags, LinterSettings}; @@ -180,7 +181,7 @@ impl AddAssign for FixMap { /// Lint the source code at the given `Path`. pub(crate) fn lint_path( path: &Path, - package: Option<&Path>, + package: Option>, settings: &LinterSettings, cache: Option<&Cache>, noqa: flags::Noqa, @@ -373,7 +374,7 @@ pub(crate) fn lint_path( /// stdin. pub(crate) fn lint_stdin( path: Option<&Path>, - package: Option<&Path>, + package: Option>, contents: String, settings: &Settings, noqa: flags::Noqa, diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index 450e8e7158..eb00486a9a 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -8,6 +8,7 @@ use std::process::Command; use std::str; use anyhow::Result; +use assert_fs::fixture::{ChildPath, FileTouch, PathChild}; use insta_cmd::{assert_cmd_snapshot, get_cargo_bin}; use tempfile::TempDir; @@ -1224,10 +1225,7 @@ fn negated_per_file_ignores_absolute() -> Result<()> { let ignored = tempdir.path().join("ignored.py"); fs::write(ignored, "")?; - insta::with_settings!({filters => vec![ - // Replace windows paths - (r"\\", "/"), - ]}, { + insta::with_settings!({filters => vec![(r"\\", "/")]}, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .args(STDIN_BASE_OPTIONS) .arg("--config") @@ -1918,3 +1916,58 @@ fn checks_notebooks_in_stable() -> anyhow::Result<()> { "###); Ok(()) } + +/// Verify that implicit namespace packages are detected even when they are nested. +/// +/// See: +#[test] +fn nested_implicit_namespace_package() -> Result<()> { + let tempdir = TempDir::new()?; + let root = ChildPath::new(tempdir.path()); + + root.child("foo").child("__init__.py").touch()?; + root.child("foo") + .child("bar") + .child("baz") + .child("__init__.py") + .touch()?; + root.child("foo") + .child("bar") + .child("baz") + .child("bop.py") + .touch()?; + + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .arg("--select") + .arg("INP") + .current_dir(&tempdir) + , @r###" + success: true + exit_code: 0 + ----- stdout ----- + All checks passed! + + ----- stderr ----- + "###); + + insta::with_settings!({filters => vec![(r"\\", "/")]}, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .arg("--select") + .arg("INP") + .arg("--preview") + .current_dir(&tempdir) + , @r###" + success: false + exit_code: 1 + ----- stdout ----- + foo/bar/baz/__init__.py:1:1: INP001 File `foo/bar/baz/__init__.py` declares a package, but is nested under an implicit namespace package. Add an `__init__.py` to `foo/bar`. + Found 1 error. + + ----- stderr ----- + "###); + }); + + Ok(()) +} diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 86852bf950..ce3dcaf9b0 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -66,6 +66,7 @@ use crate::checkers::ast::annotation::AnnotationContext; use crate::docstrings::extraction::ExtractionTarget; use crate::importer::Importer; use crate::noqa::NoqaMapping; +use crate::package::PackageRoot; use crate::registry::Rule; use crate::rules::{flake8_pyi, flake8_type_checking, pyflakes, pyupgrade}; use crate::settings::{flags, LinterSettings}; @@ -186,7 +187,7 @@ pub(crate) struct Checker<'a> { /// The [`Path`] to the file under analysis. path: &'a Path, /// The [`Path`] to the package containing the current file. - package: Option<&'a Path>, + package: Option>, /// The module representation of the current file (e.g., `foo.bar`). module: Module<'a>, /// The [`PySourceType`] of the current file. @@ -238,7 +239,7 @@ impl<'a> Checker<'a> { noqa_line_for: &'a NoqaMapping, noqa: flags::Noqa, path: &'a Path, - package: Option<&'a Path>, + package: Option>, module: Module<'a>, locator: &'a Locator, stylist: &'a Stylist, @@ -247,7 +248,7 @@ impl<'a> Checker<'a> { cell_offsets: Option<&'a CellOffsets>, notebook_index: Option<&'a NotebookIndex>, ) -> Checker<'a> { - Checker { + Self { parsed, parsed_type_annotation: None, parsed_annotations_cache: ParsedAnnotationsCache::new(parsed_annotations_arena), @@ -383,7 +384,7 @@ impl<'a> Checker<'a> { } /// The [`Path`] to the package containing the current file. - pub(crate) const fn package(&self) -> Option<&'a Path> { + pub(crate) const fn package(&self) -> Option> { self.package } @@ -2483,12 +2484,14 @@ pub(crate) fn check_ast( settings: &LinterSettings, noqa: flags::Noqa, path: &Path, - package: Option<&Path>, + package: Option>, source_type: PySourceType, cell_offsets: Option<&CellOffsets>, notebook_index: Option<&NotebookIndex>, ) -> Vec { - let module_path = package.and_then(|package| to_module_path(package, path)); + let module_path = package + .map(PackageRoot::path) + .and_then(|package| to_module_path(package, path)); let module = Module { kind: if path.ends_with("__init__.py") { ModuleKind::Package diff --git a/crates/ruff_linter/src/checkers/filesystem.rs b/crates/ruff_linter/src/checkers/filesystem.rs index 669ee4877f..15ba2ed5f6 100644 --- a/crates/ruff_linter/src/checkers/filesystem.rs +++ b/crates/ruff_linter/src/checkers/filesystem.rs @@ -3,6 +3,7 @@ use std::path::Path; use ruff_diagnostics::Diagnostic; use ruff_python_trivia::CommentRanges; +use crate::package::PackageRoot; use crate::registry::Rule; use crate::rules::flake8_builtins::rules::builtin_module_shadowing; use crate::rules::flake8_no_pep420::rules::implicit_namespace_package; @@ -12,7 +13,7 @@ use crate::Locator; pub(crate) fn check_file_path( path: &Path, - package: Option<&Path>, + package: Option>, locator: &Locator, comment_ranges: &CommentRanges, settings: &LinterSettings, @@ -28,6 +29,7 @@ pub(crate) fn check_file_path( comment_ranges, &settings.project_root, &settings.src, + settings.preview, ) { diagnostics.push(diagnostic); } diff --git a/crates/ruff_linter/src/checkers/imports.rs b/crates/ruff_linter/src/checkers/imports.rs index d956167553..7b3f1462c2 100644 --- a/crates/ruff_linter/src/checkers/imports.rs +++ b/crates/ruff_linter/src/checkers/imports.rs @@ -1,5 +1,4 @@ //! Lint rules based on import analysis. -use std::path::Path; use ruff_diagnostics::Diagnostic; use ruff_notebook::CellOffsets; @@ -10,6 +9,7 @@ use ruff_python_index::Indexer; use ruff_python_parser::Parsed; use crate::directives::IsortDirectives; +use crate::package::PackageRoot; use crate::registry::Rule; use crate::rules::isort; use crate::rules::isort::block::{Block, BlockBuilder}; @@ -24,7 +24,7 @@ pub(crate) fn check_imports( directives: &IsortDirectives, settings: &LinterSettings, stylist: &Stylist, - package: Option<&Path>, + package: Option>, source_type: PySourceType, cell_offsets: Option<&CellOffsets>, ) -> Vec { diff --git a/crates/ruff_linter/src/lib.rs b/crates/ruff_linter/src/lib.rs index e9641737d8..5c024cb5f8 100644 --- a/crates/ruff_linter/src/lib.rs +++ b/crates/ruff_linter/src/lib.rs @@ -32,6 +32,7 @@ mod locator; pub mod logging; pub mod message; mod noqa; +pub mod package; pub mod packaging; pub mod pyproject_toml; pub mod registry; diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 375c81715f..9d08f30135 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -28,6 +28,7 @@ use crate::doc_lines::{doc_lines_from_ast, doc_lines_from_tokens}; use crate::fix::{fix_file, FixResult}; use crate::message::Message; use crate::noqa::add_noqa; +use crate::package::PackageRoot; use crate::registry::{AsRule, Rule, RuleSet}; #[cfg(any(feature = "test-rules", test))] use crate::rules::ruff::rules::test_rules::{self, TestRule, TEST_RULES}; @@ -60,7 +61,7 @@ pub struct FixerResult<'a> { #[allow(clippy::too_many_arguments)] pub fn check_path( path: &Path, - package: Option<&Path>, + package: Option>, locator: &Locator, stylist: &Stylist, indexer: &Indexer, @@ -323,7 +324,7 @@ const MAX_ITERATIONS: usize = 100; /// Add any missing `# noqa` pragmas to the source code at the given `Path`. pub fn add_noqa_to_path( path: &Path, - package: Option<&Path>, + package: Option>, source_kind: &SourceKind, source_type: PySourceType, settings: &LinterSettings, @@ -380,7 +381,7 @@ pub fn add_noqa_to_path( /// code. pub fn lint_only( path: &Path, - package: Option<&Path>, + package: Option>, settings: &LinterSettings, noqa: flags::Noqa, source_kind: &SourceKind, @@ -467,7 +468,7 @@ fn diagnostics_to_messages( #[allow(clippy::too_many_arguments)] pub fn lint_fix<'a>( path: &Path, - package: Option<&Path>, + package: Option>, noqa: flags::Noqa, unsafe_fixes: UnsafeFixes, settings: &LinterSettings, diff --git a/crates/ruff_linter/src/package.rs b/crates/ruff_linter/src/package.rs new file mode 100644 index 0000000000..dd11f17b12 --- /dev/null +++ b/crates/ruff_linter/src/package.rs @@ -0,0 +1,40 @@ +use std::path::Path; + +/// The root directory of a Python package. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PackageRoot<'a> { + /// A normal package root. + Root { path: &'a Path }, + /// A nested package root. That is, a package root that's a subdirectory (direct or indirect) of + /// another Python package root. + /// + /// For example, `foo/bar/baz` in: + /// ```text + /// foo/ + /// ├── __init__.py + /// └── bar/ + /// └── baz/ + /// └── __init__.py + /// ``` + Nested { path: &'a Path }, +} + +impl<'a> PackageRoot<'a> { + /// Create a [`PackageRoot::Root`] variant. + pub fn root(path: &'a Path) -> Self { + Self::Root { path } + } + + /// Create a [`PackageRoot::Nested`] variant. + pub fn nested(path: &'a Path) -> Self { + Self::Nested { path } + } + + /// Return the [`Path`] of the package root. + pub fn path(self) -> &'a Path { + match self { + Self::Root { path } => path, + Self::Nested { path } => path, + } + } +} diff --git a/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_module_shadowing.rs b/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_module_shadowing.rs index 14d42fb7e9..254b390561 100644 --- a/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_module_shadowing.rs +++ b/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_module_shadowing.rs @@ -1,5 +1,7 @@ use std::path::Path; +use crate::package::PackageRoot; +use crate::settings::types::PythonVersion; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::PySourceType; @@ -7,8 +9,6 @@ use ruff_python_stdlib::path::is_module_file; use ruff_python_stdlib::sys::is_known_standard_library; use ruff_text_size::TextRange; -use crate::settings::types::PythonVersion; - /// ## What it does /// Checks for modules that use the same names as Python builtin modules. /// @@ -39,7 +39,7 @@ impl Violation for BuiltinModuleShadowing { /// A005 pub(crate) fn builtin_module_shadowing( path: &Path, - package: Option<&Path>, + package: Option>, allowed_modules: &[String], target_version: PythonVersion, ) -> Option { @@ -49,7 +49,7 @@ pub(crate) fn builtin_module_shadowing( if let Some(package) = package { let module_name = if is_module_file(path) { - package.file_name().unwrap().to_string_lossy() + package.path().file_name().unwrap().to_string_lossy() } else { path.file_stem().unwrap().to_string_lossy() }; 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 cd2df547be..f02022737e 100644 --- a/crates/ruff_linter/src/rules/flake8_no_pep420/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_no_pep420/mod.rs @@ -8,8 +8,9 @@ mod tests { use anyhow::Result; use test_case::test_case; - use crate::assert_messages; use crate::registry::Rule; + + use crate::assert_messages; use crate::settings::LinterSettings; use crate::test::{test_path, test_resource_path}; @@ -22,7 +23,7 @@ mod tests { #[test_case(Path::new("test_pass_pyi"), Path::new("example.pyi"))] #[test_case(Path::new("test_pass_script"), Path::new("script"))] #[test_case(Path::new("test_pass_shebang"), Path::new("example.py"))] - fn test_flake8_no_pep420(path: &Path, filename: &Path) -> Result<()> { + fn default(path: &Path, filename: &Path) -> Result<()> { let snapshot = format!("{}", path.to_string_lossy()); let p = PathBuf::from(format!( "flake8_no_pep420/{}/{}", 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 dd8da4a599..7590092eee 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 @@ -9,6 +9,8 @@ use ruff_text_size::{TextRange, TextSize}; use crate::comments::shebang::ShebangDirective; use crate::fs; +use crate::package::PackageRoot; +use crate::settings::types::PreviewMode; use crate::Locator; /// ## What it does @@ -32,24 +34,33 @@ use crate::Locator; #[violation] pub struct ImplicitNamespacePackage { filename: String, + parent: Option, } impl Violation for ImplicitNamespacePackage { #[derive_message_formats] fn message(&self) -> String { - let ImplicitNamespacePackage { filename } = self; - format!("File `{filename}` is part of an implicit namespace package. Add an `__init__.py`.") + let ImplicitNamespacePackage { filename, parent } = self; + match parent { + None => { + format!("File `{filename}` is part of an implicit namespace package. Add an `__init__.py`.") + } + Some(parent) => { + format!("File `{filename}` declares a package, but is nested under an implicit namespace package. Add an `__init__.py` to `{parent}`.") + } + } } } /// INP001 pub(crate) fn implicit_namespace_package( path: &Path, - package: Option<&Path>, + package: Option>, locator: &Locator, comment_ranges: &CommentRanges, project_root: &Path, src: &[PathBuf], + preview: PreviewMode, ) -> Option { if package.is_none() // Ignore non-`.py` files, which don't require an `__init__.py`. @@ -73,13 +84,39 @@ pub(crate) fn implicit_namespace_package( let path = path .to_string_lossy() .replace(std::path::MAIN_SEPARATOR, "/"); // The snapshot test expects / as the path separator. - Some(Diagnostic::new( + return Some(Diagnostic::new( ImplicitNamespacePackage { filename: fs::relativize_path(path), + parent: None, }, TextRange::default(), - )) - } else { - None + )); } + + if preview.is_enabled() { + if let Some(PackageRoot::Nested { path: root }) = package.as_ref() { + if path.ends_with("__init__.py") { + // Identify the intermediary package that's missing the `__init__.py` file. + if let Some(parent) = root + .ancestors() + .find(|parent| !parent.join("__init__.py").exists()) + { + #[cfg(all(test, windows))] + let path = path + .to_string_lossy() + .replace(std::path::MAIN_SEPARATOR, "/"); // The snapshot test expects / as the path separator. + + return Some(Diagnostic::new( + ImplicitNamespacePackage { + filename: fs::relativize_path(path), + parent: Some(fs::relativize_path(parent)), + }, + TextRange::default(), + )); + } + } + } + } + + None } diff --git a/crates/ruff_linter/src/rules/isort/categorize.rs b/crates/ruff_linter/src/rules/isort/categorize.rs index 4c82122611..0b85b54146 100644 --- a/crates/ruff_linter/src/rules/isort/categorize.rs +++ b/crates/ruff_linter/src/rules/isort/categorize.rs @@ -8,11 +8,11 @@ use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet}; use serde::{Deserialize, Serialize}; use strum_macros::EnumIter; -use ruff_macros::CacheKey; -use ruff_python_stdlib::sys::is_known_standard_library; - +use crate::package::PackageRoot; use crate::settings::types::PythonVersion; use crate::warn_user_once; +use ruff_macros::CacheKey; +use ruff_python_stdlib::sys::is_known_standard_library; use super::types::{ImportBlock, Importable}; @@ -93,7 +93,7 @@ pub(crate) fn categorize<'a>( module_name: &str, is_relative: bool, src: &[PathBuf], - package: Option<&Path>, + package: Option>, detect_same_package: bool, known_modules: &'a KnownModules, target_version: PythonVersion, @@ -153,8 +153,10 @@ pub(crate) fn categorize<'a>( import_type } -fn same_package(package: Option<&Path>, module_base: &str) -> bool { - package.is_some_and(|package| package.ends_with(module_base)) +fn same_package(package: Option>, module_base: &str) -> bool { + package + .map(PackageRoot::path) + .is_some_and(|package| package.ends_with(module_base)) } fn match_sources<'a>(paths: &'a [PathBuf], base: &str) -> Option<&'a Path> { @@ -177,7 +179,7 @@ fn match_sources<'a>(paths: &'a [PathBuf], base: &str) -> Option<&'a Path> { pub(crate) fn categorize_imports<'a>( block: ImportBlock<'a>, src: &[PathBuf], - package: Option<&Path>, + package: Option>, detect_same_package: bool, known_modules: &'a KnownModules, target_version: PythonVersion, diff --git a/crates/ruff_linter/src/rules/isort/mod.rs b/crates/ruff_linter/src/rules/isort/mod.rs index 613284b34b..afd3bc6fab 100644 --- a/crates/ruff_linter/src/rules/isort/mod.rs +++ b/crates/ruff_linter/src/rules/isort/mod.rs @@ -1,6 +1,6 @@ //! Rules from [isort](https://pypi.org/project/isort/). -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use annotate::annotate_imports; use block::{Block, Trailer}; @@ -18,6 +18,7 @@ use types::EitherImport::{Import, ImportFrom}; use types::{AliasData, ImportBlock, TrailingComma}; use crate::line_width::{LineLength, LineWidthBuilder}; +use crate::package::PackageRoot; use crate::settings::types::PythonVersion; use crate::Locator; @@ -71,7 +72,7 @@ pub(crate) fn format_imports( indentation_width: LineWidthBuilder, stylist: &Stylist, src: &[PathBuf], - package: Option<&Path>, + package: Option>, source_type: PySourceType, target_version: PythonVersion, settings: &Settings, @@ -155,7 +156,7 @@ fn format_import_block( indentation_width: LineWidthBuilder, stylist: &Stylist, src: &[PathBuf], - package: Option<&Path>, + package: Option>, target_version: PythonVersion, settings: &Settings, ) -> String { diff --git a/crates/ruff_linter/src/rules/isort/rules/organize_imports.rs b/crates/ruff_linter/src/rules/isort/rules/organize_imports.rs index a9f159e090..4b67296ea1 100644 --- a/crates/ruff_linter/src/rules/isort/rules/organize_imports.rs +++ b/crates/ruff_linter/src/rules/isort/rules/organize_imports.rs @@ -1,5 +1,3 @@ -use std::path::Path; - use itertools::{EitherOrBoth, Itertools}; use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; @@ -13,12 +11,12 @@ use ruff_python_trivia::{leading_indentation, textwrap::indent, PythonWhitespace use ruff_source_file::{LineRanges, UniversalNewlines}; use ruff_text_size::{Ranged, TextRange}; -use crate::line_width::LineWidthBuilder; -use crate::settings::LinterSettings; -use crate::Locator; - use super::super::block::Block; use super::super::{comments, format_imports}; +use crate::line_width::LineWidthBuilder; +use crate::package::PackageRoot; +use crate::settings::LinterSettings; +use crate::Locator; /// ## What it does /// De-duplicates, groups, and sorts imports based on the provided `isort` settings. @@ -87,7 +85,7 @@ pub(crate) fn organize_imports( stylist: &Stylist, indexer: &Indexer, settings: &LinterSettings, - package: Option<&Path>, + package: Option>, source_type: PySourceType, tokens: &Tokens, ) -> Option { diff --git a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_module_name.rs b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_module_name.rs index d525aa1d62..1637bd5742 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_module_name.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/rules/invalid_module_name.rs @@ -1,6 +1,8 @@ use std::ffi::OsStr; use std::path::Path; +use crate::package::PackageRoot; +use crate::rules::pep8_naming::settings::IgnoreNames; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::PySourceType; @@ -8,8 +10,6 @@ use ruff_python_stdlib::identifiers::{is_migration_name, is_module_name}; use ruff_python_stdlib::path::is_module_file; use ruff_text_size::TextRange; -use crate::rules::pep8_naming::settings::IgnoreNames; - /// ## What it does /// Checks for module names that do not follow the `snake_case` naming /// convention or are otherwise invalid. @@ -51,7 +51,7 @@ impl Violation for InvalidModuleName { /// N999 pub(crate) fn invalid_module_name( path: &Path, - package: Option<&Path>, + package: Option>, ignore_names: &IgnoreNames, ) -> Option { if !PySourceType::try_from_path(path).is_some_and(PySourceType::is_py_file_or_stub) { @@ -60,7 +60,7 @@ pub(crate) fn invalid_module_name( if let Some(package) = package { let module_name = if is_module_file(path) { - package.file_name().unwrap().to_string_lossy() + package.path().file_name().unwrap().to_string_lossy() } else { path.file_stem().unwrap().to_string_lossy() }; diff --git a/crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs b/crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs index 275f5f2183..7681d69228 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs @@ -9,6 +9,7 @@ use ruff_python_semantic::{FromImport, Import, Imported, ResolvedReference, Scop use ruff_text_size::Ranged; use crate::checkers::ast::Checker; +use crate::package::PackageRoot; /// ## What it does /// Checks for import statements that import a private name (a name starting @@ -106,6 +107,7 @@ pub(crate) fn import_private_name( // Ex) `from foo import _bar` within `foo/baz.py` if checker .package() + .map(PackageRoot::path) .is_some_and(|path| path.ends_with(root_module)) { continue; diff --git a/crates/ruff_linter/src/test.rs b/crates/ruff_linter/src/test.rs index a0c1fe4fba..6ded54565e 100644 --- a/crates/ruff_linter/src/test.rs +++ b/crates/ruff_linter/src/test.rs @@ -24,6 +24,7 @@ use ruff_text_size::Ranged; use crate::fix::{fix_file, FixResult}; use crate::linter::check_path; use crate::message::{Emitter, EmitterContext, Message, TextEmitter}; +use crate::package::PackageRoot; use crate::packaging::detect_package_root; use crate::registry::AsRule; use crate::settings::types::UnsafeFixes; @@ -122,7 +123,8 @@ pub(crate) fn test_contents<'a>( let diagnostics = check_path( path, path.parent() - .and_then(|parent| detect_package_root(parent, &settings.namespace_packages)), + .and_then(|parent| detect_package_root(parent, &settings.namespace_packages)) + .map(|path| PackageRoot::Root { path }), &locator, &stylist, &indexer, diff --git a/crates/ruff_server/src/fix.rs b/crates/ruff_server/src/fix.rs index 2b80f80a8f..616cf4caff 100644 --- a/crates/ruff_server/src/fix.rs +++ b/crates/ruff_server/src/fix.rs @@ -2,20 +2,20 @@ use std::borrow::Cow; use rustc_hash::FxHashMap; -use ruff_linter::{ - linter::{FixerResult, LinterResult}, - packaging::detect_package_root, - settings::{flags, types::UnsafeFixes, LinterSettings}, -}; -use ruff_notebook::SourceValue; -use ruff_source_file::LineIndex; - use crate::{ edit::{Replacement, ToRangeExt}, resolve::is_document_excluded, session::DocumentQuery, PositionEncoding, }; +use ruff_linter::package::PackageRoot; +use ruff_linter::{ + linter::{FixerResult, LinterResult}, + packaging::detect_package_root, + settings::{flags, types::UnsafeFixes, LinterSettings}, +}; +use ruff_notebook::SourceValue; +use ruff_source_file::LineIndex; /// A simultaneous fix made across a single text document or among an arbitrary /// number of notebook cells. @@ -49,6 +49,7 @@ pub(crate) fn fix_all( .expect("a path to a document should have a parent path"), &linter_settings.namespace_packages, ) + .map(PackageRoot::root) } else { None }; diff --git a/crates/ruff_server/src/lint.rs b/crates/ruff_server/src/lint.rs index cb3fe3499b..5b95488b32 100644 --- a/crates/ruff_server/src/lint.rs +++ b/crates/ruff_server/src/lint.rs @@ -3,7 +3,14 @@ use rustc_hash::FxHashMap; use serde::{Deserialize, Serialize}; +use crate::{ + edit::{NotebookRange, ToRangeExt}, + resolve::is_document_excluded, + session::DocumentQuery, + PositionEncoding, DIAGNOSTIC_NAME, +}; use ruff_diagnostics::{Applicability, Diagnostic, DiagnosticKind, Edit, Fix}; +use ruff_linter::package::PackageRoot; use ruff_linter::{ directives::{extract_directives, Flags}, generate_noqa_edits, @@ -21,13 +28,6 @@ use ruff_python_parser::ParseError; use ruff_source_file::LineIndex; use ruff_text_size::{Ranged, TextRange}; -use crate::{ - edit::{NotebookRange, ToRangeExt}, - resolve::is_document_excluded, - session::DocumentQuery, - PositionEncoding, DIAGNOSTIC_NAME, -}; - /// This is serialized on the diagnostic `data` field. #[derive(Serialize, Deserialize, Debug, Clone)] pub(crate) struct AssociatedDiagnosticData { @@ -89,6 +89,7 @@ pub(crate) fn check( .expect("a path to a document should have a parent path"), &linter_settings.namespace_packages, ) + .map(PackageRoot::root) } else { None }; diff --git a/crates/ruff_workspace/src/resolver.rs b/crates/ruff_workspace/src/resolver.rs index ae9f46f1a9..fd77839c1d 100644 --- a/crates/ruff_workspace/src/resolver.rs +++ b/crates/ruff_workspace/src/resolver.rs @@ -2,6 +2,7 @@ //! filesystem. use std::cmp::Ordering; +use std::collections::BTreeSet; use std::ffi::OsStr; use std::path::{Path, PathBuf}; use std::sync::RwLock; @@ -18,6 +19,7 @@ use path_slash::PathExt; use rustc_hash::{FxHashMap, FxHashSet}; use ruff_linter::fs; +use ruff_linter::package::PackageRoot; use ruff_linter::packaging::is_package; use crate::configuration::Configuration; @@ -147,8 +149,8 @@ impl<'a> Resolver<'a> { fn add(&mut self, path: &Path, settings: Settings) { self.settings.push(settings); - // normalize the path to use `/` separators and escape the '{' and '}' characters, - // which matchit uses for routing parameters + // Normalize the path to use `/` separators and escape the '{' and '}' characters, + // which matchit uses for routing parameters. let path = path.to_slash_lossy().replace('{', "{{").replace('}', "}}"); match self @@ -181,7 +183,10 @@ impl<'a> Resolver<'a> { } /// Return a mapping from Python package to its package root. - pub fn package_roots(&'a self, files: &[&'a Path]) -> FxHashMap<&'a Path, Option<&'a Path>> { + pub fn package_roots( + &'a self, + files: &[&'a Path], + ) -> FxHashMap<&'a Path, Option>> { // Pre-populate the module cache, since the list of files could (but isn't // required to) contain some `__init__.py` files. let mut package_cache: FxHashMap<&Path, bool> = FxHashMap::default(); @@ -200,7 +205,7 @@ impl<'a> Resolver<'a> { .any(|settings| !settings.linter.namespace_packages.is_empty()); // Search for the package root for each file. - let mut package_roots: FxHashMap<&Path, Option<&Path>> = FxHashMap::default(); + let mut package_roots: FxHashMap<&Path, Option>> = FxHashMap::default(); for file in files { if let Some(package) = file.parent() { package_roots.entry(package).or_insert_with(|| { @@ -210,10 +215,41 @@ impl<'a> Resolver<'a> { &[] }; detect_package_root_with_cache(package, namespace_packages, &mut package_cache) + .map(|path| PackageRoot::Root { path }) }); } } + // Discard any nested roots. + // + // For example, if `./foo/__init__.py` is a root, and then `./foo/bar` is empty, and + // `./foo/bar/baz/__init__.py` was detected as a root, we should only consider + // `./foo/__init__.py`. + let mut non_roots = FxHashSet::default(); + let mut router: Router<&Path> = Router::new(); + for root in package_roots + .values() + .flatten() + .copied() + .map(PackageRoot::path) + .collect::>() + { + // Normalize the path to use `/` separators and escape the '{' and '}' characters, + // which matchit uses for routing parameters. + let path = root.to_slash_lossy().replace('{', "{{").replace('}', "}}"); + if let Ok(matched) = router.at_mut(&path) { + debug!( + "Ignoring nested package root: {} (under {})", + root.display(), + matched.value.display() + ); + package_roots.insert(root, Some(PackageRoot::nested(root))); + non_roots.insert(root); + } else { + let _ = router.insert(format!("{path}/{{*filepath}}"), root); + } + } + package_roots }