#31070: Add a check for URLconfs that mix named and unnamed capture groups
-------------------------------------+-------------------------------------
Reporter: Baptiste Mispelon | Owner: Baptiste
| Mispelon
Type: New feature | Status: assigned
Component: Core (System | Version: master
checks) |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Baptiste Mispelon):
While working on this, I also realized that it would be fairly easy to
check whether a view's signature matches the capture groups. This would
help catch errors like in #31069.
Here's an example implementation using a warning (it could probably be
upgraded to an error but I don't know if I might be missing weird
edgecases):
{{{
diff --git a/django/urls/resolvers.py b/django/urls/resolvers.py
index f60d19d756..e33b687e5e 100644
--- a/django/urls/resolvers.py
+++ b/django/urls/resolvers.py
@@ -357,6 +357,7 @@ class URLPattern:
def check(self):
warnings = self._check_pattern_name()
warnings.extend(self.pattern.check())
+ warnings.extend(self._check_view_signature())
return warnings
def _check_pattern_name(self):
@@ -373,6 +374,36 @@ class URLPattern:
else:
return []
+ def _check_view_signature(self):
+ """
+ Try to see if the callback's signature matches the regex capture
groups
+ and the default_args
+ """
+ try:
+ signature = inspect.signature(self.callback)
+ except (ValueError, TypeError):
+ return []
+
+ if self.pattern.regex.groupindex:
+ # pattern has named capture groups
+ args, kwargs = (), {**self.default_args,
**self.pattern.regex.groupindex}
+ else:
+ args, kwargs = (None,) * self.pattern.regex.groups,
self.default_args
+
+ args = (None,) + args # for the request object
+
+ try:
+ signature.bind(*args, **kwargs)
+ except TypeError:
+ return [Warning(
+ "Your URL pattern {} doesn't match the signature of the "
+ "corresponding view. Consider adding a `default_kwargs`
to "
+ "the path.".format(self.pattern.describe()),
+ id="urls.W010",
+ )]
+ else:
+ return []
+
def resolve(self, path):
match = self.pattern.match(path)
if match:
}}}
Should I open a separate ticket for this?
--
Ticket URL: <https://code.djangoproject.com/ticket/31070#comment:2>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/067.094fab6b21cb5d1863fabe0316d6ae18%40djangoproject.com.
