Deprecate strings indicating attribute names

Using strings to represent relationship names in ORM operations such as
:meth:`_orm.Query.join`, as well as strings for all ORM attribute names
in loader options like :func:`_orm.selectinload`
is deprecated and will be removed in SQLAlchemy 2.0.  The class-bound
attribute should be passed instead.  This provides much better specificity
to the given method, allows for modifiers such as ``of_type()``, and
reduces internal complexity.

Additionally, the ``aliased`` and ``from_joinpoint`` parameters to
:meth:`_orm.Query.join` are also deprecated.   The :func:`_orm.aliased`
construct now provides for a great deal of flexibility and capability
and should be used directly.

Fixes: #4705
Fixes: #5202
Change-Id: I32f61663d68026154906932913c288f269991adc

5 files changed, 119 insertions, 10 deletions

diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index e278d81f3..df1a048f3 100644
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -718,9 +718,12 @@ class Query(
         """
         self._compile_options += {"_current_path": path}
 
-    # TODO: removed in 2.0
     @_generative
     @_assertions(_no_clauseelement_condition)
+    @util.deprecated_20(
+        ":meth:`_orm.Query.with_polymorphic`",
+        alternative="Use the orm.with_polymorphic() standalone function",
+    )
     def with_polymorphic(
         self, cls_or_mappers, selectable=None, polymorphic_on=None
     ):
@@ -2094,6 +2097,9 @@ class Query(
 
         **Legacy Features of Query.join()**
 
+        .. deprecated:: 1.4 The following features are deprecated and will
+           be removed in SQLAlchemy 2.0.
+
         The :meth:`_query.Query.join` method currently supports several
         usage patterns and arguments that are considered to be legacy
         as of SQLAlchemy 1.3.   A deprecation path will follow
@@ -2218,6 +2224,14 @@ class Query(
             kwargs.pop("isouter", False),
             kwargs.pop("full", False),
         )
+
+        if aliased or from_joinpoint:
+            util.warn_deprecated_20(
+                "The ``aliased`` and ``from_joinpoint`` keyword arguments "
+                "to Query.join() are deprecated and will be removed "
+                "in SQLAlchemy 2.0."
+            )
+
         if kwargs:
             raise TypeError(
                 "unknown arguments: %s" % ", ".join(sorted(kwargs))
@@ -2249,6 +2263,10 @@ class Query(
             _single = []
             for prop in (target,) + props:
                 if isinstance(prop, tuple):
+                    util.warn_deprecated_20(
+                        "Query.join() will no longer accept tuples as "
+                        "arguments in SQLAlchemy 2.0."
+                    )
                     if _single:
                         _props.extend((_s,) for _s in _single)
                     _single = []
@@ -2286,7 +2304,7 @@ class Query(
 
         # legacy ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-        self._legacy_setup_joins += tuple(
+        joins_to_add = tuple(
             (
                 coercions.expect(
                     roles.JoinTargetRole,
@@ -2295,9 +2313,9 @@ class Query(
                     apply_propagate_attrs=self,
                 ),
                 (
-                    coercions.expect(roles.OnClauseRole, prop[1])
-                    if not isinstance(prop[1], str)
-                    else prop[1]
+                    coercions.expect(roles.OnClauseRole, prop[1], legacy=True)
+                    #                    if not isinstance(prop[1], str)
+                    #                    else prop[1]
                 )
                 if len(prop) == 2
                 else None,
@@ -2313,6 +2331,14 @@ class Query(
             for i, prop in enumerate(_props)
         )
 
+        if len(joins_to_add) > 1:
+            util.warn_deprecated_20(
+                "Passing a chain of multiple join conditions to Query.join() "
+                "is deprecated and will be removed in SQLAlchemy 2.0. "
+                "Please use individual join() calls per relationship."
+            )
+        self._legacy_setup_joins += joins_to_add
+
         self.__dict__.pop("_last_joined_entity", None)
 
     def outerjoin(self, target, *props, **kwargs):
@@ -3038,9 +3064,7 @@ class Query(
 
         """
 
-        bulk_del = BulkDelete(
-            self,
-        )
+        bulk_del = BulkDelete(self)
         if self.dispatch.before_compile_delete:
             for fn in self.dispatch.before_compile_delete:
                 new_query = fn(bulk_del.query, bulk_del)
diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py
index 53350cb8e..1795fe6e5 100644
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -193,6 +193,12 @@ class Load(Generative, LoaderOption):
             else:
                 ent = path.entity
 
+            util.warn_deprecated_20(
+                "Using strings to indicate column or "
+                "relationship paths in loader options is deprecated "
+                "and will be removed in SQLAlchemy 2.0.  Please use "
+                "the class-bound attribute directly."
+            )
             try:
                 # use getattr on the class to work around
                 # synonyms, hybrids, etc.
diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index f902014ef..7d1650a1a 100644
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -1628,12 +1628,36 @@ def with_parent(instance, prop, from_entity=None):
     :func:`_orm.relationship()`
     configuration.
 
+    E.g.::
+
+        stmt = select(Address).where(with_parent(some_user, Address.user))
+
+
     The SQL rendered is the same as that rendered when a lazy loader
     would fire off from the given parent on that attribute, meaning
     that the appropriate state is taken from the parent object in
     Python without the need to render joins to the parent table
     in the rendered statement.
 
+    The given property may also make use of :meth:`_orm.PropComparator.of_type`
+    to indicate the left side of the criteria::
+
+
+        a1 = aliased(Address)
+        a2 = aliased(Address)
+        stmt = select(a1, a2).where(
+            with_parent(u1, User.addresses.of_type(a2))
+        )
+
+    The above use is equivalent to using the
+    :func:`_orm.with_parent.from_entity` argument::
+
+        a1 = aliased(Address)
+        a2 = aliased(Address)
+        stmt = select(a1, a2).where(
+            with_parent(u1, User.addresses, from_entity=a2)
+        )
+
     :param instance:
       An instance which has some :func:`_orm.relationship`.
 
@@ -1642,6 +1666,9 @@ def with_parent(instance, prop, from_entity=None):
       what relationship from the instance should be used to reconcile the
       parent/child relationship.
 
+      .. deprecated:: 1.4 Using strings is deprecated and will be removed
+         in SQLAlchemy 2.0.  Please use the class-bound attribute directly.
+
     :param from_entity:
       Entity in which to consider as the left side.  This defaults to the
       "zero" entity of the :class:`_query.Query` itself.
@@ -1650,9 +1677,16 @@ def with_parent(instance, prop, from_entity=None):
 
     """
     if isinstance(prop, util.string_types):
+        util.warn_deprecated_20(
+            "Using strings to indicate relationship names in the ORM "
+            "with_parent() function is deprecated and will be removed "
+            "SQLAlchemy 2.0.  Please use the class-bound attribute directly."
+        )
         mapper = object_mapper(instance)
         prop = getattr(mapper.class_, prop).property
     elif isinstance(prop, attributes.QueryableAttribute):
+        if prop._of_type:
+            from_entity = prop._of_type
         prop = prop.property
 
     return prop._with_parent(instance, from_entity=from_entity)
diff --git a/lib/sqlalchemy/sql/coercions.py b/lib/sqlalchemy/sql/coercions.py
index 558ced8bd..9b3acf5ad 100644
--- a/lib/sqlalchemy/sql/coercions.py
+++ b/lib/sqlalchemy/sql/coercions.py
@@ -352,7 +352,7 @@ class _CoerceLiterals(object):
             if self._coerce_star and element == "*":
                 return elements.ColumnClause("*", is_literal=True)
             else:
-                return self._text_coercion(element, argname)
+                return self._text_coercion(element, argname, **kw)
 
         if self._coerce_consts:
             if element is None:
@@ -528,6 +528,32 @@ class OnClauseImpl(_CoerceLiterals, _ColumnCoercions, RoleImpl):
 
     _coerce_consts = True
 
+    def _implicit_coercions(
+        self, original_element, resolved, argname=None, legacy=False, **kw
+    ):
+        if legacy and isinstance(resolved, str):
+            return resolved
+        else:
+            return super(OnClauseImpl, self)._implicit_coercions(
+                original_element,
+                resolved,
+                argname=argname,
+                legacy=legacy,
+                **kw
+            )
+
+    def _text_coercion(self, element, argname=None, legacy=False):
+        if legacy and isinstance(element, str):
+            util.warn_deprecated_20(
+                "Using strings to indicate relationship names in "
+                "Query.join() is deprecated and will be removed in "
+                "SQLAlchemy 2.0.  Please use the class-bound attribute "
+                "directly."
+            )
+            return element
+
+        return super(OnClauseImpl, self)._text_coercion(element, argname)
+
     def _post_coercion(self, resolved, original_element=None, **kw):
         # this is a hack right now as we want to use coercion on an
         # ORM InstrumentedAttribute, but we want to return the object
@@ -802,7 +828,15 @@ class JoinTargetImpl(RoleImpl):
     ):
         if isinstance(original_element, roles.JoinTargetRole):
             return original_element
-        elif legacy and isinstance(resolved, (str, roles.WhereHavingRole)):
+        elif legacy and isinstance(resolved, str):
+            util.warn_deprecated_20(
+                "Using strings to indicate relationship names in "
+                "Query.join() is deprecated and will be removed in "
+                "SQLAlchemy 2.0.  Please use the class-bound attribute "
+                "directly."
+            )
+            return resolved
+        elif legacy and isinstance(resolved, roles.WhereHavingRole):
             return resolved
         elif legacy and resolved._is_select_statement:
             util.warn_deprecated(
diff --git a/lib/sqlalchemy/testing/warnings.py b/lib/sqlalchemy/testing/warnings.py
index 5704cf2a6..bed58f777 100644
--- a/lib/sqlalchemy/testing/warnings.py
+++ b/lib/sqlalchemy/testing/warnings.py
@@ -105,6 +105,17 @@ def setup_filters():
         r"The Query\.with_parent\(\) function",
         r"The Query\.with_parent\(\) function",
         r"The Query\.select_entity_from\(\) function",
+        r"The ``aliased`` and ``from_joinpoint`` keyword arguments",
+        r"Using strings to indicate relationship names in Query.join",
+        r"Using strings to indicate column or relationship paths in "
+        "loader options",
+        r"Using strings to indicate relationship names in the ORM "
+        r"with_parent\(\)",
+        r"The Query.with_polymorphic\(\) function/method is considered "
+        "legacy as of the 1.x series",
+        r"Passing a chain of multiple join conditions to Query.join\(\) "
+        r"is deprecated and will be removed in SQLAlchemy 2.0.",
+        r"Query.join\(\) will no longer accept tuples as arguments",
         #
         # ORM Session
         #