Fixed #24977 -- Made template tags treat undefined variables not equal to None. #7901
Conversation
django/template/base.py
Outdated
| self.string_rep = string_rep | ||
|
|
||
| def __nonzero__(self): | ||
| return False |
There was a problem hiding this comment.
Use __bool__ as master is Python 3 only.
django/template/base.py
Outdated
| return self.string_rep | ||
|
|
||
| def __str__(self): | ||
| return unicode(self).encode('utf-8') |
There was a problem hiding this comment.
Make __str__ return self.string_rep and nuke __unicode__.
django/template/base.py
Outdated
| obj = string_if_invalid | ||
| obj = UndefinedVariable(string_if_invalid) | ||
| else: | ||
| obj = UndefinedVariable(string_if_invalid) |
There was a problem hiding this comment.
Might want to pass an explicit empty string here or make UndefinedVariable.string_rep default to it and don't pass any arg along.
django/template/base.py
Outdated
| return isinstance(other, UndefinedVariable) | ||
|
|
||
| def __ne__(self, other): | ||
| return not isinstance(other, UndefinedVariable) |
There was a problem hiding this comment.
Not required as __ne__ defaults to not self.__eq__(other) on Python 3.
| @@ -682,22 +714,19 @@ def __init__(self, token, parser): | |||
| self.filters = filters | |||
| self.var = var_obj | |||
|
|
|||
| def resolve(self, context, ignore_failures=False): | |||
| def resolve(self, context): | |||
There was a problem hiding this comment.
I guess ignore_failures should go through a deprecation path?
There was a problem hiding this comment.
I was assuming this wasn't necessary since FilterExpression isn't part of the public API, and all the internal usages have been updated.
There was a problem hiding this comment.
Some discussion in the "backwards incompatible changes" section of docs/releases/2.0.txt summarizing these changes would be useful. While these changes might be considered bug fixes, I could imagine they might affect a few projects that are inadvertently broken.
| else: | ||
| with self.assertRaises(TemplateSyntaxError): | ||
| self.engine.render_to_string('exception02') | ||
| with self.assertRaises(TemplateSyntaxError) as exception_ctx: |
There was a problem hiding this comment.
You can use assertRaisesMessage here rather than checking the message separately.
| @setup({'if-tag-invalid-member': '{% if x.non_existent == None %}yes{% else %}no{% endif %}'}) | ||
| def test_if_tag_invalid_member(self): | ||
| """ | ||
| Invalid members should not be regarded as equal to None |
| @@ -166,7 +166,7 @@ def test_url_fail09(self): | |||
|
|
|||
| @setup({'url-fail11': '{% url named_url %}'}) | |||
| def test_url_fail11(self): | |||
| with self.assertRaises(NoReverseMatch): | |||
| with self.assertRaises(TemplateSyntaxError): | |||
docs/ref/templates/builtins.txt
Outdated
| @@ -1501,6 +1501,11 @@ For example:: | |||
|
|
|||
| If ``value`` is ``None``, the output will be the string ``"nothing"``. | |||
|
|
|||
| .. versionchanged:: 2.0 | |||
There was a problem hiding this comment.
Given that the behavior of undefined variables isn't documented, I don't see a need for this note unless you also add that documentation. The note should be self-contained as described in https://docs.djangoproject.com/en/dev/internals/contributing/writing-documentation/#documenting-new-features.
django/template/base.py
Outdated
|
|
||
| def __init__(self, string_rep=""): | ||
| """ | ||
| :param string_rep: The string value that this expression should |
There was a problem hiding this comment.
We don't use JavaDoc style.
`string_repo` is the ...
would be fine. Maybe call it var_repr (usually variables aren't named after the type that they are).
timgraham
changed the title
The template engine no longer treats undefined variables as being None, but instead as an instance of a special UndefinedVariable class.
There was a problem hiding this comment.
I left an idea on the mailing list thread about possible security implications that this change could have.
It could be useful to break this up into separate commits, one for each tag that's changed. It should make it easier to review and some changes like {% url %} are more straightforward and less risky than {% if %}.
| else: | ||
| with self.assertRaises(TemplateSyntaxError): | ||
| self.engine.render_to_string('exception02') | ||
| with self.assertRaisesRegexp(TemplateSyntaxError, |
There was a problem hiding this comment.
Use:
msg = "Got this from the 'nonexistent' variable"
with self.assertRaisesMessage(TemplateSyntaxError, msg):
`assertRraisesMessage
| context. All instances are treated as being equal to each other. | ||
| """ | ||
|
|
||
| def __init__(self, var_rep=""): |
There was a problem hiding this comment.
I think var_repr is more explanatory than just rep. Also, use single quotes rather than double quotes unless the string contains a single quote.
| @@ -599,6 +601,14 @@ def test_if_is_not_variable_missing(self): | |||
| @setup({'template': '{% if foo is not bar %}yes{% else %}no{% endif %}'}) | |||
| def test_if_is_not_both_variables_missing(self): | |||
| output = self.engine.render_to_string('template', {}) | |||
| self.assertEqual(output, 'yes') | |||
|
|
|||
| @setup({'if-tag-invalid-member': '{% if x.non_existent == None %}yes{% else %}no{% endif %}'}) | |||
| @@ -271,6 +271,25 @@ If you wish to keep this restriction in the admin when editing users, set | |||
| admin.site.unregister(User) | |||
| admin.site.register(User, MyUserAdmin) | |||
|
|
|||
| Undefined variables in templates are distinguished from `None` | |||
There was a problem hiding this comment.
double backticks must be used for correct formatting of the rendered docs
| Undefined variables in templates are distinguished from `None` | ||
| -------------------------------------------------------------- | ||
|
|
||
| In templates, undefined values and non-existent attributes of objects |
|
Closing per ticket (wontfix). |
The template engine no longer treats undefined variables as being None,
but instead as an instance of a special UndefinedVariable class.