Add typing to filepath (#4980)

* Change tests for ``filepath`` changes * Add pylint/typing.py and FileItem NamedTuple * Use NamedTuple more efficiently * Fix errors and tests after adding warning * Add deprecation for future API change in Checker Co-authored-by: Marc Mueller <30130371+cdce8p@users.noreply.github.com> Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
pylint-dev · Sep 16, 2021 · 0d246e5 · 0d246e5
1 parent dc0c7e9
commit 0d246e5
Show file tree

Hide file tree

Showing 20 changed files with 164 additions and 115 deletions.
diff --git a/pylint/config/option_manager_mixin.py b/pylint/config/option_manager_mixin.py
@@ -332,7 +332,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
@@ -4,6 +4,8 @@
 
 from astroid import modutils
 
+from pylint.typing import ErrorDescriptionDict, ModuleDescriptionDict
+
 
 def _modpath_from_file(filename, is_namespace, path=None):
     def _is_package_cb(path, parts):
@@ -42,12 +44,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[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 = []
-    errors = []
+    result: List[ModuleDescriptionDict] = []
+    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
@@ -3,12 +3,12 @@
 
 import collections
 import functools
-from typing import TYPE_CHECKING, Dict, List, 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
 from pylint.message import Message
-from pylint.typing import CheckerStats
+from pylint.typing import CheckerStats, FileItem
 
 if TYPE_CHECKING:
     from typing import Counter  # typing.Counter added in Python 3.6.1
@@ -67,11 +67,13 @@ def _worker_initialize(linter, arguments=None):
     _patch_sys_path(arguments or ())
 
 
-def _worker_check_single_file(file_item):
-    name, filepath, modname = file_item
-
+def _worker_check_single_file(
+    file_item: FileItem,
+) -> Tuple[int, Any, str, Any, List[Tuple[Any, ...]], Any, Any, DefaultDict[Any, List]]:
+    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_item(file_item)
     mapreduce_data = collections.defaultdict(list)
     for checker in _worker_linter.get_checkers():
         try:
@@ -84,7 +86,7 @@ def _worker_check_single_file(file_item):
     return (
         id(multiprocessing.current_process()),
         _worker_linter.current_name,
-        filepath,
+        file_item.filepath,
         _worker_linter.file_state.base_name,
         msgs,
         _worker_linter.stats,
@@ -114,7 +116,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: 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
@@ -156,7 +158,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  # 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

diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py
@@ -11,6 +11,7 @@
 import traceback
 import warnings
 from io import TextIOWrapper
+from typing import Iterable, Iterator, List, Optional, Sequence, Union
 
 import astroid
 from astroid import AstroidError, nodes
@@ -31,7 +32,7 @@
 )
 from pylint.message import MessageDefinitionStore, MessagesHandlerMixIn
 from pylint.reporters.ureports import nodes as report_nodes
-from pylint.typing import CheckerStats
+from pylint.typing import CheckerStats, FileItem, ModuleDescriptionDict
 from pylint.utils import ASTWalker, FileState, utils
 from pylint.utils.pragma_parser import (
     OPTION_PO,
@@ -938,16 +939,20 @@ 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: Union[Sequence[str], 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.
         """
         self.initialize()
-
         if not isinstance(files_or_modules, (list, tuple)):
-            files_or_modules = (files_or_modules,)
-
+            # 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,
+            )
+            files_or_modules = (files_or_modules,)  # type: ignore
         if self.config.from_stdin:
             if len(files_or_modules) != 1:
                 raise exceptions.InvalidArgsError(
@@ -973,66 +978,65 @@ def check(self, files_or_modules):
                 files_or_modules,
             )
 
-    def check_single_file(self, name, filepath, modname):
-        """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 be removed. "
+            "Use check_single_file_item instead.",
+            DeprecationWarning,
+        )
+        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
 
         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
-            )
-
-    def _check_files(self, get_ast, file_descrs):
-        """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
-        """
+            self._check_file(self.get_ast, check_astroid_module, file)
+
+    def _check_files(
+        self,
+        get_ast,
+        file_descrs: Iterable[FileItem],
+    ) -> None:
+        """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, 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"
-                        msg = (filepath, msg)
+                        self.add_message(symbol, args=(file.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, 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
         - filepath: path to the file to check
         - 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
         """
-        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
@@ -1045,7 +1049,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) -> 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
@@ -1059,19 +1063,19 @@ def _get_file_descr_from_stdin(filepath):
         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):
+    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.
         """
         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):
+    def _expand_files(self, modules) -> List[ModuleDescriptionDict]:
         """get modules and errors from a list of modules and handle errors"""
         result, errors = expand_modules(
             modules,
@@ -1088,7 +1092,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
         """
@@ -1097,10 +1101,10 @@ def set_current_module(self, modname, filepath=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/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
@@ -3,7 +3,7 @@
 
 import os
 import sys
-from typing import List
+from typing import List, Optional
 
 from pylint.message import Message
 from pylint.typing import CheckerStats
@@ -63,7 +63,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(

diff --git 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, Optional, Union
+from typing import IO, Any, AnyStr, Callable, List, Optional
 
 from pylint.interfaces import IReporter
 from pylint.message import Message
@@ -12,7 +12,6 @@
 from pylint.typing import CheckerStats
 
 AnyFile = IO[AnyStr]
-AnyPath = Union[str, bytes, os.PathLike]
 PyLinter = Any
 
 
@@ -89,7 +88,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
@@ -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):