[Django] #35233: Push templates checks down to backend engine classes

[Django]

#35233: Push templates checks down to backend engine classes
------------------------------------------------+------------------------
Reporter: Adam Johnson | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (System checks) | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
Currently, the three system checks for template settings are individual
functions in
[https://github.com/django/django/blob/main/django/core/checks/templates.py
django.core.checks.templates]. This structure leads to some issues:

1. The checks are specific to DTL (Django Template Language), but get run
for all backends. DTL-specific code gets loaded and run even when not
using it, notably the fairly slow `get_template_tag_modules()`.
2. `check_for_template_tags_with_the_same_name` is inaccurate because it
combines `libraries` from all template backends, so might report an issue
when none exists.
3. The checks are still run when no template backend is configured
(`settings.TEMPLATES = []`).

I propose the checks be restructured to live within the engine classes in
`django.template.backends`, adopting the same pattern used for model and
field checks, admin checks, etc.

In practice, this would mean adding a dummy `BaseEngine.check()` method,
then moving the existing checks into methods called by an overridden
`DjangoTemplates.check()`. A single function left in
`django.core.checks.templates` would loop over
`django.template.engines.all()` to combine results from backends’
`check()` methods.
--
Ticket URL: <https://code.djangoproject.com/ticket/35233>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35233: Push templates checks down to backend engine classes

--------------------------------------+------------------------------------

Reporter: Adam Johnson | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (System checks) | Version: dev

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by Simon Charette):

* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/35233#comment:1>

[Django]

#35233: Push templates checks down to backend engine classes

-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Giannis
Type: | Terzopoulos
Cleanup/optimization | Status: assigned
Component: Core (System | Version: dev
checks) |

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Giannis Terzopoulos):

* owner: nobody => Giannis Terzopoulos
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35233#comment:2>

[Django]

#35233: Push templates checks down to backend engine classes
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Giannis
Type: | Terzopoulos
Cleanup/optimization | Status: assigned
Component: Core (System | Version: dev
checks) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Adam Johnson):

The staticfiles finders checks are actually the perfect template:

https://github.com/django/django/blob/35e63b7f2896959d3f1939346fd467af92f0d80f/django/contrib/staticfiles/checks.py#L11-L21
https://github.com/django/django/blob/35e63b7f2896959d3f1939346fd467af92f0d80f/django/contrib/staticfiles/finders.py#L23-L27
https://github.com/django/django/blob/35e63b7f2896959d3f1939346fd467af92f0d80f/django/contrib/staticfiles/finders.py#L74
--
Ticket URL: <https://code.djangoproject.com/ticket/35233#comment:3>

[Django]

#35233: Push templates checks down to backend engine classes
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Giannis
Type: | Terzopoulos
Cleanup/optimization | Status: assigned
Component: Core (System | Version: dev
checks) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Giannis Terzopoulos):

Thank you for the pointers Adam! I finally found the time to look into
this and I have some questions.

> 2. `check_for_template_tags_with_the_same_name` is inaccurate because it
combines `libraries` from all template backends, so might report an issue
when none exists.

Regarding this, am I right in assuming that we want to allow same names
for tags across different engines; Should this test pass?

{{{
#!python
def test_different_backends_may_have_same_tags(self):
with self.settings(
TEMPLATES=[
{"BACKEND": "django.template.backends.django.DjangoTemplates",
"OPTIONS": {"libraries": {
"same_tags":
"check_framework.template_test_apps.same_tags_app_1"
".templatetags.same_tags"}}},
{"BACKEND": "django.template.backends.OtherBackend",
"OPTIONS": {"libraries": {
"same_tags":
"check_framework.template_test_apps.same_tags_app_2"
".templatetags.same_tags"}}}
]
):
self.assertListEqual(check_for_template_tags_with_the_same_name(None), [])
}}}

And one more thing, I am noticing that the `E001` logic is duplicated:
- in `Engine.__init__` - raises `ImproperlyConfigured`
(`template_backends.test_utils.TemplateUtilsTests.test_backend_improperly_configured`)
- and in `check_setting_app_dirs_loaders` (tests under
`check_framework.test_templates.CheckTemplateSettingsAppDirsTest`)
and I am wondering if it's enough to keep one of the two, perhaps the
check? 🤔
--
Ticket URL: <https://code.djangoproject.com/ticket/35233#comment:4>

[Django]

#35233: Push templates checks down to backend engine classes
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Giannis
Type: | Terzopoulos
Cleanup/optimization | Status: assigned
Component: Core (System | Version: dev
checks) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Adam Johnson):

> Regarding this, am I right in assuming that we want to allow same names
for tags across different engines; Should this test pass?

Yes, I think so. You can use two `DjangoTemplates` backends and it would
still be valid.

> And one more thing, I am noticing that the E001 logic is duplicated:

Indeed, that is true. The system check was added in #24922 because the
exception would only appear at runtime, because the backend would only be
created lazily. But by actioning this ticket, we’ll force backends to all
be instantiated during checks, which will make the exception appear then.
So we should be able to remove E001 as part of this work.
--
Ticket URL: <https://code.djangoproject.com/ticket/35233#comment:5>

[Django]

#35233: Push templates checks down to backend engine classes
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Giannis
Type: | Terzopoulos
Cleanup/optimization | Status: assigned
Component: Core (System | Version: dev
checks) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Giannis Terzopoulos):

* has_patch: 0 => 1

Comment:

Thank you for the explanation. I opened a PR for this
[https://github.com/django/django/pull/17981 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/35233#comment:6>

[Django]

#35233: Push templates checks down to backend engine classes
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Giannis
Type: | Terzopoulos
Cleanup/optimization | Status: assigned
Component: Core (System | Version: dev
checks) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35233#comment:7>

[Django]

#35233: Push templates checks down to backend engine classes
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Giannis
Type: | Terzopoulos
Cleanup/optimization | Status: assigned
Component: Core (System | Version: dev
checks) |
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Adam Johnson):

* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/35233#comment:8>

[Django]

#35233: Push templates checks down to backend engine classes
-------------------------------------+-------------------------------------
Reporter: Adam Johnson | Owner: Giannis
Type: | Terzopoulos

Cleanup/optimization | Status: closed

Component: Core (System | Version: dev
checks) |

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"d658a3162fbeb68d148d1b2fcf4da4fe1437eddb" d658a31]:
{{{#!CommitTicketReference repository=""
revision="d658a3162fbeb68d148d1b2fcf4da4fe1437eddb"
Fixed #35233 -- Moved template engine system checks to backend methods.

Thanks Adam Johnson for reviews.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35233#comment:9>