Merge pull request #126 from spookylukey/specific-silencing

Added fine grained method for silencing errors

README.md

@@ -107,7 +107,6 @@ output will be:
 ```
 
 * TODO
-    - Fine-grained methods for silencing checks.
     - Should only warn for each unhandled Converter once.
     - Regex patterns perhaps? (only RoutePattern supported at the moment).
 

django_urlconfchecks/check.py

@@ -1,4 +1,5 @@
 """Quick and dirty URL checker."""
+import fnmatch
 import typing as t
 import uuid
 from inspect import Parameter, _empty, signature  # type: ignore[attr-defined]
@@ -8,9 +9,21 @@
 from django.urls import URLPattern, URLResolver, converters, get_resolver
 from django.urls.resolvers import RoutePattern
 
+Problem = t.Union[checks.Error, checks.Warning]
+
+# Update docs about default value if you change this:
+_DEFAULT_SILENCED_VIEWS = {
+    # CBVs use **kwargs, and have a `__name__` that ends up looking like this in URLconf
+    "*.View.as_view.<locals>.view": "W001",
+    # RedirectView is used in a way that makes it appear directly, and it has **kwargs
+    "django.views.generic.base.RedirectView": "W001",
+    # Django contrib views currently don’t have type annotations:
+    "django.contrib.*": "W003",
+}
+
 
 @checks.register(checks.Tags.urls)
-def check_url_signatures(app_configs, **kwargs) -> t.List[t.Union[checks.Error, checks.Warning]]:
+def check_url_signatures(app_configs, **kwargs) -> t.List[Problem]:
     """Check that all callbacks in the main urlconf have the correct signature.
 
     Args:
@@ -25,15 +38,14 @@ def check_url_signatures(app_configs, **kwargs) -> t.List[t.Union[checks.Error,
 
     resolver = get_resolver()
     errors = []
+    silencers = _build_view_silencers(getattr(settings, "URLCONFCHECKS_SILENCED_VIEWS", _DEFAULT_SILENCED_VIEWS))
     for route in get_all_routes(resolver):
         errors.extend(check_url_args_match(route))
-    return errors
+    return _filter_errors(errors, silencers)
 
 
 def get_all_routes(resolver: URLResolver) -> t.Iterable[URLPattern]:
     """Recursively get all routes from the resolver."""
-    if resolver.app_name == 'admin':
-        return []
     for pattern in resolver.url_patterns:
         if isinstance(pattern, URLResolver):
             yield from get_all_routes(pattern)
@@ -42,7 +54,7 @@ def get_all_routes(resolver: URLResolver) -> t.Iterable[URLPattern]:
                 yield pattern
 
 
-def check_url_args_match(url_pattern: URLPattern) -> t.List[t.Union[checks.Error, checks.Warning]]:
+def check_url_args_match(url_pattern: URLPattern) -> t.List[Problem]:
     """Check that all callbacks in the main urlconf have the correct signature."""
     callback = url_pattern.callback
     callback_repr = f'{callback.__module__}.{callback.__qualname__}'
@@ -108,7 +120,7 @@ def check_url_args_match(url_pattern: URLPattern) -> t.List[t.Union[checks.Error
             elif found_type == Parameter.empty:
                 errors.append(
                     checks.Warning(
-                        f'Missing type annotation for parameter `{name}`, can\'t check type.',
+                        f'View {callback_repr} missing type annotation for parameter `{name}`, can\'t check type.',
                         obj=url_pattern,
                         id='urlchecker.W003',
                     )
@@ -177,3 +189,35 @@ def get_converter_output_type(converter) -> t.Union[int, str, uuid.UUID, t.Type[
         return sig.return_annotation
 
     return Parameter.empty
+
+
+class ViewSilencer:
+    """Utility that checks whether errors for a view or set of views should be ignored."""
+
+    def __init__(self, view_glob: str, errors: t.Iterable[str]):
+        self.view_glob = view_glob
+        self.errors = set(errors)
+
+    def matches(self, error: Problem):
+        """Returns True if this silencer matches the given error or warning."""
+        url_pattern = error.obj
+        if not isinstance(url_pattern, URLPattern):
+            # Some other error, eg. for a convertor
+            return False
+        if error.id not in self.errors:
+            return False
+        view_name = f"{url_pattern.callback.__module__}.{url_pattern.callback.__qualname__}"
+        if fnmatch.fnmatch(view_name, self.view_glob):
+            return True
+        return False
+
+
+def _build_view_silencers(silenced_views: t.Dict[str, str]) -> t.List[ViewSilencer]:
+    return [
+        ViewSilencer(view_glob=view_glob, errors=[f"urlchecker.{error}" for error in error_list.split(",")])
+        for view_glob, error_list in silenced_views.items()
+    ]
+
+
+def _filter_errors(errors: t.List[Problem], silencers: t.List[ViewSilencer]) -> t.List[Problem]:
+    return [error for error in errors if not any(silencer.matches(error) for silencer in silencers)]

docs/usage.md

@@ -63,3 +63,42 @@ Then, add the following to your `.pre-commit-config.yaml` file:
 ```
 
 Run `pre-commit run` to check all URLConfs for errors.
