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

Fix AstroidError in similarity checker with imports/signatures ignored #6357

Merged
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
b0d5dd0
Fix AstroidError in similarity checker with imports/signatures ignored
jacobtylerwalls Apr 16, 2022
4821234
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 16, 2022
de330e4
Whitespace
jacobtylerwalls Apr 16, 2022
4f55194
Minor optimization
jacobtylerwalls Apr 16, 2022
0e66357
Docstring style
jacobtylerwalls Apr 16, 2022
ca35944
Simplify
jacobtylerwalls Apr 16, 2022
d2c2a83
All tests with ignore imports/signatures
jacobtylerwalls Apr 16, 2022
433941f
Clarify comment
jacobtylerwalls Apr 16, 2022
84173b9
Look ahead to next line
jacobtylerwalls Apr 16, 2022
4e8ce79
This test still fails on 2.13
jacobtylerwalls Apr 16, 2022
27c62fb
Fix expected result and refactor
jacobtylerwalls Apr 16, 2022
0eb0ce6
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 16, 2022
76f26a0
Refactor
jacobtylerwalls Apr 16, 2022
df5bb80
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 16, 2022
2723edd
add disable
jacobtylerwalls Apr 16, 2022
343923b
typo/reword
jacobtylerwalls Apr 16, 2022
dabc26b
Alternate solution
jacobtylerwalls Apr 16, 2022
17dfbc7
Remove mooted test condition
jacobtylerwalls Apr 16, 2022
0cda0c0
avoid some iteration
jacobtylerwalls Apr 16, 2022
8763570
Merge 'main'
jacobtylerwalls Apr 16, 2022
d5cdbbf
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 16, 2022
6e01357
Remove overkill tests (now that no lines are altered before ast re-pa…
jacobtylerwalls Apr 16, 2022
9dcfeda
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 16, 2022
3dfba99
add --
jacobtylerwalls Apr 16, 2022
cab9ef4
better typing import
jacobtylerwalls Apr 16, 2022
62028e1
Get the except in the right place
jacobtylerwalls Apr 16, 2022
53660be
More specific typing
jacobtylerwalls Apr 17, 2022
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
5 changes: 5 additions & 0 deletions ChangeLog
Expand Up @@ -192,6 +192,11 @@ Release date: TBA
Closes #5815
Closes #5406

* Fixed an ``AstroidError`` in 2.13.0 raised by the ``duplicate-code`` checker with
``ignore-imports`` or ``ignore-signatures`` enabled.

Closes #6301


What's New in Pylint 2.13.5?
============================
Expand Down
5 changes: 5 additions & 0 deletions doc/whatsnew/2.13.rst
Expand Up @@ -205,6 +205,11 @@ Other Changes
Closes #5815
Closes #5406

* Fixed an ``AstroidError`` in 2.13.0 raised by the ```duplicate-code``` checker with
``ignore-imports`` or ``ignore-signatures`` enabled.

Closes #6301

* Use the ``tomli`` package instead of ``toml`` to parse ``.toml`` files.

Closes #5885
Expand Down
57 changes: 33 additions & 24 deletions pylint/checkers/similar.py
Expand Up @@ -35,10 +35,12 @@
from typing import (
TYPE_CHECKING,
Any,
Callable,
Dict,
List,
NamedTuple,
NewType,
Optional,
TextIO,
Tuple,
Union,
Expand Down Expand Up @@ -366,28 +368,20 @@ def append_stream(
readlines = decoding_stream(stream, encoding).readlines
else:
readlines = stream.readlines # type: ignore[assignment] # hint parameter is incorrectly typed as non-optional
try:
active_lines: list[str] = []
if hasattr(self, "linter"):
# Remove those lines that should be ignored because of disables
for index, line in enumerate(readlines()):
if self.linter._is_one_message_enabled("R0801", index + 1): # type: ignore[attr-defined]
active_lines.append(line)
else:
active_lines = readlines()

self.linesets.append(
LineSet(
streamid,
active_lines,
self.namespace.ignore_comments,
self.namespace.ignore_docstrings,
self.namespace.ignore_imports,
self.namespace.ignore_signatures,
)

self.linesets.append(
LineSet(
streamid,
readlines(),
self.namespace.ignore_comments,
self.namespace.ignore_docstrings,
self.namespace.ignore_imports,
self.namespace.ignore_signatures,
line_enabled_callback=self.linter._is_one_message_enabled # type: ignore[attr-defined]
if hasattr(self, "linter")
else None,
)
except UnicodeDecodeError:
pass
)

def run(self) -> None:
"""Start looking for similarities and display results on stdout."""
Expand Down Expand Up @@ -563,6 +557,7 @@ def stripped_lines(
ignore_docstrings: bool,
ignore_imports: bool,
ignore_signatures: bool,
line_enabled_callback: Callable | None = None,
Copy link
Member Author

Choose a reason for hiding this comment

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

I fought with this locally and finally just no-verified it. Why is this syntax okay with our minimum python version of 3.7.2? (Well, 3.6.2, since we are talking about backporting, potentially.)

Copy link
Member Author

Choose a reason for hiding this comment

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

oh, did we future import annotations everywhere? ah. but still a problem with 3.6, then.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess we don't have to backport this if only one user reported it and 2.14 is coming soon.

This comment was marked as resolved.

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool. LMK if I should move the release note to 2.14. If we backport I guess we can just use different type annotation syntax.

) -> list[LineSpecifs]:
"""Return tuples of line/line number/line type with leading/trailing whitespace and any ignored code features removed.

Expand All @@ -571,6 +566,7 @@ def stripped_lines(
:param ignore_docstrings: if true, any line that is a docstring is removed from the result
:param ignore_imports: if true, any line that is an import is removed from the result
:param ignore_signatures: if true, any line that is part of a function signature is removed from the result
:param line_enabled_callback: If called with "R0801" and a line number, a return value of False will disregard the line
:return: the collection of line/line number/line type tuples
"""
if ignore_imports or ignore_signatures:
Expand Down Expand Up @@ -622,6 +618,10 @@ def _get_functions(
strippedlines = []
docstring = None
for lineno, line in enumerate(lines, start=1):
if line_enabled_callback is not None and not line_enabled_callback(
"R0801", lineno
):
continue
line = line.strip()
if ignore_docstrings:
if not docstring:
Expand Down Expand Up @@ -668,12 +668,21 @@ def __init__(
ignore_docstrings: bool = False,
ignore_imports: bool = False,
ignore_signatures: bool = False,
line_enabled_callback: Callable | None = None,
jacobtylerwalls marked this conversation as resolved.
Show resolved Hide resolved
) -> None:
self.name = name
self._real_lines = lines
self._stripped_lines = stripped_lines(
lines, ignore_comments, ignore_docstrings, ignore_imports, ignore_signatures
)
try:
self._stripped_lines = stripped_lines(
lines,
ignore_comments,
ignore_docstrings,
ignore_imports,
ignore_signatures,
line_enabled_callback=line_enabled_callback,
)
except UnicodeDecodeError:
self._stripped_lines = []

def __str__(self):
return f"<Lineset for {self.name}>"
Expand Down
@@ -0,0 +1,12 @@
r"""A raw docstring.
"""


def look_busy():
xxxx = 1 # pylint: disable=duplicate-code
yyyy = 2 # pylint: disable=duplicate-code
zzzz = 3
wwww = 4
vvvv = xxxx + yyyy + zzzz + wwww
vvvv *= 2
return vvvv
@@ -0,0 +1,12 @@
r"""A raw docstring.
"""


def look_busy():
xxxx = 1
yyyy = 2
zzzz = 3
wwww = 4
vvvv = xxxx + yyyy + zzzz + wwww
vvvv *= 2
return vvvv
23 changes: 21 additions & 2 deletions tests/test_similar.py
Expand Up @@ -35,7 +35,13 @@ def _runtest(self, args: list[str], code: int) -> None:
@staticmethod
def _run_pylint(args: list[str], out: TextIO) -> int:
"""Runs pylint with a patched output."""
args = args + ["--persistent=no"]
args = args + [
"--persistent=no",
"--enable=astroid-error",
# Enable functionality that will build another ast
"ignore-imports=y",
"ignore-signatures=y",
]
with _patch_streams(out):
with pytest.raises(SystemExit) as cm:
with warnings.catch_warnings():
Expand All @@ -54,8 +60,10 @@ def _test_output(self, args: list[str], expected_output: str) -> None:
out = StringIO()
self._run_pylint(args, out=out)
actual_output = self._clean_paths(out.getvalue())
actual_output_stripped = actual_output.strip()
expected_output = self._clean_paths(expected_output)
assert expected_output.strip() in actual_output.strip()
assert expected_output.strip() in actual_output_stripped
assert "Fatal error" not in actual_output_stripped

def test_duplicate_code_raw_strings_all(self) -> None:
"""Test similar lines in 3 similar files."""
Expand Down Expand Up @@ -108,6 +116,17 @@ def test_duplicate_code_raw_strings_disable_line_end(self) -> None:
expected_output=expected_output,
)

def test_duplicate_code_raw_strings_disable_line_some(self) -> None:
"""Tests disabling duplicate-code in the first two lines but not all lines
of a piece of similar code.
"""
path = join(DATA, "raw_strings_disable_line_some")
expected_output = "Similar lines in 2 files"
self._test_output(
[path, "--disable=all", "--enable=duplicate-code"],
expected_output=expected_output,
)

def test_duplicate_code_raw_strings_disable_scope(self) -> None:
"""Tests disabling duplicate-code at an inner scope level."""
path = join(DATA, "raw_strings_disable_scope")
Expand Down