Skip to content

[sqlalchemy] Implement first SQLAlchemy rule (SA201)#18065

Open
kreathon wants to merge 2 commits into
astral-sh:mainfrom
kreathon:add-missing-mapped-type-annotation-rule
Open

[sqlalchemy] Implement first SQLAlchemy rule (SA201)#18065
kreathon wants to merge 2 commits into
astral-sh:mainfrom
kreathon:add-missing-mapped-type-annotation-rule

Conversation

@kreathon

@kreathon kreathon commented May 13, 2025

Copy link
Copy Markdown

Summary

Due to the interest in #16864 in the rules I proposed in miketheman/flake8-sqlalchemy#54 (but were never implemented), I created this draft to add the first SQLAlchemy rule.

See changes in crates/ruff_linter/src/rules/sqlalchemy/rules/missing_mapped_type_annotation.rs for the motivation of this rule.

Implementation and Limitations

Currently, it supports most commonly used column mappings (mapped_column, relationship, ...), but still lacking support for e.g. composite.

Auto fixes might be possible in some cases, but can be tricky because it would require an understanding of type_annotation_map.

Test Plan

SA001.py snapshot test was added.

@MichaReiser MichaReiser added needs-decision Awaiting a decision from a maintainer rule Implementing or modifying a lint rule labels May 13, 2025
@kreathon

Copy link
Copy Markdown
Author

Here is a little bit more background:

Docs

The mapped_column() construct in modern Python is normally augmented by the use of PEP 484 Python type annotations, where it is capable of deriving its column-configuration information from type annotations associated with the attribute as declared in the Declarative mapped class. These type annotations, if used, must be present within a special SQLAlchemy type called Mapped, which is a generic type that indicates a specific Python type within it.

-- https://docs.sqlalchemy.org/en/20/orm/declarative_tables.html#orm-annotated-declarative-automated-mapping-with-type-annotations

Both forms are used in the examples ( tagged as Annotated Example and Non-Annotated Example).

The Non-Annotated form is also a natural migration step to SQLAlchemy 2.0:

Users of 1.x SQLAlchemy will note the use of the mapped_column() construct, which is new as of the SQLAlchemy 2.0 series. This ORM-specific construct is intended first and foremost to be a drop-in replacement for the use of Column within Declarative mappings only, adding new ORM-specific convenience features such as the ability to establish mapped_column.deferred within the construct, and most importantly to indicate to typing tools such as Mypy and Pylance an accurate representation of how the attribute will behave at runtime at both the class level as well as the instance level. As will be seen in the following sections, it’s also at the forefront of a new annotation-driven configuration style introduced in SQLAlchemy 2.0.

-- https://docs.sqlalchemy.org/en/20/orm/declarative_tables.html#orm-annotated-declarative-automated-mapping-with-type-annotations

Personal Background

In a codebase I am working with, in some places the annotations were missing (for whatever reason). We were not aware that these columns were treated as Any causing some bugs in the past.

@Daverball

Copy link
Copy Markdown
Contributor

I'd suggest collecting all the SQAlchemy 2.0 specific rules into the SA2 prefix, so making this SA201 rather than SA001. That way we leave the door open to put generally useful rules into SA0 and legacy rules into SA1, making it easier to read through the rules once there are more of them.

@kreathon kreathon changed the title [sqlalchemy] Implement first SQLAlchemy rule (SA001) [sqlalchemy] Implement first SQLAlchemy rule (SA201) May 15, 2025
@nidhishgajjar

This comment was marked as spam.

1 similar comment
@nidhishgajjar

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-decision Awaiting a decision from a maintainer rule Implementing or modifying a lint rule

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants