Opened 3 years ago

Last modified 9 months ago

#24199 new Cleanup/optimization

string_if_invalid doesn't provide information in many cases

Reported by: adam-iris Owned by: nobody
Component: Template system Version: master
Severity: Normal Keywords:
Cc: django@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

There's a check for "%s" and variable information is added, but only when handling VariableDoesNotExist for simple variables:

            except VariableDoesNotExist:
                if ignore_failures:
                    obj = None
                else:
                    string_if_invalid = context.engine.string_if_invalid
                    if string_if_invalid:
                        if '%s' in string_if_invalid:
                            return string_if_invalid % self.var
                        else:
                            return string_if_invalid
                    else:
                        obj = string_if_invalid

But string_if_invalid is also used in other cases, for example if a TypeError is raised from a method:

                if callable(current):
                    if getattr(current, 'do_not_call_in_templates', False):
                        pass
                    elif getattr(current, 'alters_data', False):
                        current = context.engine.string_if_invalid
                    else:
                        try:  # method call (assuming no args required)
                            current = current()
                        except TypeError:
                            try:
                                getcallargs(current)
                            except TypeError:  # arguments *were* required
                                current = context.engine.string_if_invalid  # invalid method call

In those cases, there's no check for "%s" and nothing about the method or the exception is included, making this largely useless for debugging.

Change History (3)

comment:1 Changed 3 years ago by Tim Graham

The documentation discourages the use of string_if_invalid. I'm not sure there's much value in trying to expand its usefulness, but I'll leave the ticket for a second opinion.

comment:2 Changed 3 years ago by Aymeric Augustin

Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

Could we refactor this code into a method of Engine?

    def invalid_value(self, value):
        # ...

comment:3 Changed 9 months ago by Daniel Hahler

Cc: django@… added

Just for information: pytest-django provides a helper (--fail-on-template-vars) that enables this.
It goes through some hoops to get to the required information (template): https://github.com/pytest-dev/pytest-django/blob/ca3b5a2498ac2714f22305fded1c1518a66a07f6/pytest_django/plugin.py#L476.

I've looked at it when trying to skip it with the default filter, but then created a PR for Django directly: https://github.com/django/django/pull/8200.

Note: See TracTickets for help on using tickets.
Back to Top