diff --git a/django/core/checks/templates.py b/django/core/checks/templates.py index 681aa1f31722..72a3212e560b 100644 --- a/django/core/checks/templates.py +++ b/django/core/checks/templates.py @@ -1,75 +1,12 @@ -import copy -from collections import defaultdict - -from django.conf import settings -from django.template.backends.django import get_template_tag_modules - -from . import Error, Tags, Warning, register - -E001 = Error( - "You have 'APP_DIRS': True in your TEMPLATES but also specify 'loaders' " - "in OPTIONS. Either remove APP_DIRS or remove the 'loaders' option.", - id="templates.E001", -) -E002 = Error( - "'string_if_invalid' in TEMPLATES OPTIONS must be a string but got: {} ({}).", - id="templates.E002", -) -W003 = Warning( - "{} is used for multiple template tag modules: {}", - id="templates.E003", -) +from . import Tags, register @register(Tags.templates) -def check_setting_app_dirs_loaders(app_configs, **kwargs): - return ( - [E001] - if any( - conf.get("APP_DIRS") and "loaders" in conf.get("OPTIONS", {}) - for conf in settings.TEMPLATES - ) - else [] - ) - - -@register(Tags.templates) -def check_string_if_invalid_is_string(app_configs, **kwargs): - errors = [] - for conf in settings.TEMPLATES: - string_if_invalid = conf.get("OPTIONS", {}).get("string_if_invalid", "") - if not isinstance(string_if_invalid, str): - error = copy.copy(E002) - error.msg = error.msg.format( - string_if_invalid, type(string_if_invalid).__name__ - ) - errors.append(error) - return errors - +def check_templates(app_configs, **kwargs): + """Check all registered template engines.""" + from django.template import engines -@register(Tags.templates) -def check_for_template_tags_with_the_same_name(app_configs, **kwargs): errors = [] - libraries = defaultdict(set) - - for conf in settings.TEMPLATES: - custom_libraries = conf.get("OPTIONS", {}).get("libraries", {}) - for module_name, module_path in custom_libraries.items(): - libraries[module_name].add(module_path) - - for module_name, module_path in get_template_tag_modules(): - libraries[module_name].add(module_path) - - for library_name, items in libraries.items(): - if len(items) > 1: - errors.append( - Warning( - W003.msg.format( - repr(library_name), - ", ".join(repr(item) for item in sorted(items)), - ), - id=W003.id, - ) - ) - + for engine in engines.all(): + errors.extend(engine.check()) return errors diff --git a/django/template/backends/base.py b/django/template/backends/base.py index 991ce64cb75e..f08eb2464be0 100644 --- a/django/template/backends/base.py +++ b/django/template/backends/base.py @@ -23,6 +23,9 @@ def __init__(self, params): "Unknown parameters: {}".format(", ".join(params)) ) + def check(self, **kwargs): + return [] + @property def app_dirname(self): raise ImproperlyConfigured( diff --git a/django/template/backends/django.py b/django/template/backends/django.py index ba561bba9f61..cf6874c408ef 100644 --- a/django/template/backends/django.py +++ b/django/template/backends/django.py @@ -1,8 +1,10 @@ +from collections import defaultdict from importlib import import_module from pkgutil import walk_packages from django.apps import apps from django.conf import settings +from django.core.checks import Error, Warning from django.template import TemplateDoesNotExist from django.template.context import make_context from django.template.engine import Engine @@ -25,6 +27,50 @@ def __init__(self, params): super().__init__(params) self.engine = Engine(self.dirs, self.app_dirs, **options) + def check(self, **kwargs): + return [ + *self._check_string_if_invalid_is_string(), + *self._check_for_template_tags_with_the_same_name(), + ] + + def _check_string_if_invalid_is_string(self): + value = self.engine.string_if_invalid + if not isinstance(value, str): + return [ + Error( + "'string_if_invalid' in TEMPLATES OPTIONS must be a string but " + "got: %r (%s)." % (value, type(value)), + obj=self, + id="templates.E002", + ) + ] + return [] + + def _check_for_template_tags_with_the_same_name(self): + libraries = defaultdict(set) + + for module_name, module_path in get_template_tag_modules(): + libraries[module_name].add(module_path) + + for module_name, module_path in self.engine.libraries.items(): + libraries[module_name].add(module_path) + + errors = [] + + for library_name, items in libraries.items(): + if len(items) > 1: + items = ", ".join(repr(item) for item in sorted(items)) + errors.append( + Warning( + f"{library_name!r} is used for multiple template tag modules: " + f"{items}", + obj=self, + id="templates.W003", + ) + ) + + return errors + def from_string(self, template_code): return Template(self.engine.from_string(template_code), self) diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index cf0ab32efa74..efc8cf666a23 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -575,7 +575,9 @@ configured: * **templates.E001**: You have ``'APP_DIRS': True`` in your :setting:`TEMPLATES` but also specify ``'loaders'`` in ``OPTIONS``. Either - remove ``APP_DIRS`` or remove the ``'loaders'`` option. + remove ``APP_DIRS`` or remove the ``'loaders'`` option. *This check is + removed in Django 5.1 as system checks may now raise* + ``ImproperlyConfigured`` *instead.* * **templates.E002**: ``string_if_invalid`` in :setting:`TEMPLATES` :setting:`OPTIONS ` must be a string but got: ``{value}`` (``{type}``). diff --git a/docs/releases/5.1.txt b/docs/releases/5.1.txt index 2dbacf08f746..fbb05d754873 100644 --- a/docs/releases/5.1.txt +++ b/docs/releases/5.1.txt @@ -307,6 +307,9 @@ Templates example, to generate a link to the next page while keeping any filtering options in place. +* :ref:`Template engines ` now implement a ``check()`` method + that is already registered with the check framework. + Tests ~~~~~ diff --git a/docs/topics/checks.txt b/docs/topics/checks.txt index 3e3bbe19d658..94ba66f0db9a 100644 --- a/docs/topics/checks.txt +++ b/docs/topics/checks.txt @@ -130,18 +130,18 @@ The code below is equivalent to the code above:: .. _field-checking: -Field, model, manager, and database checks ------------------------------------------- +Field, model, manager, template engine, and database checks +----------------------------------------------------------- In some cases, you won't need to register your check function -- you can piggyback on an existing registration. -Fields, models, model managers, and database backends all implement a -``check()`` method that is already registered with the check framework. If you -want to add extra checks, you can extend the implementation on the base class, -perform any extra checks you need, and append any messages to those generated -by the base class. It's recommended that you delegate each check to separate -methods. +Fields, models, model managers, template engines, and database backends all +implement a ``check()`` method that is already registered with the check +framework. If you want to add extra checks, you can extend the implementation +on the base class, perform any extra checks you need, and append any messages +to those generated by the base class. It's recommended that you delegate each +check to separate methods. Consider an example where you are implementing a custom field named ``RangedIntegerField``. This field adds ``min`` and ``max`` arguments to the @@ -195,6 +195,10 @@ the only difference is that the check is a classmethod, not an instance method:: # ... your own checks ... return errors +.. versionchanged:: 5.1 + + In older versions, template engines didn't implement a ``check()`` method. + Writing tests ------------- diff --git a/tests/check_framework/test_templates.py b/tests/check_framework/test_templates.py index c8a2f83b8a34..18f705ddb8e4 100644 --- a/tests/check_framework/test_templates.py +++ b/tests/check_framework/test_templates.py @@ -1,128 +1,105 @@ -from copy import copy, deepcopy - -from django.core.checks import Warning -from django.core.checks.templates import ( - E001, - E002, - W003, - check_for_template_tags_with_the_same_name, - check_setting_app_dirs_loaders, - check_string_if_invalid_is_string, -) +from copy import deepcopy +from itertools import chain + +from django.core.checks import Error, Warning +from django.core.checks.templates import check_templates +from django.template import engines +from django.template.backends.base import BaseEngine from django.test import SimpleTestCase from django.test.utils import override_settings -class CheckTemplateSettingsAppDirsTest(SimpleTestCase): - TEMPLATES_APP_DIRS_AND_LOADERS = [ - { - "BACKEND": "django.template.backends.django.DjangoTemplates", - "APP_DIRS": True, - "OPTIONS": { - "loaders": ["django.template.loaders.filesystem.Loader"], - }, - }, - ] +class ErrorEngine(BaseEngine): + def __init__(self, params): + params.pop("OPTIONS") + super().__init__(params) - @override_settings(TEMPLATES=TEMPLATES_APP_DIRS_AND_LOADERS) - def test_app_dirs_and_loaders(self): - """ - Error if template loaders are specified and APP_DIRS is True. - """ - self.assertEqual(check_setting_app_dirs_loaders(None), [E001]) + def check(self, **kwargs): + return [Error("Example")] - def test_app_dirs_removed(self): - TEMPLATES = deepcopy(self.TEMPLATES_APP_DIRS_AND_LOADERS) - del TEMPLATES[0]["APP_DIRS"] - with self.settings(TEMPLATES=TEMPLATES): - self.assertEqual(check_setting_app_dirs_loaders(None), []) - def test_loaders_removed(self): - TEMPLATES = deepcopy(self.TEMPLATES_APP_DIRS_AND_LOADERS) - del TEMPLATES[0]["OPTIONS"]["loaders"] - with self.settings(TEMPLATES=TEMPLATES): - self.assertEqual(check_setting_app_dirs_loaders(None), []) +class CheckTemplatesTests(SimpleTestCase): + @override_settings( + TEMPLATES=[ + {"BACKEND": f"{__name__}.{ErrorEngine.__qualname__}", "NAME": "backend_1"}, + {"BACKEND": f"{__name__}.{ErrorEngine.__qualname__}", "NAME": "backend_2"}, + ] + ) + def test_errors_aggregated(self): + errors = check_templates(None) + self.assertEqual(errors, [Error("Example")] * 2) class CheckTemplateStringIfInvalidTest(SimpleTestCase): TEMPLATES_STRING_IF_INVALID = [ { "BACKEND": "django.template.backends.django.DjangoTemplates", + "NAME": "backend_1", "OPTIONS": { "string_if_invalid": False, }, }, { "BACKEND": "django.template.backends.django.DjangoTemplates", + "NAME": "backend_2", "OPTIONS": { "string_if_invalid": 42, }, }, ] - @classmethod - def setUpClass(cls): - super().setUpClass() - cls.error1 = copy(E002) - cls.error2 = copy(E002) - string_if_invalid1 = cls.TEMPLATES_STRING_IF_INVALID[0]["OPTIONS"][ - "string_if_invalid" - ] - string_if_invalid2 = cls.TEMPLATES_STRING_IF_INVALID[1]["OPTIONS"][ - "string_if_invalid" - ] - cls.error1.msg = cls.error1.msg.format( - string_if_invalid1, type(string_if_invalid1).__name__ + def _get_error_for_engine(self, engine): + value = engine.engine.string_if_invalid + return Error( + "'string_if_invalid' in TEMPLATES OPTIONS must be a string but got: %r " + "(%s)." % (value, type(value)), + obj=engine, + id="templates.E002", ) - cls.error2.msg = cls.error2.msg.format( - string_if_invalid2, type(string_if_invalid2).__name__ + + def _check_engines(self, engines): + return list( + chain.from_iterable(e._check_string_if_invalid_is_string() for e in engines) ) @override_settings(TEMPLATES=TEMPLATES_STRING_IF_INVALID) def test_string_if_invalid_not_string(self): - self.assertEqual( - check_string_if_invalid_is_string(None), [self.error1, self.error2] - ) + _engines = engines.all() + errors = [ + self._get_error_for_engine(_engines[0]), + self._get_error_for_engine(_engines[1]), + ] + self.assertEqual(self._check_engines(_engines), errors) def test_string_if_invalid_first_is_string(self): TEMPLATES = deepcopy(self.TEMPLATES_STRING_IF_INVALID) TEMPLATES[0]["OPTIONS"]["string_if_invalid"] = "test" with self.settings(TEMPLATES=TEMPLATES): - self.assertEqual(check_string_if_invalid_is_string(None), [self.error2]) + _engines = engines.all() + errors = [self._get_error_for_engine(_engines[1])] + self.assertEqual(self._check_engines(_engines), errors) def test_string_if_invalid_both_are_strings(self): TEMPLATES = deepcopy(self.TEMPLATES_STRING_IF_INVALID) TEMPLATES[0]["OPTIONS"]["string_if_invalid"] = "test" TEMPLATES[1]["OPTIONS"]["string_if_invalid"] = "test" with self.settings(TEMPLATES=TEMPLATES): - self.assertEqual(check_string_if_invalid_is_string(None), []) + self.assertEqual(self._check_engines(engines.all()), []) def test_string_if_invalid_not_specified(self): TEMPLATES = deepcopy(self.TEMPLATES_STRING_IF_INVALID) del TEMPLATES[1]["OPTIONS"]["string_if_invalid"] with self.settings(TEMPLATES=TEMPLATES): - self.assertEqual(check_string_if_invalid_is_string(None), [self.error1]) + _engines = engines.all() + errors = [self._get_error_for_engine(_engines[0])] + self.assertEqual(self._check_engines(_engines), errors) class CheckTemplateTagLibrariesWithSameName(SimpleTestCase): - @classmethod - def setUpClass(cls): - super().setUpClass() - cls.warning_same_tags = Warning( - W003.msg.format( - "'same_tags'", - "'check_framework.template_test_apps.same_tags_app_1." - "templatetags.same_tags', " - "'check_framework.template_test_apps.same_tags_app_2." - "templatetags.same_tags'", - ), - id=W003.id, - ) - - @staticmethod - def get_settings(module_name, module_path): + def get_settings(self, module_name, module_path, name="django"): return { "BACKEND": "django.template.backends.django.DjangoTemplates", + "NAME": name, "OPTIONS": { "libraries": { module_name: f"check_framework.template_test_apps.{module_path}", @@ -130,6 +107,20 @@ def get_settings(module_name, module_path): }, } + def _get_error_for_engine(self, engine, modules): + return Warning( + f"'same_tags' is used for multiple template tag modules: {modules}", + obj=engine, + id="templates.W003", + ) + + def _check_engines(self, engines): + return list( + chain.from_iterable( + e._check_for_template_tags_with_the_same_name() for e in engines + ) + ) + @override_settings( INSTALLED_APPS=[ "check_framework.template_test_apps.same_tags_app_1", @@ -137,26 +128,32 @@ def get_settings(module_name, module_path): ] ) def test_template_tags_with_same_name(self): - self.assertEqual( - check_for_template_tags_with_the_same_name(None), - [self.warning_same_tags], + _engines = engines.all() + modules = ( + "'check_framework.template_test_apps.same_tags_app_1.templatetags" + ".same_tags', 'check_framework.template_test_apps.same_tags_app_2" + ".templatetags.same_tags'" ) + errors = [self._get_error_for_engine(_engines[0], modules)] + self.assertEqual(self._check_engines(_engines), errors) - def test_template_tags_with_same_library_name(self): + def test_template_tags_for_separate_backends(self): + # The "libraries" names are the same, but the backends are different. with self.settings( TEMPLATES=[ self.get_settings( - "same_tags", "same_tags_app_1.templatetags.same_tags" + "same_tags", + "same_tags_app_1.templatetags.same_tags", + name="backend_1", ), self.get_settings( - "same_tags", "same_tags_app_2.templatetags.same_tags" + "same_tags", + "same_tags_app_2.templatetags.same_tags", + name="backend_2", ), ] ): - self.assertEqual( - check_for_template_tags_with_the_same_name(None), - [self.warning_same_tags], - ) + self.assertEqual(self._check_engines(engines.all()), []) @override_settings( INSTALLED_APPS=["check_framework.template_test_apps.same_tags_app_1"] @@ -169,48 +166,44 @@ def test_template_tags_same_library_in_installed_apps_libraries(self): ), ] ): - self.assertEqual(check_for_template_tags_with_the_same_name(None), []) + self.assertEqual(self._check_engines(engines.all()), []) @override_settings( INSTALLED_APPS=["check_framework.template_test_apps.same_tags_app_1"] ) def test_template_tags_with_same_library_name_and_module_name(self): + modules = ( + "'check_framework.template_test_apps.different_tags_app.templatetags" + ".different_tags', 'check_framework.template_test_apps.same_tags_app_1" + ".templatetags.same_tags'" + ) with self.settings( TEMPLATES=[ self.get_settings( - "same_tags", - "different_tags_app.templatetags.different_tags", + "same_tags", "different_tags_app.templatetags.different_tags" ), ] ): - self.assertEqual( - check_for_template_tags_with_the_same_name(None), - [ - Warning( - W003.msg.format( - "'same_tags'", - "'check_framework.template_test_apps.different_tags_app." - "templatetags.different_tags', " - "'check_framework.template_test_apps.same_tags_app_1." - "templatetags.same_tags'", - ), - id=W003.id, - ) - ], - ) + _engines = engines.all() + errors = [self._get_error_for_engine(_engines[0], modules)] + self.assertEqual(self._check_engines(_engines), errors) def test_template_tags_with_different_library_name(self): with self.settings( TEMPLATES=[ self.get_settings( - "same_tags", "same_tags_app_1.templatetags.same_tags" + "same_tags", + "same_tags_app_1.templatetags.same_tags", + name="backend_1", ), self.get_settings( - "not_same_tags", "same_tags_app_2.templatetags.same_tags" + "not_same_tags", + "same_tags_app_2.templatetags.same_tags", + name="backend_2", ), ] ): - self.assertEqual(check_for_template_tags_with_the_same_name(None), []) + self.assertEqual(self._check_engines(engines.all()), []) @override_settings( INSTALLED_APPS=[ @@ -219,4 +212,4 @@ def test_template_tags_with_different_library_name(self): ] ) def test_template_tags_with_different_name(self): - self.assertEqual(check_for_template_tags_with_the_same_name(None), []) + self.assertEqual(self._check_engines(engines.all()), [])