identify unresolvable Mapped types

Fixed issue where use of an unknown datatype within a :class:`.Mapped`
annotation for a column-based attribute would silently fail to map the
attribute, rather than reporting an exception; an informative exception
message is now raised.

tighten up iteration of names on mapped classes to more fully
exclude a large number of underscored names, so that we can avoid trying
to look at annotations for them or anything else.  centralize the
"list of names we care about" more fully within _cls_attr_resolver
and base it on underscore conventions we should usually ignore,
with the exception of the few underscore names we want to see.

Fixes: #8888
Change-Id: I3c0a1666579fe67b3c40cc74fa443b6f1de354ce

4 files changed, 85 insertions, 17 deletions

diff --git a/test/orm/declarative/test_basic.py b/test/orm/declarative/test_basic.py
index 3dfc59827..475b5e39b 100644
--- a/test/orm/declarative/test_basic.py
+++ b/test/orm/declarative/test_basic.py
@@ -46,6 +46,7 @@ from sqlalchemy.testing import assert_raises
 from sqlalchemy.testing import assert_raises_message
 from sqlalchemy.testing import assertions
 from sqlalchemy.testing import eq_
+from sqlalchemy.testing import expect_raises_message
 from sqlalchemy.testing import expect_warnings
 from sqlalchemy.testing import fixtures
 from sqlalchemy.testing import is_
@@ -1048,27 +1049,46 @@ class DeclarativeMultiBaseTest(
             configure_mappers,
         )
 
-    def test_reserved_identifiers(self):
-        def go1():
-            class User1(Base):
-                __tablename__ = "user1"
-                id = Column(Integer, primary_key=True)
-                metadata = Column(Integer)
+    # currently "registry" is allowed, "metadata" is not.
+    @testing.combinations(
+        ("metadata", True), ("registry", False), argnames="name, expect_raise"
+    )
+    @testing.variation("attrtype", ["column", "relationship"])
+    def test_reserved_identifiers(
+        self, decl_base, name, expect_raise, attrtype
+    ):
 
-        def go2():
-            class User2(Base):
-                __tablename__ = "user2"
+        if attrtype.column:
+            clsdict = {
+                "__tablename__": "user",
+                "id": Column(Integer, primary_key=True),
+                name: Column(Integer),
+            }
+        elif attrtype.relationship:
+            clsdict = {
+                "__tablename__": "user",
+                "id": Column(Integer, primary_key=True),
+                name: relationship("Address"),
+            }
+
+            class Address(decl_base):
+                __tablename__ = "address"
                 id = Column(Integer, primary_key=True)
-                metadata = relationship("Address")
+                user_id = Column(ForeignKey("user.id"))
 
-        for go in (go1, go2):
-            assert_raises_message(
+        else:
+            assert False
+
+        if expect_raise:
+            with expect_raises_message(
                 exc.InvalidRequestError,
-                "Attribute name 'metadata' is reserved "
-                "for the MetaData instance when using a "
-                "declarative base class.",
-                go,
-            )
+                f"Attribute name '{name}' is reserved "
+                "when using the Declarative API.",
+            ):
+                type("User", (decl_base,), clsdict)
+        else:
+            User = type("User", (decl_base,), clsdict)
+            assert getattr(User, name).property
 
     def test_recompile_on_othermapper(self):
         """declarative version of the same test in mappers.py"""
diff --git a/test/orm/declarative/test_tm_future_annotations.py b/test/orm/declarative/test_tm_future_annotations.py
index 1e8913368..b66d67a77 100644
--- a/test/orm/declarative/test_tm_future_annotations.py
+++ b/test/orm/declarative/test_tm_future_annotations.py
@@ -160,6 +160,22 @@ class MappedColumnTest(_MappedColumnTest):
         is_(MyClass.id.expression.type._type_affinity, Integer)
         is_(MyClass.data.expression.type._type_affinity, Uuid)
 
+    def test_dont_ignore_unresolvable(self, decl_base):
+        """test #8888"""
+
+        with expect_raises_message(
+            exc.ArgumentError,
+            r"Could not resolve all types within mapped annotation: "
+            r"\"Mapped\[fake\]\".  Ensure all types are written correctly and "
+            r"are imported within the module in use.",
+        ):
+
+            class A(decl_base):
+                __tablename__ = "a"
+
+                id: Mapped[int] = mapped_column(primary_key=True)
+                data: Mapped[fake]  # noqa
+
 
 class MappedOneArg(KeyFuncDict[str, _R]):
     pass
diff --git a/test/orm/declarative/test_tm_future_annotations_sync.py b/test/orm/declarative/test_tm_future_annotations_sync.py
index 1a4ac6de3..7358f385d 100644
--- a/test/orm/declarative/test_tm_future_annotations_sync.py
+++ b/test/orm/declarative/test_tm_future_annotations_sync.py
@@ -1118,6 +1118,22 @@ class MappedColumnTest(fixtures.TestBase, testing.AssertsCompiledSQL):
 
         assert isinstance(MyClass.__table__.c.data.type, sqltype)
 
+    def test_dont_ignore_unresolvable(self, decl_base):
+        """test #8888"""
+
+        with expect_raises_message(
+            sa_exc.ArgumentError,
+            r"Could not resolve all types within mapped annotation: "
+            r"\".*Mapped\[.*fake.*\]\".  Ensure all types are written "
+            r"correctly and are imported within the module in use.",
+        ):
+
+            class A(decl_base):
+                __tablename__ = "a"
+
+                id: Mapped[int] = mapped_column(primary_key=True)
+                data: Mapped["fake"]  # noqa
+
 
 class MixinTest(fixtures.TestBase, testing.AssertsCompiledSQL):
     __dialect__ = "default"
diff --git a/test/orm/declarative/test_typed_mapping.py b/test/orm/declarative/test_typed_mapping.py
index 05ceee3f8..ba099412f 100644
--- a/test/orm/declarative/test_typed_mapping.py
+++ b/test/orm/declarative/test_typed_mapping.py
@@ -1109,6 +1109,22 @@ class MappedColumnTest(fixtures.TestBase, testing.AssertsCompiledSQL):
 
         assert isinstance(MyClass.__table__.c.data.type, sqltype)
 
+    def test_dont_ignore_unresolvable(self, decl_base):
+        """test #8888"""
+
+        with expect_raises_message(
+            sa_exc.ArgumentError,
+            r"Could not resolve all types within mapped annotation: "
+            r"\".*Mapped\[.*fake.*\]\".  Ensure all types are written "
+            r"correctly and are imported within the module in use.",
+        ):
+
+            class A(decl_base):
+                __tablename__ = "a"
+
+                id: Mapped[int] = mapped_column(primary_key=True)
+                data: Mapped["fake"]  # noqa
+
 
 class MixinTest(fixtures.TestBase, testing.AssertsCompiledSQL):
     __dialect__ = "default"