From dca394035268a234b29d0c103a4fcc201c84061f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 15 Dec 2022 09:20:18 +0100 Subject: [PATCH] Fix inconsistent argument exit code when argparse exit with its own error code (#7931) (#7943) Returning 2 here is confusing as it doesn't match the documentation: https://pylint.pycqa.org/en/latest/user_guide/usage/run.html#exit-codes * pylint: use exit code 32 when invalid arguments are passed * pylint: add failing test when ambiguous abbreviated parameters are set in a config file This is confusing behaviour. The output is: ``` usage: pylint [options] pylint: error: ambiguous option: --no could match --notes, --notes-rgx, --no-docstring-rgx ``` The exit code is 2 which doesn't match the documentation: https://pylint.pycqa.org/en/latest/user_guide/usage/run.html#exit-codes * pylint: use exit code 32 when ambiguous abbreviated parameters are set in a config file Co-authored-by: Pierre Sassoulas (cherry picked from commit 62232b33a525a2b8ef8638a59a03e459ca069c4c) Co-authored-by: David Lawson --- doc/whatsnew/fragments/7931.bugfix | 3 +++ pylint/config/arguments_manager.py | 9 ++++--- pylint/config/config_initialization.py | 5 +++- tests/lint/test_run_pylint.py | 36 ++++++++++++++++++++++++++ tests/lint/unittest_expand_modules.py | 9 +++++++ 5 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 doc/whatsnew/fragments/7931.bugfix create mode 100644 tests/lint/test_run_pylint.py diff --git a/doc/whatsnew/fragments/7931.bugfix b/doc/whatsnew/fragments/7931.bugfix new file mode 100644 index 0000000000..fe42346f43 --- /dev/null +++ b/doc/whatsnew/fragments/7931.bugfix @@ -0,0 +1,3 @@ +When pylint exit due to bad arguments being provided the exit code will now be the expected ``32``. + +Refs #7931 diff --git a/pylint/config/arguments_manager.py b/pylint/config/arguments_manager.py index 40058071cc..6b9a8dce29 100644 --- a/pylint/config/arguments_manager.py +++ b/pylint/config/arguments_manager.py @@ -252,9 +252,12 @@ def _load_default_argument_values(self) -> None: def _parse_configuration_file(self, arguments: list[str]) -> None: """Parse the arguments found in a configuration file into the namespace.""" - self.config, parsed_args = self._arg_parser.parse_known_args( - arguments, self.config - ) + try: + self.config, parsed_args = self._arg_parser.parse_known_args( + arguments, self.config + ) + except SystemExit: + sys.exit(32) unrecognized_options: list[str] = [] for opt in parsed_args: if opt.startswith("--"): diff --git a/pylint/config/config_initialization.py b/pylint/config/config_initialization.py index b903a58026..d26f0e8c58 100644 --- a/pylint/config/config_initialization.py +++ b/pylint/config/config_initialization.py @@ -87,7 +87,10 @@ def _config_initialization( unrecognized_options.append(opt[1:]) if unrecognized_options: msg = ", ".join(unrecognized_options) - linter._arg_parser.error(f"Unrecognized option found: {msg}") + try: + linter._arg_parser.error(f"Unrecognized option found: {msg}") + except SystemExit: + sys.exit(32) # Now that config file and command line options have been loaded # with all disables, it is safe to emit messages diff --git a/tests/lint/test_run_pylint.py b/tests/lint/test_run_pylint.py new file mode 100644 index 0000000000..73dc26331b --- /dev/null +++ b/tests/lint/test_run_pylint.py @@ -0,0 +1,36 @@ +# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html +# For details: https://github.com/PyCQA/pylint/blob/main/LICENSE +# Copyright (c) https://github.com/PyCQA/pylint/blob/main/CONTRIBUTORS.txt + +from pathlib import Path + +import pytest +from _pytest.capture import CaptureFixture + +from pylint import run_pylint + + +def test_run_pylint_with_invalid_argument(capsys: CaptureFixture[str]) -> None: + """Check that appropriate exit code is used with invalid argument.""" + with pytest.raises(SystemExit) as ex: + run_pylint(["--never-use-this"]) + captured = capsys.readouterr() + assert captured.err.startswith("usage: pylint [options]") + assert ex.value.code == 32 + + +def test_run_pylint_with_invalid_argument_in_config( + capsys: CaptureFixture[str], tmp_path: Path +) -> None: + """Check that appropriate exit code is used with an ambiguous + argument in a config file. + """ + test_file = tmp_path / "testpylintrc" + with open(test_file, "w", encoding="utf-8") as f: + f.write("[MASTER]\nno=") + + with pytest.raises(SystemExit) as ex: + run_pylint(["--rcfile", f"{test_file}"]) + captured = capsys.readouterr() + assert captured.err.startswith("usage: pylint [options]") + assert ex.value.code == 32 diff --git a/tests/lint/unittest_expand_modules.py b/tests/lint/unittest_expand_modules.py index 3336c47bde..005eef387a 100644 --- a/tests/lint/unittest_expand_modules.py +++ b/tests/lint/unittest_expand_modules.py @@ -69,6 +69,14 @@ def test__is_in_ignore_list_re_match() -> None: "path": str(TEST_DIRECTORY / "lint/test_utils.py"), } +test_run_pylint = { + "basename": "lint", + "basepath": INIT_PATH, + "isarg": False, + "name": "lint.test_run_pylint", + "path": str(TEST_DIRECTORY / "lint/test_run_pylint.py"), +} + test_pylinter = { "basename": "lint", "basepath": INIT_PATH, @@ -102,6 +110,7 @@ def _list_expected_package_modules( init_of_package, test_caching, test_pylinter, + test_run_pylint, test_utils, this_file_from_init_deduplicated if deduplicating else this_file_from_init, unittest_lint,