Warnings for @declared_attr.cascadingticket_4091

A warning is emitted if a subclass attempts to override an attribute
that was declared on a superclass using ``@declared_attr.cascading``
that the overridden attribute will be ignored. This use
case cannot be fully supported down to further subclasses without more
complex development efforts, so for consistency the "cascading" is
honored all the way down regardless of overriding attributes.

A warning is emitted if the ``@declared_attr.cascading`` attribute is
used with a special declarative name such as ``__tablename__``, as this
has no effect.

Ensure that documenation refers to the current inconsistency that
__tablename__ can be overridden by subclasses however
@declared_attr.cascading cannot.

Fixes: #4091
Fixes: #4092
Change-Id: I3aecdb2f99d408e404a1223f5ad86ae3c7fdf036

2 files changed, 52 insertions, 8 deletions

diff --git a/lib/sqlalchemy/ext/declarative/api.py b/lib/sqlalchemy/ext/declarative/api.py
index ee13a90f3..0773922d0 100644
--- a/lib/sqlalchemy/ext/declarative/api.py
+++ b/lib/sqlalchemy/ext/declarative/api.py
@@ -199,11 +199,24 @@ class declared_attr(interfaces._MappedAttribute, property):
         or MapperProperty-based declared attribute should be configured
         distinctly per mapped subclass, within a mapped-inheritance scenario.
 
-        .. note::
+        .. warning::
 
-            The :attr:`.declared_attr.cascading` modifier **only** applies
-            to the use of :class:`.declared_attr` on declarative mixin classes
-            and ``__abstract__`` classes.
+            The :attr:`.declared_attr.cascading` modifier has several
+            limitations:
+
+            * The flag **only** applies to the use of :class:`.declared_attr`
+              on declarative mixin classes and ``__abstract__`` classes; it
+              currently has no effect when used on a mapped class directly.
+
+            * The flag **only** applies to normally-named attributes, e.g.
+              not any special underscore attributes such as ``__tablename__``.
+              On these attributes it has **no** effect.
+
+            * The flag currently **does not allow further overrides** down
+              the class hierarchy; if a subclass tries to override the
+              attribute, a warning is emitted and the overridden attribute
+              is skipped.  This is a limitation that it is hoped will be
+              resolved at some point.
 
         Below, both MyClass as well as MySubClass will have a distinct
         ``id`` Column object established::
diff --git a/lib/sqlalchemy/ext/declarative/base.py b/lib/sqlalchemy/ext/declarative/base.py
index 3a2ac66ae..255373597 100644
--- a/lib/sqlalchemy/ext/declarative/base.py
+++ b/lib/sqlalchemy/ext/declarative/base.py
@@ -88,6 +88,19 @@ def _as_declarative(cls, classname, dict_):
     _MapperConfig.setup_mapping(cls, classname, dict_)
 
 
+def _check_declared_props_nocascade(obj, name, cls):
+
+    if isinstance(obj, declarative_props):
+        if getattr(obj, '_cascading', False):
+            util.warn(
+                "@declared_attr.cascading is not supported on the %s "
+                "attribute on class %s.  This attribute invokes for "
+                "subclasses in any case." % (name, cls))
+        return True
+    else:
+        return False
+
+
 class _MapperConfig(object):
 
     @classmethod
@@ -167,9 +180,11 @@ class _MapperConfig(object):
 
             for name, obj in vars(base).items():
                 if name == '__mapper_args__':
+                    check_decl = \
+                        _check_declared_props_nocascade(obj, name, cls)
                     if not mapper_args_fn and (
                         not class_mapped or
-                        isinstance(obj, declarative_props)
+                        check_decl
                     ):
                         # don't even invoke __mapper_args__ until
                         # after we've determined everything about the
@@ -177,17 +192,21 @@ class _MapperConfig(object):
                         # make a copy of it so a class-level dictionary
                         # is not overwritten when we update column-based
                         # arguments.
-                        mapper_args_fn = lambda: dict(cls.__mapper_args__)
+                        mapper_args_fn = lambda: dict(cls.__mapper_args__)  # noqa
                 elif name == '__tablename__':
+                    check_decl = \
+                        _check_declared_props_nocascade(obj, name, cls)
                     if not tablename and (
                         not class_mapped or
-                        isinstance(obj, declarative_props)
+                        check_decl
                     ):
                         tablename = cls.__tablename__
                 elif name == '__table_args__':
+                    check_decl = \
+                        _check_declared_props_nocascade(obj, name, cls)
                     if not table_args and (
                         not class_mapped or
-                        isinstance(obj, declarative_props)
+                        check_decl
                     ):
                         table_args = cls.__table_args__
                         if not isinstance(
@@ -220,6 +239,18 @@ class _MapperConfig(object):
                     elif isinstance(obj, declarative_props):
                         oldclassprop = isinstance(obj, util.classproperty)
                         if not oldclassprop and obj._cascading:
+                            if name in dict_:
+                                # unfortunately, while we can use the user-
+                                # defined attribute here to allow a clean
+                                # override, if there's another
+                                # subclass below then it still tries to use
+                                # this.  not sure if there is enough information
+                                # here to add this as a feature later on.
+                                util.warn(
+                                    "Attribute '%s' on class %s cannot be "
+                                    "processed due to "
+                                    "@declared_attr.cascading; "
+                                    "skipping" % (name, cls))
                             dict_[name] = column_copies[obj] = \
                                 ret = obj.__get__(obj, cls)
                             setattr(cls, name, ret)