From f5274bfeb892328720c1ff77505ae1153cb3fe3a Mon Sep 17 00:00:00 2001 From: Pierre Sassoulas Date: Fri, 7 Jun 2024 07:04:33 +0200 Subject: [PATCH] [symilar] Migrate from optparse / getopt to argparse --- doc/whatsnew/fragments/9999.user_action | 5 ++ pylint/checkers/similar.py | 83 ++++++++----------------- tests/checkers/unittest_symilar.py | 24 ++++--- 3 files changed, 45 insertions(+), 67 deletions(-) create mode 100644 doc/whatsnew/fragments/9999.user_action diff --git a/doc/whatsnew/fragments/9999.user_action b/doc/whatsnew/fragments/9999.user_action new file mode 100644 index 00000000000..664a9bc551d --- /dev/null +++ b/doc/whatsnew/fragments/9999.user_action @@ -0,0 +1,5 @@ +We migrated ``symilar`` to argparse, from getopt, so the error and help output changed +for the better). We exit with 2 instead of sometime 1, sometime 2, and the error output +is not captured by the runner anymore. + +Refs #9999 diff --git a/pylint/checkers/similar.py b/pylint/checkers/similar.py index ada141da364..80dceac2a2d 100644 --- a/pylint/checkers/similar.py +++ b/pylint/checkers/similar.py @@ -33,13 +33,13 @@ import copy import functools import itertools +import logging import operator import re import sys import warnings from collections import defaultdict from collections.abc import Callable, Generator, Iterable, Sequence -from getopt import GetoptError, getopt from io import BufferedIOBase, BufferedReader, BytesIO from itertools import chain from typing import ( @@ -890,67 +890,34 @@ def register(linter: PyLinter) -> None: linter.register_checker(SimilarChecker(linter)) -def usage(status: int = 0) -> NoReturn: - """Display command line usage information.""" - print("finds copy pasted blocks in a set of files") - print() - print( - "Usage: symilar [-d|--duplicates min_duplicated_lines] \ -[-i|--ignore-comments] [--ignore-docstrings] [--ignore-imports] [--ignore-signatures] file1..." - ) - sys.exit(status) - - def Run(argv: Sequence[str] | None = None) -> NoReturn: """Standalone command line access point.""" - if argv is None: - argv = sys.argv[1:] - - s_opts = "hd:i:" - l_opts = [ - "help", - "duplicates=", - "ignore-comments", - "ignore-imports", - "ignore-docstrings", - "ignore-signatures", - ] - min_lines = DEFAULT_MIN_SIMILARITY_LINE - ignore_comments = False - ignore_docstrings = False - ignore_imports = False - ignore_signatures = False - 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"}: - 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"}: - ignore_comments = True - elif opt in {"--ignore-docstrings"}: - ignore_docstrings = True - elif opt in {"--ignore-imports"}: - ignore_imports = True - elif opt in {"--ignore-signatures"}: - ignore_signatures = True - if not args: - usage(1) - sim = Similar( - min_lines, ignore_comments, ignore_docstrings, ignore_imports, ignore_signatures + logging.error(argv) + parser = argparse.ArgumentParser( + prog="symilar", description="Finds copy pasted blocks in a set of files." + ) + parser.add_argument("files", nargs="+") + parser.add_argument( + "-d", "--duplicates", type=int, default=DEFAULT_MIN_SIMILARITY_LINE + ) + parser.add_argument("-i", "--ignore-comments", default=False) + parser.add_argument("--ignore-docstrings", default=False) + parser.add_argument("--ignore-imports", default=False) + parser.add_argument("--ignore-signatures", default=False) + parsed_args = parser.parse_args(args=argv) + similar_runner = Similar( + min_lines=parsed_args.duplicates, + ignore_comments=parsed_args.ignore_comments, + ignore_docstrings=parsed_args.ignore_docstrings, + ignore_imports=parsed_args.ignore_imports, + ignore_signatures=parsed_args.ignore_signatures, ) - for filename in args: + logging.error(parsed_args.files) + for filename in parsed_args.files: with open(filename, encoding="utf-8") as stream: - sim.append_stream(filename, stream) - sim.run() + similar_runner.append_stream(filename, stream) + similar_runner.run() + # the sys exit must be kept because of the unit tests that rely on it sys.exit(0) diff --git a/tests/checkers/unittest_symilar.py b/tests/checkers/unittest_symilar.py index 91a9e5e545e..ceabd2f1c1e 100644 --- a/tests/checkers/unittest_symilar.py +++ b/tests/checkers/unittest_symilar.py @@ -7,6 +7,7 @@ from pathlib import Path import pytest +from _pytest.capture import CaptureFixture from pylint.checkers import similar from pylint.constants import IS_PYPY, PY39_PLUS @@ -369,13 +370,16 @@ def test_help() -> None: pytest.fail("not system exit") -def test_no_args() -> None: +def test_no_args(capsys: CaptureFixture) -> None: output = StringIO() with redirect_stdout(output): try: similar.Run([]) except SystemExit as ex: - assert ex.code == 1 + assert ex.code == 2 + out, err = capsys.readouterr() + assert not out + assert "the following arguments are required: files" in err else: pytest.fail("not system exit") @@ -499,14 +503,14 @@ def test_set_duplicate_lines_to_zero() -> None: assert output.getvalue() == "" -@pytest.mark.parametrize("v", ["d"]) -def test_bad_equal_short_form_option(v: str) -> None: +@pytest.mark.parametrize("v", ["d", "i"]) +def test_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() + similar.Run([f"-{v}=2", SIMILAR1, SIMILAR2]) + assert ex.value.code == 0 + assert "similar lines in" in output.getvalue() @pytest.mark.parametrize("v", ["i", "d"]) @@ -519,10 +523,12 @@ def test_space_short_form_option(v: str) -> None: assert "similar lines in" in output.getvalue() -def test_bad_short_form_option() -> None: +def test_bad_short_form_option(capsys: CaptureFixture) -> 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]) + out, err = capsys.readouterr() assert ex.value.code == 2 - assert "option -j not recognized" in output.getvalue() + assert not out + assert "unrecognized arguments: -j=0" in err