mirror of https://github.com/astral-sh/ruff
Stabilize new strategy for classifying imports as first party (#20268)
This stabilizes the behavior introduced in #16565 which (roughly) tries to match an import like `import a.b.c` to an actual directory path `a/b/c` in order to label it as first-party, rather than simply looking for a directory `a`. Mainly this affects the sorting of imports in the presence of namespace packages, but a few other rules are affected as well.
This commit is contained in:
Dylan
Brent Westbrook
parent
5dec37fbaf
commit
512395f4e6
|
|
@ -55,6 +55,10 @@ either a redundant alias or, if already present in the file, an `__all__` entry.
|
|||
to remove third-party and standard library imports -- the fix is unsafe because the module's
|
||||
interface changes.
|
||||
|
||||
See [this FAQ section](https://docs.astral.sh/ruff/faq/#how-does-ruff-determine-which-of-my-imports-are-first-party-third-party-etc)
|
||||
for more details on how Ruff
|
||||
determines whether an import is first or third-party.
|
||||
|
||||
## Example
|
||||
|
||||
```python
|
||||
|
|
@ -83,11 +87,6 @@ else:
|
|||
print("numpy is not installed")
|
||||
```
|
||||
|
||||
## Preview
|
||||
When [preview](https://docs.astral.sh/ruff/preview/) is enabled,
|
||||
the criterion for determining whether an import is first-party
|
||||
is stricter, which could affect the suggested fix. See [this FAQ section](https://docs.astral.sh/ruff/faq/#how-does-ruff-determine-which-of-my-imports-are-first-party-third-party-etc) for more details.
|
||||
|
||||
## Options
|
||||
- `lint.ignore-init-module-imports`
|
||||
- `lint.pyflakes.allowed-unused-imports`
|
||||
|
|
|
|||
|
|
@ -95,7 +95,7 @@ exit_code: 1
|
|||
"rules": [
|
||||
{
|
||||
"fullDescription": {
|
||||
"text": "## What it does\nChecks for unused imports.\n\n## Why is this bad?\nUnused imports add a performance overhead at runtime, and risk creating\nimport cycles. They also increase the cognitive load of reading the code.\n\nIf an import statement is used to check for the availability or existence\nof a module, consider using `importlib.util.find_spec` instead.\n\nIf an import statement is used to re-export a symbol as part of a module's\npublic interface, consider using a \"redundant\" import alias, which\ninstructs Ruff (and other tools) to respect the re-export, and avoid\nmarking it as unused, as in:\n\n```python\nfrom module import member as member\n```\n\nAlternatively, you can use `__all__` to declare a symbol as part of the module's\ninterface, as in:\n\n```python\n# __init__.py\nimport some_module\n\n__all__ = [\"some_module\"]\n```\n\n## Fix safety\n\nFixes to remove unused imports are safe, except in `__init__.py` files.\n\nApplying fixes to `__init__.py` files is currently in preview. The fix offered depends on the\ntype of the unused import. Ruff will suggest a safe fix to export first-party imports with\neither a redundant alias or, if already present in the file, an `__all__` entry. If multiple\n`__all__` declarations are present, Ruff will not offer a fix. Ruff will suggest an unsafe fix\nto remove third-party and standard library imports -- the fix is unsafe because the module's\ninterface changes.\n\n## Example\n\n```python\nimport numpy as np # unused import\n\n\ndef area(radius):\n return 3.14 * radius**2\n```\n\nUse instead:\n\n```python\ndef area(radius):\n return 3.14 * radius**2\n```\n\nTo check the availability of a module, use `importlib.util.find_spec`:\n\n```python\nfrom importlib.util import find_spec\n\nif find_spec(\"numpy\") is not None:\n print(\"numpy is installed\")\nelse:\n print(\"numpy is not installed\")\n```\n\n## Preview\nWhen [preview](https://docs.astral.sh/ruff/preview/) is enabled,\nthe criterion for determining whether an import is first-party\nis stricter, which could affect the suggested fix. See [this FAQ section](https://docs.astral.sh/ruff/faq/#how-does-ruff-determine-which-of-my-imports-are-first-party-third-party-etc) for more details.\n\n## Options\n- `lint.ignore-init-module-imports`\n- `lint.pyflakes.allowed-unused-imports`\n\n## References\n- [Python documentation: `import`](https://docs.python.org/3/reference/simple_stmts.html#the-import-statement)\n- [Python documentation: `importlib.util.find_spec`](https://docs.python.org/3/library/importlib.html#importlib.util.find_spec)\n- [Typing documentation: interface conventions](https://typing.python.org/en/latest/spec/distributing.html#library-interface-public-and-private-symbols)\n"
|
||||
"text": "## What it does\nChecks for unused imports.\n\n## Why is this bad?\nUnused imports add a performance overhead at runtime, and risk creating\nimport cycles. They also increase the cognitive load of reading the code.\n\nIf an import statement is used to check for the availability or existence\nof a module, consider using `importlib.util.find_spec` instead.\n\nIf an import statement is used to re-export a symbol as part of a module's\npublic interface, consider using a \"redundant\" import alias, which\ninstructs Ruff (and other tools) to respect the re-export, and avoid\nmarking it as unused, as in:\n\n```python\nfrom module import member as member\n```\n\nAlternatively, you can use `__all__` to declare a symbol as part of the module's\ninterface, as in:\n\n```python\n# __init__.py\nimport some_module\n\n__all__ = [\"some_module\"]\n```\n\n## Fix safety\n\nFixes to remove unused imports are safe, except in `__init__.py` files.\n\nApplying fixes to `__init__.py` files is currently in preview. The fix offered depends on the\ntype of the unused import. Ruff will suggest a safe fix to export first-party imports with\neither a redundant alias or, if already present in the file, an `__all__` entry. If multiple\n`__all__` declarations are present, Ruff will not offer a fix. Ruff will suggest an unsafe fix\nto remove third-party and standard library imports -- the fix is unsafe because the module's\ninterface changes.\n\nSee [this FAQ section](https://docs.astral.sh/ruff/faq/#how-does-ruff-determine-which-of-my-imports-are-first-party-third-party-etc)\nfor more details on how Ruff\ndetermines whether an import is first or third-party.\n\n## Example\n\n```python\nimport numpy as np # unused import\n\n\ndef area(radius):\n return 3.14 * radius**2\n```\n\nUse instead:\n\n```python\ndef area(radius):\n return 3.14 * radius**2\n```\n\nTo check the availability of a module, use `importlib.util.find_spec`:\n\n```python\nfrom importlib.util import find_spec\n\nif find_spec(\"numpy\") is not None:\n print(\"numpy is installed\")\nelse:\n print(\"numpy is not installed\")\n```\n\n## Options\n- `lint.ignore-init-module-imports`\n- `lint.pyflakes.allowed-unused-imports`\n\n## References\n- [Python documentation: `import`](https://docs.python.org/3/reference/simple_stmts.html#the-import-statement)\n- [Python documentation: `importlib.util.find_spec`](https://docs.python.org/3/library/importlib.html#importlib.util.find_spec)\n- [Typing documentation: interface conventions](https://typing.python.org/en/latest/spec/distributing.html#library-interface-public-and-private-symbols)\n"
|
||||
},
|
||||
"help": {
|
||||
"text": "`{name}` imported but unused; consider using `importlib.util.find_spec` to test for availability"
|
||||
|
|
|
|||
|
|
@ -81,7 +81,7 @@ expression: value
|
|||
"rules": [
|
||||
{
|
||||
"fullDescription": {
|
||||
"text": "## What it does\nChecks for unused imports.\n\n## Why is this bad?\nUnused imports add a performance overhead at runtime, and risk creating\nimport cycles. They also increase the cognitive load of reading the code.\n\nIf an import statement is used to check for the availability or existence\nof a module, consider using `importlib.util.find_spec` instead.\n\nIf an import statement is used to re-export a symbol as part of a module's\npublic interface, consider using a \"redundant\" import alias, which\ninstructs Ruff (and other tools) to respect the re-export, and avoid\nmarking it as unused, as in:\n\n```python\nfrom module import member as member\n```\n\nAlternatively, you can use `__all__` to declare a symbol as part of the module's\ninterface, as in:\n\n```python\n# __init__.py\nimport some_module\n\n__all__ = [\"some_module\"]\n```\n\n## Fix safety\n\nFixes to remove unused imports are safe, except in `__init__.py` files.\n\nApplying fixes to `__init__.py` files is currently in preview. The fix offered depends on the\ntype of the unused import. Ruff will suggest a safe fix to export first-party imports with\neither a redundant alias or, if already present in the file, an `__all__` entry. If multiple\n`__all__` declarations are present, Ruff will not offer a fix. Ruff will suggest an unsafe fix\nto remove third-party and standard library imports -- the fix is unsafe because the module's\ninterface changes.\n\n## Example\n\n```python\nimport numpy as np # unused import\n\n\ndef area(radius):\n return 3.14 * radius**2\n```\n\nUse instead:\n\n```python\ndef area(radius):\n return 3.14 * radius**2\n```\n\nTo check the availability of a module, use `importlib.util.find_spec`:\n\n```python\nfrom importlib.util import find_spec\n\nif find_spec(\"numpy\") is not None:\n print(\"numpy is installed\")\nelse:\n print(\"numpy is not installed\")\n```\n\n## Preview\nWhen [preview](https://docs.astral.sh/ruff/preview/) is enabled,\nthe criterion for determining whether an import is first-party\nis stricter, which could affect the suggested fix. See [this FAQ section](https://docs.astral.sh/ruff/faq/#how-does-ruff-determine-which-of-my-imports-are-first-party-third-party-etc) for more details.\n\n## Options\n- `lint.ignore-init-module-imports`\n- `lint.pyflakes.allowed-unused-imports`\n\n## References\n- [Python documentation: `import`](https://docs.python.org/3/reference/simple_stmts.html#the-import-statement)\n- [Python documentation: `importlib.util.find_spec`](https://docs.python.org/3/library/importlib.html#importlib.util.find_spec)\n- [Typing documentation: interface conventions](https://typing.python.org/en/latest/spec/distributing.html#library-interface-public-and-private-symbols)\n"
|
||||
"text": "## What it does\nChecks for unused imports.\n\n## Why is this bad?\nUnused imports add a performance overhead at runtime, and risk creating\nimport cycles. They also increase the cognitive load of reading the code.\n\nIf an import statement is used to check for the availability or existence\nof a module, consider using `importlib.util.find_spec` instead.\n\nIf an import statement is used to re-export a symbol as part of a module's\npublic interface, consider using a \"redundant\" import alias, which\ninstructs Ruff (and other tools) to respect the re-export, and avoid\nmarking it as unused, as in:\n\n```python\nfrom module import member as member\n```\n\nAlternatively, you can use `__all__` to declare a symbol as part of the module's\ninterface, as in:\n\n```python\n# __init__.py\nimport some_module\n\n__all__ = [\"some_module\"]\n```\n\n## Fix safety\n\nFixes to remove unused imports are safe, except in `__init__.py` files.\n\nApplying fixes to `__init__.py` files is currently in preview. The fix offered depends on the\ntype of the unused import. Ruff will suggest a safe fix to export first-party imports with\neither a redundant alias or, if already present in the file, an `__all__` entry. If multiple\n`__all__` declarations are present, Ruff will not offer a fix. Ruff will suggest an unsafe fix\nto remove third-party and standard library imports -- the fix is unsafe because the module's\ninterface changes.\n\nSee [this FAQ section](https://docs.astral.sh/ruff/faq/#how-does-ruff-determine-which-of-my-imports-are-first-party-third-party-etc)\nfor more details on how Ruff\ndetermines whether an import is first or third-party.\n\n## Example\n\n```python\nimport numpy as np # unused import\n\n\ndef area(radius):\n return 3.14 * radius**2\n```\n\nUse instead:\n\n```python\ndef area(radius):\n return 3.14 * radius**2\n```\n\nTo check the availability of a module, use `importlib.util.find_spec`:\n\n```python\nfrom importlib.util import find_spec\n\nif find_spec(\"numpy\") is not None:\n print(\"numpy is installed\")\nelse:\n print(\"numpy is not installed\")\n```\n\n## Options\n- `lint.ignore-init-module-imports`\n- `lint.pyflakes.allowed-unused-imports`\n\n## References\n- [Python documentation: `import`](https://docs.python.org/3/reference/simple_stmts.html#the-import-statement)\n- [Python documentation: `importlib.util.find_spec`](https://docs.python.org/3/library/importlib.html#importlib.util.find_spec)\n- [Typing documentation: interface conventions](https://typing.python.org/en/latest/spec/distributing.html#library-interface-public-and-private-symbols)\n"
|
||||
},
|
||||
"help": {
|
||||
"text": "`{name}` imported but unused; consider using `importlib.util.find_spec` to test for availability"
|
||||
|
|
|
|||
|
|
@ -11,11 +11,6 @@ pub(crate) const fn is_py314_support_enabled(settings: &LinterSettings) -> bool
|
|||
settings.preview.is_enabled()
|
||||
}
|
||||
|
||||
// https://github.com/astral-sh/ruff/pull/16565
|
||||
pub(crate) const fn is_full_path_match_source_strategy_enabled(settings: &LinterSettings) -> bool {
|
||||
settings.preview.is_enabled()
|
||||
}
|
||||
|
||||
// Rule-specific behavior
|
||||
|
||||
// https://github.com/astral-sh/ruff/pull/15541
|
||||
|
|
|
|||
|
|
@ -11,12 +11,10 @@ use crate::checkers::ast::{Checker, DiagnosticGuard};
|
|||
use crate::codes::Rule;
|
||||
use crate::fix;
|
||||
use crate::importer::ImportedMembers;
|
||||
use crate::preview::is_full_path_match_source_strategy_enabled;
|
||||
use crate::rules::flake8_type_checking::helpers::{
|
||||
TypingReference, filter_contained, quote_annotation,
|
||||
};
|
||||
use crate::rules::flake8_type_checking::imports::ImportBinding;
|
||||
use crate::rules::isort::categorize::MatchSourceStrategy;
|
||||
use crate::rules::isort::{ImportSection, ImportType, categorize};
|
||||
use crate::{Fix, FixAvailability, Violation};
|
||||
|
||||
|
|
@ -67,10 +65,6 @@ use crate::{Fix, FixAvailability, Violation};
|
|||
///
|
||||
///
|
||||
/// ## Preview
|
||||
/// When [preview](https://docs.astral.sh/ruff/preview/) is enabled,
|
||||
/// the criterion for determining whether an import is first-party
|
||||
/// is stricter, which could affect whether this lint is triggered vs [`TC001`](https://docs.astral.sh/ruff/rules/typing-only-third-party-import/). See [this FAQ section](https://docs.astral.sh/ruff/faq/#how-does-ruff-determine-which-of-my-imports-are-first-party-third-party-etc) for more details.
|
||||
///
|
||||
/// If [`lint.future-annotations`] is set to `true`, `from __future__ import
|
||||
/// annotations` will be added if doing so would enable an import to be moved into an `if
|
||||
/// TYPE_CHECKING:` block. This takes precedence over the
|
||||
|
|
@ -154,10 +148,6 @@ impl Violation for TypingOnlyFirstPartyImport {
|
|||
/// ```
|
||||
///
|
||||
/// ## Preview
|
||||
/// When [preview](https://docs.astral.sh/ruff/preview/) is enabled,
|
||||
/// the criterion for determining whether an import is first-party
|
||||
/// is stricter, which could affect whether this lint is triggered vs [`TC001`](https://docs.astral.sh/ruff/rules/typing-only-first-party-import/). See [this FAQ section](https://docs.astral.sh/ruff/faq/#how-does-ruff-determine-which-of-my-imports-are-first-party-third-party-etc) for more details.
|
||||
///
|
||||
/// If [`lint.future-annotations`] is set to `true`, `from __future__ import
|
||||
/// annotations` will be added if doing so would enable an import to be moved into an `if
|
||||
/// TYPE_CHECKING:` block. This takes precedence over the
|
||||
|
|
@ -347,13 +337,6 @@ pub(crate) fn typing_only_runtime_import(
|
|||
let source_name = import.source_name().join(".");
|
||||
|
||||
// Categorize the import, using coarse-grained categorization.
|
||||
let match_source_strategy =
|
||||
if is_full_path_match_source_strategy_enabled(checker.settings()) {
|
||||
MatchSourceStrategy::FullPath
|
||||
} else {
|
||||
MatchSourceStrategy::Root
|
||||
};
|
||||
|
||||
let import_type = match categorize(
|
||||
&source_name,
|
||||
qualified_name.is_unresolved_import(),
|
||||
|
|
@ -365,7 +348,6 @@ pub(crate) fn typing_only_runtime_import(
|
|||
checker.settings().isort.no_sections,
|
||||
&checker.settings().isort.section_order,
|
||||
&checker.settings().isort.default_section,
|
||||
match_source_strategy,
|
||||
) {
|
||||
ImportSection::Known(ImportType::LocalFolder | ImportType::FirstParty) => {
|
||||
ImportType::FirstParty
|
||||
|
|
|
|||
|
|
@ -1,6 +1,5 @@
|
|||
use std::collections::BTreeMap;
|
||||
use std::fmt;
|
||||
use std::fs;
|
||||
use std::iter;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
|
|
@ -101,7 +100,6 @@ pub(crate) fn categorize<'a>(
|
|||
no_sections: bool,
|
||||
section_order: &'a [ImportSection],
|
||||
default_section: &'a ImportSection,
|
||||
match_source_strategy: MatchSourceStrategy,
|
||||
) -> &'a ImportSection {
|
||||
let module_base = module_name.split('.').next().unwrap();
|
||||
let (mut import_type, mut reason) = {
|
||||
|
|
@ -129,7 +127,7 @@ pub(crate) fn categorize<'a>(
|
|||
&ImportSection::Known(ImportType::FirstParty),
|
||||
Reason::SamePackage,
|
||||
)
|
||||
} else if let Some(src) = match_sources(src, module_name, match_source_strategy) {
|
||||
} else if let Some(src) = match_sources(src, module_name) {
|
||||
(
|
||||
&ImportSection::Known(ImportType::FirstParty),
|
||||
Reason::SourceMatch(src),
|
||||
|
|
@ -161,61 +159,29 @@ fn same_package(package: Option<PackageRoot<'_>>, module_base: &str) -> bool {
|
|||
/// Returns the source path with respect to which the module `name`
|
||||
/// should be considered first party, or `None` if no path is found.
|
||||
///
|
||||
/// The [`MatchSourceStrategy`] is the criterion used to decide whether
|
||||
/// the module path matches a given source directory.
|
||||
///
|
||||
/// # Examples
|
||||
///
|
||||
/// - The module named `foo` will match `[SRC]` if `[SRC]/foo` is a directory,
|
||||
/// no matter the strategy.
|
||||
/// - The module named `foo` will match `[SRC]` if `[SRC]/foo` is a directory
|
||||
///
|
||||
/// - With `match_source_strategy == MatchSourceStrategy::Root`, the module
|
||||
/// named `foo.baz` will match `[SRC]` if `[SRC]/foo` is a
|
||||
/// directory or `[SRC]/foo.py` exists.
|
||||
///
|
||||
/// - With `match_source_stratgy == MatchSourceStrategy::FullPath`, the module
|
||||
/// named `foo.baz` will match `[SRC]` only if `[SRC]/foo/baz` is a directory,
|
||||
/// or `[SRC]/foo/baz.py` exists or `[SRC]/foo/baz.pyi` exists.
|
||||
fn match_sources<'a>(
|
||||
paths: &'a [PathBuf],
|
||||
name: &str,
|
||||
match_source_strategy: MatchSourceStrategy,
|
||||
) -> Option<&'a Path> {
|
||||
match match_source_strategy {
|
||||
MatchSourceStrategy::Root => {
|
||||
let base = name.split('.').next()?;
|
||||
for path in paths {
|
||||
if let Ok(metadata) = fs::metadata(path.join(base)) {
|
||||
if metadata.is_dir() {
|
||||
return Some(path);
|
||||
}
|
||||
}
|
||||
if let Ok(metadata) = fs::metadata(path.join(format!("{base}.py"))) {
|
||||
if metadata.is_file() {
|
||||
return Some(path);
|
||||
}
|
||||
}
|
||||
}
|
||||
None
|
||||
/// - The module named `foo.baz` will match `[SRC]` only if `[SRC]/foo/baz`
|
||||
/// is a directory, or `[SRC]/foo/baz.py` exists,
|
||||
/// or `[SRC]/foo/baz.pyi` exists.
|
||||
fn match_sources<'a>(paths: &'a [PathBuf], name: &str) -> Option<&'a Path> {
|
||||
let relative_path: PathBuf = name.split('.').collect();
|
||||
relative_path.components().next()?;
|
||||
for root in paths {
|
||||
let candidate = root.join(&relative_path);
|
||||
if candidate.is_dir() {
|
||||
return Some(root);
|
||||
}
|
||||
MatchSourceStrategy::FullPath => {
|
||||
let relative_path: PathBuf = name.split('.').collect();
|
||||
relative_path.components().next()?;
|
||||
for root in paths {
|
||||
let candidate = root.join(&relative_path);
|
||||
if candidate.is_dir() {
|
||||
return Some(root);
|
||||
}
|
||||
if ["py", "pyi"]
|
||||
.into_iter()
|
||||
.any(|extension| candidate.with_extension(extension).is_file())
|
||||
{
|
||||
return Some(root);
|
||||
}
|
||||
}
|
||||
None
|
||||
if ["py", "pyi"]
|
||||
.into_iter()
|
||||
.any(|extension| candidate.with_extension(extension).is_file())
|
||||
{
|
||||
return Some(root);
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
#[expect(clippy::too_many_arguments)]
|
||||
|
|
@ -229,7 +195,6 @@ pub(crate) fn categorize_imports<'a>(
|
|||
no_sections: bool,
|
||||
section_order: &'a [ImportSection],
|
||||
default_section: &'a ImportSection,
|
||||
match_source_strategy: MatchSourceStrategy,
|
||||
) -> BTreeMap<&'a ImportSection, ImportBlock<'a>> {
|
||||
let mut block_by_type: BTreeMap<&ImportSection, ImportBlock> = BTreeMap::default();
|
||||
// Categorize `Stmt::Import`.
|
||||
|
|
@ -245,7 +210,6 @@ pub(crate) fn categorize_imports<'a>(
|
|||
no_sections,
|
||||
section_order,
|
||||
default_section,
|
||||
match_source_strategy,
|
||||
);
|
||||
block_by_type
|
||||
.entry(import_type)
|
||||
|
|
@ -266,7 +230,6 @@ pub(crate) fn categorize_imports<'a>(
|
|||
no_sections,
|
||||
section_order,
|
||||
default_section,
|
||||
match_source_strategy,
|
||||
);
|
||||
block_by_type
|
||||
.entry(classification)
|
||||
|
|
@ -287,7 +250,6 @@ pub(crate) fn categorize_imports<'a>(
|
|||
no_sections,
|
||||
section_order,
|
||||
default_section,
|
||||
match_source_strategy,
|
||||
);
|
||||
block_by_type
|
||||
.entry(classification)
|
||||
|
|
@ -308,7 +270,6 @@ pub(crate) fn categorize_imports<'a>(
|
|||
no_sections,
|
||||
section_order,
|
||||
default_section,
|
||||
match_source_strategy,
|
||||
);
|
||||
block_by_type
|
||||
.entry(classification)
|
||||
|
|
@ -463,25 +424,9 @@ impl fmt::Display for KnownModules {
|
|||
}
|
||||
}
|
||||
|
||||
/// Rule to determine whether a module path matches
|
||||
/// a relative path from a source directory.
|
||||
#[derive(Debug, Clone, Copy)]
|
||||
pub(crate) enum MatchSourceStrategy {
|
||||
/// Matches if first term in module path is found in file system
|
||||
///
|
||||
/// # Example
|
||||
/// Module is `foo.bar.baz` and `[SRC]/foo` exists
|
||||
Root,
|
||||
/// Matches only if full module path is reflected in file system
|
||||
///
|
||||
/// # Example
|
||||
/// Module is `foo.bar.baz` and `[SRC]/foo/bar/baz` exists
|
||||
FullPath,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use crate::rules::isort::categorize::{MatchSourceStrategy, match_sources};
|
||||
use crate::rules::isort::categorize::match_sources;
|
||||
|
||||
use std::fs;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
|
@ -522,49 +467,17 @@ mod tests {
|
|||
|
||||
let paths = vec![project_dir.clone()];
|
||||
|
||||
// Test with Root strategy
|
||||
|
||||
assert_eq!(
|
||||
match_sources(&paths, "mypackage", MatchSourceStrategy::Root),
|
||||
match_sources(&paths, "mypackage"),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
match_sources(&paths, "mypackage.module1", MatchSourceStrategy::Root),
|
||||
match_sources(&paths, "mypackage.module1"),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
match_sources(&paths, "mypackage.nonexistent", MatchSourceStrategy::Root),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
match_sources(&paths, "nonexistent", MatchSourceStrategy::Root),
|
||||
None
|
||||
);
|
||||
|
||||
// Test with FullPath strategy
|
||||
|
||||
assert_eq!(
|
||||
match_sources(&paths, "mypackage", MatchSourceStrategy::FullPath),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
match_sources(&paths, "mypackage.module1", MatchSourceStrategy::FullPath),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
|
||||
// Differs in behavior from [`MatchSourceStrategy::Root`]
|
||||
assert_eq!(
|
||||
match_sources(
|
||||
&paths,
|
||||
"mypackage.nonexistent",
|
||||
MatchSourceStrategy::FullPath
|
||||
),
|
||||
None
|
||||
);
|
||||
assert_eq!(match_sources(&paths, "mypackage.nonexistent",), None);
|
||||
}
|
||||
|
||||
/// Tests a src-based Python package layout:
|
||||
|
|
@ -588,39 +501,12 @@ mod tests {
|
|||
|
||||
let paths = vec![src_dir.clone()];
|
||||
|
||||
// Test with Root strategy
|
||||
|
||||
assert_eq!(
|
||||
match_sources(&paths, "mypackage", MatchSourceStrategy::Root),
|
||||
match_sources(&paths, "mypackage.module1"),
|
||||
Some(src_dir.as_path())
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
match_sources(&paths, "mypackage.module1", MatchSourceStrategy::Root),
|
||||
Some(src_dir.as_path())
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
match_sources(&paths, "mypackage.nonexistent", MatchSourceStrategy::Root),
|
||||
Some(src_dir.as_path())
|
||||
);
|
||||
|
||||
// Test with FullPath strategy
|
||||
|
||||
assert_eq!(
|
||||
match_sources(&paths, "mypackage.module1", MatchSourceStrategy::FullPath),
|
||||
Some(src_dir.as_path())
|
||||
);
|
||||
|
||||
// Differs in behavior from [`MatchSourceStrategy::Root`]
|
||||
assert_eq!(
|
||||
match_sources(
|
||||
&paths,
|
||||
"mypackage.nonexistent",
|
||||
MatchSourceStrategy::FullPath
|
||||
),
|
||||
None
|
||||
);
|
||||
assert_eq!(match_sources(&paths, "mypackage.nonexistent"), None);
|
||||
}
|
||||
|
||||
/// Tests a nested package layout:
|
||||
|
|
@ -647,35 +533,13 @@ mod tests {
|
|||
|
||||
let paths = vec![project_dir.clone()];
|
||||
|
||||
// Test with Root strategy
|
||||
assert_eq!(
|
||||
match_sources(&paths, "mypackage", MatchSourceStrategy::Root),
|
||||
match_sources(&paths, "mypackage.subpackage.module2"),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
match_sources(&paths, "mypackage.subpackage", MatchSourceStrategy::Root),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
|
||||
// Test with FullPath strategy
|
||||
|
||||
assert_eq!(
|
||||
match_sources(
|
||||
&paths,
|
||||
"mypackage.subpackage.module2",
|
||||
MatchSourceStrategy::FullPath
|
||||
),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
|
||||
// Differs in behavior from [`MatchSourceStrategy::Root`]
|
||||
assert_eq!(
|
||||
match_sources(
|
||||
&paths,
|
||||
"mypackage.subpackage.nonexistent",
|
||||
MatchSourceStrategy::FullPath
|
||||
),
|
||||
match_sources(&paths, "mypackage.subpackage.nonexistent"),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
|
@ -699,52 +563,17 @@ mod tests {
|
|||
create_file(project_dir.join("namespace/package1/module1.py"));
|
||||
|
||||
let paths = vec![project_dir.clone()];
|
||||
// Test with Root strategy
|
||||
|
||||
assert_eq!(
|
||||
match_sources(&paths, "namespace", MatchSourceStrategy::Root),
|
||||
match_sources(&paths, "namespace.package1"),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
match_sources(&paths, "namespace.package1", MatchSourceStrategy::Root),
|
||||
match_sources(&paths, "namespace.package1.module1"),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
match_sources(
|
||||
&paths,
|
||||
"namespace.package2.module1",
|
||||
MatchSourceStrategy::Root
|
||||
),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
|
||||
// Test with FullPath strategy
|
||||
|
||||
assert_eq!(
|
||||
match_sources(&paths, "namespace.package1", MatchSourceStrategy::FullPath),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
match_sources(
|
||||
&paths,
|
||||
"namespace.package1.module1",
|
||||
MatchSourceStrategy::FullPath
|
||||
),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
|
||||
// Differs in behavior from [`MatchSourceStrategy::Root`]
|
||||
assert_eq!(
|
||||
match_sources(
|
||||
&paths,
|
||||
"namespace.package2.module1",
|
||||
MatchSourceStrategy::FullPath
|
||||
),
|
||||
None
|
||||
);
|
||||
assert_eq!(match_sources(&paths, "namespace.package2.module1"), None);
|
||||
}
|
||||
|
||||
/// Tests a package with type stubs (.pyi files):
|
||||
|
|
@ -764,12 +593,11 @@ mod tests {
|
|||
create_file(project_dir.join("mypackage/__init__.py"));
|
||||
create_file(project_dir.join("mypackage/module1.pyi")); // Only create .pyi file, not .py
|
||||
|
||||
// Test with FullPath strategy
|
||||
let paths = vec![project_dir.clone()];
|
||||
|
||||
// Module "mypackage.module1" should match project_dir using .pyi file
|
||||
assert_eq!(
|
||||
match_sources(&paths, "mypackage.module1", MatchSourceStrategy::FullPath),
|
||||
match_sources(&paths, "mypackage.module1"),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
}
|
||||
|
|
@ -796,30 +624,17 @@ mod tests {
|
|||
create_file(project_dir.join("mypackage/feature/__init__.py"));
|
||||
create_file(project_dir.join("mypackage/feature/submodule.py"));
|
||||
|
||||
// Test with Root strategy
|
||||
let paths = vec![project_dir.clone()];
|
||||
|
||||
// Module "mypackage.feature" should match project_dir (matches the file first)
|
||||
assert_eq!(
|
||||
match_sources(&paths, "mypackage.feature", MatchSourceStrategy::Root),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
|
||||
// Test with FullPath strategy
|
||||
|
||||
// Module "mypackage.feature" should match project_dir
|
||||
assert_eq!(
|
||||
match_sources(&paths, "mypackage.feature", MatchSourceStrategy::FullPath),
|
||||
match_sources(&paths, "mypackage.feature"),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
|
||||
// Module "mypackage.feature.submodule" should match project_dir
|
||||
assert_eq!(
|
||||
match_sources(
|
||||
&paths,
|
||||
"mypackage.feature.submodule",
|
||||
MatchSourceStrategy::FullPath
|
||||
),
|
||||
match_sources(&paths, "mypackage.feature.submodule"),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
}
|
||||
|
|
@ -857,13 +672,13 @@ mod tests {
|
|||
|
||||
// Module "package1" should match project1_dir
|
||||
assert_eq!(
|
||||
match_sources(&paths, "package1", MatchSourceStrategy::Root),
|
||||
match_sources(&paths, "package1"),
|
||||
Some(project1_dir.as_path())
|
||||
);
|
||||
|
||||
// Module "package2" should match project2_dir
|
||||
assert_eq!(
|
||||
match_sources(&paths, "package2", MatchSourceStrategy::Root),
|
||||
match_sources(&paths, "package2"),
|
||||
Some(project2_dir.as_path())
|
||||
);
|
||||
|
||||
|
|
@ -872,7 +687,7 @@ mod tests {
|
|||
|
||||
// Module "package1" should still match project1_dir
|
||||
assert_eq!(
|
||||
match_sources(&paths_reversed, "package1", MatchSourceStrategy::Root),
|
||||
match_sources(&paths_reversed, "package1"),
|
||||
Some(project1_dir.as_path())
|
||||
);
|
||||
}
|
||||
|
|
@ -885,8 +700,7 @@ mod tests {
|
|||
///
|
||||
/// In theory this should never happen since we expect
|
||||
/// module names to have been normalized by the time we
|
||||
/// call `match_sources`. But it is worth noting that the
|
||||
/// behavior is different depending on the [`MatchSourceStrategy`]
|
||||
/// call `match_sources`.
|
||||
#[test]
|
||||
fn test_empty_module_name() {
|
||||
let temp_dir = tempdir().unwrap();
|
||||
|
|
@ -894,16 +708,9 @@ mod tests {
|
|||
|
||||
create_dir(project_dir.join("mypackage"));
|
||||
|
||||
let paths = vec![project_dir.clone()];
|
||||
let paths = vec![project_dir];
|
||||
|
||||
assert_eq!(
|
||||
match_sources(&paths, "", MatchSourceStrategy::Root),
|
||||
Some(project_dir.as_path())
|
||||
);
|
||||
assert_eq!(
|
||||
match_sources(&paths, "", MatchSourceStrategy::FullPath),
|
||||
None
|
||||
);
|
||||
assert_eq!(match_sources(&paths, ""), None);
|
||||
}
|
||||
|
||||
/// Tests behavior with an empty list of source paths
|
||||
|
|
@ -911,14 +718,6 @@ mod tests {
|
|||
fn test_empty_paths() {
|
||||
let paths: Vec<PathBuf> = vec![];
|
||||
|
||||
// Empty paths should return None
|
||||
assert_eq!(
|
||||
match_sources(&paths, "mypackage", MatchSourceStrategy::Root),
|
||||
None
|
||||
);
|
||||
assert_eq!(
|
||||
match_sources(&paths, "mypackage", MatchSourceStrategy::FullPath),
|
||||
None
|
||||
);
|
||||
assert_eq!(match_sources(&paths, "mypackage"), None);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,8 +5,8 @@ use std::path::PathBuf;
|
|||
use annotate::annotate_imports;
|
||||
use block::{Block, Trailer};
|
||||
pub(crate) use categorize::categorize;
|
||||
use categorize::categorize_imports;
|
||||
pub use categorize::{ImportSection, ImportType};
|
||||
use categorize::{MatchSourceStrategy, categorize_imports};
|
||||
use comments::Comment;
|
||||
use normalize::normalize_imports;
|
||||
use order::order_imports;
|
||||
|
|
@ -76,7 +76,6 @@ pub(crate) fn format_imports(
|
|||
source_type: PySourceType,
|
||||
target_version: PythonVersion,
|
||||
settings: &Settings,
|
||||
match_source_strategy: MatchSourceStrategy,
|
||||
tokens: &Tokens,
|
||||
) -> String {
|
||||
let trailer = &block.trailer;
|
||||
|
|
@ -104,7 +103,6 @@ pub(crate) fn format_imports(
|
|||
package,
|
||||
target_version,
|
||||
settings,
|
||||
match_source_strategy,
|
||||
);
|
||||
|
||||
if !block_output.is_empty() && !output.is_empty() {
|
||||
|
|
@ -161,7 +159,6 @@ fn format_import_block(
|
|||
package: Option<PackageRoot<'_>>,
|
||||
target_version: PythonVersion,
|
||||
settings: &Settings,
|
||||
match_source_strategy: MatchSourceStrategy,
|
||||
) -> String {
|
||||
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
|
||||
enum LineInsertion {
|
||||
|
|
@ -182,7 +179,6 @@ fn format_import_block(
|
|||
settings.no_sections,
|
||||
&settings.section_order,
|
||||
&settings.default_section,
|
||||
match_source_strategy,
|
||||
);
|
||||
|
||||
let mut output = String::new();
|
||||
|
|
|
|||
|
|
@ -14,9 +14,7 @@ use crate::Locator;
|
|||
use crate::checkers::ast::LintContext;
|
||||
use crate::line_width::LineWidthBuilder;
|
||||
use crate::package::PackageRoot;
|
||||
use crate::preview::is_full_path_match_source_strategy_enabled;
|
||||
use crate::rules::isort::block::Block;
|
||||
use crate::rules::isort::categorize::MatchSourceStrategy;
|
||||
use crate::rules::isort::{comments, format_imports};
|
||||
use crate::settings::LinterSettings;
|
||||
use crate::{Edit, Fix, FixAvailability, Violation};
|
||||
|
|
@ -40,12 +38,6 @@ use crate::{Edit, Fix, FixAvailability, Violation};
|
|||
/// import pandas
|
||||
/// ```
|
||||
///
|
||||
/// ## Preview
|
||||
/// When [`preview`](https://docs.astral.sh/ruff/preview/) mode is enabled, Ruff applies a stricter criterion
|
||||
/// for determining whether an import should be classified as first-party.
|
||||
/// Specifically, for an import of the form `import foo.bar.baz`, Ruff will
|
||||
/// check that `foo/bar`, relative to a [user-specified `src`](https://docs.astral.sh/ruff/settings/#src) directory, contains either
|
||||
/// the directory `baz` or else a file with the name `baz.py` or `baz.pyi`.
|
||||
#[derive(ViolationMetadata)]
|
||||
pub(crate) struct UnsortedImports;
|
||||
|
||||
|
|
@ -129,12 +121,6 @@ pub(crate) fn organize_imports(
|
|||
trailing_lines_end(block.imports.last().unwrap(), locator.contents())
|
||||
};
|
||||
|
||||
let match_source_strategy = if is_full_path_match_source_strategy_enabled(settings) {
|
||||
MatchSourceStrategy::FullPath
|
||||
} else {
|
||||
MatchSourceStrategy::Root
|
||||
};
|
||||
|
||||
// Generate the sorted import block.
|
||||
let expected = format_imports(
|
||||
block,
|
||||
|
|
@ -148,7 +134,6 @@ pub(crate) fn organize_imports(
|
|||
source_type,
|
||||
target_version,
|
||||
&settings.isort,
|
||||
match_source_strategy,
|
||||
tokens,
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -15,11 +15,8 @@ use ruff_text_size::{Ranged, TextRange};
|
|||
|
||||
use crate::checkers::ast::Checker;
|
||||
use crate::fix;
|
||||
use crate::preview::{
|
||||
is_dunder_init_fix_unused_import_enabled, is_full_path_match_source_strategy_enabled,
|
||||
};
|
||||
use crate::preview::is_dunder_init_fix_unused_import_enabled;
|
||||
use crate::registry::Rule;
|
||||
use crate::rules::isort::categorize::MatchSourceStrategy;
|
||||
use crate::rules::{isort, isort::ImportSection, isort::ImportType};
|
||||
use crate::{Applicability, Fix, FixAvailability, Violation};
|
||||
|
||||
|
|
@ -63,6 +60,10 @@ use crate::{Applicability, Fix, FixAvailability, Violation};
|
|||
/// to remove third-party and standard library imports -- the fix is unsafe because the module's
|
||||
/// interface changes.
|
||||
///
|
||||
/// See [this FAQ section](https://docs.astral.sh/ruff/faq/#how-does-ruff-determine-which-of-my-imports-are-first-party-third-party-etc)
|
||||
/// for more details on how Ruff
|
||||
/// determines whether an import is first or third-party.
|
||||
///
|
||||
/// ## Example
|
||||
///
|
||||
/// ```python
|
||||
|
|
@ -91,11 +92,6 @@ use crate::{Applicability, Fix, FixAvailability, Violation};
|
|||
/// print("numpy is not installed")
|
||||
/// ```
|
||||
///
|
||||
/// ## Preview
|
||||
/// When [preview](https://docs.astral.sh/ruff/preview/) is enabled,
|
||||
/// the criterion for determining whether an import is first-party
|
||||
/// is stricter, which could affect the suggested fix. See [this FAQ section](https://docs.astral.sh/ruff/faq/#how-does-ruff-determine-which-of-my-imports-are-first-party-third-party-etc) for more details.
|
||||
///
|
||||
/// ## Options
|
||||
/// - `lint.ignore-init-module-imports`
|
||||
/// - `lint.pyflakes.allowed-unused-imports`
|
||||
|
|
@ -231,11 +227,6 @@ enum UnusedImportContext {
|
|||
|
||||
fn is_first_party(import: &AnyImport, checker: &Checker) -> bool {
|
||||
let source_name = import.source_name().join(".");
|
||||
let match_source_strategy = if is_full_path_match_source_strategy_enabled(checker.settings()) {
|
||||
MatchSourceStrategy::FullPath
|
||||
} else {
|
||||
MatchSourceStrategy::Root
|
||||
};
|
||||
let category = isort::categorize(
|
||||
&source_name,
|
||||
import.qualified_name().is_unresolved_import(),
|
||||
|
|
@ -247,7 +238,6 @@ fn is_first_party(import: &AnyImport, checker: &Checker) -> bool {
|
|||
checker.settings().isort.no_sections,
|
||||
&checker.settings().isort.section_order,
|
||||
&checker.settings().isort.default_section,
|
||||
match_source_strategy,
|
||||
);
|
||||
matches! {
|
||||
category,
|
||||
|
|
|
|||
|
|
@ -310,8 +310,7 @@ my_project
|
|||
When Ruff sees an import like `import foo`, it will then iterate over the `src` directories,
|
||||
looking for a corresponding Python module (in reality, a directory named `foo` or a file named
|
||||
`foo.py`). For module paths with multiple components like `import foo.bar`,
|
||||
the default behavior is to search only for a directory named `foo` or a file
|
||||
named `foo.py`. However, if `preview` is enabled, Ruff will require that the full relative path `foo/bar` exists as a directory, or that `foo/bar.py` or `foo/bar.pyi` exist as files. Finally, imports of the form `from foo import bar`, Ruff will only use `foo` when determining whether a module is first-party or third-party.
|
||||
Ruff will require that the full relative path `foo/bar` exists as a directory, or that `foo/bar.py` or `foo/bar.pyi` exist as files. Finally, for imports of the form `from foo import bar`, Ruff will only use `foo` when determining whether a module is first-party or third-party.
|
||||
|
||||
If there is a directory
|
||||
whose name matches a third-party package, but does not contain Python code,
|
||||
|
|
|
|||
Loading…
Reference in New Issue