diff --git a/scripts/docstrings_checker_test.py b/scripts/docstrings_checker_test.py index 73bb6da65682..7a73ca27becc 100644 --- a/scripts/docstrings_checker_test.py +++ b/scripts/docstrings_checker_test.py @@ -16,8 +16,16 @@ """Unit tests for scripts/docstrings_checker.""" +import ast +import contextlib import unittest -import docstrings_checker # pylint: disable=relative-import + +# pylint: disable=wrong-import-position +import astroid +import docstrings_checker # pylint: disable=relative-import + +from pylint.checkers import utils # isort:skip +# pylint: enable=wrong-import-position class ASTDocstringsCheckerTest(unittest.TestCase): @@ -165,3 +173,90 @@ def test_compare_arg_order_multi_line_descriptions_success(self): expected_result = [] result = docstring_checker.compare_arg_order(func_args, docstring_args) self.assertEqual(result, expected_result) + + def test_space_indentation(self): + sample_string = ' This is a sample string.' + self.assertEqual(docstrings_checker.space_indentation(sample_string), 5) + + def test_check_docstrings_arg_order(self): + docstring_checker = docstrings_checker.ASTDocStringChecker() + + ast_file = ast.walk(ast.parse( + """ +def func(test_var_one, test_var_two): #@ + \"\"\"Function to test docstring parameters. + + Args: + test_var_one: int. First test variable. + test_var_two: str. Second test variable. + + Returns: + int. The test result. + \"\"\" + result = test_var_one + test_var_two + return result""")) + + func_defs = [n for n in ast_file if isinstance(n, ast.FunctionDef)] + self.assertEqual(len(func_defs), 1) + + func_result = docstring_checker.check_docstrings_arg_order(func_defs[0]) + self.assertEqual(func_result, []) + + def test_possible_exc_types_with_inference_error(self): + + @contextlib.contextmanager + def swap(obj, attr, newvalue): + """Swap an object's attribute value within the context of a + 'with' statement. The object can be anything that supports + getattr and setattr, such as class instances, modules, etc. + """ + original = getattr(obj, attr) + setattr(obj, attr, newvalue) + try: + yield + finally: + setattr(obj, attr, original) + + raise_node = astroid.extract_node(""" + def func(): + raise Exception('An exception.') #@ + """) + node_ignores_exception_swap = swap( + utils, 'node_ignores_exception', + lambda _, __: (_ for _ in ()).throw(astroid.InferenceError())) + + with node_ignores_exception_swap: + exceptions = docstrings_checker.possible_exc_types(raise_node) + self.assertEqual(exceptions, set([])) + + def test_possible_exc_types_with_exception_message(self): + raise_node = astroid.extract_node(""" + def func(): + \"\"\"Function to test raising exceptions.\"\"\" + raise Exception('An exception.') #@ + """) + + exceptions = docstrings_checker.possible_exc_types(raise_node) + self.assertEqual(exceptions, set(['Exception'])) + + def test_possible_exc_types_with_no_exception(self): + raise_node = astroid.extract_node(""" + def func(): + \"\"\"Function to test raising exceptions.\"\"\" + raise #@ + """) + + exceptions = docstrings_checker.possible_exc_types(raise_node) + self.assertEqual(exceptions, set([])) + + def test_possible_exc_types_with_exception_inside_function(self): + raise_node = astroid.extract_node(""" + def func(): + try: + raise Exception('An exception.') + except Exception: + raise #@ + """) + + exceptions = docstrings_checker.possible_exc_types(raise_node) + self.assertEqual(exceptions, set(['Exception'])) diff --git a/scripts/pylint_extensions.py b/scripts/pylint_extensions.py index 181567cf1b05..00fb459adac6 100644 --- a/scripts/pylint_extensions.py +++ b/scripts/pylint_extensions.py @@ -19,6 +19,7 @@ """ import re + import astroid import docstrings_checker # pylint: disable=relative-import @@ -69,9 +70,6 @@ def visit_call(self, node): # Build the set of keyword arguments and count the positional arguments. call_site = astroid.arguments.CallSite.from_call(node) - if call_site.has_invalid_arguments() or ( - call_site.has_invalid_keywords()): - return num_positional_args = len(call_site.positional_arguments) keyword_args = list(call_site.keyword_arguments.keys()) @@ -107,10 +105,7 @@ def visit_call(self, node): # been called explicitly. for [(name, defval), _] in parameters: if defval: - if name is None: - display_name = '' - else: - display_name = repr(name) + display_name = repr(name) if name not in keyword_args and ( num_positional_args_unused > ( @@ -122,10 +117,7 @@ def visit_call(self, node): try: func_name = node.func.attrname except AttributeError: - try: - func_name = node.func.name - except AttributeError: - func_name = node.func + func_name = node.func.name self.add_message( 'non-explicit-keyword-args', node=node, @@ -443,8 +435,6 @@ def visit_return(self, node): return func_node = node.frame() - if not isinstance(func_node, astroid.FunctionDef): - return doc = docstrings_checker.docstringify(func_node.doc) if not doc.is_valid() and self.config.accept_no_return_doc: @@ -475,19 +465,13 @@ def visit_yield(self, node): method definition in the AST. """ func_node = node.frame() - if not isinstance(func_node, astroid.FunctionDef): - return doc = docstrings_checker.docstringify(func_node.doc) if not doc.is_valid() and self.config.accept_no_yields_doc: return - if doc.supports_yields: - doc_has_yields = doc.has_yields() - doc_has_yields_type = doc.has_yields_type() - else: - doc_has_yields = doc.has_returns() - doc_has_yields_type = doc.has_rtype() + doc_has_yields = doc.has_yields() + doc_has_yields_type = doc.has_yields_type() if not doc_has_yields: self.add_message( @@ -648,7 +632,6 @@ def check_single_constructor_params(self, class_doc, init_doc, class_node): def _handle_no_raise_doc(self, excs, node): """Checks whether the raised exception in a function has been documented, add a message otherwise. - Args: excs: list(str). A list of exception types. node: astroid.scoped_nodes.Function. Node to access module content. @@ -722,8 +705,6 @@ def visit_importfrom(self, node): node=node, args=(name, modname), ) - except astroid.AstroidBuildingException: - pass class BackslashContinuationChecker(checkers.BaseChecker): diff --git a/scripts/pylint_extensions_test.py b/scripts/pylint_extensions_test.py index 5445d35f4a59..134d95b829b4 100644 --- a/scripts/pylint_extensions_test.py +++ b/scripts/pylint_extensions_test.py @@ -36,6 +36,7 @@ import astroid # isort:skip import pylint_extensions # isort:skip from pylint import testutils # isort:skip +from pylint import lint # isort:skip # pylint: enable=wrong-import-position # pylint: enable=relative-import @@ -47,16 +48,32 @@ def test_finds_non_explicit_keyword_args(self): checker_test_object.CHECKER_CLASS = ( pylint_extensions.ExplicitKeywordArgsChecker) checker_test_object.setup_method() - func_call_node_one, func_call_node_two, func_call_node_three = ( - astroid.extract_node(""" + ( + func_call_node_one, func_call_node_two, func_call_node_three, + func_call_node_four, func_call_node_five, func_call_node_six, + class_call_node) = astroid.extract_node(""" + class TestClass(): + pass + def test(test_var_one, test_var_two=4, test_var_three=5, test_var_four="test_checker"): test_var_five = test_var_two + test_var_three return test_var_five + def test_1(test_var_one, test_var_one): + pass + + def test_2((a, b)): + pass + test(2, 5, test_var_three=6) #@ test(2) #@ test(2, 6, test_var_two=5, test_var_four="test_checker") #@ - """)) + max(5, 1) #@ + test_1(1, 2) #@ + test_2((1, 2)) #@ + + TestClass() #@ + """) with checker_test_object.assertAddsMessages( testutils.Message( msg_id='non-explicit-keyword-args', @@ -89,6 +106,22 @@ def test(test_var_one, test_var_two=4, test_var_three=5, test_var_four="test_che checker_test_object.checker.visit_call( func_call_node_three) + with checker_test_object.assertNoMessages(): + checker_test_object.checker.visit_call(class_call_node) + + with checker_test_object.assertNoMessages(): + checker_test_object.checker.visit_call(func_call_node_four) + + with checker_test_object.assertNoMessages(): + checker_test_object.checker.visit_call(func_call_node_five) + + with checker_test_object.assertNoMessages(): + checker_test_object.checker.visit_call(func_call_node_six) + + def test_register(self): + pylinter_instance = lint.PyLinter() + pylint_extensions.register(pylinter_instance) + class HangingIndentCheckerTests(unittest.TestCase): @@ -121,7 +154,7 @@ def test_finds_hanging_indent(self): ): temp_file.close() - node_no_err_message = astroid.scoped_nodes.Module( + node_with_no_error_message = astroid.scoped_nodes.Module( name='test', doc='Custom test') @@ -129,30 +162,313 @@ def test_finds_hanging_indent(self): filename = temp_file.name with open(filename, 'w') as tmp: tmp.write( - """master_translation_dict = json.loads( + """\"""Some multiline + docstring. + \""" + # Load JSON. + master_translation_dict = json.loads( utils.get_file_contents(os.path.join( os.getcwd(), 'assets', 'i18n', 'en.json'))) """) - node_no_err_message.file = filename - node_no_err_message.path = filename + node_with_no_error_message.file = filename + node_with_no_error_message.path = filename - checker_test_object.checker.process_module(node_no_err_message) + checker_test_object.checker.process_module(node_with_no_error_message) with checker_test_object.assertNoMessages(): temp_file.close() + node_with_no_error_message = astroid.scoped_nodes.Module( + name='test', + doc='Custom test') + + temp_file = tempfile.NamedTemporaryFile() + filename = temp_file.name + with open(filename, 'w') as tmp: + tmp.write( + """self.post_json('/', + self.payload, expect_errors=True, expected_status_int=401)""") + node_with_no_error_message.file = filename + node_with_no_error_message.path = filename + + checker_test_object.checker.process_module(node_with_no_error_message) + + with checker_test_object.assertNoMessages(): + temp_file.close() + + class DocstringParameterCheckerTests(unittest.TestCase): + def setUp(self): + super(DocstringParameterCheckerTests, self).setUp() + self.checker_test_object = testutils.CheckerTestCase() + self.checker_test_object.CHECKER_CLASS = ( + pylint_extensions.DocstringParameterChecker) + self.checker_test_object.setup_method() + def test_finds_docstring_parameter(self): - checker_test_object = testutils.CheckerTestCase() - checker_test_object.CHECKER_CLASS = ( + self.checker_test_object = testutils.CheckerTestCase() + self.checker_test_object.CHECKER_CLASS = ( pylint_extensions.DocstringParameterChecker) - checker_test_object.setup_method() - func_node = astroid.extract_node(""" + self.checker_test_object.setup_method() + valid_func_node, valid_return_node = astroid.extract_node(""" def test(test_var_one, test_var_two): #@ \"\"\"Function to test docstring parameters. + Args: + test_var_one: int. First test variable. + test_var_two: str. Second test variable. + + Returns: + int. The test result. + \"\"\" + result = test_var_one + test_var_two + return result #@ + """) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_functiondef(valid_func_node) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_return(valid_return_node) + + valid_func_node, valid_yield_node = astroid.extract_node(""" + def test(test_var_one, test_var_two): #@ + \"\"\"Function to test docstring parameters.\"\"\" + result = test_var_one + test_var_two + yield result #@ + """) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_functiondef(valid_func_node) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_yield(valid_yield_node) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_return(valid_yield_node) + + ( + missing_yield_type_func_node, + missing_yield_type_yield_node) = astroid.extract_node(""" + class Test(object): + def __init__(self, test_var_one, test_var_two): #@ + \"\"\"Function to test docstring parameters. + + Args: + test_var_one: int. First test variable. + test_var_two: str. Second test variable. + + Returns: + int. The test result. + \"\"\" + result = test_var_one + test_var_two + yield result #@ + """) + with self.checker_test_object.assertAddsMessages( + testutils.Message( + msg_id='redundant-returns-doc', + node=missing_yield_type_func_node + ), + ): + self.checker_test_object.checker.visit_functiondef( + missing_yield_type_func_node) + with self.checker_test_object.assertAddsMessages( + testutils.Message( + msg_id='missing-yield-doc', + node=missing_yield_type_func_node + ), testutils.Message( + msg_id='missing-yield-type-doc', + node=missing_yield_type_func_node + ), + ): + self.checker_test_object.checker.visit_yieldfrom( + missing_yield_type_yield_node) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_return( + missing_yield_type_yield_node) + + ( + missing_return_type_func_node, + missing_return_type_return_node) = astroid.extract_node(""" + class Test(object): + def __init__(self, test_var_one, test_var_two): #@ + \"\"\"Function to test docstring parameters. + + Args: + test_var_one: int. First test variable. + test_var_two: str. Second test variable. + + Yields: + int. The test result. + \"\"\" + result = test_var_one + test_var_two + return result #@ + """) + with self.checker_test_object.assertAddsMessages( + testutils.Message( + msg_id='redundant-yields-doc', + node=missing_return_type_func_node + ), + ): + self.checker_test_object.checker.visit_functiondef( + missing_return_type_func_node) + with self.checker_test_object.assertAddsMessages( + testutils.Message( + msg_id='missing-return-doc', + node=missing_return_type_func_node + ), testutils.Message( + msg_id='missing-return-type-doc', + node=missing_return_type_func_node + ), + ): + self.checker_test_object.checker.visit_return( + missing_return_type_return_node) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_yield( + missing_return_type_return_node) + + valid_raise_node = astroid.extract_node(""" + def func(test_var_one, test_var_two): + \"\"\"Function to test docstring parameters. + + Args: + test_var_one: int. First test variable. + test_var_two: str. Second test variable. + + Raises: + Exception: An exception. + \"\"\" + raise Exception #@ + """) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_raise(valid_raise_node) + + ( + missing_raise_type_func_node, + missing_raise_type_raise_node) = astroid.extract_node(""" + def func(test_var_one, test_var_two): #@ + \"\"\"Function to test raising exceptions. + + Args: + test_var_one: int. First test variable. + test_var_two: str. Second test variable. + \"\"\" + raise Exception #@ + """) + with self.checker_test_object.assertAddsMessages( + testutils.Message( + msg_id='missing-raises-doc', + args=('Exception',), + node=missing_raise_type_func_node + ), + ): + self.checker_test_object.checker.visit_raise( + missing_raise_type_raise_node) + + valid_raise_node = astroid.extract_node(""" + class Test(object): + raise Exception #@ + """) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_raise(valid_raise_node) + + valid_raise_node = astroid.extract_node(""" + class Test(): + @property + def decorator_func(self): + pass + + @decorator_func.setter + @property + def func(self): + raise Exception #@ + """) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_raise(valid_raise_node) + + valid_raise_node = astroid.extract_node(""" + class Test(): + def func(self): + raise Exception #@ + """) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_raise(valid_raise_node) + + valid_raise_node = astroid.extract_node(""" + def func(): + try: + raise Exception #@ + except Exception: + pass + """) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_raise(valid_raise_node) + + valid_raise_node = astroid.extract_node(""" + def func(): + \"\"\"Function to test raising exceptions.\"\"\" + raise Exception #@ + """) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_raise(valid_raise_node) + + valid_raise_node = astroid.extract_node(""" + def my_func(self): + \"\"\"This is a docstring. + :raises NameError: Never. + \"\"\" + def ex_func(val): + return RuntimeError(val) + raise ex_func('hi') #@ + raise NameError('hi') + """) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_raise(valid_raise_node) + + valid_raise_node = astroid.extract_node(""" + from unknown import Unknown + def my_func(self): + \"\"\"This is a docstring. + :raises NameError: Never. + \"\"\" + raise Unknown('hi') #@ + raise NameError('hi') + """) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_raise(valid_raise_node) + + valid_raise_node = astroid.extract_node(""" + def my_func(self): + \"\"\"This is a docstring. + :raises NameError: Never. + \"\"\" + def ex_func(val): + def inner_func(value): + return OSError(value) + return RuntimeError(val) + raise ex_func('hi') #@ + raise NameError('hi') + """) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_raise(valid_raise_node) + + valid_return_node = astroid.extract_node(""" + def func(): + \"\"\"Function to test return values.\"\"\" + return None #@ + """) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_return(valid_return_node) + + valid_return_node = astroid.extract_node(""" + def func(): + \"\"\"Function to test return values.\"\"\" + return #@ + """) + with self.checker_test_object.assertNoMessages(): + self.checker_test_object.checker.visit_return(valid_return_node) + + missing_param_func_node = astroid.extract_node(""" + def func(test_var_one, test_var_two, *args, **kwargs): #@ + \"\"\"Function to test docstring parameters. + Args: test_var_one: int. First test variable. test_var_two: str. Second test variable. @@ -163,8 +479,104 @@ def test(test_var_one, test_var_two): #@ result = test_var_one + test_var_two return result """) - with checker_test_object.assertNoMessages(): - checker_test_object.checker.visit_functiondef(func_node) + with self.checker_test_object.assertAddsMessages( + testutils.Message( + msg_id='missing-param-doc', + node=missing_param_func_node, + args=('args, kwargs',), + ), + ): + self.checker_test_object.checker.visit_functiondef( + missing_param_func_node) + + missing_param_func_node = astroid.extract_node(""" + def func(test_var_one, test_var_two): #@ + \"\"\"Function to test docstring parameters. + + Args: + test_var_one: int. First test variable. + invalid_var_name: str. Second test variable. + + Returns: + int. The test result. + \"\"\" + result = test_var_one + test_var_two + return result + """) + with self.checker_test_object.assertAddsMessages( + testutils.Message( + msg_id='missing-param-doc', + node=missing_param_func_node, + args=('test_var_two',), + ), testutils.Message( + msg_id='missing-type-doc', + node=missing_param_func_node, + args=('test_var_two',), + ), testutils.Message( + msg_id='differing-param-doc', + node=missing_param_func_node, + args=('invalid_var_name',), + ), testutils.Message( + msg_id='differing-type-doc', + node=missing_param_func_node, + args=('invalid_var_name',), + ), + ): + self.checker_test_object.checker.visit_functiondef( + missing_param_func_node) + + class_node, multiple_constructor_func_node = astroid.extract_node(""" + class Test(): #@ + \"\"\"Function to test docstring parameters. + + Args: + test_var_one: int. First test variable. + test_var_two: str. Second test variable. + + Returns: + int. The test result. + \"\"\" + + def __init__(self, test_var_one, test_var_two): #@ + \"\"\"Function to test docstring parameters. + + Args: + test_var_one: int. First test variable. + test_var_two: str. Second test variable. + + Returns: + int. The test result. + \"\"\" + result = test_var_one + test_var_two + return result + """) + with self.checker_test_object.assertAddsMessages( + testutils.Message( + msg_id='multiple-constructor-doc', + node=class_node, + args=(class_node.name,), + ), + ): + self.checker_test_object.checker.visit_functiondef( + multiple_constructor_func_node) + + def test_visit_raise_warns_unknown_style(self): + self.checker_test_object.checker.config.accept_no_raise_doc = False + node = astroid.extract_node(""" + def my_func(self): + \"\"\"This is a docstring.\"\"\" + raise RuntimeError('hi') + """) + raise_node = node.body[0] + func_node = raise_node.frame() + with self.checker_test_object.assertAddsMessages( + testutils.Message( + msg_id='missing-raises-doc', + args=('RuntimeError',), + node=func_node + ), + ): + self.checker_test_object.checker.visit_raise(raise_node) class ImportOnlyModulesCheckerTests(unittest.TestCase): @@ -195,6 +607,36 @@ def test_finds_import_from(self): checker_test_object.checker.visit_importfrom( importfrom_node2) + importfrom_node3 = astroid.extract_node(""" + from invalid_module import invalid_module #@ + """) + with checker_test_object.assertNoMessages(): + checker_test_object.checker.visit_importfrom(importfrom_node3) + + importfrom_node4 = astroid.extract_node(""" + from constants import constants #@ + """) + with checker_test_object.assertNoMessages(): + checker_test_object.checker.visit_importfrom(importfrom_node4) + + importfrom_node5 = astroid.extract_node(""" + from os import invalid_module #@ + """) + with checker_test_object.assertAddsMessages( + testutils.Message( + msg_id='import-only-modules', + node=importfrom_node5, + args=('invalid_module', 'os') + ), + ): + checker_test_object.checker.visit_importfrom(importfrom_node5) + + importfrom_node6 = astroid.extract_node(""" + from .constants import constants #@ + """, module_name='.constants') + with checker_test_object.assertNoMessages(): + checker_test_object.checker.visit_importfrom(importfrom_node6) + class BackslashContinuationCheckerTests(unittest.TestCase): @@ -265,6 +707,19 @@ def test(test_var_one, test_var_two, self): #@ ): checker_test_object.checker.visit_functiondef(functiondef_node2) + functiondef_node3 = astroid.extract_node(""" + def test(test_var_one, test_var_two, cls): #@ + result = test_var_one + test_var_two + return result + """) + with checker_test_object.assertAddsMessages( + testutils.Message( + msg_id='function-args-order-cls', + node=functiondef_node3 + ), + ): + checker_test_object.checker.visit_functiondef(functiondef_node3) + class RestrictedImportCheckerTests(unittest.TestCase): @@ -422,7 +877,7 @@ def test_checks_single_char_and_newline_eof(self): ): temp_file.close() - node_no_err_message = astroid.scoped_nodes.Module( + node_with_no_error_message = astroid.scoped_nodes.Module( name='test', doc='Custom test') @@ -430,10 +885,10 @@ def test_checks_single_char_and_newline_eof(self): filename = temp_file.name with open(filename, 'w') as tmp: tmp.write("""x = 'something dummy'""") - node_no_err_message.file = filename - node_no_err_message.path = filename + node_with_no_error_message.file = filename + node_with_no_error_message.path = filename - checker_test_object.checker.process_module(node_no_err_message) + checker_test_object.checker.process_module(node_with_no_error_message) with checker_test_object.assertNoMessages(): temp_file.close()