]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
Fix typing for `hybrid_property.__set__` to properly validate setter values
authorMicah Denbraver <micah.denbraver@whatnot.com>
Wed, 20 Aug 2025 20:58:02 +0000 (16:58 -0400)
committerFederico Caselli <cfederico87@gmail.com>
Wed, 20 Aug 2025 21:09:00 +0000 (23:09 +0200)
While iterating on some typing improvements, my colleague @seamuswn pointed out mypy wasn't catching when values with invalid types were set using a `hybrid_property` setter. I believe this is all that's needed to fix the typing.

### Description
Adjust `hybrid_property.__set__` to expect a value of the type that matches the generic's type variable.

### Checklist
<!-- go over following points. check them with an `x` if they do apply, (they turn into clickable checkboxes once the PR is submitted, so no need to do everything at once)

-->

This pull request is:

- [x] A documentation / typographical / small typing error fix
- Good to go, no issue or tests are needed
- [ ] A short code fix
- please include the issue number, and create an issue if none exists, which
  must include a complete example of the issue.  one line code fixes without an
  issue and demonstration will not be accepted.
- Please include: `Fixes: #<issue number>` in the commit message
- please include tests.   one line code fixes without tests will not be accepted.
- [ ] A new feature implementation
- please include the issue number, and create an issue if none exists, which must
  include a complete example of how the feature would look.
- Please include: `Fixes: #<issue number>` in the commit message
- please include tests.

**Have a nice day!**

Closes: #12814
Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/12814
Pull-request-sha: 8a17b26aea264acf70c52de324b8ccb92b469f2d

Change-Id: Ic99ccc68a32354ef6fe013ec17242058ad2d6d63
(cherry picked from commit 7a68e2aeffd43fc5b78df6182969e031e31043b9)

lib/sqlalchemy/ext/hybrid.py
test/typing/plain_files/ext/hybrid/hybrid_five.py [new file with mode: 0644]
test/typing/plain_files/ext/hybrid/hybrid_two.py

index c1c46e7c5f597d65d7cf78d623826c60f24759f7..13e8fbad195c0515acdbac3d3375da21870010ee 100644 (file)
@@ -1140,10 +1140,12 @@ class hybrid_property(interfaces.InspectionAttrInfo, ORMDescriptor[_T]):
         else:
             return self.fget(instance)
 
-    def __set__(self, instance: object, value: Any) -> None:
+    def __set__(
+        self, instance: object, value: Union[SQLCoreOperations[_T], _T]
+    ) -> None:
         if self.fset is None:
             raise AttributeError("can't set attribute")
-        self.fset(instance, value)
+        self.fset(instance, value)  # type: ignore[arg-type]
 
     def __delete__(self, instance: object) -> None:
         if self.fdel is None:
diff --git a/test/typing/plain_files/ext/hybrid/hybrid_five.py b/test/typing/plain_files/ext/hybrid/hybrid_five.py
new file mode 100644 (file)
index 0000000..24f5737
--- /dev/null
@@ -0,0 +1,33 @@
+from sqlalchemy import func
+from sqlalchemy import select
+from sqlalchemy.ext.hybrid import hybrid_property
+from sqlalchemy.orm import DeclarativeBase
+from sqlalchemy.orm import Mapped
+from sqlalchemy.orm import mapped_column
+from sqlalchemy.sql.elements import SQLCoreOperations
+
+
+class Base(DeclarativeBase):
+    pass
+
+
+class MyModel(Base):
+    __tablename__ = "my_model"
+
+    id: Mapped[int] = mapped_column(primary_key=True)
+
+    int_col: Mapped[int | None] = mapped_column()
+
+    @hybrid_property
+    def some_col(self) -> int:
+        return (self.int_col or 0) + 1
+
+    @some_col.inplace.setter
+    def _str_col_setter(self, value: int | SQLCoreOperations[int]) -> None:
+        self.int_col = value - 1
+
+
+m = MyModel(id=42, int_col=1)
+m.some_col = 42
+m.some_col = select(func.max(MyModel.id)).scalar_subquery()
+m.some_col = func.max(MyModel.id)
index db50d7678e0ef42f36e66a9e85ba3e026de1158d..b4f2aca769527ccdc9bda6b6276439e0fcf4345f 100644 (file)
@@ -133,7 +133,6 @@ class Foo(Base):
 
     def needs_update_getter(self) -> bool:
         return self.val
-        ...
 
     def needs_update_setter(self, value: bool) -> None:
         self.val = value