astral-sh / ruff

An extremely fast Python linter and code formatter, written in Rust.
https://docs.astral.sh/ruff
MIT License
30.94k stars 1.02k forks source link

Third-party namespace packages can be incorrectly considered first-party packages by `isort` rules #12984

Open tehkirill opened 3 weeks ago

tehkirill commented 3 weeks ago

Hi, team. Thanks for your hard work.

I noticed a change in how known-first-party imports get classified recently. If I'm working on my_namespace.a project, I expect my_namespace.b to be classified as a Known(ThirdParty) (so does isort). This was the case with ruff as well, up until version 0.5.7 .

Working on my_namespace.a project, with the following stucture:

ruff-namespace-src-imports-test/
  ├─ main.py
  ├─ ruff.toml
  └─ src/
     └─ my_namespace/
        └─ a/
           └─ ...
# ruff.toml
[lint.isort]
known-first-party = ["my_namespace.a"]
# main.py
import os

import my_namespace.b
import numpy

import my_namespace.a

^ I see it as a valid import order, so does v0.5.6:

$ python3.12 -m ruff check main.py --select I --diff -v
[2024-08-19][11:11:40][ruff::resolve][DEBUG] Using configuration file (via parent) at: C:\ruff-namespace-src-imports-test\ruff.toml
[2024-08-19][11:11:40][ruff_workspace::pyproject][DEBUG] `project.requires_python` in `pyproject.toml` will not be used to set `target_version` when using `ruff.toml`.
[2024-08-19][11:11:40][ruff::commands::check][DEBUG] Identified files to lint in: 4.5661ms
[2024-08-19][11:11:40][ruff::diagnostics][DEBUG] Checking: C:\ruff-namespace-src-imports-test\main.py
[2024-08-19][11:11:40][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'my_namespace.a' as Known(FirstParty) (KnownFirstParty)
[2024-08-19][11:11:40][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'os' as Known(StandardLibrary) (KnownStandardLibrary)
[2024-08-19][11:11:40][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'my_namespace.b' as Known(ThirdParty) (NoMatch)
[2024-08-19][11:11:40][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'numpy' as Known(ThirdParty) (NoMatch)
[2024-08-19][11:11:40][ruff::commands::check][DEBUG] Checked 1 files in: 2.7995ms

But 0.5.7+ (including the latest 0.6.1) sees otherwise:

$ python3.12 -m ruff check main.py --select I --diff -v
[2024-08-19][11:12:44][ruff::resolve][DEBUG] Using configuration file (via parent) at: C:\ruff-namespace-src-imports-test\ruff.toml
[2024-08-19][11:12:44][ruff_workspace::pyproject][DEBUG] `project.requires_python` in `pyproject.toml` will not be used to set `target_version` when using `ruff.toml`.
[2024-08-19][11:12:44][ruff::commands::check][DEBUG] Identified files to lint in: 4.8756ms
[2024-08-19][11:12:44][ruff::diagnostics][DEBUG] Checking: C:\ruff-namespace-src-imports-test\main.py
[2024-08-19][11:12:44][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'my_namespace.a' as Known(FirstParty) (KnownFirstParty)
[2024-08-19][11:12:44][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'os' as Known(StandardLibrary) (KnownStandardLibrary)
[2024-08-19][11:12:44][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'my_namespace.b' as Known(FirstParty) (SourceMatch("C:\\ruff-namespace-src-imports-test\\src"))
[2024-08-19][11:12:44][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'numpy' as Known(ThirdParty) (NoMatch)
[2024-08-19][11:12:44][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'my_namespace.b' as Known(FirstParty) (SourceMatch("C:\\ruff-namespace-src-imports-test\\src"))
[2024-08-19][11:12:44][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'os' as Known(StandardLibrary) (KnownStandardLibrary)
[2024-08-19][11:12:44][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'numpy' as Known(ThirdParty) (NoMatch)
[2024-08-19][11:12:44][ruff_linter::rules::isort::categorize][DEBUG] Categorized 'my_namespace.a' as Known(FirstParty) (KnownFirstParty)
--- main.py
+++ main.py
@@ -1,6 +1,6 @@
 import os

-import my_namespace.b
 import numpy

 import my_namespace.a
+import my_namespace.b

[2024-08-19][11:12:44][ruff::commands::check][DEBUG] Checked 1 files in: 2.8168ms
Would fix 1 error.

Removing src/my_namespace correctly classifies my_namespace.b as known-third-party again.

MichaReiser commented 3 weeks ago

Hi @tehkirill

This behavior changed with Ruff 0.6 where the directory src is now included in the src setting. You can get back the old behavior by setting src = ["."] which was the default until Ruff 0.6.

You can read more about it in our blog post.

tehkirill commented 3 weeks ago

Thanks for the prompt response and linking to where it was documented, @MichaReiser In my case - I am using src/ layout - don't think I should exclude it to make this work.

The way it's implemented at the moment makes sense for projects without namespaces, eg ./src/requests from requests repo.

It does, however, incorrectly identifies a third-party import under the same namespace as first-party, e.g. using an example from packaging.python.org:

mynamespace-subpackage-a/
    pyproject.toml
    src/
        mynamespace/
            subpackage_a/
                __init__.py

mynamespace-subpackage-b/
    pyproject.toml
    src/
        mynamespace/
            subpackage_b/
                __init__.py
            module_b.py

When working on mynamespace-subpackage-a, mynamespace-subpackage-b would be identified as first-party (in fact, anything starting with mynamespace. would), I don't think it's an expected behaviour.

AlexWaygood commented 3 weeks ago

When working on mynamespace-subpackage-a, mynamespace-subpackage-b would be identified as first-party (in fact, anything starting with mynamespace. would), I don't think it's an expected behaviour.

That's an interesting edge case! This seems to me to be a general issue with the way our isort rules detect first-party packages when it comes to namespace packages, that has been exposed in your project by the recent change to isort's configuration defaults, rather than an issue with the change in defaults itself.