diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index f44a3922..6f1fc917 100644 --- a/CONTRIBUTORS.md +++ b/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) diff --git a/README.rst b/README.rst index cd2d2f43..874671d8 100644 --- a/README.rst +++ b/README.rst @@ -124,6 +124,10 @@ a test name, and insert into it some code. The tests will run pylint against these modules. If the idea is that no messages now occur, then that is fine, just check to see if it works by running ``scripts/test.sh``. +Any command line argument passed to ``scripts/test.sh`` will be passed to the internal invocation of ``pytest``. +For example if you want to debug the tests you can execute ``scripts/test.sh --capture=no``. +A specific test case can be run by filtering based on the file name of the test case ``./scripts/test.sh -k 'func_noerror_views'``. + Ideally, add some pylint error suppression messages to the file to prevent spurious warnings, since these are all tiny little modules not designed to do anything so there's no need to be perfect. @@ -132,7 +136,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 diff --git a/pylint_django/augmentations/__init__.py b/pylint_django/augmentations/__init__.py index 17757d40..3f7ac9bf 100644 --- a/pylint_django/augmentations/__init__.py +++ b/pylint_django/augmentations/__init__.py @@ -644,23 +644,17 @@ 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): +def ignore_unused_argument_warnings_for_request(orig_method, self, stmt, name): """ - Checks that node is get or post method of the View class and it has valid arguments. + Ignore unused-argument warnings for function arguments named "request". - TODO: Bad checkings, need to be more smart. + The signature of Django view functions require the request argument but it is okay if the request is not used. + This function should be used as a wrapper for the `VariablesChecker._is_name_ignored` method. """ - if not is_model_view_subclass_method_shouldnt_be_function(node): - return False - - return is_argument_named_request(node) + if name == 'request': + return True - -def is_argument_named_request(node): - """ - If an unused-argument is named 'request' ignore that! - """ - return 'request' in node.argnames() + return orig_method(self, stmt, name) def is_model_field_display_method(node): @@ -742,7 +736,7 @@ def is_class(class_name): def wrap(orig_method, with_method): @functools.wraps(orig_method) def wrap_func(*args, **kwargs): - with_method(orig_method, *args, **kwargs) + return with_method(orig_method, *args, **kwargs) return wrap_func @@ -759,6 +753,31 @@ def pylint_newstyle_classdef_compat(linter, warning_name, augment): suppress_message(linter, getattr(NewStyleConflictChecker, 'visit_classdef'), warning_name, augment) +def apply_wrapped_augmentations(): + """ + Apply augmentation and supression rules through monkey patching of pylint. + """ + # NOTE: The monkey patching is done with wrap and needs to be done in a thread safe manner to support the + # parallel option of pylint (-j). + # This is achieved by comparing __name__ of the monkey patched object to the original value and only patch it if + # these are equal. + + # Unused argument 'request' (get, post) + current_is_name_ignored = VariablesChecker._is_name_ignored # pylint: disable=protected-access + if current_is_name_ignored.__name__ == '_is_name_ignored': + # pylint: disable=protected-access + VariablesChecker._is_name_ignored = wrap(current_is_name_ignored, ignore_unused_argument_warnings_for_request) + + # ForeignKey and OneToOneField + current_leave_module = VariablesChecker.leave_module + if current_leave_module.__name__ == 'leave_module': + # current_leave_module is not wrapped + # Two threads may hit the next assignment concurrently, but the result is the same + VariablesChecker.leave_module = wrap(current_leave_module, ignore_import_warnings_for_related_fields) + # VariablesChecker.leave_module is now wrapped + # else VariablesChecker.leave_module is already wrapped + + # augment things def apply_augmentations(linter): """Apply augmentation and suppression rules.""" @@ -826,10 +845,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) @@ -841,15 +856,7 @@ def apply_augmentations(linter): suppress_message(linter, TypeChecker.visit_attribute, 'no-member', is_model_factory) suppress_message(linter, ClassChecker.visit_functiondef, 'no-self-argument', is_factory_post_generation_method) - # ForeignKey and OneToOneField - # Must update this in a thread safe way to support the parallel option on pylint (-j) - current_leave_module = VariablesChecker.leave_module - if current_leave_module.__name__ == 'leave_module': - # current_leave_module is not wrapped - # Two threads may hit the next assignment concurrently, but the result is the same - VariablesChecker.leave_module = wrap(current_leave_module, ignore_import_warnings_for_related_fields) - # VariablesChecker.leave_module is now wrapped - # else VariablesChecker.leave_module is already wrapped - # wsgi.py suppress_message(linter, NameChecker.visit_assignname, 'invalid-name', is_wsgi_application) + + apply_wrapped_augmentations() diff --git a/pylint_django/tests/input/func_unused_arguments.py b/pylint_django/tests/input/func_unused_arguments.py new file mode 100644 index 00000000..61b2e23a --- /dev/null +++ b/pylint_django/tests/input/func_unused_arguments.py @@ -0,0 +1,40 @@ +""" +Checks that Pylint still complains about unused-arguments for other +arguments if a function/method contains an argument named `request`. +""" +# pylint: disable=missing-docstring + +from django.http import JsonResponse +from django.views import View + +# Pylint generates the warning `redefined-outer-name` if an argument name shadows +# a variable name from an outer scope. But if that argument name is ignored this +# warning will not be generated. +# Therefore define request here to cover this behaviour in this test case. + +request = None # pylint: disable=invalid-name + + +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'}) + + +# The following views are already covered in other test cases. +# They are included here for completeness sake. + +def welcome_view(request): + # just don't use `request' b/c we could have Django views + # which never use it! + return JsonResponse({'message': 'welcome'}) + + +class CBV(View): + def get(self, request): + return JsonResponse({'message': 'hello world'}) diff --git a/pylint_django/tests/input/func_unused_arguments.txt b/pylint_django/tests/input/func_unused_arguments.txt new file mode 100644 index 00000000..cd3d120e --- /dev/null +++ b/pylint_django/tests/input/func_unused_arguments.txt @@ -0,0 +1,2 @@ +unused-argument:18:user_detail:Unused argument 'user_id':HIGH +unused-argument:24:UserView.get:Unused argument 'user_id':INFERENCE diff --git a/pylint_django/tests/test_func.py b/pylint_django/tests/test_func.py index 19516219..3b3cb5a2 100644 --- a/pylint_django/tests/test_func.py +++ b/pylint_django/tests/test_func.py @@ -29,6 +29,7 @@ class PylintDjangoLintModuleTest(test_functional.LintModuleTest): def __init__(self, test_file): super(PylintDjangoLintModuleTest, self).__init__(test_file) self._linter.load_plugin_modules(['pylint_django']) + self._linter.load_plugin_configuration() class PylintDjangoDbPerformanceTest(PylintDjangoLintModuleTest): @@ -39,6 +40,7 @@ class PylintDjangoDbPerformanceTest(PylintDjangoLintModuleTest): def __init__(self, test_file): super(PylintDjangoDbPerformanceTest, self).__init__(test_file) self._linter.load_plugin_modules(['pylint_django.checkers.db_performance']) + self._linter.load_plugin_configuration() def get_tests(input_dir='input', sort=False): diff --git a/scripts/test.sh b/scripts/test.sh index b1377d77..ca5be46c 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -1,2 +1,2 @@ #!/bin/bash -python pylint_django/tests/test_func.py -v +python pylint_django/tests/test_func.py -v "$@"