Skip to content

Commit

Permalink
Merge branch 'main' of https://github.com/PyCQA/pylint into type-chec…
Browse files Browse the repository at this point in the history
…king-elif-fp
  • Loading branch information
zenlyj committed Mar 29, 2023
2 parents 873572b + 155298f commit 258981a
Show file tree
Hide file tree
Showing 39 changed files with 387 additions and 197 deletions.
4 changes: 2 additions & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ repos:
doc/data/messages/m/missing-final-newline/bad.py|
)$
- repo: https://github.com/charliermarsh/ruff-pre-commit
rev: "v0.0.257"
rev: "v0.0.259"
hooks:
- id: ruff
args: ["--fix"]
exclude: &fixtures tests(/\w*)*/functional/|tests/input|doc/data/messages|tests(/\w*)*data/
- repo: https://github.com/charliermarsh/ruff-pre-commit
rev: "v0.0.257"
rev: "v0.0.259"
hooks:
- id: ruff
name: ruff-doc
Expand Down
15 changes: 8 additions & 7 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,14 @@ It can also be integrated in most editors or IDEs. More information can be found

.. This is used inside the doc to recover the end of the short text for installation
What differentiate Pylint?
--------------------------
What differentiates Pylint?
---------------------------

Pylint is not trusting your typing and is inferring the actual value of nodes (for a
start because there was no typing when pylint started off) using its internal code
representation (astroid). If your code is ``import logging as argparse``, Pylint
can check and know that ``argparse.error(...)`` is in fact a logging call and not an
argparse call. This makes pylint slower, but it also let pylint find more issues if
argparse call. This makes pylint slower, but it also lets pylint find more issues if
your code is not fully typed.

[inference] is the killer feature that keeps us using [pylint] in our project despite how painfully slow it is.
Expand All @@ -94,7 +94,8 @@ your code is not fully typed.
.. _`Realist pylint user`: https://github.com/charliermarsh/ruff/issues/970#issuecomment-1381067064

pylint, not afraid of being a little slower than it already is, is also a lot more thorough than other linters.
There is more checks, some opinionated one that are deactivated by default, but can be enabled using configuration.
There are more checks, including some opinionated ones that are deactivated by default
but can be enabled using configuration.

How to use pylint
-----------------
Expand All @@ -103,12 +104,12 @@ Pylint isn't smarter than you: it may warn you about things that you have
conscientiously done or check for some things that you don't care about.
During adoption, especially in a legacy project where pylint was never enforced,
it's best to start with the ``--errors-only`` flag, then disable
convention and refactor message with ``--disable=C,R`` and progressively
convention and refactor messages with ``--disable=C,R`` and progressively
re-evaluate and re-enable messages as your priorities evolve.

Pylint is highly configurable and permits to write plugins in order to add your
own checks (for example, for internal libraries or an internal rule). Pylint also has an
ecosystem of existing plugins for popular frameworks and third party libraries.
ecosystem of existing plugins for popular frameworks and third-party libraries.

.. note::

Expand All @@ -128,7 +129,7 @@ Advised linters alongside pylint
--------------------------------

