Skip to content

Commit 965a4dd

Browse files
authored
[isort] Check full module path against project root(s) when categorizing first-party (#16565)
When attempting to determine whether `import foo.bar.baz` is a known first-party import relative to [user-provided source paths](https://docs.astral.sh/ruff/settings/#src), when `preview` is enabled we now check that `SRC/foo/bar/baz` is a directory or `SRC/foo/bar/baz.py` or `SRC/foo/bar/baz.pyi` exist. Previously, we just checked the analogous thing for `SRC/foo`, but this can be misleading in situations with disjoint namespace packages that share a common base name (e.g. we may be working inside the namespace package `foo.buzz` and importing `foo.bar` from elsewhere). Supersedes #12987 Closes #12984
1 parent 5e2c818 commit 965a4dd

12 files changed

Lines changed: 652 additions & 21 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/ruff/tests/snapshots/integration_test__rule_f401.snap

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ info:
55
args:
66
- rule
77
- F401
8-
snapshot_kind: text
98
---
109
success: true
1110
exit_code: 0
@@ -84,6 +83,11 @@ else:
8483
print("numpy is not installed")
8584
```
8685

86+
## Preview
87+
When [preview](https://docs.astral.sh/ruff/preview/) is enabled,
88+
the criterion for determining whether an import is first-party
89+
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.
90+
8791
## Options
8892
- `lint.ignore-init-module-imports`
8993
- `lint.pyflakes.allowed-unused-imports`

crates/ruff_linter/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ insta = { workspace = true, features = ["filters", "json", "redactions"] }
7676
test-case = { workspace = true }
7777
# Disable colored output in tests
7878
colored = { workspace = true, features = ["no-color"] }
79+
tempfile = { workspace = true }
7980

8081
[features]
8182
default = []

crates/ruff_linter/src/message/snapshots/ruff_linter__message__sarif__tests__results.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ expression: value
8181
"rules": [
8282
{
8383
"fullDescription": {
84-
"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## 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/source/libraries.html#library-interface-public-and-private-symbols)\n"
84+
"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/source/libraries.html#library-interface-public-and-private-symbols)\n"
8585
},
8686
"help": {
8787
"text": "`{name}` imported but unused; consider using `importlib.util.find_spec` to test for availability"

crates/ruff_linter/src/preview.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ pub(crate) const fn is_py314_support_enabled(settings: &LinterSettings) -> bool
2222
settings.preview.is_enabled()
2323
}
2424

25+
// https://github.com/astral-sh/ruff/pull/16565
26+
pub(crate) const fn is_full_path_match_source_strategy_enabled(settings: &LinterSettings) -> bool {
27+
settings.preview.is_enabled()
28+
}
29+
2530
// Rule-specific behavior
2631

2732
// https://github.com/astral-sh/ruff/pull/17136

crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,12 @@ use crate::checkers::ast::Checker;
1212
use crate::codes::Rule;
1313
use crate::fix;
1414
use crate::importer::ImportedMembers;
15+
use crate::preview::is_full_path_match_source_strategy_enabled;
1516
use crate::rules::flake8_type_checking::helpers::{
1617
filter_contained, is_typing_reference, quote_annotation,
1718
};
1819
use crate::rules::flake8_type_checking::imports::ImportBinding;
20+
use crate::rules::isort::categorize::MatchSourceStrategy;
1921
use crate::rules::isort::{categorize, ImportSection, ImportType};
2022

2123
/// ## What it does
@@ -63,6 +65,12 @@ use crate::rules::isort::{categorize, ImportSection, ImportType};
6365
/// return len(sized)
6466
/// ```
6567
///
68+
///
69+
/// ## Preview
70+
/// When [preview](https://docs.astral.sh/ruff/preview/) is enabled,
71+
/// the criterion for determining whether an import is first-party
72+
/// 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.
73+
///
6674
/// ## Options
6775
/// - `lint.flake8-type-checking.quote-annotations`
6876
/// - `lint.flake8-type-checking.runtime-evaluated-base-classes`
@@ -138,6 +146,11 @@ impl Violation for TypingOnlyFirstPartyImport {
138146
/// return len(df)
139147
/// ```
140148
///
149+
/// ## Preview
150+
/// When [preview](https://docs.astral.sh/ruff/preview/) is enabled,
151+
/// the criterion for determining whether an import is first-party
152+
/// 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.
153+
///
141154
/// ## Options
142155
/// - `lint.flake8-type-checking.quote-annotations`
143156
/// - `lint.flake8-type-checking.runtime-evaluated-base-classes`
@@ -299,9 +312,18 @@ pub(crate) fn typing_only_runtime_import(
299312
continue;
300313
}
301314

315+
let source_name = import.source_name().join(".");
316+
302317
// Categorize the import, using coarse-grained categorization.
318+
let match_source_strategy =
319+
if is_full_path_match_source_strategy_enabled(checker.settings) {
320+
MatchSourceStrategy::FullPath
321+
} else {
322+
MatchSourceStrategy::Root
323+
};
324+
303325
let import_type = match categorize(
304-
&qualified_name.to_string(),
326+
&source_name,
305327
qualified_name.is_unresolved_import(),
306328
&checker.settings.src,
307329
checker.package(),
@@ -311,6 +333,7 @@ pub(crate) fn typing_only_runtime_import(
311333
checker.settings.isort.no_sections,
312334
&checker.settings.isort.section_order,
313335
&checker.settings.isort.default_section,
336+
match_source_strategy,
314337
) {
315338
ImportSection::Known(ImportType::LocalFolder | ImportType::FirstParty) => {
316339
ImportType::FirstParty

0 commit comments

Comments
 (0)