Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add typing to filepath #4980

Merged
merged 21 commits into from Sep 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion pylint/config/option_manager_mixin.py
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions pylint/lint/expand_modules.py
Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand Down
20 changes: 11 additions & 9 deletions pylint/lint/parallel.py
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mpy complains on the following two lines about the possibility that _work_linter might be None.
We currently already test for this, but do so with catching an AttributeError on L73. See:
https://github.com/PyCQA/pylint/blob/fab2de272b3fc36dc4480d89840b8bfe0fbfbd0c/tests/test_check_parallel.py#L177-L182
I think making this error more explicit as mypy wants makes sense. Although this is probably the wrong Exception type.

_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:
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
92 changes: 48 additions & 44 deletions pylint/lint/pylinter.py
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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(
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
"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(
Expand All @@ -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:
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
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):
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
"""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
Expand All @@ -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
Expand All @@ -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)
Pierre-Sassoulas marked this conversation as resolved.
Show resolved Hide resolved

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,
Expand All @@ -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
"""
Expand All @@ -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):
Expand Down
9 changes: 3 additions & 6 deletions pylint/lint/utils.py
Expand Up @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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. "
Expand Down
4 changes: 2 additions & 2 deletions pylint/reporters/base_reporter.py
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
5 changes: 2 additions & 3 deletions pylint/reporters/multi_reporter.py
Expand Up @@ -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
Expand All @@ -12,7 +12,6 @@
from pylint.typing import CheckerStats

AnyFile = IO[AnyStr]
AnyPath = Union[str, bytes, os.PathLike]
PyLinter = Any


Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion pylint/reporters/text.py
Expand Up @@ -25,6 +25,7 @@
import os
import sys
import warnings
from typing import Optional

from pylint import utils
from pylint.interfaces import IReporter
Expand Down Expand Up @@ -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):
Expand Down