Skip to content

Commit

Permalink
use fully qualified, locatable names for all use of api.named_type()
Browse files Browse the repository at this point in the history
Fixed mypy regression where the release of mypy 0.930 added additional
internal checks to the format of "named types", requiring that they be
fully qualified and locatable. This broke the mypy plugin for SQLAlchemy,
raising an assertion error, as there was use of symbols such as
``__builtins__`` and other un-locatable or unqualified names that
previously had not raised any assertions.

Fixes: #7496
Change-Id: I037680606a1d51158ef6503508ec76c5d5adc946
  • Loading branch information
zzzeek committed Dec 22, 2021
1 parent ab75ec2 commit aded8b1
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 11 deletions.
11 changes: 11 additions & 0 deletions doc/build/changelog/unreleased_14/7496.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
.. change::
:tags: bug, mypy
:tickets: 7496

Fixed mypy regression where the release of mypy 0.930 added additional
internal checks to the format of "named types", requiring that they be
fully qualified and locatable. This broke the mypy plugin for SQLAlchemy,
raising an assertion error, as there was use of symbols such as
``__builtins__`` and other un-locatable or unqualified names that
previously had not raised any assertions.

9 changes: 5 additions & 4 deletions lib/sqlalchemy/ext/mypy/apply.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

from . import infer
from . import util
from .names import NAMED_TYPE_SQLA_MAPPED


def apply_mypy_mapped_attr(
Expand Down Expand Up @@ -134,7 +135,7 @@ def re_apply_declarative_assignments(
and isinstance(stmt.rvalue.callee.expr, NameExpr)
and stmt.rvalue.callee.expr.node is not None
and stmt.rvalue.callee.expr.node.fullname
== "sqlalchemy.orm.attributes.Mapped"
== NAMED_TYPE_SQLA_MAPPED
and stmt.rvalue.callee.name == "_empty_constructor"
and isinstance(stmt.rvalue.args[0], CallExpr)
and isinstance(stmt.rvalue.args[0].callee, RefExpr)
Expand Down Expand Up @@ -165,7 +166,7 @@ def re_apply_declarative_assignments(

if python_type_for_type is not None:
left_node.type = api.named_type(
"__sa_Mapped", [python_type_for_type]
NAMED_TYPE_SQLA_MAPPED, [python_type_for_type]
)

if update_cls_metadata:
Expand Down Expand Up @@ -202,12 +203,12 @@ class User(Base):

if left_hand_explicit_type is not None:
left_node.type = api.named_type(
"__sa_Mapped", [left_hand_explicit_type]
NAMED_TYPE_SQLA_MAPPED, [left_hand_explicit_type]
)
else:
lvalue.is_inferred_def = False
left_node.type = api.named_type(
"__sa_Mapped",
NAMED_TYPE_SQLA_MAPPED,
[] if python_type_for_type is None else [python_type_for_type],
)

Expand Down
2 changes: 1 addition & 1 deletion lib/sqlalchemy/ext/mypy/decl_class.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ class MyClass:
)

left_node.node.type = api.named_type(
"__sa_Mapped", [left_hand_explicit_type]
names.NAMED_TYPE_SQLA_MAPPED, [left_hand_explicit_type]
)

# this will ignore the rvalue entirely
Expand Down
10 changes: 6 additions & 4 deletions lib/sqlalchemy/ext/mypy/infer.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ class MyClass:
type_is_a_collection = True
if python_type_for_type is not None:
python_type_for_type = api.named_type(
"__builtins__.list", [python_type_for_type]
names.NAMED_TYPE_BUILTINS_LIST, [python_type_for_type]
)
elif (
uselist_arg is None or api.parse_bool(uselist_arg) is True
Expand Down Expand Up @@ -438,7 +438,7 @@ def _infer_type_from_left_and_inferred_right(

if not is_subtype(left_hand_explicit_type, python_type_for_type):
effective_type = api.named_type(
"__sa_Mapped", [orig_python_type_for_type]
names.NAMED_TYPE_SQLA_MAPPED, [orig_python_type_for_type]
)

msg = (
Expand Down Expand Up @@ -507,7 +507,9 @@ def infer_type_from_left_hand_type_only(
)
util.fail(api, msg.format(node.name), node)

return api.named_type("__sa_Mapped", [AnyType(TypeOfAny.special_form)])
return api.named_type(
names.NAMED_TYPE_SQLA_MAPPED, [AnyType(TypeOfAny.special_form)]
)

else:
# use type from the left hand side
Expand All @@ -529,7 +531,7 @@ def extract_python_type_from_typeengine(
return Instance(first_arg.node, [])
# TODO: support other pep-435 types here
else:
return api.named_type("__builtins__.str", [])
return api.named_type(names.NAMED_TYPE_BUILTINS_STR, [])

assert node.has_base("sqlalchemy.sql.type_api.TypeEngine"), (
"could not extract Python type from node: %s" % node
Expand Down
6 changes: 6 additions & 0 deletions lib/sqlalchemy/ext/mypy/names.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@
DECLARATIVE_MIXIN: int = util.symbol("DECLARATIVE_MIXIN") # type: ignore
QUERY_EXPRESSION: int = util.symbol("QUERY_EXPRESSION") # type: ignore

# names that must succeed with mypy.api.named_type
NAMED_TYPE_BUILTINS_OBJECT = "builtins.object"
NAMED_TYPE_BUILTINS_STR = "builtins.str"
NAMED_TYPE_BUILTINS_LIST = "builtins.list"
NAMED_TYPE_SQLA_MAPPED = "sqlalchemy.orm.attributes.Mapped"

_lookup: Dict[str, Tuple[int, Set[str]]] = {
"Column": (
COLUMN,
Expand Down
4 changes: 2 additions & 2 deletions lib/sqlalchemy/ext/mypy/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def _dynamic_class_hook(ctx: DynamicClassDefContext) -> None:
)
info.bases = [Instance(cls_arg.node, [])]
else:
obj = ctx.api.named_type("__builtins__.object")
obj = ctx.api.named_type(names.NAMED_TYPE_BUILTINS_OBJECT)

info.bases = [obj]

Expand All @@ -152,7 +152,7 @@ def _dynamic_class_hook(ctx: DynamicClassDefContext) -> None:
util.fail(
ctx.api, "Not able to calculate MRO for declarative base", ctx.call
)
obj = ctx.api.named_type("__builtins__.object")
obj = ctx.api.named_type(names.NAMED_TYPE_BUILTINS_OBJECT)
info.bases = [obj]
info.fallback_to_any = True

Expand Down

0 comments on commit aded8b1

Please sign in to comment.