Add checks for common URLpattern errors

[Julian Wachholz]

The system checks framework can be expanded to help less experienced users help with common mistakes. One source of those is the urlpatterns we have to configure. It would be nice to have checks in place that verify the validity and sanity of urls.

One particular example that comes to mind (from my own experience):

urlpatterns = [
    url(r'^my_app/$', include('my_app')),
]

This would trigger a warning that the trailing '$' in this regular expression is most likely not intended. :-)


Please do suggest any additional checks that could be added as well.

[Tim Graham]

An idea raised in ​pull request #3808 is to disallow pattern names that contain a colon to avoid ambiguous namespace lookups.

[Alasdair Nicol]

In the last few weeks, I've helped a couple of beginners caught out by the trailing $ when using include. I've had a go at this ticket. I don't think it's ready to check in yet, so I haven't opened a pull request. I'd appreciate a code review and any feedback.

​https://github.com/alasdairnicol/django/tree/urlchecks

[Josh Smeaton]

After a quick look your changes look good to me. A PR is much easier to review though, even if you feel it's not quite ready to be merged. We can leave feedback on specific lines and also trigger the CI to run its tests. So please, open up a PR and we'll see if we can get this in on time for 1.9.

[Alasdair Nicol]

Replying to jarshwah:

After a quick look your changes look good to me. A PR is much easier to review though, even if you feel it's not quite ready to be merged. We can leave feedback on specific lines and also trigger the CI to run its tests. So please, open up a PR and we'll see if we can get this in on time for 1.9.

Thanks, I've opened a pull request ​https://github.com/django/django/pull/5301. In particular, I thought maybe the warning messages and documentation might need improving.

[Josh Smeaton]

There are a couple of failing tests. See here: ​http://djangoci.com/job/pull-requests-trusty/3493/

You'll also need to add some release notes.

I've added a couple of quick comments on the PR.

[Alasdair Nicol]

Replying to jarshwah:

There are a couple of failing tests. See here: ​http://djangoci.com/job/pull-requests-trusty/3493/

You'll also need to add some release notes.

I've added a couple of quick comments on the PR.

I've updated the docs as suggested, and added a sentence to the 1.9 release notes.

I tried to look at the failing tests, but I was a bit confused, maybe because I'm unfamiliar with Jenkins. The failing tests seemed to be testing the cache. I wasn't expecting these changes to break those tests.

[Tim Graham]

Sorry about those old test failures, that was a transient issue while I was making some updates to Jenkins.

As noted on the pull request, the current warnings don't show which url triggered the warning. I don't think it's acceptable to make the user hunt their entire project to track down the cause of the warning. :-)

[Alasdair Nicol]

Replying to timgraham:

As noted on the pull request, the current warnings don't show which url triggered the warning. I don't think it's acceptable to make the user hunt their entire project to track down the cause of the warning. :-)

I've updated the pull request. The warning message now includes the regex (and name if it has one) of the URL pattern which triggered the warning.

[Josh Smeaton <josh.smeaton@…>]

In fe3fc52:

Fixed #23813 -- Added checks for common URL pattern errors

Thanks jwa and lamby for the suggestions, and timgraham and jarshwah
for their reviews.

[Tim Graham <timograham@…>]

In f2975c0:

Refs #23813 -- Moved URLconfs into module and tidied docstrings.