Commit eb5d7bc2 authored by Alasdair Nicol's avatar Alasdair Nicol Committed by Tim Graham
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.
parent 914c72be
Loading
Loading
Loading
Loading
+33 −2
Original line number Diff line number Diff line
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))

        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.
+2 −0
Original line number Diff line number Diff line
@@ -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.
+29 −1
Original line number Diff line number Diff line
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/\$', <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 @@ 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)
+3 −0
Original line number Diff line number Diff line
urlpatterns = [
    (r'^tuple/$', lambda x: x),
]