mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-08-30 23:27:27 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Detect empty implicit namespace packages (#14236)

Browse source 
## Summary

The implicit namespace package rule currently fails to detect cases like
the following:

```text
foo/
├── __init__.py
└── bar/
    └── baz/
        └── __init__.py
```

The problem is that we detect a root at `foo`, and then an independent
root at `baz`. We _would_ detect that `bar` is an implicit namespace
package, but it doesn't contain any files! So we never check it, and
have no place to raise the diagnostic.

This PR adds detection for these kinds of nested packages, and augments
the `INP` rule to flag the `__init__.py` file above with a specialized
message. As a side effect, I've introduced a dedicated `PackageRoot`
struct which we can pass around in lieu of Yet Another `Path`.

For now, I'm only enabling this in preview (and the approach doesn't
affect any other rules). It's a bug fix, but it may end up expanding the
rule.

Closes https://github.com/astral-sh/ruff/issues/13519.
This commit is contained in:
Charlie Marsh 2024-11-09 22:03:34 -05:00 • committed by GitHub
parent 94dee2a36d
commit c7d48e10e6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
26 changed files with 282 additions and 83 deletions
Download patch file Download diff file Expand all files Collapse all files

0
crates/ruff_linter/__init__.py → bar/baz.py
View file

14
crates/ruff/src/cache.rs
View file

@ -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<PackageRoot<'a>>>,
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<Diagnostics, anyhow::Error> {
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,

8
crates/ruff/src/commands/analyze_graph.rs
View file

@ -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::<Vec<_>>(),
)
.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::<FxHashMap<_, _>>();
// Create a database from the source roots.

7
crates/ruff/src/commands/check.rs
View file

@ -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<PackageRoot<'_>>,
settings: &LinterSettings,
cache: Option<&Cache>,
noqa: flags::Noqa,

3
crates/ruff/src/commands/check_stdin.rs
View file

@ -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,

5
crates/ruff/src/commands/format.rs
View file

@ -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(

5
crates/ruff/src/diagnostics.rs
View file

@ -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<PackageRoot<'_>>,
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<PackageRoot<'_>>,
contents: String,
settings: &Settings,
noqa: flags::Noqa,

61
crates/ruff/tests/lint.rs
View file

@ -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: <https://github.com/astral-sh/ruff/issues/13519>
#[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(())
}

15
crates/ruff_linter/src/checkers/ast/mod.rs
View file

@ -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<PackageRoot<'a>>,
/// 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<PackageRoot<'a>>,
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<PackageRoot<'_>> {
self.package
}
@ -2483,12 +2484,14 @@ pub(crate) fn check_ast(
settings: &LinterSettings,
noqa: flags::Noqa,
path: &Path,
package: Option<&Path>,
package: Option<PackageRoot<'_>>,
source_type: PySourceType,
cell_offsets: Option<&CellOffsets>,
notebook_index: Option<&NotebookIndex>,
) -> Vec<Diagnostic> {
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

4
crates/ruff_linter/src/checkers/filesystem.rs
View file

@ -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<PackageRoot<'_>>,
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);
}

4
crates/ruff_linter/src/checkers/imports.rs
View file

@ -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<PackageRoot<'_>>,
source_type: PySourceType,
cell_offsets: Option<&CellOffsets>,
) -> Vec<Diagnostic> {

1
crates/ruff_linter/src/lib.rs
View file

@ -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;

9
crates/ruff_linter/src/linter.rs
View file

@ -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<PackageRoot<'_>>,
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<PackageRoot<'_>>,
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<PackageRoot<'_>>,
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<PackageRoot<'_>>,
noqa: flags::Noqa,
unsafe_fixes: UnsafeFixes,
settings: &LinterSettings,

40
crates/ruff_linter/src/package.rs Normal file
View file

@ -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,
}
}
}

8
crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_module_shadowing.rs
View file

@ -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<PackageRoot<'_>>,
allowed_modules: &[String],
target_version: PythonVersion,
) -> Option<Diagnostic> {
@ -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()
};

5
crates/ruff_linter/src/rules/flake8_no_pep420/mod.rs
View file

@ -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/{}/{}",

51
crates/ruff_linter/src/rules/flake8_no_pep420/rules/implicit_namespace_package.rs
View file

@ -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<String>,
}
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<PackageRoot<'_>>,
locator: &Locator,
comment_ranges: &CommentRanges,
project_root: &Path,
src: &[PathBuf],
preview: PreviewMode,
) -> Option<Diagnostic> {
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
}

16
crates/ruff_linter/src/rules/isort/categorize.rs
View file

@ -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<PackageRoot<'_>>,
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<PackageRoot<'_>>, 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<PackageRoot<'_>>,
detect_same_package: bool,
known_modules: &'a KnownModules,
target_version: PythonVersion,

7
crates/ruff_linter/src/rules/isort/mod.rs
View file

@ -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<PackageRoot<'_>>,
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<PackageRoot<'_>>,
target_version: PythonVersion,
settings: &Settings,
) -> String {

12
crates/ruff_linter/src/rules/isort/rules/organize_imports.rs
View file

@ -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<PackageRoot<'_>>,
source_type: PySourceType,
tokens: &Tokens,
) -> Option<Diagnostic> {

8
crates/ruff_linter/src/rules/pep8_naming/rules/invalid_module_name.rs
View file

@ -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<PackageRoot<'_>>,
ignore_names: &IgnoreNames,
) -> Option<Diagnostic> {
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()
};

2
crates/ruff_linter/src/rules/pylint/rules/import_private_name.rs
View file

@ -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;

4
crates/ruff_linter/src/test.rs
View file

@ -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,

17
crates/ruff_server/src/fix.rs
View file

@ -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
};

15
crates/ruff_server/src/lint.rs
View file

@ -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
};

44
crates/ruff_workspace/src/resolver.rs
View file

@ -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<PackageRoot<'_>>> {
// 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<PackageRoot<'_>>> = 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::<BTreeSet<_>>()
{
// 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
}