diff --git a/crates/ruff/tests/snapshots/integration_test__rule_f401.snap b/crates/ruff/tests/snapshots/integration_test__rule_f401.snap index 20e77e7894..55c35568cd 100644 --- a/crates/ruff/tests/snapshots/integration_test__rule_f401.snap +++ b/crates/ruff/tests/snapshots/integration_test__rule_f401.snap @@ -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` diff --git a/crates/ruff/tests/snapshots/lint__output_format_sarif.snap b/crates/ruff/tests/snapshots/lint__output_format_sarif.snap index 28f7438945..9eba120fae 100644 --- a/crates/ruff/tests/snapshots/lint__output_format_sarif.snap +++ b/crates/ruff/tests/snapshots/lint__output_format_sarif.snap @@ -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" diff --git a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__sarif__tests__results.snap b/crates/ruff_linter/src/message/snapshots/ruff_linter__message__sarif__tests__results.snap index 6bb1cc667f..6465d94a7c 100644 --- a/crates/ruff_linter/src/message/snapshots/ruff_linter__message__sarif__tests__results.snap +++ b/crates/ruff_linter/src/message/snapshots/ruff_linter__message__sarif__tests__results.snap @@ -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" diff --git a/crates/ruff_linter/src/preview.rs b/crates/ruff_linter/src/preview.rs index c734b0f960..3eeed95e27 100644 --- a/crates/ruff_linter/src/preview.rs +++ b/crates/ruff_linter/src/preview.rs @@ -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 diff --git a/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs b/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs index 9789ce417d..1d216a6680 100644 --- a/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs +++ b/crates/ruff_linter/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs @@ -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 diff --git a/crates/ruff_linter/src/rules/isort/categorize.rs b/crates/ruff_linter/src/rules/isort/categorize.rs index 7486cdbb0d..a6af5b4ca2 100644 --- a/crates/ruff_linter/src/rules/isort/categorize.rs +++ b/crates/ruff_linter/src/rules/isort/categorize.rs @@ -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>, 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 = 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); } } diff --git a/crates/ruff_linter/src/rules/isort/mod.rs b/crates/ruff_linter/src/rules/isort/mod.rs index 6f18ce05b8..7368088f36 100644 --- a/crates/ruff_linter/src/rules/isort/mod.rs +++ b/crates/ruff_linter/src/rules/isort/mod.rs @@ -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>, 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(); 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 4f379b503a..a8d5d68ffd 100644 --- a/crates/ruff_linter/src/rules/isort/rules/organize_imports.rs +++ b/crates/ruff_linter/src/rules/isort/rules/organize_imports.rs @@ -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, ); diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs index 130c614ae7..9f73bb3b97 100644 --- a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs +++ b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs @@ -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, diff --git a/docs/faq.md b/docs/faq.md index c4b358b092..68e69ee9bd 100644 --- a/docs/faq.md +++ b/docs/faq.md @@ -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,