Permalink
Browse files

Fixed #26440 -- Added a warning for non-url()s in urlpatterns.

Thanks Burhan Khalid for the initial patch and knbk/timgraham
for review.
  • Loading branch information...
alasdairnicol authored and timgraham committed Apr 2, 2016
1 parent 914c72b commit eb5d7bc2f465b055f4eb54a3d238502bdddb6d7e
View
@@ -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.
View
@@ -631,3 +631,5 @@ The following checks are performed on your URL configuration:
* **urls.W003**: Your URL pattern ``<pattern>`` has a ``name``
including a ``:``. Remove the colon, to avoid ambiguous namespace
references.
+* **urls.E004**: Your URL pattern ``<pattern>`` is invalid. Ensure that
+ ``urlpatterns`` is a list of :func:`~django.conf.urls.url()` instances.
@@ -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 @@ def test_include_with_dollar(self):
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/\$', <function <lambda> 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 @@ def test_no_root_urlconf_in_settings(self):
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)
@@ -0,0 +1,3 @@
+urlpatterns = [
+ (r'^tuple/$', lambda x: x),
+]

0 comments on commit eb5d7bc

Please sign in to comment.