Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

F821 false-positive for SQLAlchemy mappings with not yet declared classes #10451

Closed
aberres opened this issue Mar 18, 2024 · 14 comments · Fixed by #10513
Closed

F821 false-positive for SQLAlchemy mappings with not yet declared classes #10451

aberres opened this issue Mar 18, 2024 · 14 comments · Fixed by #10513
Labels
bug Something isn't working linter Related to the linter rule Implementing or modifying a lint rule

Comments

@aberres
Copy link

aberres commented Mar 18, 2024

The described behavior happens since 0.3.3. It might be related to #10340

It is triggered throughout my SQLAlchemy models in cases where a model is not defined yet while used in a type annotation

Minimal repro:

from __future__ import annotations

from sqlalchemy.orm import DeclarativeBase, Mapped


class Base(DeclarativeBase):
    some_mapping: Mapped[list[Bar]] | None = None # F821 Undefined name `Bar`
    simplified: list[Bar] | None = None # F821 Undefined name `Bar`


class Bar:
    pass

I can only trigger the error when inheriting from DeclarativeBase. Without this inheritance or when inheriting from another class, things are fine.

@AlexWaygood AlexWaygood added rule Implementing or modifying a lint rule linter Related to the linter labels Mar 18, 2024
@AlexWaygood
Copy link
Member

Hi! I can't reproduce your error locally or in the Ruff playground: https://play.ruff.rs/1f9ab24b-394d-46c3-923a-ecf866705693

Can you paste a snippet that reproduces the error in the Ruff playground?

@aberres
Copy link
Author

aberres commented Mar 18, 2024

Does the playground have a real installation of SQLAlchemy?

I can only reproduce it locally with the actual sqlalchemy.orm.DeclarativeBase. Other classes as a base class are fine.

@AlexWaygood
Copy link
Member

AlexWaygood commented Mar 18, 2024

I don't think it should matter from Ruff's perspective whether sqlalchemy is installed into a Python environment or not, but let me check that :-)

@AlexWaygood
Copy link
Member

AlexWaygood commented Mar 18, 2024

I can't reproduce locally with SQLAlchemy installed either.

Does it still reproduce for you after running ruff clean?

@carl-baillargeon
Copy link

We are also hitting this issue in this project: https://github.com/arista-netdevops-community/anta

If you install the project locally with 0.3.3 you should see a bunch of F821 errors:

(.anta-dev) ~/git_projects/anta (main ✗) ruff check 
anta/models.py:273:27: F821 Undefined name `ResultOverwrite`
anta/models.py:274:18: F821 Undefined name `Filters`
anta/tests/bfd.py:40:25: F821 Undefined name `BFDPeer`
anta/tests/bfd.py:93:25: F821 Undefined name `BFDPeer`
anta/tests/connectivity.py:35:21: F821 Undefined name `Host`
anta/tests/connectivity.py:94:25: F821 Undefined name `Neighbor`
anta/tests/interfaces.py:176:26: F821 Undefined name `InterfaceState`
anta/tests/interfaces.py:535:26: F821 Undefined name `InterfaceDetail`
anta/tests/routing/bgp.py:175:32: F821 Undefined name `BgpAfi`
anta/tests/routing/bgp.py:285:32: F821 Undefined name `BgpAfi`
anta/tests/routing/bgp.py:397:32: F821 Undefined name `BgpAfi`
anta/tests/routing/bgp.py:509:25: F821 Undefined name `BgpNeighbor`
anta/tests/routing/bgp.py:581:25: F821 Undefined name `BgpPeer`
anta/tests/routing/bgp.py:653:25: F821 Undefined name `BgpPeer`
anta/tests/routing/bgp.py:719:25: F821 Undefined name `BgpPeer`
anta/tests/routing/bgp.py:785:25: F821 Undefined name `BgpPeer`
anta/tests/routing/bgp.py:846:31: F821 Undefined name `VxlanEndpoint`
anta/tests/routing/bgp.py:909:25: F821 Undefined name `BgpPeer`
anta/tests/routing/bgp.py:968:25: F821 Undefined name `BgpPeer`
anta/tests/security.py:303:28: F821 Undefined name `APISSLCertificate`
anta/tests/security.py:462:33: F821 Undefined name `IPv4ACL`
anta/tests/security.py:471:27: F821 Undefined name `IPv4ACLEntry`
anta/tests/services.py:106:27: F821 Undefined name `DnsServer`
anta/tests/services.py:159:23: F821 Undefined name `ErrDisableReason`
Found 24 errors.

Thanks

@charliermarsh
Copy link
Member

I also can't reproduce this in the playground, but I can reproduce by running cargo run check anta/anta/models.py --select F821 -n locally.

@charliermarsh
Copy link
Member

(I can't reproduce by running locally over the originating snippet.)

@charliermarsh
Copy link
Member

This reproduces:

from __future__ import annotations

from pydantic import BaseModel

 
class Input(BaseModel):

    result_overwrite: ResultOverwrite | None = None
    filters: Filters | None = None

    class ResultOverwrite(BaseModel):
        ...

    class Filters(BaseModel):
        ...

@charliermarsh
Copy link
Member

The original snippet doesn't reproduce because DeclarativeBase needs to be declared as runtime-required. Now it reproduces: https://play.ruff.rs/825f89f6-d49c-4688-bfe7-6866319fa764

@AlexWaygood
Copy link
Member

Okay, so lint.flake8-type-checking.runtime-evaluated-base-classes needs to be set to ["sqlalchemy.orm.DeclarativeBase"] in order for it to reproduce

@carl-baillargeon
Copy link

This reproduces:

from __future__ import annotations

from pydantic import BaseModel

 
class Input(BaseModel):

    result_overwrite: ResultOverwrite | None = None
    filters: Filters | None = None

    class ResultOverwrite(BaseModel):
        ...

    class Filters(BaseModel):
        ...

Should I open a separate issue to track it?

@aberres
Copy link
Author

aberres commented Mar 19, 2024

@carl-baillargeon Should be the same issue due to pydantic.BaseModel being listed in lint.flake8-type-checking.runtime-evaluated-base-classes.

@AlexWaygood
Copy link
Member

Confirmed that this (unsurprisingly) bisects to 704fefc. Sorry for the regression!

@zanieb zanieb added the bug Something isn't working label Mar 20, 2024
@insilications
Copy link

@carl-baillargeon Should be the same issue due to pydantic.BaseModel being listed in lint.flake8-type-checking.runtime-evaluated-base-classes.

Confirmed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working linter Related to the linter rule Implementing or modifying a lint rule
Projects
None yet
6 participants