Projects that you might want to use alongside pylint include ruff_ (**really** fast,
with builtin auto-fix, and a growing number of checks taken from popular
with builtin auto-fix and a growing number of checks taken from popular
linters but implemented in ``rust``) or flake8_ (faster and simpler checks with very few false positives),
mypy_, pyright_ or pyre_ (typing checks), bandit_ (security oriented checks), black_ and
isort_ (auto-formatting), autoflake_ (automated removal of unused imports or variables),
Expand Down
10 changes: 5 additions & 5 deletions doc/data/messages/i/invalid-name/details.rst
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,11 @@ The following type of names are checked with a predefined pattern:
| ``typevar`` | ``T``, ``_CallableT``, ``_T_co``, ``AnyStr``, | ``DICT_T``, ``CALLABLE_T``, ``ENUM_T``, ``DeviceType``, |
| | ``DeviceTypeT``, ``IPAddressT`` | ``_StrType`` |
+--------------------+-------------------------------------------------------+------------------------------------------------------------+
| ``typealias`` | ``GoodName``, ``_GoodName``, ``IPAddressType`` and | ``BadNameT``, ``badName``, ``TBadName``, ``TypeBadName`` |
| | other PascalCase variants that don't start with ``T``| |
| | or ``Type``. This is to distinguish them from | |
| | ``typevars``. Note that ``TopName`` is allowed but | |
| | ``TTopName`` isn't. | |
| ``typealias`` | ``GoodName``, ``_GoodName``, ``IPAddressType``, | ``BadNameT``, ``badName``, ``TBadName``, ``TypeBadName``, |
| | ``GoodName2`` and other PascalCase variants that | ``_1BadName`` |
| | don't start with ``T`` or ``Type``. This is to | |
| | distinguish them from ``typevars``. Note that | |
| | ``TopName`` is allowed but ``TTopName`` isn't. | |
+--------------------+-------------------------------------------------------+------------------------------------------------------------+

Custom regular expressions
Expand Down
5 changes: 5 additions & 0 deletions doc/whatsnew/fragments/8474.internal
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Following a deprecation period, the ``OutputLine`` class now requires
the right number of argument all the time. The functional output can be
regenerated automatically to achieve that easily.

Refs #8474
5 changes: 5 additions & 0 deletions doc/whatsnew/fragments/8485.false_positive
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
``invalid-name`` now allows for integers in ``typealias`` names:
- now valid: ``Good2Name``, ``GoodName2``.
- still invalid: ``_1BadName``.

Closes #8485
5 changes: 5 additions & 0 deletions doc/whatsnew/fragments/8496.false_positive
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
``unnecessary-lambda`` no longer warns on lambdas which use its parameters in
their body (other than the final arguments), e.g.
``lambda foo: (bar if foo else baz)(foo)``.

Closes #8496
8 changes: 8 additions & 0 deletions pylint/checkers/base/basic_checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,7 @@ def _has_variadic_argument(
)

@utils.only_required_for_messages("unnecessary-lambda")
# pylint: disable-next=too-many-return-statements
def visit_lambda(self, node: nodes.Lambda) -> None:
"""Check whether the lambda is suspicious."""
# if the body of the lambda is a call expression with the same
Expand Down Expand Up @@ -576,6 +577,13 @@ def visit_lambda(self, node: nodes.Lambda) -> None:
if arg.name != passed_arg.name:
return

# The lambda is necessary if it uses its parameter in the function it is
# calling in the lambda's body
# e.g. lambda foo: (func1 if foo else func2)(foo)
for name in call.func.nodes_of_class(nodes.Name):
if name.lookup(name.name)[0] is node:
return

self.add_message("unnecessary-lambda", line=node.fromlineno, node=node)

@utils.only_required_for_messages("dangerous-default-value")
Expand Down
4 changes: 3 additions & 1 deletion pylint/checkers/base/name_checker/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@
"typevar": re.compile(
r"^_{0,2}(?!T[A-Z])(?:[A-Z]+|(?:[A-Z]+[a-z]+)+T?(?<!Type))(?:_co(?:ntra)?)?$"
),
"typealias": re.compile(r"^_{0,2}(?!T[A-Z]|Type)[A-Z]+[a-z]+(?:[A-Z][a-z]+)*$"),
"typealias": re.compile(
r"^_{0,2}(?!T[A-Z]|Type)[A-Z]+[a-z0-9]+(?:[A-Z][a-z0-9]+)*$"
),
}

BUILTIN_PROPERTY = "builtins.property"
Expand Down
59 changes: 50 additions & 9 deletions pylint/lint/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@
from __future__ import annotations

import contextlib
import platform
import sys
import traceback
from collections.abc import Iterator, Sequence
from datetime import datetime
from pathlib import Path

from pylint.constants import PYLINT_HOME
from pylint.constants import PYLINT_HOME, full_version


