From 2aaa045c61a3d4b325f964aa01554107b47b9774 Mon Sep 17 00:00:00 2001 From: e0ne Date: Fri, 13 Sep 2013 18:11:48 +0300 Subject: [PATCH] Fixed #13408 -- Deprecated silent unpacking exception passing in for template tag. Thanks peterbe for the suggestion. --- django/template/defaulttags.py | 19 ++++++++++++++++++- docs/internals/deprecation.txt | 3 +++ docs/releases/1.8.txt | 6 ++++++ tests/template_tests/tests.py | 18 ++++++++++++------ 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/django/template/defaulttags.py b/django/template/defaulttags.py index 64fd86c042..c08f869e5f 100644 --- a/django/template/defaulttags.py +++ b/django/template/defaulttags.py @@ -17,6 +17,7 @@ from django.template.base import (Node, NodeList, Template, Context, Library, render_value_in_context) from django.template.smartif import IfParser, Literal from django.template.defaultfilters import date +from django.utils.deprecation import RemovedInDjango20Warning from django.utils.encoding import force_text, smart_text from django.utils.safestring import mark_safe from django.utils.html import format_html @@ -158,7 +159,8 @@ class ForNode(Node): nodelist = [] if self.is_reversed: values = reversed(values) - unpack = len(self.loopvars) > 1 + num_loopvars = len(self.loopvars) + unpack = num_loopvars > 1 # Create a forloop value in the context. We'll update counters on each # iteration just below. loop_dict = context['forloop'] = {'parentloop': parentloop} @@ -177,6 +179,21 @@ class ForNode(Node): if unpack: # If there are multiple loop variables, unpack the item into # them. + + # To complete this deprecation, remove from here to the + # try/except block as well as the try/except itself, + # leaving `unpacked_vars = ...` and the "else" statements. + if not isinstance(item, (list, tuple)): + len_item = 1 + else: + len_item = len(item) + # Check loop variable count before unpacking + if num_loopvars != len_item: + warnings.warn( + "Need {0} values to unpack in for loop; got {1}. " + "This will raise an exception in Django 2.0." + .format(num_loopvars, len_item), + RemovedInDjango20Warning) try: unpacked_vars = dict(zip(self.loopvars, item)) except TypeError: diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 49eaa90ccb..a0d3b8a80c 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -22,6 +22,9 @@ about each item can often be found in the release notes of two versions prior. * ``SimpleTestCase.urls`` will be removed. +* Using an incorrect count of unpacked values in the ``for`` template tag + will raise an exception rather than fail silently. + .. _deprecation-removed-in-1.9: 1.9 diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index 46bbbdbb3e..79b7168030 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -270,3 +270,9 @@ removed in Django 2.0. Use :func:`@override_settings(ROOT_URLCONF=...) Related to the previous item, the ``prefix`` argument to :func:`django.conf.urls.i18n.i18n_patterns` has been deprecated. Simply pass a list of :func:`django.conf.urls.url` instances instead. + +Using an incorrect count of unpacked values in the :ttag:`for` template tag +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Using an incorrect count of unpacked values in :ttag:`for` tag will raise an +exception rather than fail silently in Django 2.0. diff --git a/tests/template_tests/tests.py b/tests/template_tests/tests.py index cd4a6b783a..17030c7560 100644 --- a/tests/template_tests/tests.py +++ b/tests/template_tests/tests.py @@ -602,7 +602,10 @@ class TemplateTests(TestCase): failures.append("Template test (Cached='%s', TEMPLATE_STRING_IF_INVALID='%s', TEMPLATE_DEBUG=%s): %s -- FAILED. Template loading invoked method that shouldn't have been invoked." % (is_cached, invalid_str, template_debug, name)) try: - output = self.render(test_template, vals) + with warnings.catch_warnings(): + # Ignore deprecations of using the wrong number of variables with the 'for' tag. + warnings.filterwarnings("ignore", category=RemovedInDjango20Warning, module="django.template.defaulttags") + output = self.render(test_template, vals) except ShouldNotExecuteException: failures.append("Template test (Cached='%s', TEMPLATE_STRING_IF_INVALID='%s', TEMPLATE_DEBUG=%s): %s -- FAILED. Template rendering invoked method that shouldn't have been invoked." % (is_cached, invalid_str, template_debug, name)) except ContextStackException: @@ -954,18 +957,21 @@ class TemplateTests(TestCase): 'for-tag-unpack08': ("{% for key,value, in items %}{{ key }}:{{ value }}/{% endfor %}", {"items": (('one', 1), ('two', 2))}, template.TemplateSyntaxError), # Ensure that a single loopvar doesn't truncate the list in val. 'for-tag-unpack09': ("{% for val in items %}{{ val.0 }}:{{ val.1 }}/{% endfor %}", {"items": (('one', 1), ('two', 2))}, "one:1/two:2/"), - # Otherwise, silently truncate if the length of loopvars differs to the length of each set of items. - 'for-tag-unpack10': ("{% for x,y in items %}{{ x }}:{{ y }}/{% endfor %}", {"items": (('one', 1, 'carrot'), ('two', 2, 'orange'))}, "one:1/two:2/"), - 'for-tag-unpack11': ("{% for x,y,z in items %}{{ x }}:{{ y }},{{ z }}/{% endfor %}", {"items": (('one', 1), ('two', 2))}, ("one:1,/two:2,/", "one:1,INVALID/two:2,INVALID/")), - 'for-tag-unpack12': ("{% for x,y,z in items %}{{ x }}:{{ y }},{{ z }}/{% endfor %}", {"items": (('one', 1, 'carrot'), ('two', 2))}, ("one:1,carrot/two:2,/", "one:1,carrot/two:2,INVALID/")), 'for-tag-unpack13': ("{% for x,y,z in items %}{{ x }}:{{ y }},{{ z }}/{% endfor %}", {"items": (('one', 1, 'carrot'), ('two', 2, 'cheese'))}, ("one:1,carrot/two:2,cheese/", "one:1,carrot/two:2,cheese/")), - 'for-tag-unpack14': ("{% for x,y in items %}{{ x }}:{{ y }}/{% endfor %}", {"items": (1, 2)}, (":/:/", "INVALID:INVALID/INVALID:INVALID/")), 'for-tag-empty01': ("{% for val in values %}{{ val }}{% empty %}empty text{% endfor %}", {"values": [1, 2, 3]}, "123"), 'for-tag-empty02': ("{% for val in values %}{{ val }}{% empty %}values array empty{% endfor %}", {"values": []}, "values array empty"), 'for-tag-empty03': ("{% for val in values %}{{ val }}{% empty %}values array not found{% endfor %}", {}, "values array not found"), # Ticket 19882 'for-tag-filter-ws': ("{% load custom %}{% for x in s|noop:'x y' %}{{ x }}{% endfor %}", {'s': 'abc'}, 'abc'), + # These tests raise deprecation warnings and will raise an exception + # in Django 2.0. The existing behavior is silent truncation if the + # length of loopvars differs to the length of each set of items. + 'for-tag-unpack10': ("{% for x,y in items %}{{ x }}:{{ y }}/{% endfor %}", {"items": (('one', 1, 'carrot'), ('two', 2, 'orange'))}, "one:1/two:2/"), + 'for-tag-unpack11': ("{% for x,y,z in items %}{{ x }}:{{ y }},{{ z }}/{% endfor %}", {"items": (('one', 1), ('two', 2))}, ("one:1,/two:2,/", "one:1,INVALID/two:2,INVALID/")), + 'for-tag-unpack12': ("{% for x,y,z in items %}{{ x }}:{{ y }},{{ z }}/{% endfor %}", {"items": (('one', 1, 'carrot'), ('two', 2))}, ("one:1,carrot/two:2,/", "one:1,carrot/two:2,INVALID/")), + 'for-tag-unpack14': ("{% for x,y in items %}{{ x }}:{{ y }}/{% endfor %}", {"items": (1, 2)}, (":/:/", "INVALID:INVALID/INVALID:INVALID/")), + ### IF TAG ################################################################ 'if-tag01': ("{% if foo %}yes{% else %}no{% endif %}", {"foo": True}, "yes"), 'if-tag02': ("{% if foo %}yes{% else %}no{% endif %}", {"foo": False}, "no"),