From dc3696f9770bd548a3f2f6f7bfa8e522823d8e0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 7 Sep 2021 11:36:39 +0200 Subject: [PATCH 01/18] Add typing to ``filepath`` --- pylint/config/option_manager_mixin.py | 3 ++- pylint/lint/expand_modules.py | 27 +++++++++++++++++--- pylint/lint/parallel.py | 11 ++++++--- pylint/lint/pylinter.py | 34 ++++++++++++++------------ pylint/lint/utils.py | 9 +++---- pylint/reporters/base_reporter.py | 4 +-- pylint/reporters/multi_reporter.py | 5 ++-- pylint/reporters/text.py | 3 ++- pylint/testutils/reporter_for_tests.py | 8 +++--- 9 files changed, 65 insertions(+), 39 deletions(-) diff --git a/pylint/config/option_manager_mixin.py b/pylint/config/option_manager_mixin.py index 106ec47cca..5db2381b71 100644 --- a/pylint/config/option_manager_mixin.py +++ b/pylint/config/option_manager_mixin.py @@ -10,6 +10,7 @@ import optparse # pylint: disable=deprecated-module import os import sys +from typing import List import toml @@ -326,7 +327,7 @@ def load_configuration_from_config(self, config): provider = self._all_options[opt] provider.set_option(opt, opt_value) - def load_command_line_configuration(self, args=None): + def load_command_line_configuration(self, args=None) -> List[str]: """Override configuration according to command line parameters return additional arguments diff --git a/pylint/lint/expand_modules.py b/pylint/lint/expand_modules.py index 11927eb32d..aaca19ab16 100644 --- a/pylint/lint/expand_modules.py +++ b/pylint/lint/expand_modules.py @@ -1,9 +1,28 @@ import os import sys -from typing import List, Pattern, Tuple +from typing import List, Pattern, Tuple, Union from astroid import modutils +if sys.version_info >= (3, 8): + from typing import Literal, TypedDict +else: + from typing_extensions import Literal, TypedDict + + +class MODULE_DESCRIPTION_DICT(TypedDict): + path: str + name: str + isarg: bool + basepath: str + basename: str + + +class ERROR_DESCRIPTION_DICT(TypedDict): + key: Literal["fatal"] + mod: str + ex: Union[ImportError, SyntaxError] + def _modpath_from_file(filename, is_namespace, path=None): def _is_package_cb(path, parts): @@ -42,12 +61,12 @@ def expand_modules( ignore_list: List[str], ignore_list_re: List[Pattern], ignore_list_paths_re: List[Pattern], -) -> Tuple[List[dict], List[dict]]: +) -> Tuple[List[MODULE_DESCRIPTION_DICT], List[ERROR_DESCRIPTION_DICT]]: """take a list of files/modules/packages and return the list of tuple (file, module name) which have to be actually checked """ - result = [] - errors = [] + result: List[MODULE_DESCRIPTION_DICT] = [] + errors: List[ERROR_DESCRIPTION_DICT] = [] path = sys.path.copy() for something in files_or_modules: diff --git a/pylint/lint/parallel.py b/pylint/lint/parallel.py index ad6830721d..cf19d0499b 100644 --- a/pylint/lint/parallel.py +++ b/pylint/lint/parallel.py @@ -3,6 +3,7 @@ import collections import functools +from typing import Any, DefaultDict, Iterator, List, Tuple from pylint import reporters from pylint.lint.utils import _patch_sys_path @@ -62,9 +63,13 @@ def _worker_initialize(linter, arguments=None): _patch_sys_path(arguments or ()) -def _worker_check_single_file(file_item): +def _worker_check_single_file( + file_item: Tuple[str, str, str] +) -> Tuple[int, Any, str, Any, List[Tuple[Any, ...]], Any, Any, DefaultDict[Any, List]]: name, filepath, modname = file_item + if not _worker_linter: + raise Exception("Worker linter not yet initialised") _worker_linter.open() _worker_linter.check_single_file(name, filepath, modname) mapreduce_data = collections.defaultdict(list) @@ -109,7 +114,7 @@ def _merge_mapreduce_data(linter, all_mapreduce_data): checker.reduce_map_data(linter, collated_map_reduce_data[checker.name]) -def check_parallel(linter, jobs, files, arguments=None): +def check_parallel(linter, jobs, files: Iterator[Tuple[str, str, str]], arguments=None): """Use the given linter to lint the files with given amount of workers (jobs) This splits the work filestream-by-filestream. If you need to do work across multiple files, as in the similarity-checker, then inherit from MapReduceMixin and @@ -151,7 +156,7 @@ def check_parallel(linter, jobs, files, arguments=None): linter.set_current_module(module, file_path) for msg in messages: msg = Message(*msg) - linter.reporter.handle_message(msg) + linter.reporter.handle_message(msg) # type: ignore # L134 makes linter always have a reporter attribute all_stats.append(stats) all_mapreduce_data[worker_idx].append(mapreduce_data) linter.msg_status |= msg_status diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index 59276a50a2..24650fd34b 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -11,13 +11,14 @@ import traceback import warnings from io import TextIOWrapper +from typing import Iterator, List, Optional, Tuple, Union import astroid from astroid import AstroidError from pylint import checkers, config, exceptions, interfaces, reporters from pylint.constants import MAIN_CHECKER_NAME, MSG_TYPES -from pylint.lint.expand_modules import expand_modules +from pylint.lint.expand_modules import MODULE_DESCRIPTION_DICT, expand_modules from pylint.lint.parallel import check_parallel from pylint.lint.report_functions import ( report_messages_by_module_stats, @@ -935,16 +936,13 @@ def initialize(self): if not msg.may_be_emitted(): self._msgs_state[msg.msgid] = False - def check(self, files_or_modules): + def check(self, files_or_modules: List[str]) -> None: """main checking entry: check a list of files or modules from their name. - files_or_modules is either a string or list of strings presenting modules to check. + files_or_modules is a list of strings presenting modules to check. """ self.initialize() - if not isinstance(files_or_modules, (list, tuple)): - files_or_modules = (files_or_modules,) - if self.config.from_stdin: if len(files_or_modules) != 1: raise exceptions.InvalidArgsError( @@ -970,7 +968,7 @@ def check(self, files_or_modules): files_or_modules, ) - def check_single_file(self, name, filepath, modname): + def check_single_file(self, name: str, filepath: str, modname: str) -> None: """Check single file The arguments are the same that are documented in _check_files @@ -982,7 +980,11 @@ def check_single_file(self, name, filepath, modname): self.get_ast, check_astroid_module, name, filepath, modname ) - def _check_files(self, get_ast, file_descrs): + def _check_files( + self, + get_ast, + file_descrs: Union[List[Tuple[str, str, str]], Iterator[Tuple[str, str, str]]], + ): """Check all files from file_descrs The file_descrs should be iterable of tuple (name, filepath, modname) @@ -1004,12 +1006,14 @@ def _check_files(self, get_ast, file_descrs): msg = get_fatal_error_message(filepath, template_path) if isinstance(ex, AstroidError): symbol = "astroid-error" - msg = (filepath, msg) + self.add_message(symbol, args=(filepath, msg)) else: symbol = "fatal" - self.add_message(symbol, args=msg) + self.add_message(symbol, args=msg) - def _check_file(self, get_ast, check_astroid_module, name, filepath, modname): + def _check_file( + self, get_ast, check_astroid_module, name: str, filepath: str, modname: str + ): """Check a file using the passed utility functions (get_ast and check_astroid_module) :param callable get_ast: callable returning AST from defined file taking the following arguments @@ -1042,7 +1046,7 @@ def _check_file(self, get_ast, check_astroid_module, name, filepath, modname): self.add_message(msgid, line, None, args) @staticmethod - def _get_file_descr_from_stdin(filepath): + def _get_file_descr_from_stdin(filepath: str) -> Tuple[str, str, str]: """Return file description (tuple of module name, file path, base name) from given file path This method is used for creating suitable file description for _check_files when the @@ -1058,7 +1062,7 @@ def _get_file_descr_from_stdin(filepath): return (modname, filepath, filepath) - def _iterate_file_descrs(self, files_or_modules): + def _iterate_file_descrs(self, files_or_modules) -> Iterator[Tuple[str, str, str]]: """Return generator yielding file descriptions (tuples of module name, file path, base name) The returned generator yield one item for each Python module that should be linted. @@ -1068,7 +1072,7 @@ def _iterate_file_descrs(self, files_or_modules): if self.should_analyze_file(name, filepath, is_argument=is_arg): yield (name, filepath, descr["basename"]) - def _expand_files(self, modules): + def _expand_files(self, modules) -> List[MODULE_DESCRIPTION_DICT]: """get modules and errors from a list of modules and handle errors""" result, errors = expand_modules( modules, @@ -1085,7 +1089,7 @@ def _expand_files(self, modules): self.add_message(key, args=message) return result - def set_current_module(self, modname, filepath=None): + def set_current_module(self, modname, filepath: Optional[str] = None): """set the name of the currently analyzed module and init statistics for it """ diff --git a/pylint/lint/utils.py b/pylint/lint/utils.py index 280b144b72..ed0cefb770 100644 --- a/pylint/lint/utils.py +++ b/pylint/lint/utils.py @@ -5,8 +5,7 @@ import sys import traceback from datetime import datetime -from pathlib import Path, PosixPath -from typing import Union +from pathlib import Path from pylint.config import PYLINT_HOME from pylint.lint.expand_modules import get_python_path @@ -16,9 +15,7 @@ class ArgumentPreprocessingError(Exception): """Raised if an error occurs during argument preprocessing.""" -def prepare_crash_report( - ex: Exception, filepath: PosixPath, crash_file_path: Union[Path, str] -) -> Path: +def prepare_crash_report(ex: Exception, filepath: str, crash_file_path: str) -> Path: issue_template_path = ( Path(PYLINT_HOME) / datetime.now().strftime(str(crash_file_path)) ).resolve() @@ -63,7 +60,7 @@ def prepare_crash_report( return issue_template_path -def get_fatal_error_message(filepath: str, issue_template_path: str) -> str: +def get_fatal_error_message(filepath: str, issue_template_path: Path) -> str: return ( f"Fatal error while checking '{filepath}'. " f"Please open an issue in our bug tracker so we address this. " diff --git a/pylint/reporters/base_reporter.py b/pylint/reporters/base_reporter.py index fda99f4ef9..1f5f1e56f7 100644 --- a/pylint/reporters/base_reporter.py +++ b/pylint/reporters/base_reporter.py @@ -3,7 +3,7 @@ import os import sys -from typing import List +from typing import List, Optional from pylint.message import Message @@ -62,7 +62,7 @@ def display_messages(self, layout): # Event callbacks - def on_set_current_module(self, module, filepath): + def on_set_current_module(self, module: str, filepath: Optional[str]) -> None: """Hook called when a module starts to be analysed.""" def on_close(self, stats, previous_stats): diff --git a/pylint/reporters/multi_reporter.py b/pylint/reporters/multi_reporter.py index a4dbae53b7..7bd85b2245 100644 --- a/pylint/reporters/multi_reporter.py +++ b/pylint/reporters/multi_reporter.py @@ -3,7 +3,7 @@ import os -from typing import IO, Any, AnyStr, Callable, List, Mapping, Optional, Union +from typing import IO, Any, AnyStr, Callable, List, Mapping, Optional from pylint.interfaces import IReporter from pylint.message import Message @@ -11,7 +11,6 @@ from pylint.reporters.ureports.nodes import BaseLayout AnyFile = IO[AnyStr] -AnyPath = Union[str, bytes, os.PathLike] PyLinter = Any @@ -88,7 +87,7 @@ def display_messages(self, layout: BaseLayout) -> None: for rep in self._sub_reporters: rep.display_messages(layout) - def on_set_current_module(self, module: str, filepath: Optional[AnyPath]) -> None: + def on_set_current_module(self, module: str, filepath: Optional[str]) -> None: """hook called when a module starts to be analysed""" for rep in self._sub_reporters: rep.on_set_current_module(module, filepath) diff --git a/pylint/reporters/text.py b/pylint/reporters/text.py index 6d9a05f783..32c75fd7fb 100644 --- a/pylint/reporters/text.py +++ b/pylint/reporters/text.py @@ -25,6 +25,7 @@ import os import sys import warnings +from typing import Optional from pylint import utils from pylint.interfaces import IReporter @@ -136,7 +137,7 @@ def __init__(self, output=None): self._modules = set() self._template = self.line_format - def on_set_current_module(self, module, filepath): + def on_set_current_module(self, module: str, filepath: Optional[str]) -> None: self._template = str(self.linter.config.msg_template or self._template) def write_message(self, msg): diff --git a/pylint/testutils/reporter_for_tests.py b/pylint/testutils/reporter_for_tests.py index b9e01b5a44..9527544a06 100644 --- a/pylint/testutils/reporter_for_tests.py +++ b/pylint/testutils/reporter_for_tests.py @@ -3,7 +3,7 @@ from io import StringIO from os import getcwd, linesep, sep -from typing import Dict, List +from typing import Dict, List, Optional from pylint import interfaces from pylint.message import Message @@ -51,7 +51,7 @@ def finalize(self): return result # pylint: disable=unused-argument - def on_set_current_module(self, module, filepath): + def on_set_current_module(self, module: str, filepath: Optional[str]) -> None: pass # pylint: enable=unused-argument @@ -63,14 +63,14 @@ def display_reports(self, layout): class MinimalTestReporter(BaseReporter): - def on_set_current_module(self, module, filepath): + def on_set_current_module(self, module: str, filepath: Optional[str]) -> None: self.messages = [] _display = None class FunctionalTestReporter(BaseReporter): - def on_set_current_module(self, module, filepath): + def on_set_current_module(self, module: str, filepath: Optional[str]) -> None: self.messages = [] def display_reports(self, layout): From 56b106117fac04dbfb8db3d218cca8a81edff273 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 7 Sep 2021 11:50:07 +0200 Subject: [PATCH 02/18] Change tests for ``filepath`` changes --- tests/checkers/unittest_variables.py | 2 +- tests/extensions/test_broad_try_clause.py | 3 ++- tests/extensions/test_comparetozero.py | 3 ++- tests/lint/test_utils.py | 6 +++--- tests/lint/unittest_lint.py | 10 ++++----- tests/test_check_parallel.py | 12 +++++++---- tests/test_import_graph.py | 2 +- tests/test_regr.py | 26 +++++++++++------------ 8 files changed, 35 insertions(+), 29 deletions(-) diff --git a/tests/checkers/unittest_variables.py b/tests/checkers/unittest_variables.py index 7ab79bf6f4..62a9ca3d0e 100644 --- a/tests/checkers/unittest_variables.py +++ b/tests/checkers/unittest_variables.py @@ -368,7 +368,7 @@ def test_package_all() -> None: sys.path.insert(0, REGR_DATA_DIR) try: - linter.check(os.path.join(REGR_DATA_DIR, "package_all")) + linter.check([os.path.join(REGR_DATA_DIR, "package_all")]) got = linter.reporter.finalize().strip() assert got == "E: 3: Undefined variable name 'missing' in __all__" finally: diff --git a/tests/extensions/test_broad_try_clause.py b/tests/extensions/test_broad_try_clause.py index 21c79fed06..a41a7ab3dc 100644 --- a/tests/extensions/test_broad_try_clause.py +++ b/tests/extensions/test_broad_try_clause.py @@ -13,6 +13,7 @@ """Tests for the pylint checker in :mod:`pylint.extensions.broad_try_clause`""" import unittest from os import path as osp +from typing import Optional from pylint import checkers from pylint.extensions.broad_try_clause import BroadTryClauseChecker @@ -21,7 +22,7 @@ class BroadTryClauseTestReporter(BaseReporter): - def on_set_current_module(self, module: str, filepath: str) -> None: + def on_set_current_module(self, module: str, filepath: Optional[str]) -> None: self.messages = [] def _display(self, layout): diff --git a/tests/extensions/test_comparetozero.py b/tests/extensions/test_comparetozero.py index 167a615efc..c90c211834 100644 --- a/tests/extensions/test_comparetozero.py +++ b/tests/extensions/test_comparetozero.py @@ -14,6 +14,7 @@ import os import unittest +from typing import Optional from pylint import checkers from pylint.extensions.comparetozero import CompareToZeroChecker @@ -22,7 +23,7 @@ class CompareToZeroTestReporter(BaseReporter): - def on_set_current_module(self, module: str, filepath: str) -> None: + def on_set_current_module(self, module: str, filepath: Optional[str]) -> None: self.messages = [] def _display(self, layout): diff --git a/tests/lint/test_utils.py b/tests/lint/test_utils.py index 80e7287f0b..f7294f353c 100644 --- a/tests/lint/test_utils.py +++ b/tests/lint/test_utils.py @@ -1,4 +1,4 @@ -from pathlib import PosixPath +from pathlib import Path, PosixPath from pylint.lint.utils import get_fatal_error_message, prepare_crash_report @@ -13,7 +13,7 @@ def test_prepare_crash_report(tmp_path: PosixPath) -> None: raise Exception(exception_content) except Exception as ex: # pylint: disable=broad-except template_path = prepare_crash_report( - ex, python_file, tmp_path / "pylint-crash-%Y.txt" + ex, str(python_file), str(tmp_path / "pylint-crash-%Y.txt") ) assert str(tmp_path) in str(template_path) with open(template_path, encoding="utf8") as f: @@ -27,7 +27,7 @@ def test_prepare_crash_report(tmp_path: PosixPath) -> None: def test_get_fatal_error_message() -> None: python_path = "mypath.py" crash_path = "crash.txt" - msg = get_fatal_error_message(python_path, crash_path) + msg = get_fatal_error_message(python_path, Path(crash_path)) assert python_path in msg assert crash_path in msg assert "open an issue" in msg diff --git a/tests/lint/unittest_lint.py b/tests/lint/unittest_lint.py index 18656ad740..ef004a08a8 100644 --- a/tests/lint/unittest_lint.py +++ b/tests/lint/unittest_lint.py @@ -269,7 +269,7 @@ def visit_class(self, _): linter.open() out = StringIO() linter.set_reporter(text.TextReporter(out)) - linter.check("abc") + linter.check(["abc"]) def test_enable_message(init_linter: PyLinter) -> None: @@ -573,7 +573,7 @@ def test_init_hooks_called_before_load_plugins() -> None: def test_analyze_explicit_script(linter: PyLinter) -> None: linter.set_reporter(testutils.GenericTestReporter()) - linter.check(os.path.join(DATA_DIR, "ascript")) + linter.check([os.path.join(DATA_DIR, "ascript")]) assert ["C: 2: Line too long (175/100)"] == linter.reporter.messages @@ -826,15 +826,15 @@ def test_filename_with__init__(init_linter: PyLinter) -> None: def test_by_module_statement_value(init_linter: PyLinter) -> None: """Test "statement" for each module analized of computed correctly.""" linter = init_linter - linter.check(os.path.join(os.path.dirname(__file__), "data")) + linter.check([os.path.join(os.path.dirname(__file__), "data")]) for module, module_stats in linter.stats["by_module"].items(): linter2 = init_linter if module == "data": - linter2.check(os.path.join(os.path.dirname(__file__), "data/__init__.py")) + linter2.check([os.path.join(os.path.dirname(__file__), "data/__init__.py")]) else: - linter2.check(os.path.join(os.path.dirname(__file__), module)) + linter2.check([os.path.join(os.path.dirname(__file__), module)]) # Check that the by_module "statement" is equal to the global "statement" # computed for that module diff --git a/tests/test_check_parallel.py b/tests/test_check_parallel.py index f87f818450..ef12604ce9 100644 --- a/tests/test_check_parallel.py +++ b/tests/test_check_parallel.py @@ -177,7 +177,7 @@ def test_worker_initialize(self) -> None: def test_worker_check_single_file_uninitialised(self) -> None: pylint.lint.parallel._worker_linter = None with pytest.raises( # Objects that do not match the linter interface will fail - AttributeError, match="'NoneType' object has no attribute 'open'" + Exception, match="Worker linter not yet initialised" ): worker_check_single_file(_gen_file_data()) @@ -289,7 +289,9 @@ def test_sequential_checkers_work(self) -> None: # Invoke the lint process in a multiprocess way, although we only specify one # job. - check_parallel(linter, jobs=1, files=single_file_container, arguments=None) + check_parallel( + linter, jobs=1, files=iter(single_file_container), arguments=None + ) assert len(linter.get_checkers()) == 2, ( "We should only have the 'master' and 'sequential-checker' " "checkers registered" @@ -318,7 +320,7 @@ def test_sequential_checkers_work(self) -> None: # now run the regular mode of checking files and check that, in this proc, we # collect the right data - filepath = single_file_container[0][1] # get the filepath element + filepath = [single_file_container[0][1]] # get the filepath element linter.check(filepath) assert { "by_module": { @@ -356,7 +358,9 @@ def test_invoke_single_job(self) -> None: # Invoke the lint process in a multiprocess way, although we only specify one # job. - check_parallel(linter, jobs=1, files=single_file_container, arguments=None) + check_parallel( + linter, jobs=1, files=iter(single_file_container), arguments=None + ) assert { "by_module": { diff --git a/tests/test_import_graph.py b/tests/test_import_graph.py index 8029707189..42e3baf56b 100644 --- a/tests/test_import_graph.py +++ b/tests/test_import_graph.py @@ -106,7 +106,7 @@ def test_checker_dep_graphs(linter: PyLinter) -> None: linter.global_set_option("int-import-graph", "int_import.dot") # ignore this file causing spurious MemoryError w/ some python version (>=2.3?) linter.global_set_option("ignore", ("func_unknown_encoding.py",)) - linter.check("input") + linter.check(["input"]) linter.generate_reports() assert exists("import.dot") assert exists("ext_import.dot") diff --git a/tests/test_regr.py b/tests/test_regr.py index 793a196129..ac9e5c5805 100644 --- a/tests/test_regr.py +++ b/tests/test_regr.py @@ -61,15 +61,15 @@ def Equals(expected): @pytest.mark.parametrize( "file_name, check", [ - ("package.__init__", Equals("")), - ("precedence_test", Equals("")), - ("import_package_subpackage_module", Equals("")), - ("pylint.checkers.__init__", lambda x: "__path__" not in x), - (join(REGR_DATA, "classdoc_usage.py"), Equals("")), - (join(REGR_DATA, "module_global.py"), Equals("")), - (join(REGR_DATA, "decimal_inference.py"), Equals("")), - (join(REGR_DATA, "absimp", "string.py"), Equals("")), - (join(REGR_DATA, "bad_package"), lambda x: "Unused import missing" in x), + (["package.__init__"], Equals("")), + (["precedence_test"], Equals("")), + (["import_package_subpackage_module"], Equals("")), + (["pylint.checkers.__init__"], lambda x: "__path__" not in x), + ([join(REGR_DATA, "classdoc_usage.py")], Equals("")), + ([join(REGR_DATA, "module_global.py")], Equals("")), + ([join(REGR_DATA, "decimal_inference.py")], Equals("")), + ([join(REGR_DATA, "absimp", "string.py")], Equals("")), + ([join(REGR_DATA, "bad_package")], lambda x: "Unused import missing" in x), ], ) def test_package(finalize_linter, file_name, check): @@ -94,7 +94,7 @@ def test_crash(finalize_linter, file_name): "fname", [x for x in os.listdir(REGR_DATA) if x.endswith("_crash.py")] ) def test_descriptor_crash(fname: str, finalize_linter: PyLinter) -> None: - finalize_linter.check(join(REGR_DATA, fname)) + finalize_linter.check([join(REGR_DATA, fname)]) finalize_linter.reporter.finalize().strip() @@ -109,13 +109,13 @@ def modify_path() -> Iterator: @pytest.mark.usefixtures("modify_path") def test_check_package___init__(finalize_linter: PyLinter) -> None: - filename = "package.__init__" + filename = ["package.__init__"] finalize_linter.check(filename) checked = list(finalize_linter.stats["by_module"].keys()) - assert checked == [filename] + assert checked == filename os.chdir(join(REGR_DATA, "package")) - finalize_linter.check("__init__") + finalize_linter.check(["__init__"]) checked = list(finalize_linter.stats["by_module"].keys()) assert checked == ["__init__"] From 565e245323dab3292cb54a494901430c831806b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 14 Sep 2021 22:06:25 +0200 Subject: [PATCH 03/18] Code review --- pylint/lint/expand_modules.py | 10 +++++----- pylint/lint/pylinter.py | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pylint/lint/expand_modules.py b/pylint/lint/expand_modules.py index aaca19ab16..3781789b2c 100644 --- a/pylint/lint/expand_modules.py +++ b/pylint/lint/expand_modules.py @@ -10,7 +10,7 @@ from typing_extensions import Literal, TypedDict -class MODULE_DESCRIPTION_DICT(TypedDict): +class ModuleDescriptionDict(TypedDict): path: str name: str isarg: bool @@ -18,7 +18,7 @@ class MODULE_DESCRIPTION_DICT(TypedDict): basename: str -class ERROR_DESCRIPTION_DICT(TypedDict): +class ErrorDescriptionTest(TypedDict): key: Literal["fatal"] mod: str ex: Union[ImportError, SyntaxError] @@ -61,12 +61,12 @@ def expand_modules( ignore_list: List[str], ignore_list_re: List[Pattern], ignore_list_paths_re: List[Pattern], -) -> Tuple[List[MODULE_DESCRIPTION_DICT], List[ERROR_DESCRIPTION_DICT]]: +) -> Tuple[List[ModuleDescriptionDict], List[ErrorDescriptionTest]]: """take a list of files/modules/packages and return the list of tuple (file, module name) which have to be actually checked """ - result: List[MODULE_DESCRIPTION_DICT] = [] - errors: List[ERROR_DESCRIPTION_DICT] = [] + result: List[ModuleDescriptionDict] = [] + errors: List[ErrorDescriptionTest] = [] path = sys.path.copy() for something in files_or_modules: diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index 24650fd34b..9949307361 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -18,7 +18,7 @@ from pylint import checkers, config, exceptions, interfaces, reporters from pylint.constants import MAIN_CHECKER_NAME, MSG_TYPES -from pylint.lint.expand_modules import MODULE_DESCRIPTION_DICT, expand_modules +from pylint.lint.expand_modules import ModuleDescriptionDict, expand_modules from pylint.lint.parallel import check_parallel from pylint.lint.report_functions import ( report_messages_by_module_stats, @@ -1072,7 +1072,7 @@ def _iterate_file_descrs(self, files_or_modules) -> Iterator[Tuple[str, str, str if self.should_analyze_file(name, filepath, is_argument=is_arg): yield (name, filepath, descr["basename"]) - def _expand_files(self, modules) -> List[MODULE_DESCRIPTION_DICT]: + def _expand_files(self, modules) -> List[ModuleDescriptionDict]: """get modules and errors from a list of modules and handle errors""" result, errors = expand_modules( modules, From 2b46031afd204096500f0c0d63eae133a4870b60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 14 Sep 2021 22:08:31 +0200 Subject: [PATCH 04/18] Additional code review --- pylint/lint/parallel.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/lint/parallel.py b/pylint/lint/parallel.py index cf19d0499b..15a064bf36 100644 --- a/pylint/lint/parallel.py +++ b/pylint/lint/parallel.py @@ -156,7 +156,7 @@ def check_parallel(linter, jobs, files: Iterator[Tuple[str, str, str]], argument linter.set_current_module(module, file_path) for msg in messages: msg = Message(*msg) - linter.reporter.handle_message(msg) # type: ignore # L134 makes linter always have a reporter attribute + linter.reporter.handle_message(msg) # type: ignore # set_reporter() call above makes linter have a reporter attr all_stats.append(stats) all_mapreduce_data[worker_idx].append(mapreduce_data) linter.msg_status |= msg_status From 7bd3810c40b1d3ecb0dbc92b51e57775efd5e52a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 14 Sep 2021 22:42:42 +0200 Subject: [PATCH 05/18] Update pylint/lint/parallel.py Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com> --- pylint/lint/parallel.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/lint/parallel.py b/pylint/lint/parallel.py index 15a064bf36..e501363408 100644 --- a/pylint/lint/parallel.py +++ b/pylint/lint/parallel.py @@ -156,7 +156,7 @@ def check_parallel(linter, jobs, files: Iterator[Tuple[str, str, str]], argument linter.set_current_module(module, file_path) for msg in messages: msg = Message(*msg) - linter.reporter.handle_message(msg) # type: ignore # set_reporter() call above makes linter have a reporter attr + linter.reporter.handle_message(msg) # type: ignore # linter.set_reporter() call above makes linter have a reporter attr all_stats.append(stats) all_mapreduce_data[worker_idx].append(mapreduce_data) linter.msg_status |= msg_status From f4a7045b99a08c08bc480f2262a444c79f0d2713 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 14 Sep 2021 23:02:47 +0200 Subject: [PATCH 06/18] Add pylint/typing.py and FileItem NamedTuple --- pylint/lint/parallel.py | 5 +++-- pylint/lint/pylinter.py | 13 +++++++------ pylint/typing.py | 12 ++++++++++++ tests/test_check_parallel.py | 9 +++++---- 4 files changed, 27 insertions(+), 12 deletions(-) create mode 100644 pylint/typing.py diff --git a/pylint/lint/parallel.py b/pylint/lint/parallel.py index e501363408..1d7f9d9c4e 100644 --- a/pylint/lint/parallel.py +++ b/pylint/lint/parallel.py @@ -8,6 +8,7 @@ from pylint import reporters from pylint.lint.utils import _patch_sys_path from pylint.message import Message +from pylint.typing import FileItem try: import multiprocessing @@ -64,7 +65,7 @@ def _worker_initialize(linter, arguments=None): def _worker_check_single_file( - file_item: Tuple[str, str, str] + file_item: FileItem, ) -> Tuple[int, Any, str, Any, List[Tuple[Any, ...]], Any, Any, DefaultDict[Any, List]]: name, filepath, modname = file_item @@ -114,7 +115,7 @@ def _merge_mapreduce_data(linter, all_mapreduce_data): checker.reduce_map_data(linter, collated_map_reduce_data[checker.name]) -def check_parallel(linter, jobs, files: Iterator[Tuple[str, str, str]], arguments=None): +def check_parallel(linter, jobs, files: Iterator[FileItem], arguments=None): """Use the given linter to lint the files with given amount of workers (jobs) This splits the work filestream-by-filestream. If you need to do work across multiple files, as in the similarity-checker, then inherit from MapReduceMixin and diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index d12f19f2ce..d760b889f2 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -11,7 +11,7 @@ import traceback import warnings from io import TextIOWrapper -from typing import Iterator, List, Optional, Tuple, Union +from typing import Iterator, List, Optional, Union import astroid from astroid import AstroidError, nodes @@ -32,6 +32,7 @@ ) from pylint.message import MessageDefinitionStore, MessagesHandlerMixIn from pylint.reporters.ureports import nodes as report_nodes +from pylint.typing import FileItem from pylint.utils import ASTWalker, FileState, utils from pylint.utils.pragma_parser import ( OPTION_PO, @@ -983,7 +984,7 @@ def check_single_file(self, name: str, filepath: str, modname: str) -> None: def _check_files( self, get_ast, - file_descrs: Union[List[Tuple[str, str, str]], Iterator[Tuple[str, str, str]]], + file_descrs: Union[List[FileItem], Iterator[FileItem]], ): """Check all files from file_descrs @@ -1046,7 +1047,7 @@ def _check_file( self.add_message(msgid, line, None, args) @staticmethod - def _get_file_descr_from_stdin(filepath: str) -> Tuple[str, str, str]: + def _get_file_descr_from_stdin(filepath: str) -> FileItem: """Return file description (tuple of module name, file path, base name) from given file path This method is used for creating suitable file description for _check_files when the @@ -1060,9 +1061,9 @@ def _get_file_descr_from_stdin(filepath: str) -> Tuple[str, str, str]: except ImportError: modname = os.path.splitext(os.path.basename(filepath))[0] - return (modname, filepath, filepath) + return FileItem(modname, filepath, filepath) - def _iterate_file_descrs(self, files_or_modules) -> Iterator[Tuple[str, str, str]]: + def _iterate_file_descrs(self, files_or_modules) -> Iterator[FileItem]: """Return generator yielding file descriptions (tuples of module name, file path, base name) The returned generator yield one item for each Python module that should be linted. @@ -1070,7 +1071,7 @@ def _iterate_file_descrs(self, files_or_modules) -> Iterator[Tuple[str, str, str for descr in self._expand_files(files_or_modules): name, filepath, is_arg = descr["name"], descr["path"], descr["isarg"] if self.should_analyze_file(name, filepath, is_argument=is_arg): - yield (name, filepath, descr["basename"]) + yield FileItem(name, filepath, descr["basename"]) def _expand_files(self, modules) -> List[ModuleDescriptionDict]: """get modules and errors from a list of modules and handle errors""" diff --git a/pylint/typing.py b/pylint/typing.py new file mode 100644 index 0000000000..4aa9691ce5 --- /dev/null +++ b/pylint/typing.py @@ -0,0 +1,12 @@ +# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html +# For details: https://github.com/PyCQA/pylint/blob/main/LICENSE + +"""A collection of typing utilities.""" +from typing import NamedTuple + + +# Represents data about a file handled by pylint +class FileItem(NamedTuple): + name: str + filepath: str + modpath: str diff --git a/tests/test_check_parallel.py b/tests/test_check_parallel.py index 102c0f1225..8ba9659b8d 100644 --- a/tests/test_check_parallel.py +++ b/tests/test_check_parallel.py @@ -11,7 +11,7 @@ import collections import os -from typing import List, Tuple +from typing import List import pytest from astroid import nodes @@ -24,14 +24,15 @@ from pylint.lint.parallel import _worker_initialize as worker_initialize from pylint.lint.parallel import check_parallel from pylint.testutils import GenericTestReporter as Reporter +from pylint.typing import FileItem -def _gen_file_data(idx: int = 0) -> Tuple[str, str, str]: +def _gen_file_data(idx: int = 0) -> FileItem: """Generates a file to use as a stream""" filepath = os.path.abspath( os.path.join(os.path.dirname(__file__), "input", "similar1") ) - file_data = ( + file_data = FileItem( f"--test-file_data-name-{idx}--", filepath, f"--test-file_data-modname-{idx}--", @@ -39,7 +40,7 @@ def _gen_file_data(idx: int = 0) -> Tuple[str, str, str]: return file_data -def _gen_file_datas(count: int = 1) -> List[Tuple[str, str, str]]: +def _gen_file_datas(count: int = 1) -> List[FileItem]: return [_gen_file_data(idx) for idx in range(count)] From 6af1902c84140a2b22d8a16f42698803903aa4f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Tue, 14 Sep 2021 23:45:55 +0200 Subject: [PATCH 07/18] Code review --- pylint/lint/expand_modules.py | 25 ++++--------------------- pylint/lint/parallel.py | 10 ++++------ pylint/lint/pylinter.py | 29 ++++++++++++----------------- pylint/typing.py | 29 +++++++++++++++++++++++++++-- tests/unittest_reporting.py | 3 ++- 5 files changed, 49 insertions(+), 47 deletions(-) diff --git a/pylint/lint/expand_modules.py b/pylint/lint/expand_modules.py index 3781789b2c..1d4432322b 100644 --- a/pylint/lint/expand_modules.py +++ b/pylint/lint/expand_modules.py @@ -1,27 +1,10 @@ import os import sys -from typing import List, Pattern, Tuple, Union +from typing import List, Pattern, Tuple from astroid import modutils -if sys.version_info >= (3, 8): - from typing import Literal, TypedDict -else: - from typing_extensions import Literal, TypedDict - - -class ModuleDescriptionDict(TypedDict): - path: str - name: str - isarg: bool - basepath: str - basename: str - - -class ErrorDescriptionTest(TypedDict): - key: Literal["fatal"] - mod: str - ex: Union[ImportError, SyntaxError] +from pylint.typing import ErrorDescriptionDict, ModuleDescriptionDict def _modpath_from_file(filename, is_namespace, path=None): @@ -61,12 +44,12 @@ def expand_modules( ignore_list: List[str], ignore_list_re: List[Pattern], ignore_list_paths_re: List[Pattern], -) -> Tuple[List[ModuleDescriptionDict], List[ErrorDescriptionTest]]: +) -> Tuple[List[ModuleDescriptionDict], List[ErrorDescriptionDict]]: """take a list of files/modules/packages and return the list of tuple (file, module name) which have to be actually checked """ result: List[ModuleDescriptionDict] = [] - errors: List[ErrorDescriptionTest] = [] + errors: List[ErrorDescriptionDict] = [] path = sys.path.copy() for something in files_or_modules: diff --git a/pylint/lint/parallel.py b/pylint/lint/parallel.py index 1d7f9d9c4e..322d9dbd8f 100644 --- a/pylint/lint/parallel.py +++ b/pylint/lint/parallel.py @@ -3,7 +3,7 @@ import collections import functools -from typing import Any, DefaultDict, Iterator, List, Tuple +from typing import Any, DefaultDict, Iterable, List, Tuple from pylint import reporters from pylint.lint.utils import _patch_sys_path @@ -67,12 +67,10 @@ def _worker_initialize(linter, arguments=None): def _worker_check_single_file( file_item: FileItem, ) -> Tuple[int, Any, str, Any, List[Tuple[Any, ...]], Any, Any, DefaultDict[Any, List]]: - name, filepath, modname = file_item - if not _worker_linter: raise Exception("Worker linter not yet initialised") _worker_linter.open() - _worker_linter.check_single_file(name, filepath, modname) + _worker_linter.check_single_file(file_item) mapreduce_data = collections.defaultdict(list) for checker in _worker_linter.get_checkers(): try: @@ -85,7 +83,7 @@ def _worker_check_single_file( return ( id(multiprocessing.current_process()), _worker_linter.current_name, - filepath, + file_item[1], _worker_linter.file_state.base_name, msgs, _worker_linter.stats, @@ -115,7 +113,7 @@ def _merge_mapreduce_data(linter, all_mapreduce_data): checker.reduce_map_data(linter, collated_map_reduce_data[checker.name]) -def check_parallel(linter, jobs, files: Iterator[FileItem], arguments=None): +def check_parallel(linter, jobs, files: Iterable[FileItem], arguments=None): """Use the given linter to lint the files with given amount of workers (jobs) This splits the work filestream-by-filestream. If you need to do work across multiple files, as in the similarity-checker, then inherit from MapReduceMixin and diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index d760b889f2..2f9be4b2e3 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -11,14 +11,14 @@ import traceback import warnings from io import TextIOWrapper -from typing import Iterator, List, Optional, Union +from typing import Iterable, Iterator, List, Optional, Sequence import astroid from astroid import AstroidError, nodes from pylint import checkers, config, exceptions, interfaces, reporters from pylint.constants import MAIN_CHECKER_NAME, MSG_TYPES -from pylint.lint.expand_modules import ModuleDescriptionDict, expand_modules +from pylint.lint.expand_modules import expand_modules from pylint.lint.parallel import check_parallel from pylint.lint.report_functions import ( report_messages_by_module_stats, @@ -32,7 +32,7 @@ ) from pylint.message import MessageDefinitionStore, MessagesHandlerMixIn from pylint.reporters.ureports import nodes as report_nodes -from pylint.typing import FileItem +from pylint.typing import FileItem, ModuleDescriptionDict from pylint.utils import ASTWalker, FileState, utils from pylint.utils.pragma_parser import ( OPTION_PO, @@ -937,7 +937,7 @@ def initialize(self): if not msg.may_be_emitted(): self._msgs_state[msg.msgid] = False - def check(self, files_or_modules: List[str]) -> None: + def check(self, files_or_modules: Sequence[str]) -> None: """main checking entry: check a list of files or modules from their name. files_or_modules is a list of strings presenting modules to check. @@ -969,7 +969,7 @@ def check(self, files_or_modules: List[str]) -> None: files_or_modules, ) - def check_single_file(self, name: str, filepath: str, modname: str) -> None: + def check_single_file(self, file: FileItem) -> None: """Check single file The arguments are the same that are documented in _check_files @@ -977,15 +977,13 @@ def check_single_file(self, name: str, filepath: str, modname: str) -> None: The initialize() method should be called before calling this method """ with self._astroid_module_checker() as check_astroid_module: - self._check_file( - self.get_ast, check_astroid_module, name, filepath, modname - ) + self._check_file(self.get_ast, check_astroid_module, file) def _check_files( self, get_ast, - file_descrs: Union[List[FileItem], Iterator[FileItem]], - ): + file_descrs: Iterable[FileItem], + ) -> None: """Check all files from file_descrs The file_descrs should be iterable of tuple (name, filepath, modname) @@ -998,7 +996,7 @@ def _check_files( for name, filepath, modname in file_descrs: try: self._check_file( - get_ast, check_astroid_module, name, filepath, modname + get_ast, check_astroid_module, FileItem(name, filepath, modname) ) except Exception as ex: # pylint: disable=broad-except template_path = prepare_crash_report( @@ -1012,9 +1010,7 @@ def _check_files( symbol = "fatal" self.add_message(symbol, args=msg) - def _check_file( - self, get_ast, check_astroid_module, name: str, filepath: str, modname: str - ): + def _check_file(self, get_ast, check_astroid_module, file: FileItem): """Check a file using the passed utility functions (get_ast and check_astroid_module) :param callable get_ast: callable returning AST from defined file taking the following arguments @@ -1022,10 +1018,9 @@ def _check_file( - name: Python module name :param callable check_astroid_module: callable checking an AST taking the following arguments - ast: AST of the module - :param str name: full name of the module - :param str filepath: path to checked file - :param str modname: name of the checked Python module + :param FileItem file: data about the file """ + name, filepath, modname = file self.set_current_module(name, filepath) # get the module representation ast_node = get_ast(filepath, name) diff --git a/pylint/typing.py b/pylint/typing.py index 4aa9691ce5..882f74aadd 100644 --- a/pylint/typing.py +++ b/pylint/typing.py @@ -2,11 +2,36 @@ # For details: https://github.com/PyCQA/pylint/blob/main/LICENSE """A collection of typing utilities.""" -from typing import NamedTuple +import sys +from typing import NamedTuple, Union + +if sys.version_info >= (3, 8): + from typing import Literal, TypedDict +else: + from typing_extensions import Literal, TypedDict -# Represents data about a file handled by pylint class FileItem(NamedTuple): + """Represents data about a file handled by pylint""" + name: str filepath: str modpath: str + + +class ModuleDescriptionDict(TypedDict): + """Represents data about a checked module""" + + path: str + name: str + isarg: bool + basepath: str + basename: str + + +class ErrorDescriptionDict(TypedDict): + """Represents data about errors collected during checking of a module""" + + key: Literal["fatal"] + mod: str + ex: Union[ImportError, SyntaxError] diff --git a/tests/unittest_reporting.py b/tests/unittest_reporting.py index d1d483a359..07ffc27465 100644 --- a/tests/unittest_reporting.py +++ b/tests/unittest_reporting.py @@ -29,6 +29,7 @@ from pylint.lint import PyLinter from pylint.reporters import BaseReporter from pylint.reporters.text import ParseableTextReporter, TextReporter +from pylint.typing import FileItem @pytest.fixture(scope="module") @@ -120,7 +121,7 @@ def test_multi_format_output(tmp_path): linter.reporter.set_output(text) linter.open() - linter.check_single_file("somemodule", source_file, "somemodule") + linter.check_single_file(FileItem("somemodule", source_file, "somemodule")) linter.add_message("line-too-long", line=1, args=(1, 2)) linter.generate_reports() linter.reporter.writeln("direct output") From ccaaa8680eb3481e24753a0c52f8c529c134b570 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Wed, 15 Sep 2021 08:30:23 +0200 Subject: [PATCH 08/18] Apply suggestions from code review Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com> --- pylint/lint/parallel.py | 2 +- pylint/lint/pylinter.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pylint/lint/parallel.py b/pylint/lint/parallel.py index 322d9dbd8f..e31ef9faed 100644 --- a/pylint/lint/parallel.py +++ b/pylint/lint/parallel.py @@ -83,7 +83,7 @@ def _worker_check_single_file( return ( id(multiprocessing.current_process()), _worker_linter.current_name, - file_item[1], + file_item.filepath, _worker_linter.file_state.base_name, msgs, _worker_linter.stats, diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index 2f9be4b2e3..6a69910552 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -940,7 +940,7 @@ def initialize(self): def check(self, files_or_modules: Sequence[str]) -> None: """main checking entry: check a list of files or modules from their name. - files_or_modules is a list of strings presenting modules to check. + files_or_modules is a sequence of strings presenting modules to check. """ self.initialize() From 4c31342c5c44b692e131a697c97d38b9802ce087 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Wed, 15 Sep 2021 08:35:39 +0200 Subject: [PATCH 09/18] Use NamedTuple more efficiently --- pylint/lint/pylinter.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index 6a69910552..42703ed2ea 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -1020,16 +1020,15 @@ def _check_file(self, get_ast, check_astroid_module, file: FileItem): - ast: AST of the module :param FileItem file: data about the file """ - name, filepath, modname = file - self.set_current_module(name, filepath) + self.set_current_module(file.name, file.filepath) # get the module representation - ast_node = get_ast(filepath, name) + ast_node = get_ast(file.filepath, file.name) if ast_node is None: return self._ignore_file = False - self.file_state = FileState(modname) + self.file_state = FileState(file.modpath) # fix the current file (if the source file was not available or # if it's actually a c extension) self.current_file = ast_node.file # pylint: disable=maybe-no-member From b8ba102efda9d957fec609ef477ea8829f0138c5 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 15 Sep 2021 20:13:31 +0000 Subject: [PATCH 10/18] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pylint/lint/parallel.py | 2 +- pylint/lint/pylinter.py | 2 +- pylint/typing.py | 2 +- tests/test_check_parallel.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pylint/lint/parallel.py b/pylint/lint/parallel.py index 7b9d3c1a61..a4334aca17 100644 --- a/pylint/lint/parallel.py +++ b/pylint/lint/parallel.py @@ -3,7 +3,7 @@ import collections import functools -from typing import Any, DefaultDict, Iterable, List, Tuple, TYPE_CHECKING, Dict, Union +from typing import TYPE_CHECKING, Any, DefaultDict, Dict, Iterable, List, Tuple, Union from pylint import reporters from pylint.lint.utils import _patch_sys_path diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index be968b46f5..6ca90737d2 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -32,7 +32,7 @@ ) from pylint.message import MessageDefinitionStore, MessagesHandlerMixIn from pylint.reporters.ureports import nodes as report_nodes -from pylint.typing import FileItem, ModuleDescriptionDict, CheckerStats +from pylint.typing import CheckerStats, FileItem, ModuleDescriptionDict from pylint.utils import ASTWalker, FileState, utils from pylint.utils.pragma_parser import ( OPTION_PO, diff --git a/pylint/typing.py b/pylint/typing.py index 981bde79da..205229213f 100644 --- a/pylint/typing.py +++ b/pylint/typing.py @@ -3,7 +3,7 @@ """A collection of typing utilities.""" import sys -from typing import NamedTuple, Union, TYPE_CHECKING, Dict, List +from typing import TYPE_CHECKING, Dict, List, NamedTuple, Union if TYPE_CHECKING: from typing import Counter # typing.Counter added in Python 3.6.1 diff --git a/tests/test_check_parallel.py b/tests/test_check_parallel.py index cc13d7b788..35f0238b6b 100644 --- a/tests/test_check_parallel.py +++ b/tests/test_check_parallel.py @@ -24,7 +24,7 @@ from pylint.lint.parallel import _worker_initialize as worker_initialize from pylint.lint.parallel import check_parallel from pylint.testutils import GenericTestReporter as Reporter -from pylint.typing import FileItem, CheckerStats +from pylint.typing import CheckerStats, FileItem def _gen_file_data(idx: int = 0) -> FileItem: From 0910978f6284a30a9c5e823dd3c6eb36efee8494 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Wed, 15 Sep 2021 22:38:27 +0200 Subject: [PATCH 11/18] Fix tests and errors --- pylint/lint/pylinter.py | 6 +++--- tests/benchmark/test_baseline_benchmarks.py | 3 ++- tests/test_regr.py | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index 6ca90737d2..c4c8e596ca 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -1095,10 +1095,10 @@ def set_current_module(self, modname, filepath: Optional[str] = None): self.reporter.on_set_current_module(modname, filepath) self.current_name = modname self.current_file = filepath or modname - self.stats["by_module"][modname] = {} - self.stats["by_module"][modname]["statement"] = 0 + self.stats["by_module"][modname] = {} # type: ignore # Refactor of PyLinter.stats necessary + self.stats["by_module"][modname]["statement"] = 0 # type: ignore for msg_cat in MSG_TYPES.values(): - self.stats["by_module"][modname][msg_cat] = 0 + self.stats["by_module"][modname][msg_cat] = 0 # type: ignore @contextlib.contextmanager def _astroid_module_checker(self): diff --git a/tests/benchmark/test_baseline_benchmarks.py b/tests/benchmark/test_baseline_benchmarks.py index 27dada39d5..9b64cd01ce 100644 --- a/tests/benchmark/test_baseline_benchmarks.py +++ b/tests/benchmark/test_baseline_benchmarks.py @@ -22,6 +22,7 @@ from pylint.checkers.base_checker import BaseChecker from pylint.lint import PyLinter, Run, check_parallel from pylint.testutils import GenericTestReporter as Reporter +from pylint.typing import FileItem from pylint.utils import register_plugins @@ -111,7 +112,7 @@ class TestEstablishBaselineBenchmarks: impact everything else""" empty_filepath = _empty_filepath() - empty_file_info = ( + empty_file_info = FileItem( "name-emptyfile-file", _empty_filepath(), "modname-emptyfile-mod", diff --git a/tests/test_regr.py b/tests/test_regr.py index ac9e5c5805..afb5e69ac4 100644 --- a/tests/test_regr.py +++ b/tests/test_regr.py @@ -111,12 +111,12 @@ def modify_path() -> Iterator: def test_check_package___init__(finalize_linter: PyLinter) -> None: filename = ["package.__init__"] finalize_linter.check(filename) - checked = list(finalize_linter.stats["by_module"].keys()) + checked = list(finalize_linter.stats["by_module"].keys()) # type: ignore # Refactor of PyLinter.stats necessary assert checked == filename os.chdir(join(REGR_DATA, "package")) finalize_linter.check(["__init__"]) - checked = list(finalize_linter.stats["by_module"].keys()) + checked = list(finalize_linter.stats["by_module"].keys()) # type: ignore assert checked == ["__init__"] From dd67951dbc677d3cf703a24f2324637b4787262f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Thu, 16 Sep 2021 11:45:17 +0200 Subject: [PATCH 12/18] Update pylint/lint/pylinter.py Co-authored-by: Pierre Sassoulas --- pylint/lint/pylinter.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index c4c8e596ca..892b7d3cc3 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -945,7 +945,9 @@ def check(self, files_or_modules: Sequence[str]) -> None: files_or_modules is a sequence of strings presenting modules to check. """ self.initialize() - + if not isinstance(files_or_modules, (list, tuple)): + warnins.warn("In pylint 3.0, the checkers check function will only accept sequence of string") + files_or_modules = (files_or_modules,) if self.config.from_stdin: if len(files_or_modules) != 1: raise exceptions.InvalidArgsError( From 32742d2036fcad37b387a50a2e3f559990bef862 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 16 Sep 2021 09:46:04 +0000 Subject: [PATCH 13/18] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- pylint/lint/pylinter.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index 892b7d3cc3..9ebd7d54f0 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -946,7 +946,9 @@ def check(self, files_or_modules: Sequence[str]) -> None: """ self.initialize() if not isinstance(files_or_modules, (list, tuple)): - warnins.warn("In pylint 3.0, the checkers check function will only accept sequence of string") + warnins.warn( + "In pylint 3.0, the checkers check function will only accept sequence of string" + ) files_or_modules = (files_or_modules,) if self.config.from_stdin: if len(files_or_modules) != 1: From cfc01dad0db5b319c4814aff0c768ad1c86773f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Thu, 16 Sep 2021 11:53:23 +0200 Subject: [PATCH 14/18] Fix errors and tests after adding warning --- pylint/lint/pylinter.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index 9ebd7d54f0..bad74b3836 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -946,10 +946,10 @@ def check(self, files_or_modules: Sequence[str]) -> None: """ self.initialize() if not isinstance(files_or_modules, (list, tuple)): - warnins.warn( + warnings.warn( "In pylint 3.0, the checkers check function will only accept sequence of string" ) - files_or_modules = (files_or_modules,) + files_or_modules = (files_or_modules,) # type: ignore if self.config.from_stdin: if len(files_or_modules) != 1: raise exceptions.InvalidArgsError( From 21e6f76a163978bf925df9a31b4549eccf347ad3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Thu, 16 Sep 2021 12:18:47 +0200 Subject: [PATCH 15/18] Code review --- pylint/lint/pylinter.py | 39 ++++++++++++++++++------------------- pylint/typing.py | 7 ++++++- tests/unittest_reporting.py | 3 +-- 3 files changed, 26 insertions(+), 23 deletions(-) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index bad74b3836..b5b98405e6 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -11,7 +11,7 @@ import traceback import warnings from io import TextIOWrapper -from typing import Iterable, Iterator, List, Optional, Sequence +from typing import Iterable, Iterator, List, Optional, Sequence, Union import astroid from astroid import AstroidError, nodes @@ -939,7 +939,7 @@ def initialize(self): if not msg.may_be_emitted(): self._msgs_state[msg.msgid] = False - def check(self, files_or_modules: Sequence[str]) -> None: + def check(self, files_or_modules: Union[Sequence[str], str]) -> None: """main checking entry: check a list of files or modules from their name. files_or_modules is a sequence of strings presenting modules to check. @@ -947,7 +947,8 @@ def check(self, files_or_modules: Sequence[str]) -> None: self.initialize() if not isinstance(files_or_modules, (list, tuple)): warnings.warn( - "In pylint 3.0, the checkers check function will only accept sequence of string" + "In pylint 3.0, the checkers check function will only accept sequence of string", + DeprecationWarning, # TODO: Update typing of files_or_modules to Sequence[str] # pylint: disable=fixme ) files_or_modules = (files_or_modules,) # type: ignore if self.config.from_stdin: @@ -975,8 +976,15 @@ def check(self, files_or_modules: Sequence[str]) -> None: files_or_modules, ) - def check_single_file(self, file: FileItem) -> None: - """Check single file + def check_single_file(self, name: str, filepath: str, modname: str) -> None: + warnings.warn( + "In pylint 3.0, the checkers check_single_file function will only accept a FileItem", + DeprecationWarning, # TODO: Replace check_single_file with check_single_file_item # pylint: disable=fixme + ) + self.check_single_file_item(FileItem(name, filepath, modname)) + + def check_single_file_item(self, file: FileItem) -> None: + """Check single file item The arguments are the same that are documented in _check_files @@ -990,28 +998,19 @@ def _check_files( get_ast, file_descrs: Iterable[FileItem], ) -> None: - """Check all files from file_descrs - - The file_descrs should be iterable of tuple (name, filepath, modname) - where - - name: full name of the module - - filepath: path of the file - - modname: module name - """ + """Check all files from file_descrs""" with self._astroid_module_checker() as check_astroid_module: - for name, filepath, modname in file_descrs: + for file in file_descrs: try: - self._check_file( - get_ast, check_astroid_module, FileItem(name, filepath, modname) - ) + self._check_file(get_ast, check_astroid_module, file) except Exception as ex: # pylint: disable=broad-except template_path = prepare_crash_report( - ex, filepath, self.crash_file_path + ex, file.filepath, self.crash_file_path ) - msg = get_fatal_error_message(filepath, template_path) + msg = get_fatal_error_message(file.filepath, template_path) if isinstance(ex, AstroidError): symbol = "astroid-error" - self.add_message(symbol, args=(filepath, msg)) + self.add_message(symbol, args=(file.filepath, msg)) else: symbol = "fatal" self.add_message(symbol, args=msg) diff --git a/pylint/typing.py b/pylint/typing.py index 205229213f..bea6a3fefb 100644 --- a/pylint/typing.py +++ b/pylint/typing.py @@ -15,7 +15,12 @@ class FileItem(NamedTuple): - """Represents data about a file handled by pylint""" + """Represents data about a file handled by pylint + + Each file item has: + - name: full name of the module + - filepath: path of the file + - modname: module name""" name: str filepath: str diff --git a/tests/unittest_reporting.py b/tests/unittest_reporting.py index 07ffc27465..d1d483a359 100644 --- a/tests/unittest_reporting.py +++ b/tests/unittest_reporting.py @@ -29,7 +29,6 @@ from pylint.lint import PyLinter from pylint.reporters import BaseReporter from pylint.reporters.text import ParseableTextReporter, TextReporter -from pylint.typing import FileItem @pytest.fixture(scope="module") @@ -121,7 +120,7 @@ def test_multi_format_output(tmp_path): linter.reporter.set_output(text) linter.open() - linter.check_single_file(FileItem("somemodule", source_file, "somemodule")) + linter.check_single_file("somemodule", source_file, "somemodule") linter.add_message("line-too-long", line=1, args=(1, 2)) linter.generate_reports() linter.reporter.writeln("direct output") From 4df05c56ec37c3df67591303a230e0a65bff4ee0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Thu, 16 Sep 2021 12:23:34 +0200 Subject: [PATCH 16/18] Fix tests --- pylint/lint/parallel.py | 2 +- tests/unittest_reporting.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pylint/lint/parallel.py b/pylint/lint/parallel.py index a4334aca17..1896fc95f2 100644 --- a/pylint/lint/parallel.py +++ b/pylint/lint/parallel.py @@ -73,7 +73,7 @@ def _worker_check_single_file( if not _worker_linter: raise Exception("Worker linter not yet initialised") _worker_linter.open() - _worker_linter.check_single_file(file_item) + _worker_linter.check_single_file_item(file_item) mapreduce_data = collections.defaultdict(list) for checker in _worker_linter.get_checkers(): try: diff --git a/tests/unittest_reporting.py b/tests/unittest_reporting.py index d1d483a359..88f9c43b56 100644 --- a/tests/unittest_reporting.py +++ b/tests/unittest_reporting.py @@ -29,6 +29,7 @@ from pylint.lint import PyLinter from pylint.reporters import BaseReporter from pylint.reporters.text import ParseableTextReporter, TextReporter +from pylint.typing import FileItem @pytest.fixture(scope="module") @@ -120,7 +121,7 @@ def test_multi_format_output(tmp_path): linter.reporter.set_output(text) linter.open() - linter.check_single_file("somemodule", source_file, "somemodule") + linter.check_single_file_item(FileItem("somemodule", source_file, "somemodule")) linter.add_message("line-too-long", line=1, args=(1, 2)) linter.generate_reports() linter.reporter.writeln("direct output") From 58d0e880e50e3bd4ef32b2345aba260293f5263b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Thu, 16 Sep 2021 12:42:30 +0200 Subject: [PATCH 17/18] Revert docstring change --- pylint/lint/pylinter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index b5b98405e6..3d49c92d49 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -942,7 +942,7 @@ def initialize(self): def check(self, files_or_modules: Union[Sequence[str], str]) -> None: """main checking entry: check a list of files or modules from their name. - files_or_modules is a sequence of strings presenting modules to check. + files_or_modules is either a string or list of strings presenting modules to check. """ self.initialize() if not isinstance(files_or_modules, (list, tuple)): From 0cf4b1fe003df193292729d3f7a320657fd7fbb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Thu, 16 Sep 2021 12:44:23 +0200 Subject: [PATCH 18/18] Apply suggestions from code review Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com> --- pylint/lint/pylinter.py | 9 ++++++--- pylint/typing.py | 3 ++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index 3d49c92d49..e8efc2fcde 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -946,9 +946,11 @@ def check(self, files_or_modules: Union[Sequence[str], str]) -> None: """ self.initialize() if not isinstance(files_or_modules, (list, tuple)): + # pylint: disable-next=fixme + # TODO: Update typing and docstring for 'files_or_modules' when removing the deprecation warnings.warn( "In pylint 3.0, the checkers check function will only accept sequence of string", - DeprecationWarning, # TODO: Update typing of files_or_modules to Sequence[str] # pylint: disable=fixme + DeprecationWarning, ) files_or_modules = (files_or_modules,) # type: ignore if self.config.from_stdin: @@ -978,8 +980,9 @@ def check(self, files_or_modules: Union[Sequence[str], str]) -> None: def check_single_file(self, name: str, filepath: str, modname: str) -> None: warnings.warn( - "In pylint 3.0, the checkers check_single_file function will only accept a FileItem", - DeprecationWarning, # TODO: Replace check_single_file with check_single_file_item # pylint: disable=fixme + "In pylint 3.0, the checkers check_single_file function will be removed. " + "Use check_single_file_item instead.", + DeprecationWarning, ) self.check_single_file_item(FileItem(name, filepath, modname)) diff --git a/pylint/typing.py b/pylint/typing.py index bea6a3fefb..4c12710de9 100644 --- a/pylint/typing.py +++ b/pylint/typing.py @@ -20,7 +20,8 @@ class FileItem(NamedTuple): Each file item has: - name: full name of the module - filepath: path of the file - - modname: module name""" + - modname: module name + """ name: str filepath: str