mirror of https://github.com/astral-sh/ruff
Merge 5145f2f0bb into b0bc990cbf
This commit is contained in:
commit
2226487f2f
|
|
@ -0,0 +1,71 @@
|
||||||
|
from typing import List
|
||||||
|
|
||||||
|
from sqlalchemy import Integer, ForeignKey, String, func
|
||||||
|
from sqlalchemy.ext.associationproxy import association_proxy, AssociationProxy
|
||||||
|
from sqlalchemy.orm import (
|
||||||
|
Mapped,
|
||||||
|
mapped_column,
|
||||||
|
DeclarativeBase,
|
||||||
|
relationship,
|
||||||
|
column_property,
|
||||||
|
MappedSQLExpression,
|
||||||
|
synonym,
|
||||||
|
Synonym,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class Base(DeclarativeBase):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
class Parent(Base):
|
||||||
|
__tablename__ = "parent"
|
||||||
|
|
||||||
|
id: Mapped[int] = mapped_column(primary_key=True)
|
||||||
|
children: Mapped[List["Child"]] = relationship(back_populates="parent")
|
||||||
|
|
||||||
|
name: Mapped[str]
|
||||||
|
|
||||||
|
parent_name: Synonym[str] = synonym("name")
|
||||||
|
|
||||||
|
|
||||||
|
class Child(Base):
|
||||||
|
__tablename__ = "child"
|
||||||
|
|
||||||
|
id: Mapped[int] = mapped_column(primary_key=True)
|
||||||
|
parent_id: Mapped[int] = mapped_column(ForeignKey("parent.id"))
|
||||||
|
parent: Mapped["Parent"] = relationship(back_populates="children")
|
||||||
|
|
||||||
|
parent_name: AssociationProxy[str] = association_proxy("parent", "name")
|
||||||
|
|
||||||
|
first_name: Mapped[str] = mapped_column(String)
|
||||||
|
last_name: Mapped[str] = mapped_column()
|
||||||
|
|
||||||
|
name_length: MappedSQLExpression[int] = column_property(
|
||||||
|
func.length(first_name + last_name)
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class Company(Base):
|
||||||
|
__tablename__ = "company"
|
||||||
|
|
||||||
|
id = mapped_column(Integer, primary_key=True)
|
||||||
|
employees = relationship("Employee", back_populates="company")
|
||||||
|
|
||||||
|
name = mapped_column(String)
|
||||||
|
company_name = synonym("name")
|
||||||
|
|
||||||
|
|
||||||
|
class Employee(Base):
|
||||||
|
__tablename__ = "employee"
|
||||||
|
|
||||||
|
id = mapped_column(Integer, primary_key=True)
|
||||||
|
company_id = mapped_column(ForeignKey("company.id"))
|
||||||
|
company = relationship("Company", back_populates="employees")
|
||||||
|
|
||||||
|
company_name = association_proxy("company", "name")
|
||||||
|
|
||||||
|
first_name = mapped_column(String)
|
||||||
|
last_name = mapped_column(String)
|
||||||
|
|
||||||
|
name_length = column_property(func.length(first_name + last_name))
|
||||||
|
|
@ -10,7 +10,8 @@ use crate::rules::{
|
||||||
flake8_builtins, flake8_debugger, flake8_django, flake8_errmsg, flake8_import_conventions,
|
flake8_builtins, flake8_debugger, flake8_django, flake8_errmsg, flake8_import_conventions,
|
||||||
flake8_pie, flake8_pyi, flake8_pytest_style, flake8_raise, flake8_return, flake8_simplify,
|
flake8_pie, flake8_pyi, flake8_pytest_style, flake8_raise, flake8_return, flake8_simplify,
|
||||||
flake8_slots, flake8_tidy_imports, flake8_type_checking, mccabe, pandas_vet, pep8_naming,
|
flake8_slots, flake8_tidy_imports, flake8_type_checking, mccabe, pandas_vet, pep8_naming,
|
||||||
perflint, pycodestyle, pyflakes, pygrep_hooks, pylint, pyupgrade, refurb, ruff, tryceratops,
|
perflint, pycodestyle, pyflakes, pygrep_hooks, pylint, pyupgrade, refurb, ruff, sqlalchemy,
|
||||||
|
tryceratops,
|
||||||
};
|
};
|
||||||
use ruff_python_ast::PythonVersion;
|
use ruff_python_ast::PythonVersion;
|
||||||
|
|
||||||
|
|
@ -385,6 +386,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
|
||||||
if checker.is_rule_enabled(Rule::DjangoUnorderedBodyContentInModel) {
|
if checker.is_rule_enabled(Rule::DjangoUnorderedBodyContentInModel) {
|
||||||
flake8_django::rules::unordered_body_content_in_model(checker, class_def);
|
flake8_django::rules::unordered_body_content_in_model(checker, class_def);
|
||||||
}
|
}
|
||||||
|
if checker.enabled(Rule::SQLAlchemyMissingMappedTypeAnnotation) {
|
||||||
|
sqlalchemy::rules::missing_mapped_type_annotation(checker, body);
|
||||||
|
}
|
||||||
if !checker.source_type.is_stub() {
|
if !checker.source_type.is_stub() {
|
||||||
if checker.is_rule_enabled(Rule::DjangoModelWithoutDunderStr) {
|
if checker.is_rule_enabled(Rule::DjangoModelWithoutDunderStr) {
|
||||||
flake8_django::rules::model_without_dunder_str(checker, class_def);
|
flake8_django::rules::model_without_dunder_str(checker, class_def);
|
||||||
|
|
|
||||||
|
|
@ -1189,6 +1189,9 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
|
||||||
(Flake8Logging, "014") => rules::flake8_logging::rules::ExcInfoOutsideExceptHandler,
|
(Flake8Logging, "014") => rules::flake8_logging::rules::ExcInfoOutsideExceptHandler,
|
||||||
(Flake8Logging, "015") => rules::flake8_logging::rules::RootLoggerCall,
|
(Flake8Logging, "015") => rules::flake8_logging::rules::RootLoggerCall,
|
||||||
|
|
||||||
|
// SQLAlchemy
|
||||||
|
(SQLAlchemy, "201") => (RuleGroup::Preview, rules::sqlalchemy::rules::SQLAlchemyMissingMappedTypeAnnotation),
|
||||||
|
|
||||||
_ => return None,
|
_ => return None,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -209,6 +209,9 @@ pub enum Linter {
|
||||||
/// Ruff-specific rules
|
/// Ruff-specific rules
|
||||||
#[prefix = "RUF"]
|
#[prefix = "RUF"]
|
||||||
Ruff,
|
Ruff,
|
||||||
|
/// [SQLAlchemy](https://pypi.org/project/SQLAlchemy/)
|
||||||
|
#[prefix = "SA"]
|
||||||
|
SQLAlchemy,
|
||||||
/// [tryceratops](https://pypi.org/project/tryceratops/)
|
/// [tryceratops](https://pypi.org/project/tryceratops/)
|
||||||
#[prefix = "TRY"]
|
#[prefix = "TRY"]
|
||||||
Tryceratops,
|
Tryceratops,
|
||||||
|
|
|
||||||
|
|
@ -56,4 +56,5 @@ pub mod pylint;
|
||||||
pub mod pyupgrade;
|
pub mod pyupgrade;
|
||||||
pub mod refurb;
|
pub mod refurb;
|
||||||
pub mod ruff;
|
pub mod ruff;
|
||||||
|
pub mod sqlalchemy;
|
||||||
pub mod tryceratops;
|
pub mod tryceratops;
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,22 @@
|
||||||
|
use ruff_python_ast::Expr;
|
||||||
|
use ruff_python_semantic::SemanticModel;
|
||||||
|
|
||||||
|
pub(in crate::rules::sqlalchemy) fn is_mapped_attribute(
|
||||||
|
expr: &Expr,
|
||||||
|
semantic: &SemanticModel,
|
||||||
|
) -> bool {
|
||||||
|
semantic
|
||||||
|
.resolve_qualified_name(expr)
|
||||||
|
.is_some_and(|qualified_name| {
|
||||||
|
[
|
||||||
|
["sqlalchemy", "ext", "associationproxy"],
|
||||||
|
["sqlalchemy", "orm", "column_property"],
|
||||||
|
["sqlalchemy", "orm", "composite"],
|
||||||
|
["sqlalchemy", "orm", "mapped_column"],
|
||||||
|
["sqlalchemy", "orm", "relationship"],
|
||||||
|
["sqlalchemy", "orm", "synonym"],
|
||||||
|
]
|
||||||
|
.iter()
|
||||||
|
.any(|n| qualified_name.segments().starts_with(n))
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,25 @@
|
||||||
|
mod helpers;
|
||||||
|
pub(crate) mod rules;
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use std::path::Path;
|
||||||
|
|
||||||
|
use anyhow::Result;
|
||||||
|
use test_case::test_case;
|
||||||
|
|
||||||
|
use crate::registry::Rule;
|
||||||
|
use crate::test::test_path;
|
||||||
|
use crate::{assert_messages, settings};
|
||||||
|
|
||||||
|
#[test_case(Rule::SQLAlchemyMissingMappedTypeAnnotation, Path::new("SA201.py"))]
|
||||||
|
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
|
||||||
|
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
|
||||||
|
let diagnostics = test_path(
|
||||||
|
Path::new("sqlalchemy").join(path).as_path(),
|
||||||
|
&settings::LinterSettings::for_rule(rule_code),
|
||||||
|
)?;
|
||||||
|
assert_messages!(snapshot, diagnostics);
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,98 @@
|
||||||
|
use ruff_diagnostics::{Diagnostic, FixAvailability, Violation};
|
||||||
|
use ruff_macros::{derive_message_formats, ViolationMetadata};
|
||||||
|
|
||||||
|
use ruff_python_ast::{self as ast, Expr, Stmt};
|
||||||
|
use ruff_text_size::Ranged;
|
||||||
|
|
||||||
|
use crate::checkers::ast::Checker;
|
||||||
|
|
||||||
|
use crate::rules::sqlalchemy::helpers;
|
||||||
|
use ruff_python_semantic::Modules;
|
||||||
|
|
||||||
|
/// ## What it does
|
||||||
|
/// Checks for existence of `Mapped` or other ORM container class type annotations in SQLAlchemy
|
||||||
|
/// models.
|
||||||
|
///
|
||||||
|
/// ## Why is this bad?
|
||||||
|
/// If an annotation is missing, type checkers will treat the corresponding field as type `Any`.
|
||||||
|
///
|
||||||
|
/// ## Example
|
||||||
|
/// ```python
|
||||||
|
/// from sqlalchemy import Integer
|
||||||
|
/// from sqlalchemy.orm import DeclarativeBase
|
||||||
|
/// from sqlalchemy.orm import Mapped
|
||||||
|
/// from sqlalchemy.orm import mapped_column
|
||||||
|
///
|
||||||
|
/// class Base(DeclarativeBase):
|
||||||
|
/// pass
|
||||||
|
///
|
||||||
|
///
|
||||||
|
/// class MyModel(Base):
|
||||||
|
/// __tablename__ = "my_model"
|
||||||
|
/// id: Mapped[int] = mapped_column(primary_key=True)
|
||||||
|
///
|
||||||
|
/// count = mapped_column(Integer)
|
||||||
|
///
|
||||||
|
///
|
||||||
|
/// m = MyModel()
|
||||||
|
/// reveal_type(m.count) # note: Revealed type is "Any"
|
||||||
|
/// ```
|
||||||
|
///
|
||||||
|
/// Use instead:
|
||||||
|
/// ```python
|
||||||
|
/// from sqlalchemy.orm import DeclarativeBase
|
||||||
|
/// from sqlalchemy.orm import Mapped
|
||||||
|
/// from sqlalchemy.orm import mapped_column
|
||||||
|
///
|
||||||
|
/// class Base(DeclarativeBase):
|
||||||
|
/// pass
|
||||||
|
///
|
||||||
|
///
|
||||||
|
/// class MyModel(Base):
|
||||||
|
/// __tablename__ = "my_model"
|
||||||
|
/// id: Mapped[int] = mapped_column(primary_key=True)
|
||||||
|
///
|
||||||
|
/// count: Mapped[int]
|
||||||
|
///
|
||||||
|
///
|
||||||
|
/// m = MyModel()
|
||||||
|
/// reveal_type(m.count) # note: Revealed type is "builtins.int"
|
||||||
|
/// ```
|
||||||
|
#[derive(ViolationMetadata)]
|
||||||
|
pub(crate) struct SQLAlchemyMissingMappedTypeAnnotation;
|
||||||
|
|
||||||
|
impl Violation for SQLAlchemyMissingMappedTypeAnnotation {
|
||||||
|
const FIX_AVAILABILITY: FixAvailability = FixAvailability::None;
|
||||||
|
|
||||||
|
#[derive_message_formats]
|
||||||
|
fn message(&self) -> String {
|
||||||
|
"Missing `Mapped` or other ORM container class type annotation".to_string()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// SA201
|
||||||
|
pub(crate) fn missing_mapped_type_annotation(checker: &mut Checker, body: &[Stmt]) {
|
||||||
|
if !checker.semantic().seen_module(Modules::SQLALCHEMY) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
for statement in body {
|
||||||
|
let Stmt::Assign(ast::StmtAssign { value, targets, .. }) = statement else {
|
||||||
|
continue;
|
||||||
|
};
|
||||||
|
|
||||||
|
if let Expr::Call(ast::ExprCall { func, .. }) = value.as_ref() {
|
||||||
|
if helpers::is_mapped_attribute(func, checker.semantic()) {
|
||||||
|
// SQLAlchemy does not allow multiple targets for column assignments.
|
||||||
|
let [target] = targets.as_slice() else {
|
||||||
|
continue;
|
||||||
|
};
|
||||||
|
|
||||||
|
checker.report_diagnostic(Diagnostic::new(
|
||||||
|
SQLAlchemyMissingMappedTypeAnnotation {},
|
||||||
|
target.range(),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,3 @@
|
||||||
|
pub(crate) use missing_mapped_type_annotation::*;
|
||||||
|
|
||||||
|
mod missing_mapped_type_annotation;
|
||||||
|
|
@ -0,0 +1,100 @@
|
||||||
|
---
|
||||||
|
source: crates/ruff_linter/src/rules/sqlalchemy/mod.rs
|
||||||
|
---
|
||||||
|
SA201.py:52:5: SA201 Missing `Mapped` or other ORM container class type annotation
|
||||||
|
|
|
||||||
|
50 | __tablename__ = "company"
|
||||||
|
51 |
|
||||||
|
52 | id = mapped_column(Integer, primary_key=True)
|
||||||
|
| ^^ SA201
|
||||||
|
53 | employees = relationship("Employee", back_populates="company")
|
||||||
|
|
|
||||||
|
|
||||||
|
SA201.py:53:5: SA201 Missing `Mapped` or other ORM container class type annotation
|
||||||
|
|
|
||||||
|
52 | id = mapped_column(Integer, primary_key=True)
|
||||||
|
53 | employees = relationship("Employee", back_populates="company")
|
||||||
|
| ^^^^^^^^^ SA201
|
||||||
|
54 |
|
||||||
|
55 | name = mapped_column(String)
|
||||||
|
|
|
||||||
|
|
||||||
|
SA201.py:55:5: SA201 Missing `Mapped` or other ORM container class type annotation
|
||||||
|
|
|
||||||
|
53 | employees = relationship("Employee", back_populates="company")
|
||||||
|
54 |
|
||||||
|
55 | name = mapped_column(String)
|
||||||
|
| ^^^^ SA201
|
||||||
|
56 | company_name = synonym("name")
|
||||||
|
|
|
||||||
|
|
||||||
|
SA201.py:56:5: SA201 Missing `Mapped` or other ORM container class type annotation
|
||||||
|
|
|
||||||
|
55 | name = mapped_column(String)
|
||||||
|
56 | company_name = synonym("name")
|
||||||
|
| ^^^^^^^^^^^^ SA201
|
||||||
|
|
|
||||||
|
|
||||||
|
SA201.py:62:5: SA201 Missing `Mapped` or other ORM container class type annotation
|
||||||
|
|
|
||||||
|
60 | __tablename__ = "employee"
|
||||||
|
61 |
|
||||||
|
62 | id = mapped_column(Integer, primary_key=True)
|
||||||
|
| ^^ SA201
|
||||||
|
63 | company_id = mapped_column(ForeignKey("company.id"))
|
||||||
|
64 | company = relationship("Company", back_populates="employees")
|
||||||
|
|
|
||||||
|
|
||||||
|
SA201.py:63:5: SA201 Missing `Mapped` or other ORM container class type annotation
|
||||||
|
|
|
||||||
|
62 | id = mapped_column(Integer, primary_key=True)
|
||||||
|
63 | company_id = mapped_column(ForeignKey("company.id"))
|
||||||
|
| ^^^^^^^^^^ SA201
|
||||||
|
64 | company = relationship("Company", back_populates="employees")
|
||||||
|
|
|
||||||
|
|
||||||
|
SA201.py:64:5: SA201 Missing `Mapped` or other ORM container class type annotation
|
||||||
|
|
|
||||||
|
62 | id = mapped_column(Integer, primary_key=True)
|
||||||
|
63 | company_id = mapped_column(ForeignKey("company.id"))
|
||||||
|
64 | company = relationship("Company", back_populates="employees")
|
||||||
|
| ^^^^^^^ SA201
|
||||||
|
65 |
|
||||||
|
66 | company_name = association_proxy("company", "name")
|
||||||
|
|
|
||||||
|
|
||||||
|
SA201.py:66:5: SA201 Missing `Mapped` or other ORM container class type annotation
|
||||||
|
|
|
||||||
|
64 | company = relationship("Company", back_populates="employees")
|
||||||
|
65 |
|
||||||
|
66 | company_name = association_proxy("company", "name")
|
||||||
|
| ^^^^^^^^^^^^ SA201
|
||||||
|
67 |
|
||||||
|
68 | first_name = mapped_column(String)
|
||||||
|
|
|
||||||
|
|
||||||
|
SA201.py:68:5: SA201 Missing `Mapped` or other ORM container class type annotation
|
||||||
|
|
|
||||||
|
66 | company_name = association_proxy("company", "name")
|
||||||
|
67 |
|
||||||
|
68 | first_name = mapped_column(String)
|
||||||
|
| ^^^^^^^^^^ SA201
|
||||||
|
69 | last_name = mapped_column(String)
|
||||||
|
|
|
||||||
|
|
||||||
|
SA201.py:69:5: SA201 Missing `Mapped` or other ORM container class type annotation
|
||||||
|
|
|
||||||
|
68 | first_name = mapped_column(String)
|
||||||
|
69 | last_name = mapped_column(String)
|
||||||
|
| ^^^^^^^^^ SA201
|
||||||
|
70 |
|
||||||
|
71 | name_length = column_property(func.length(first_name + last_name))
|
||||||
|
|
|
||||||
|
|
||||||
|
SA201.py:71:5: SA201 Missing `Mapped` or other ORM container class type annotation
|
||||||
|
|
|
||||||
|
69 | last_name = mapped_column(String)
|
||||||
|
70 |
|
||||||
|
71 | name_length = column_property(func.length(first_name + last_name))
|
||||||
|
| ^^^^^^^^^^^ SA201
|
||||||
|
|
|
||||||
|
|
@ -1486,6 +1486,7 @@ impl<'a> SemanticModel<'a> {
|
||||||
"airflow" => self.seen.insert(Modules::AIRFLOW),
|
"airflow" => self.seen.insert(Modules::AIRFLOW),
|
||||||
"hashlib" => self.seen.insert(Modules::HASHLIB),
|
"hashlib" => self.seen.insert(Modules::HASHLIB),
|
||||||
"crypt" => self.seen.insert(Modules::CRYPT),
|
"crypt" => self.seen.insert(Modules::CRYPT),
|
||||||
|
"sqlalchemy" => self.seen.insert(Modules::SQLALCHEMY),
|
||||||
_ => {}
|
_ => {}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -2254,6 +2255,7 @@ bitflags! {
|
||||||
const AIRFLOW = 1 << 27;
|
const AIRFLOW = 1 << 27;
|
||||||
const HASHLIB = 1 << 28;
|
const HASHLIB = 1 << 28;
|
||||||
const CRYPT = 1 << 29;
|
const CRYPT = 1 << 29;
|
||||||
|
const SQLALCHEMY = 1 << 30;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -4144,6 +4144,10 @@
|
||||||
"S701",
|
"S701",
|
||||||
"S702",
|
"S702",
|
||||||
"S704",
|
"S704",
|
||||||
|
"SA",
|
||||||
|
"SA2",
|
||||||
|
"SA20",
|
||||||
|
"SA201",
|
||||||
"SIM",
|
"SIM",
|
||||||
"SIM1",
|
"SIM1",
|
||||||
"SIM10",
|
"SIM10",
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue