From eb5d7bc2f465b055f4eb54a3d238502bdddb6d7e Mon Sep 17 00:00:00 2001 From: Alasdair Nicol Date: Sat, 2 Apr 2016 12:49:12 +0200 Subject: [PATCH] Fixed #26440 -- Added a warning for non-url()s in urlpatterns. Thanks Burhan Khalid for the initial patch and knbk/timgraham for review. --- django/core/checks/urls.py | 35 ++++++++++++++++++-- docs/ref/checks.txt | 2 ++ tests/check_framework/test_urls.py | 30 ++++++++++++++++- tests/check_framework/urls/contains_tuple.py | 3 ++ 4 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 tests/check_framework/urls/contains_tuple.py diff --git a/django/core/checks/urls.py b/django/core/checks/urls.py index 176cd16190..1b4db4b921 100644 --- a/django/core/checks/urls.py +++ b/django/core/checks/urls.py @@ -1,8 +1,10 @@ from __future__ import unicode_literals +import six + from django.conf import settings -from . import Tags, Warning, register +from . import Error, Tags, Warning, register @register(Tags.urls) @@ -27,12 +29,41 @@ def check_resolver(resolver): warnings.extend(check_resolver(pattern)) elif isinstance(pattern, RegexURLPattern): warnings.extend(check_pattern_name(pattern)) + else: + # This is not a url() instance + warnings.extend(get_warning_for_invalid_pattern(pattern)) - warnings.extend(check_pattern_startswith_slash(pattern)) + if not warnings: + warnings.extend(check_pattern_startswith_slash(pattern)) return warnings +def get_warning_for_invalid_pattern(pattern): + """ + Return a list containing a warning that the pattern is invalid. + + describe_pattern() cannot be used here, because we cannot rely on the + urlpattern having regex or name attributes. + """ + if isinstance(pattern, six.string_types): + hint = ( + "Try removing the string '{}'. The list of urlpatterns should not " + "have a prefix string as the first element.".format(pattern) + ) + elif isinstance(pattern, tuple): + hint = "Try using url() instead of a tuple." + else: + hint = None + + return [Error( + "Your URL pattern {!r} is invalid. Ensure that urlpatterns is a list " + "of url() instances.".format(pattern), + hint=hint, + id="urls.E004", + )] + + def describe_pattern(pattern): """ Format the URL pattern for display in warning messages. diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index 7c15bbeb7b..486b88cc2e 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -631,3 +631,5 @@ The following checks are performed on your URL configuration: * **urls.W003**: Your URL pattern ```` has a ``name`` including a ``:``. Remove the colon, to avoid ambiguous namespace references. +* **urls.E004**: Your URL pattern ```` is invalid. Ensure that + ``urlpatterns`` is a list of :func:`~django.conf.urls.url()` instances. diff --git a/tests/check_framework/test_urls.py b/tests/check_framework/test_urls.py index a0c113aac3..a1ac727057 100644 --- a/tests/check_framework/test_urls.py +++ b/tests/check_framework/test_urls.py @@ -1,5 +1,7 @@ from django.conf import settings -from django.core.checks.urls import check_url_config +from django.core.checks.urls import ( + check_url_config, get_warning_for_invalid_pattern, +) from django.test import SimpleTestCase from django.test.utils import override_settings @@ -27,6 +29,16 @@ class CheckUrlsTest(SimpleTestCase): expected_msg = "Your URL pattern '^include-with-dollar$' uses include with a regex ending with a '$'." self.assertIn(expected_msg, warning.msg) + @override_settings(ROOT_URLCONF='check_framework.urls.contains_tuple') + def test_contains_tuple_not_url_instance(self): + result = check_url_config(None) + warning = result[0] + self.assertEqual(warning.id, 'urls.E004') + self.assertRegexpMatches(warning.msg, ( + r"^Your URL pattern \('\^tuple/\$', at 0x(\w+)>\) is " + r"invalid. Ensure that urlpatterns is a list of url\(\) instances.$" + )) + @override_settings(ROOT_URLCONF='check_framework.urls.beginning_with_slash') def test_beginning_with_slash(self): result = check_url_config(None) @@ -50,3 +62,19 @@ class CheckUrlsTest(SimpleTestCase): delattr(settings, 'ROOT_URLCONF') result = check_url_config(None) self.assertEqual(result, []) + + def test_get_warning_for_invalid_pattern_string(self): + warning = get_warning_for_invalid_pattern('')[0] + self.assertEqual( + warning.hint, + "Try removing the string ''. The list of urlpatterns should " + "not have a prefix string as the first element.", + ) + + def test_get_warning_for_invalid_pattern_tuple(self): + warning = get_warning_for_invalid_pattern((r'^$', lambda x: x))[0] + self.assertEqual(warning.hint, "Try using url() instead of a tuple.") + + def test_get_warning_for_invalid_pattern_other(self): + warning = get_warning_for_invalid_pattern(object())[0] + self.assertIsNone(warning.hint) diff --git a/tests/check_framework/urls/contains_tuple.py b/tests/check_framework/urls/contains_tuple.py new file mode 100644 index 0000000000..56aa3ea364 --- /dev/null +++ b/tests/check_framework/urls/contains_tuple.py @@ -0,0 +1,3 @@ +urlpatterns = [ + (r'^tuple/$', lambda x: x), +]