Ignore unused-argument warning for request arguments (Fixes pylint-de…

…v#249)

The previous implementation (result of pylint-dev#155) of disabling the
unused-argument warning for functions/methods having an argument named
`request` was to broad. As soon as a function/method contains a
`request` argument no unused-argument warning will be issued for this
function anymore, even though some other arguments are unused!

This commit extends the `ignored-argument-names` regex of Pylint to also
ignore arguments named `request`. This way unused-argument warnings will
still be issued for every other unused argument of the function/method.

CONTRIBUTORS.md

@@ -12,3 +12,4 @@
 * [matusvalo](https://github.com/matusvalo)
 * [fadedDexofan](https://github.com/fadeddexofan)
 * [imomaliev](https://github.com/imomaliev)
+* [psrb](https://github.com/psrb)

README.rst

@@ -135,7 +135,7 @@ It is possible to make tests with expected error output, for example, if
 adding a new message or simply accepting that pylint is supposed to warn.
 A ``test_file_name.txt`` file contains a list of expected error messages in the
 format
-``error-type:line number:class name or empty:1st line of detailed error text``.
+``error-type:line number:class name or empty:1st line of detailed error text:confidence or empty``.
 
 
 License

pylint_django/augmentations/__init__.py

@@ -644,25 +644,6 @@ def is_model_view_subclass_method_shouldnt_be_function(node):
     return parent is not None and node_is_subclass(parent, *subclass)
 
 
-def is_model_view_subclass_unused_argument(node):
-    """
-    Checks that node is get or post method of the View class and it has valid arguments.
-
-    TODO: Bad checkings, need to be more smart.
-    """
-    if not is_model_view_subclass_method_shouldnt_be_function(node):
-        return False
-
-    return is_argument_named_request(node)
-
-
-def is_argument_named_request(node):
-    """
-    If an unused-argument is named 'request' ignore that!
-    """
-    return 'request' in node.argnames()
-
-
 def is_model_field_display_method(node):
     """Accept model's fields with get_*_display names."""
     if not node.attrname.endswith('_display'):
@@ -826,10 +807,6 @@ def apply_augmentations(linter):
     # Method could be a function (get, post)
     suppress_message(linter, ClassChecker.leave_functiondef, 'no-self-use',
                      is_model_view_subclass_method_shouldnt_be_function)
-    # Unused argument 'request' (get, post)
-    suppress_message(linter, VariablesChecker.leave_functiondef, 'unused-argument',
-                     is_model_view_subclass_unused_argument)
-    suppress_message(linter, VariablesChecker.leave_functiondef, 'unused-argument', is_argument_named_request)
 
     # django-mptt
     suppress_message(linter, DocStringChecker.visit_classdef, 'missing-docstring', is_model_mpttmeta_subclass)

pylint_django/plugin.py

@@ -1,5 +1,7 @@
 """Common Django module."""
+import re
 from pylint.checkers.base import NameChecker
+from pylint.checkers.variables import VariablesChecker
 from pylint_plugin_utils import get_checker
 
 from pylint_django.checkers import register_checkers
@@ -17,6 +19,17 @@ def load_configuration(linter):
     name_checker = get_checker(linter, NameChecker)
     name_checker.config.good_names += ('qs', 'urlpatterns', 'register', 'app_name', 'handler500')
 
+    # We want to ignore the unused-argument warning for arguments named `request`. The signature of Django view
+    # functions require the request argument but it is okay if the request is not used in the function.
+    # https://github.com/PyCQA/pylint-django/issues/155
+    variables_checker = get_checker(linter, VariablesChecker)
+    old_ignored_argument_names_pattern = variables_checker.config.ignored_argument_names.pattern
+    new_ignored_argument_names_pattern = "request"
+    if old_ignored_argument_names_pattern:
+        new_ignored_argument_names_pattern = old_ignored_argument_names_pattern + "|request"
+
+    variables_checker.config.ignored_argument_names = re.compile(new_ignored_argument_names_pattern)
+
     # we don't care about South migrations
     linter.config.black_list += ('migrations', 'south_migrations')
 

pylint_django/tests/input/func_unused_arguments.py

@@ -0,0 +1,19 @@
+"""
+Checks that Pylint still complains about unused-arguments if a function/method
+contains an argument named `request`.
+"""
+# pylint: disable=missing-docstring
+
+from django.http import JsonResponse
+from django.views import View
+
+
+def user_detail(request, user_id):  # [unused-argument]
+    # nothing is done with user_id
+    return JsonResponse({'username': 'steve'})
+
+
+class UserView(View):
+    def get(self, request, user_id):  #[unused-argument]
+        # nothing is done with user_id
+        return JsonResponse({'username': 'steve'})

pylint_django/tests/input/func_unused_arguments.txt

@@ -0,0 +1,2 @@
+unused-argument:11:user_detail:Unused argument 'user_id':HIGH
+unused-argument:17:UserView.get:Unused argument 'user_id':INFERENCE