apply code review changes

- added test for `runtime.py`'s `markup_join` and fixed wrong return - reverted warning for escape and Markup imports as they will be already removed in 3.1 - changed change messages from 3.0 as PR will be part of 3.1
pallets · May 29, 2021 · 0fc2b15 · 0fc2b15
1 parent 7b7f4ef
commit 0fc2b15
Show file tree

Hide file tree

Showing 11 changed files with 97 additions and 51 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -5,6 +5,9 @@ Version 3.1.0
 
 Unreleased
 
+-   Allow custom autoescape functions :issue:`1377`
+-   Removed hardcoded Markup and escape calls
+
 
 Version 3.0.1
 -------------
@@ -26,8 +29,6 @@ Released 2021-05-11
 
 -   Drop support for Python 2.7 and 3.5.
 -   Bump MarkupSafe dependency to >=1.1.
--   Allow custom autoescape functions :issue:`1377`
--   Removed hardcoded Markup and escape calls
 -   Bump Babel optional dependency to >=2.1.
 -   Remove code that was marked deprecated.
 -   Add type hinting. :pr:`1412`

diff --git a/docs/api.rst b/docs/api.rst
@@ -224,7 +224,7 @@ useful if you want to dig deeper into Jinja or :ref:`develop extensions
 
 Safe Strings and Escaping
 -------------------------
-.. versionchanged:: 3.0
+.. versionchanged:: 3.1
 
 To handle untrusted input when rendering templates to
 avoid injection attacks Jinja uses a combination of trusted strings
@@ -236,7 +236,7 @@ multiple times and at the same time make sure, that using string
 operation like ``%`` the original escaped string stays escaped, even
 when unescaped string are thrown at it.
 
-Before Jinja 3.0 this was done by the hardcoded
+Before Jinja 3.1 this was done by the hardcoded
 :class:`markupsafe.Markup` class and
 :func:`markupsafe.escape` function from the `MarkupSafe`_  package.
 The ``escape(s: str)`` function converts the characters
@@ -257,7 +257,7 @@ This is done in a way so that the result of these operations
 in combination with an raw strings is always an escaped ``Markup``
 class by using the ``escape`` method of the ``Markup`` class.
 
-With version 3.0 this hardcoded relation to the `MarkupSafe`_ and it's
+With version 3.1 this hardcoded relation to the `MarkupSafe`_ and it's
 HTML based escaping was removed, as Jinja is intended to be a Language
 independent template system.
 It is still the default but now you are able to provide a custom escape
@@ -324,7 +324,7 @@ future.  It's recommended to configure a sensible default for
 autoescaping.  This makes it possible to enable and disable autoescaping
 on a per-template basis (HTML versus text for instance).
 
-.. versionchanged:: 3.0
+.. versionchanged:: 3.1
 
 Jinja now also allows the usage of different escape functions selected
 by template suffix.

diff --git a/src/jinja2/compiler.py b/src/jinja2/compiler.py
@@ -1702,7 +1702,7 @@ def visit_Concat(self, node: nodes.Concat, frame: Frame) -> None:
             self.visit(arg, frame)
             self.write(", ")
         self.write(")")
-        self.write(", escape_func=context.eval_ctx.autoescape")
+        self.write(", mark_safe=context.eval_ctx.mark_safe")
         self.write(")")
 
     @optimizeconst

diff --git a/src/jinja2/environment.py b/src/jinja2/environment.py
@@ -231,7 +231,7 @@ class Environment:
             return ``True`` or ``False`` depending on autoescape should be
             enabled by default.
 
-            As of Jinja 3.0 the autoescape can be even smarter.
+            As of Jinja 3.1 the autoescape can be even smarter.
             If the given function does not return a boolean but a
             function again, this function is considered to be the
             escape function that shall be used. So you can use the
@@ -246,7 +246,7 @@ class Environment:
 
             See :ref:`escaping` and :ref:`autoescaping` for details.
 
-            .. versionchanged:: 3.0
+            .. versionchanged:: 3.1
                 if the `autoescape` function doesn't return True or False but a
                 callable, it is assumed to be a custom escape function
 
@@ -264,7 +264,7 @@ class Environment:
 
             Defaults to False
 
-            .. versionadded:: 3.0
+            .. versionadded:: 3.1
 
         `default_escape`
             define a custom escape function or class.
@@ -287,7 +287,7 @@ class Environment:
             This setting will also overwrite the filter ``{{ var | safe }}``,
             ``{{ var | e }}`` and ``{{ var | escape }}`` accordingly.
 
-            .. versionadded:: 3.0
+            .. versionadded:: 3.1
 
         `loader`
             The template loader for this environment.
@@ -449,7 +449,7 @@ def get_markup_class(self, template_name: t.Optional[str] = None) -> t.Type[Mark
                               for special escpaing in the autoescape
                               settings
 
-        .. versionadded:: 3.0
+        .. versionadded:: 3.1
         """
         if callable(self.autoescape) and callable(self.autoescape(template_name)):
             return get_wrapped_escape_class(self.autoescape(template_name))
@@ -1119,7 +1119,7 @@ def get_template(
             function was calling it, i.e. 'extends' or 'include'.
             Required to define behavior for custom autoescape.
 
-        .. versionchanged:: 3.0
+        .. versionchanged:: 3.1
             Added caller parameter and a check if we need to raise an
             error due to usage different autoescape function within
             extends
@@ -1164,7 +1164,7 @@ def select_template(
             function was calling it, i.e. 'extends' or 'include'.
             Required to define behavior for custom autoescape.
 
-        .. versionchanged:: 3.0
+        .. versionchanged:: 3.1
             Added caller parameter
 
         .. versionchanged:: 3.0
@@ -1232,7 +1232,7 @@ def get_or_select_template(
             function was calling it, i.e. 'extends' or 'include'.
             Required to define behavior for custom autoescape.
 
-        .. versionchanged:: 3.0
+        .. versionchanged:: 3.1
             Added caller parameter
 
         .. versionadded:: 2.3

diff --git a/src/jinja2/filters.py b/src/jinja2/filters.py
@@ -106,7 +106,7 @@ def do_escape(eval_ctx: "EvalContext", s: t.Union[str, "HasHTML"]) -> markupsafe
     Escape a string with the escape function active in the current
     eval context
 
-    .. versionadded:: 3.0
+    .. versionadded:: 3.1
         replaced the hard coded HTML :func:`markupsafe.escape` function
         with an context aware escape function
     """
@@ -210,7 +210,7 @@ def do_forceescape(
     """
     Enforce HTML escaping.  This will probably double escape variables.
 
-    .. versionchanged:: 3.0
+    .. versionchanged:: 3.1
         made function context aware to use context based escape filter
     """
     if hasattr(value, "__html__"):
@@ -271,7 +271,7 @@ def do_replace(
         {{ "aaaaargh"|replace("a", "d'oh, ", 2) }}
             -> d'oh, d'oh, aaargh
 
-    .. versionchanged:: 3.0
+    .. versionchanged:: 3.1
         made function context aware to use context based escape filter
     """
     if count is None:
@@ -631,7 +631,7 @@ def sync_do_join(
 
         {{ users|join(', ', attribute='username') }}
 
-    .. versionchanged:: 3.0
+    .. versionchanged:: 3.1
         made function context aware to use context based escape filter
 
     .. versionadded:: 2.6
@@ -808,7 +808,7 @@ def do_urlize(
         ``env.policies["urlize.extra_schemes"]``, which defaults to no
         extra schemes.
 
-    .. versionchanged:: 3.0
+    .. versionchanged:: 3.1
         made function context aware to use context based escape filter
 
     .. versionchanged:: 3.0
@@ -874,7 +874,7 @@ def do_indent(
     :param first: Don't skip indenting the first line.
     :param blank: Don't skip indenting empty lines.
 
-    .. versionchanged:: 3.0
+    .. versionchanged:: 3.1
         made function context aware to use context based escape filter
 
     .. versionchanged:: 3.0

diff --git a/src/jinja2/nodes.py b/src/jinja2/nodes.py
@@ -70,11 +70,11 @@ class EvalContext:
     """Holds evaluation time information.  Custom attributes can be attached
     to it in extensions.
 
-    .. versionchanged:: 3.0
+    .. versionchanged:: 3.1
 
-    - Added ``get_escape_function`` and ``mark_safe`` functions
-    - allow autoescape to be not only boolean but also an
-      escape function
+        - Added ``get_escape_function`` and ``mark_safe`` functions
+        - allow autoescape to be not only boolean but also an
+          escape function
     """
 
     def __init__(
@@ -98,7 +98,7 @@ def get_escape_function(self) -> t.Callable[[t.Any], "Markup"]:
         """
         return the currently valid escape function
 
-        .. versionadded:: 3.0
+        .. versionadded:: 3.1
 
         """
         return self._markup_class.escape
@@ -111,7 +111,7 @@ def mark_safe(self, input: str) -> "Markup":
         if possible so custom escape functions
         are correctly handled by the Markup class.
 
-        .. versionadded:: 3.0
+        .. versionadded:: 3.1
 
         """
         return self._markup_class(input)

diff --git a/src/jinja2/runtime.py b/src/jinja2/runtime.py
@@ -5,8 +5,7 @@
 from collections import abc
 from itertools import chain
 
-from markupsafe import escape as html_escape  # noqa: F401
-from markupsafe import soft_str
+import markupsafe
 
 from .async_utils import auto_aiter
 from .async_utils import auto_await  # noqa: F401
@@ -41,6 +40,8 @@ def __call__(
             ...
 
 
+html_escape = markupsafe.escape
+
 # these variables are exported to the template runtime
 exported = [
     "LoopContext",
@@ -72,30 +73,32 @@ def identity(x: V) -> V:
     return x
 
 
-def markup_join(seq: t.Iterable[t.Any], escape_func: EscapeFunc = html_escape) -> str:
+def markup_join(
+    seq: t.Iterable[t.Any], mark_safe: EscapeFunc = markupsafe.Markup
+) -> t.Union[str, markupsafe.Markup]:
     """
     Concatenation that escapes if necessary and converts to string.
 
-    .. versionchanged:: 3.0
+    .. versionchanged:: 3.1
         added optional parameter escape_function to make
-        use the contex based escape function
+        use the context based escape function
     """
     buf = []
-    iterator = map(soft_str, seq)
+    iterator = map(markupsafe.soft_str, seq)
     for arg in iterator:
         buf.append(arg)
         if hasattr(arg, "__html__"):
-            return "".join(map(escape_func, chain(buf, iterator)))
+            return mark_safe("").join(chain(buf, iterator))
     return concat(buf)
 
 
-def str_join(seq: t.Iterable[t.Any], escape_func: EscapeFunc = html_escape) -> str:
+def str_join(seq: t.Iterable[t.Any], mark_safe: EscapeFunc = markupsafe.Markup) -> str:
     """
     Simple args to string conversion and concatenation.
 
-    .. versionchanged:: 3.0
+    .. versionchanged:: 3.1
             added optional and currently ignored parameter
-            ``escape_function`` to allow easier usage of ``markup_join``
+            ``mark_safe`` to allow easier usage of ``markup_join``
     """
     return concat(map(str, seq))
 

diff --git a/src/jinja2/utils.py b/src/jinja2/utils.py
@@ -318,8 +318,11 @@ def urlize(
     :param extra_schemes: Recognize URLs that start with these schemes
         in addition to the default behavior.
 
+    .. versionchanged:: 3.1
+        The ``do_escape`` parameter was added.
+
     .. versionchanged:: 3.0
-        The ``extra_schemes`` and ``do_escape`` parameter was added.
+        The ``extra_schemes`` parameter was added.
 
     .. versionchanged:: 3.0
         Generate ``https://`` links for URLs without a scheme.
@@ -422,7 +425,7 @@ def generate_lorem_ipsum(
 ) -> t.Union[markupsafe.Markup, str]:
     """Generate some lorem ipsum for the template.
 
-    .. versionchanged:: 3.0
+    .. versionchanged:: 3.1
         added mark_safe and do_escape parameter
     """
     from .constants import LOREM_IPSUM_WORDS
@@ -714,7 +717,7 @@ def select_autoescape(
 
     .. versionadded:: 2.9
         created function
-    .. versionchanged:: 3.0
+    .. versionchanged:: 3.1
         parameter ``special_extensions`` was added
     """
 
@@ -780,9 +783,12 @@ def htmlsafe_json_dumps(
     :param kwargs: Extra arguments to pass to ``dumps``. Merged onto
         ``env.policies["json.dumps_kwargs"]``.
 
+    .. versionchanged:: 3.1
+        Added required mark_safe parameter
+
     .. versionchanged:: 3.0
-        - The ``dumper`` parameter is renamed to ``dumps``.
-        - Added required mark_safe parameter
+        The ``dumper`` parameter is renamed to ``dumps``.
+
 
     .. versionadded:: 2.9
     """
@@ -820,7 +826,7 @@ def get_wrapped_escape_class(
 
     :return: a Markup class using this escape function
 
-    .. versionadded:: 3.0
+    .. versionadded:: 3.1
     """
 
     class MarkupWrapper(markupsafe.Markup):
@@ -950,9 +956,7 @@ class Markup(markupsafe.Markup):
     def __new__(cls, base, encoding=None, errors="strict"):  # type: ignore
         warnings.warn(
             "'jinja2.Markup' is deprecated and will be removed in Jinja"
-            " 3.1. Use Environment.get_markup_class and "
-            "EvalContext.mark_safe instead. "
-            "(See Escape in API Documentation)",
+            " 3.1. Import 'markupsafe.Markup' instead.",
             DeprecationWarning,
             stacklevel=2,
         )
@@ -961,10 +965,8 @@ def __new__(cls, base, encoding=None, errors="strict"):  # type: ignore
 
 def escape(s: t.Any) -> str:
     warnings.warn(
-        "'jinja2.Markup' is deprecated and will be removed in Jinja"
-        " 3.1. Use Environment.get_markup_class and "
-        "EvalContext.get_escape_function instead. "
-        "(See Escape in API Documentation)",
+        "'jinja2.escape' is deprecated and will be removed in Jinja"
+        " 3.1. Import 'markupsafe.escape' instead.",
         DeprecationWarning,
         stacklevel=2,
     )

diff --git a/tests/test_async.py b/tests/test_async.py
@@ -340,7 +340,7 @@ def test_unoptimized_scopes(self, test_env_async):
         )
         assert t.render().strip() == "(FOO)"
 
-    def test_unoptimized_scopes_autoescape(self, return_custom_autoescape):
+    def test_unoptimized_scopes_autoescape(self):
         env = Environment(
             loader=DictLoader({"o_printer": "({{ o }})"}),
             autoescape=True,