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

[symilar] Fix the short form options that weren't being processed properly #9709

Merged
merged 3 commits into from
Jun 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions doc/whatsnew/fragments/9343.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Fixed a crash in ``symilar`` when the ``-d`` or ``-i`` short option were not properly recognized.
It's still impossible to do ``-d=1`` (you must do ``-d 1``).

Closes #9343
16 changes: 12 additions & 4 deletions pylint/checkers/similar.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
import warnings
from collections import defaultdict
from collections.abc import Callable, Generator, Iterable, Sequence
from getopt import getopt
from getopt import GetoptError, getopt
from io import BufferedIOBase, BufferedReader, BytesIO
from itertools import chain
from typing import (
Expand Down Expand Up @@ -906,7 +906,7 @@ def Run(argv: Sequence[str] | None = None) -> NoReturn:
if argv is None:
argv = sys.argv[1:]

s_opts = "hdi"
s_opts = "hd:i:"
l_opts = [
"help",
"duplicates=",
Expand All @@ -920,10 +920,18 @@ def Run(argv: Sequence[str] | None = None) -> NoReturn:
ignore_docstrings = False
ignore_imports = False
ignore_signatures = False
opts, args = getopt(list(argv), s_opts, l_opts)
try:
opts, args = getopt(list(argv), s_opts, l_opts)
except GetoptError as e:
print(e)
usage(2)
for opt, val in opts:
if opt in {"-d", "--duplicates"}:
min_lines = int(val)
try:
min_lines = int(val)
except ValueError as e:
print(e)
usage(2)
elif opt in {"-h", "--help"}:
usage()
elif opt in {"-i", "--ignore-comments"}:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -497,3 +497,32 @@ def test_set_duplicate_lines_to_zero() -> None:
similar.Run(["--duplicates=0", SIMILAR1, SIMILAR2])
assert ex.value.code == 0
assert output.getvalue() == ""


@pytest.mark.parametrize("v", ["d"])
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems unnecessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but with argparse we'll be able to do both again.

def test_bad_equal_short_form_option(v: str) -> None:
"""Regression test for https://github.com/pylint-dev/pylint/issues/9343"""
output = StringIO()
with redirect_stdout(output), pytest.raises(SystemExit) as ex:
similar.Run([f"-{v}=0", SIMILAR1, SIMILAR2])
assert ex.value.code == 2
assert "invalid literal for int() with base 10: '=0'" in output.getvalue()


@pytest.mark.parametrize("v", ["i", "d"])
def test_space_short_form_option(v: str) -> None:
"""Regression test for https://github.com/pylint-dev/pylint/issues/9343"""
output = StringIO()
with redirect_stdout(output), pytest.raises(SystemExit) as ex:
similar.Run([f"-{v} 2", SIMILAR1, SIMILAR2])
assert ex.value.code == 0
assert "similar lines in" in output.getvalue()


def test_bad_short_form_option() -> None:
"""Regression test for https://github.com/pylint-dev/pylint/issues/9343"""
output = StringIO()
with redirect_stdout(output), pytest.raises(SystemExit) as ex:
similar.Run(["-j=0", SIMILAR1, SIMILAR2])
assert ex.value.code == 2
assert "option -j not recognized" in output.getvalue()
Loading