def prepare_crash_report(ex: Exception, filepath: str, crash_file_path: str) -> Path:
Expand All @@ -26,16 +27,18 @@ def prepare_crash_report(ex: Exception, filepath: str, crash_file_path: str) ->
First, please verify that the bug is not already filled:
https://github.com/PyCQA/pylint/issues/
Then create a new crash issue:
https://github.com/PyCQA/pylint/issues/new?assignees=&labels=crash%2Cneeds+triage&template=BUG-REPORT.yml
Then create a new issue:
https://github.com/PyCQA/pylint/issues/new?labels=Crash 💥%2CNeeds triage 📥
"""
template += f"""\
"""
template += f"""
Issue title:
Crash ``{ex}`` (if possible, be more specific about what made pylint crash)
Content:
When parsing the following file:
### Bug description
When parsing the following ``a.py``:
<!--
If sharing the code is not an option, please state so,
Expand All @@ -46,11 +49,49 @@ def prepare_crash_report(ex: Exception, filepath: str, crash_file_path: str) ->
{file_content}
```
pylint crashed with a ``{ex.__class__.__name__}`` and with the following stacktrace:
### Command used
```shell
pylint a.py
```
### Pylint output
<details open>
<summary>
pylint crashed with a ``{ex.__class__.__name__}`` and with the following stacktrace:
</summary>
```python
"""
template += traceback.format_exc()
template += "```\n"
template += f"""
```
</details>
### Expected behavior
No crash.
### Pylint version
```shell
{full_version}
```
### OS / Environment
{sys.platform} ({platform.system()})
### Additional dependencies
<!--
Please remove this part if you're not using any of
your dependencies in the example.
-->
"""
try:
with open(issue_template_path, "a", encoding="utf8") as f:
f.write(template)
Expand Down
100 changes: 76 additions & 24 deletions pylint/testutils/functional/find_functional_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@
from __future__ import annotations

import os
from collections.abc import Iterator
from pathlib import Path

from pylint.testutils.functional.test_file import FunctionalTestFile

REASONABLY_DISPLAYABLE_VERTICALLY = 48
"""'Wet finger' number of files that are reasonable to display by an IDE."""
SHOULD_BE_IN_THE_SAME_DIRECTORY = 5
"""'Wet finger' as in 'in my settings there are precisely this many'."""
REASONABLY_DISPLAYABLE_VERTICALLY = 49
"""'Wet finger' number of files that are reasonable to display by an IDE.
'Wet finger' as in 'in my settings there are precisely this many'.
"""

IGNORED_PARENT_DIRS = {
"deprecated_relative_import",
Expand All @@ -32,11 +34,12 @@

def get_functional_test_files_from_directory(
input_dir: Path | str,
max_file_per_directory: int = REASONABLY_DISPLAYABLE_VERTICALLY,
) -> list[FunctionalTestFile]:
"""Get all functional tests in the input_dir."""
suite = []

_check_functional_tests_structure(Path(input_dir))
_check_functional_tests_structure(Path(input_dir), max_file_per_directory)

for dirpath, dirnames, filenames in os.walk(input_dir):
if dirpath.endswith("__pycache__"):
Expand All @@ -49,39 +52,88 @@ def get_functional_test_files_from_directory(
return suite


def _check_functional_tests_structure(directory: Path) -> None:
"""Check if test directories follow correct file/folder structure."""
# Ignore underscored directories
def _check_functional_tests_structure(
directory: Path, max_file_per_directory: int
) -> None:
"""Check if test directories follow correct file/folder structure.
Ignore underscored directories or files.
"""
if Path(directory).stem.startswith("_"):
return

files: set[Path] = set()
dirs: set[Path] = set()

def _get_files_from_dir(
path: Path, violations: list[tuple[Path, int]]
) -> list[Path]:
"""Return directories and files from a directory and handles violations."""
files_without_leading_underscore = list(
p for p in path.iterdir() if not p.stem.startswith("_")
)
if len(files_without_leading_underscore) > max_file_per_directory:
violations.append((path, len(files_without_leading_underscore)))
return files_without_leading_underscore

def walk(path: Path) -> Iterator[Path]:
violations: list[tuple[Path, int]] = []
violations_msgs: set[str] = set()
parent_dir_files = _get_files_from_dir(path, violations)
error_msg = (
"The following directory contains too many functional tests files:\n"
)
for _file_or_dir in parent_dir_files:
if _file_or_dir.is_dir():
_files = _get_files_from_dir(_file_or_dir, violations)
yield _file_or_dir.resolve()
try:
yield from walk(_file_or_dir)
except AssertionError as e:
violations_msgs.add(str(e).replace(error_msg, ""))
else:
yield _file_or_dir.resolve()
if violations or violations_msgs:
_msg = error_msg
for offending_file, number in violations:
_msg += f"- {offending_file}: {number} when the max is {max_file_per_directory}\n"
for error_msg in violations_msgs:
_msg += error_msg
raise AssertionError(_msg)

# Collect all sub-directories and files in directory
for file_or_dir in directory.iterdir():
if file_or_dir.is_file():
if file_or_dir.suffix == ".py" and not file_or_dir.stem.startswith("_"):
files.add(file_or_dir)
elif file_or_dir.is_dir():
for file_or_dir in walk(directory):
if file_or_dir.is_dir():
dirs.add(file_or_dir)
_check_functional_tests_structure(file_or_dir)

assert len(files) <= REASONABLY_DISPLAYABLE_VERTICALLY, (
f"{directory} contains too many functional tests files "
+ f"({len(files)} > {REASONABLY_DISPLAYABLE_VERTICALLY})."
)
elif file_or_dir.suffix == ".py":
files.add(file_or_dir)

directory_does_not_exists: list[tuple[Path, Path]] = []
misplaced_file: list[Path] = []
for file in files:
possible_dir = file.parent / file.stem.split("_")[0]
assert not possible_dir.exists(), f"{file} should go in {possible_dir}."

if possible_dir.exists():
directory_does_not_exists.append((file, possible_dir))
# Exclude some directories as they follow a different structure
if (
not len(file.parent.stem) == 1 # First letter sub-directories
and file.parent.stem not in IGNORED_PARENT_DIRS
and file.parent.parent.stem not in IGNORED_PARENT_PARENT_DIRS
):
assert file.stem.startswith(
file.parent.stem
), f"{file} should not go in {file.parent}"
if not file.stem.startswith(file.parent.stem):
misplaced_file.append(file)

if directory_does_not_exists or misplaced_file:
msg = "The following functional tests are disorganized:\n"
for file, possible_dir in directory_does_not_exists:
msg += (
f"- In '{directory}', '{file.relative_to(directory)}' "
f"should go in '{possible_dir.relative_to(directory)}'\n"
)
for file in misplaced_file:
msg += (
f"- In '{directory}', {file.relative_to(directory)} should go in a directory"
f" that starts with the first letters"
f" of '{file.stem}' (not '{file.parent.stem}')\n"
)
raise AssertionError(msg)
4 changes: 2 additions & 2 deletions pylint/testutils/lint_module_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ def error_msg_for_unequal_messages(
expected_messages: MessageCounter,
actual_output: list[OutputLine],
) -> str:
msg = [f'Wrong results for file "{self._test_file.base}":']
msg = [f'Wrong message(s) raised for "{Path(self._test_file.source).name}":']
missing, unexpected = self.multiset_difference(
expected_messages, actual_messages
)
Expand All @@ -289,7 +289,7 @@ def error_msg_for_unequal_output(
) -> str:
missing = set(expected_lines) - set(received_lines)
unexpected = set(received_lines) - set(expected_lines)
error_msg = f"Wrong output for '{self._test_file.base}.txt':"
error_msg = f'Wrong output for "{Path(self._test_file.expected_output).name}":'
sort_by_line_number = operator.attrgetter("lineno")
if missing:
error_msg += "\n- Missing lines:\n"
Expand Down
Loading

0 comments on commit 258981a

Please sign in to comment.