Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: can't reuse a variable name in a {% for %} loop #640

Closed
timgraham opened this issue Jan 7, 2017 · 6 comments

Comments

@timgraham
Copy link

commented Jan 7, 2017

Hi, I bisected the introduction of some failures in Django's test suite to 1205e64:

$ ./tests/runtests.py forms_tests.widget_tests.test_multiplehiddeninput.MultipleHiddenInputTest.test_render_attrs

Traceback (most recent call last):
  File "/var/lib/jenkins/workspace/django-security-master/database/postgres/python/python3.6/tests/forms_tests/widget_tests/test_splithiddendatetimewidget.py", line 21, in test_render_value
    '<input type="hidden" name="date_0" value="2007-09-17" />'
  File "/var/lib/jenkins/workspace/django-security-master/database/postgres/python/python3.6/tests/forms_tests/widget_tests/base.py", line 23, in check_html
    output = widget.render(name, value, attrs=attrs, renderer=self.jinja2_renderer, **kwargs)
  File "/var/lib/jenkins/workspace/django-security-master/database/postgres/python/python3.6/django/forms/widgets.py", line 222, in render
    return self._render(self.template_name, context, renderer)
  File "/var/lib/jenkins/workspace/django-security-master/database/postgres/python/python3.6/django/forms/widgets.py", line 227, in _render
    return mark_safe(renderer.render(template_name, context))
  File "/var/lib/jenkins/workspace/django-security-master/database/postgres/python/python3.6/django/forms/renderers.py", line 32, in render
    return template.render(context, request=request).strip()
  File "/var/lib/jenkins/workspace/django-security-master/database/postgres/python/python3.6/django/template/backends/jinja2.py", line 81, in render
    return self.template.render(context)
  File "/var/lib/jenkins/workspace/django-security-master/database/postgres/python/python3.6/tests/.env/lib/python3.6/site-packages/jinja2/asyncsupport.py", line 76, in render
    return original_render(self, *args, **kwargs)
  File "/var/lib/jenkins/workspace/django-security-master/database/postgres/python/python3.6/tests/.env/lib/python3.6/site-packages/jinja2/environment.py", line 1008, in render
    return self.environment.handle_exception(exc_info, True)
  File "/var/lib/jenkins/workspace/django-security-master/database/postgres/python/python3.6/tests/.env/lib/python3.6/site-packages/jinja2/environment.py", line 780, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/var/lib/jenkins/workspace/django-security-master/database/postgres/python/python3.6/tests/.env/lib/python3.6/site-packages/jinja2/_compat.py", line 37, in reraise
    raise value.with_traceback(tb)
  File "/var/lib/jenkins/workspace/django-security-master/database/postgres/python/python3.6/django/forms/jinja2/django/forms/widgets/splithiddendatetime.html", line 1, in top-level template code
    {% include 'django/forms/widgets/multiwidget.html' %}
  File "/var/lib/jenkins/workspace/django-security-master/database/postgres/python/python3.6/django/forms/jinja2/django/forms/widgets/multiwidget.html", line 1, in top-level template code
    {% for widget in widget.subwidgets %}{% include widget.template_name %}{% endfor %}
UnboundLocalError: local variable 'l_1_widget' referenced before assignment

Let me know if I can help or provide additional info.

@davidism

This comment has been minimized.

Copy link
Member

commented Jan 7, 2017

Please include a self-contained example that produces this error.

@timgraham

This comment has been minimized.

Copy link
Author

commented Jan 7, 2017

Okay, I'm trying to write a test now. Wasn't sure if the bug would be obvious given the traceback.

@timgraham

This comment has been minimized.

Copy link
Author

commented Jan 7, 2017

I think the issue is the construct {% for widget in widget.subwidgets %} -- the widget name is reused.

@timgraham

This comment has been minimized.

Copy link
Author

commented Jan 7, 2017

Here's a test:

diff --git a/tests/test_core_tags.py b/tests/test_core_tags.py
index 0438008..6cc7dac 100644
--- a/tests/test_core_tags.py
+++ b/tests/test_core_tags.py
@@ -22,6 +22,10 @@ def env_trim():
 @pytest.mark.for_loop
 class TestForLoop(object):
 
+    def test_simple_reuse(self, env):
+        tmpl = env.from_string('{% for item in item %}{{ item }}{% endfor %}')
+        assert tmpl.render(item=list(range(10))) == '0123456789'
+
     def test_simple(self, env):
         tmpl = env.from_string('{% for item in seq %}{{ item }}{% endfor %}')
         assert tmpl.render(seq=list(range(10))) == '0123456789'

It errors out with UnboundLocalError: local variable 'l_1_item' referenced before assignment.

@ThiefMaster

This comment has been minimized.

Copy link
Member

commented Jan 7, 2017

While this looks a lot like a bug that should be fixed, is it really a good idea to write such code? Especially considering that while Jinja (probably) gets the scoping right, Python itself doesn't (since it has no block scope):

>>> foo = [1, 2, 3]
>>> for foo in foo:
...     print foo
...
1
2
3
>>> foo
3
@timgraham

This comment has been minimized.

Copy link
Author

commented Jan 7, 2017

I'm a maintainer of Django and don't have much experience with Jinja2 (and didn't write the template). In our case, I'm not sure what the alternative is:
{% for widget in widget.subwidgets %}{% include widget.template_name %}{% endfor %}
the included template relies on the variable being named widget.

@squidfunk squidfunk referenced this issue Jan 7, 2017

Closed

Status of 1.0.0 #46

51 of 56 tasks complete

timgraham added a commit to django/django that referenced this issue Jan 7, 2017

Pinned jinja2<2.9 in test requirements.
Kept the build green until a regression is fixed:
pallets/jinja#640

@timgraham timgraham changed the title Regression in Django tests: UnboundLocalError: local variable 'l_1_widget' referenced before assignment Regression: can't reuse a variable name in a {% for %} loop Jan 7, 2017

@mitsuhiko mitsuhiko self-assigned this Jan 7, 2017

@mitsuhiko mitsuhiko closed this in e302252 Jan 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.