Skip to content

Commit

Permalink
Nested namespace packages support (#10541)
Browse files Browse the repository at this point in the history
## Summary
PEP 420 says [nested namespace
packages](https://peps.python.org/pep-0420/#nested-namespace-packages)
are allowed, i.e. marking a directory as a namespace package marks all
subdirectories in the subtree as namespace packages.

`is_package` is modified to use `Path::starts_with` and the order of
checks is reversed to do in-memory checks first before hitting the disk.

## Test Plan
Added unit tests. Previously all tests were run with `namespace_packages
== &[]`. Verified that one of the tests was failing before changing the
implementation.

## Future Improvements
The `is_package_with_cache` can probably be rewritten to avoid repeated
calls to `Path::starts_with`, by caching all directories up to the
`namespace_root`:
```ruff
let namespace_root = namespace_packages
    .iter()
    .filter(|namespace_package| path.starts_with(namespace_package))
    .min();
```
  • Loading branch information
dizzy57 committed Mar 25, 2024
1 parent 9856c14 commit d625f55
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 16 deletions.
4 changes: 2 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -813,8 +813,8 @@ To understand Ruff's import categorization system, we first need to define two c
"project root".)
- "Package root": The top-most directory defining the Python package that includes a given Python
file. To find the package root for a given Python file, traverse up its parent directories until
you reach a parent directory that doesn't contain an `__init__.py` file (and isn't marked as
a [namespace package](https://docs.astral.sh/ruff/settings/#namespace-packages)); take the directory
you reach a parent directory that doesn't contain an `__init__.py` file (and isn't in a subtree
marked as a [namespace package](https://docs.astral.sh/ruff/settings/#namespace-packages)); take the directory
just before that, i.e., the first directory in the package.

For example, given:
Expand Down
50 changes: 41 additions & 9 deletions crates/ruff_linter/src/packaging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,14 @@ use std::path::{Path, PathBuf};
/// Return `true` if the directory at the given `Path` appears to be a Python
/// package.
pub fn is_package(path: &Path, namespace_packages: &[PathBuf]) -> bool {
path.join("__init__.py").is_file()
|| namespace_packages
.iter()
.any(|namespace_package| namespace_package == path)
namespace_packages
.iter()
.any(|namespace_package| path.starts_with(namespace_package))
|| path.join("__init__.py").is_file()
}

/// Return the package root for the given Python file.
pub fn detect_package_root<'a>(
path: &'a Path,
namespace_packages: &'a [PathBuf],
) -> Option<&'a Path> {
/// Return the package root for the given path to a directory with Python file.
pub fn detect_package_root<'a>(path: &'a Path, namespace_packages: &[PathBuf]) -> Option<&'a Path> {
let mut current = None;
for parent in path.ancestors() {
if !is_package(parent, namespace_packages) {
Expand Down Expand Up @@ -84,4 +81,39 @@ mod tests {
None,
);
}

#[test]
fn package_detection_with_namespace_packages() {
assert_eq!(
detect_package_root(&test_resource_path("project/python_modules/core/core"), &[],),
Some(test_resource_path("project/python_modules/core/core").as_path())
);

assert_eq!(
detect_package_root(
&test_resource_path("project/python_modules/core/core"),
&[test_resource_path("project/python_modules/core"),],
),
Some(test_resource_path("project/python_modules/core").as_path())
);

assert_eq!(
detect_package_root(
&test_resource_path("project/python_modules/core/core"),
&[
test_resource_path("project/python_modules/core"),
test_resource_path("project/python_modules"),
],
),
Some(test_resource_path("project/python_modules").as_path())
);

assert_eq!(
detect_package_root(
&test_resource_path("project/python_modules/core/core"),
&[test_resource_path("project/python_modules"),],
),
Some(test_resource_path("project/python_modules").as_path())
);
}
}
4 changes: 2 additions & 2 deletions crates/ruff_workspace/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,8 +293,8 @@ pub struct Options {
pub builtins: Option<Vec<String>>,

/// Mark the specified directories as namespace packages. For the purpose of
/// module resolution, Ruff will treat those directories as if they
/// contained an `__init__.py` file.
/// module resolution, Ruff will treat those directories and all their subdirectories
/// as if they contained an `__init__.py` file.
#[option(
default = r#"[]"#,
value_type = "list[str]",
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_workspace/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ impl<'a> Resolver<'a> {
/// A wrapper around `detect_package_root` to cache filesystem lookups.
fn detect_package_root_with_cache<'a>(
path: &'a Path,
namespace_packages: &'a [PathBuf],
namespace_packages: &[PathBuf],
package_cache: &mut FxHashMap<&'a Path, bool>,
) -> Option<&'a Path> {
let mut current = None;
Expand All @@ -219,7 +219,7 @@ fn detect_package_root_with_cache<'a>(
/// A wrapper around `is_package` to cache filesystem lookups.
fn is_package_with_cache<'a>(
path: &'a Path,
namespace_packages: &'a [PathBuf],
namespace_packages: &[PathBuf],
package_cache: &mut FxHashMap<&'a Path, bool>,
) -> bool {
*package_cache
Expand Down
2 changes: 1 addition & 1 deletion ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit d625f55

Please sign in to comment.