+
+
+## Silencing errors and warnings
+
+You can silence specific errors and warnings as described in the [System check
+framework
+documentation](https://docs.djangoproject.com/en/stable/topics/checks/).
+
+For example:
+
+```python
+SILENCED_SYSTEM_CHECKS = [
+    "urlchecker.W003“,
+]
+```
+
+However, this turns off the check for all view functions. Instead of this you
+can use the `URLCONFCHECKS_SILENCED_VIEWS` setting for more fine grained
+silencing. The value must be a dictionary:
+
+- whose keys are fully qualified dotted paths to view functions or callables,
+  with globbing syntax allowed, e.g. `"my_project.views.my_view"` or
+  `"other_project.*"`
+
+- whose values are comma separated lists of warning or error IDs (without the
+  `urlchecker` prefix), e.g. `"E001,W003”`
+
+
+The default value of `URLCONFCHECKS_SILENCED_VIEWS` is below. If you override it
+in your `settings.py`, you will probably want to include the following and add
+more items:
+
+```python
+URLCONFCHECKS_SILENCED_VIEWS = {
+    "*.View.as_view.<locals>.view": "W001",  # CBVs
+    "django.views.generic.base.RedirectView": "W001",
+    "django.contrib.*": "W003",  # admin etc.
+}
+```

setup.cfg

@@ -1,7 +1,7 @@
 [flake8]
 max-line-length = 120
 max-complexity = 18
-ignore = E203, E266, W503
+ignore = E203, E266, W503, D107
 docstring-convention = google
 per-file-ignores = __init__.py:F401
 exclude = .git,

tests/test_url_checker.py

@@ -44,6 +44,30 @@ def test_incorrect_urls():
         assert error_eql(errors[0], expected_error)
 
 
+def test_silencing():
+    """Test that fine-grained error silencing mechanisms work."""
+    # Specific view silenced
+    with override_settings(
+        ROOT_URLCONF="tests.dummy_project.urls.incorrect_urls",
+        URLCONFCHECKS_SILENCED_VIEWS={"tests.dummy_project.views.year_archive": "E002"},
+    ):
+        assert len(check_url_signatures(None)) == 0
+
+    # Glob pattern
+    with override_settings(
+        ROOT_URLCONF="tests.dummy_project.urls.incorrect_urls",
+        URLCONFCHECKS_SILENCED_VIEWS={"tests.dummy_project.views.*": "E002"},
+    ):
+        assert len(check_url_signatures(None)) == 0
+
+    # Non-matching silencer
+    with override_settings(
+        ROOT_URLCONF="tests.dummy_project.urls.incorrect_urls",
+        URLCONFCHECKS_SILENCED_VIEWS={"tests.dummy_project.views.month_archive": "E002"},
+    ):
+        assert len(check_url_signatures(None)) > 0
+
+
 def test_all_urls_checked():
     """Test that all urls are checked.
 
@@ -69,12 +93,11 @@ def test_child_urls_checked():
 
 
 def test_admin_urls_ignored():
-    """Test that admin urls are ignored.
+    """Test that admin urls errors are ignored with default settings.
 
     Returns:
         None
     """
     with override_settings(ROOT_URLCONF='tests.dummy_project.urls.admin_urls'):
-        resolver = get_resolver()
-        routes = get_all_routes(resolver)
-        assert len(list(routes)) == 0
+        errors = check_url_signatures(None)
+        assert len(errors) == 0