From ffd81327a38867708c6e38a7a37fc473cfbedc1d Mon Sep 17 00:00:00 2001 From: Mark Bell Date: Thu, 15 Jul 2021 20:52:50 +0100 Subject: [PATCH 1/6] Changed exit logic. --- CONTRIBUTORS.txt | 2 ++ pylint/lint/pylinter.py | 4 +++- pylint/lint/run.py | 13 ++++++------- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 1c053c260b..0231d0aa03 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -512,3 +512,5 @@ contributors: * Daniel Dorani (doranid): contributor * Will Shanks: contributor + +* Mark Bell: contributor diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index 563ef621f2..c2d5362117 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -707,7 +707,9 @@ def enable_fail_on_messages(self): self.fail_on_symbols.append(msg.symbol) def any_fail_on_issues(self): - return any(x in self.fail_on_symbols for x in self.stats["by_msg"]) + return self.stats is not None and any( + x in self.fail_on_symbols for x in self.stats["by_msg"] + ) def disable_noerror_messages(self): for msgcat, msgids in self.msgs_store._msgs_by_category.items(): diff --git a/pylint/lint/run.py b/pylint/lint/run.py index 9f95287d0b..6d674e4295 100644 --- a/pylint/lint/run.py +++ b/pylint/lint/run.py @@ -394,14 +394,13 @@ def __init__( if exit: if linter.config.exit_zero: sys.exit(0) + elif linter.any_fail_on_issues(): + # We need to make sure we return a failing exit code in this case. + # So we use self.linter.msg_status if that is non-zero, otherwise we just return 1. + sys.exit(self.linter.msg_status or 1) + elif score_value and score_value >= linter.config.fail_under: + sys.exit(0) else: - if ( - score_value - and score_value >= linter.config.fail_under - # detected messages flagged by --fail-on prevent non-zero exit code - and not linter.any_fail_on_issues() - ): - sys.exit(0) sys.exit(self.linter.msg_status) def version_asked(self, _, __): From c4ab928364fd3de6301b397bf7cbbccb6bfb4a47 Mon Sep 17 00:00:00 2001 From: Mark Bell Date: Thu, 15 Jul 2021 20:56:54 +0100 Subject: [PATCH 2/6] Described change in Changelog. --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index ba24ace79a..c03da1f968 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,6 +17,8 @@ Release date: TBA .. Put bug fixes that should not wait for a new minor version here +* Fix bug in which --fail-on can return a zero exit code even when the specified issue is present + * Fix hard failure when handling missing attribute in a class with duplicated bases Closes #4687 From ea031632ced90a4e7101dc27599126d0f82ef470 Mon Sep 17 00:00:00 2001 From: Mark Bell Date: Sat, 17 Jul 2021 12:58:49 +0100 Subject: [PATCH 3/6] Added unit tests and allow fail-under to apply even when the score is exactly zero. --- pylint/lint/run.py | 2 +- tests/regrtest_data/fail_on.py | 12 +++++++++ tests/regrtest_data/fail_on_info_only.py | 11 ++++++++ tests/test_self.py | 32 ++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 tests/regrtest_data/fail_on.py create mode 100644 tests/regrtest_data/fail_on_info_only.py diff --git a/pylint/lint/run.py b/pylint/lint/run.py index 6d674e4295..1f6d6b71c7 100644 --- a/pylint/lint/run.py +++ b/pylint/lint/run.py @@ -398,7 +398,7 @@ def __init__( # We need to make sure we return a failing exit code in this case. # So we use self.linter.msg_status if that is non-zero, otherwise we just return 1. sys.exit(self.linter.msg_status or 1) - elif score_value and score_value >= linter.config.fail_under: + elif score_value >= linter.config.fail_under: sys.exit(0) else: sys.exit(self.linter.msg_status) diff --git a/tests/regrtest_data/fail_on.py b/tests/regrtest_data/fail_on.py new file mode 100644 index 0000000000..6f22e5013f --- /dev/null +++ b/tests/regrtest_data/fail_on.py @@ -0,0 +1,12 @@ +""" + Pylint score: -1.67 +""" +import nonexistent +# pylint: disable=broad-except + + +def loop(): + count = 0 + for _ in range(5): + count += 1 + print(count) diff --git a/tests/regrtest_data/fail_on_info_only.py b/tests/regrtest_data/fail_on_info_only.py new file mode 100644 index 0000000000..c6baffed09 --- /dev/null +++ b/tests/regrtest_data/fail_on_info_only.py @@ -0,0 +1,11 @@ +""" + Pylint score: -1.67 +""" +# pylint: disable=broad-except + +def loop(): + """Run a loop.""" + count = 0 + for _ in range(5): + count += 1 + print(count) diff --git a/tests/test_self.py b/tests/test_self.py index a921248c81..ef70cdf333 100644 --- a/tests/test_self.py +++ b/tests/test_self.py @@ -1136,6 +1136,38 @@ def test_output_file_invalid_path_exits_with_code_32(self): output_file = "thisdirectorydoesnotexit/output.txt" self._runtest([path, f"--output={output_file}"], code=32) + @pytest.mark.parametrize("args, expected", [ + ([], 0), + (["--enable=C"], 0), + (["--fail-on=superfluous-parens"], 0), + (["--fail-on=import-error"], 6), + (["--fail-on=unused-import"], 6), + (["--fail-on=unused-import", "--enable=C"], 22), + (["--fail-on=missing-function-docstring"], 22), + (["--fail-on=useless-suppression"], 6), + (["--fail-on=useless-suppression", "--enable=C"], 22), + ]) + def test_fail_on_exit_code(self, args, expected): + path = join(HERE, "regrtest_data", "fail_on.py") + # We set fail-under to be something very low so that even with the warnings + # and errors that are generated they don't affect the exit code. + self._runtest([path, "--fail-under=-10"] + args, code=expected) + + @pytest.mark.parametrize("args, expected", [ + ([], 0), + (["--enable=C"], 0), + (["--fail-on=superfluous-parens"], 0), + (["--fail-on=import-error"], 0), + (["--fail-on=unused-import"], 0), + (["--fail-on=unused-import", "--enable=C"], 0), + (["--fail-on=missing-function-docstring"], 0), + (["--fail-on=useless-suppression"], 1), + (["--fail-on=useless-suppression", "--enable=C"], 1), + ]) + def test_fail_on_info_only_exit_code(self, args, expected): + path = join(HERE, "regrtest_data", "fail_on_info_only.py") + self._runtest([path] + args, code=expected) + @pytest.mark.parametrize( "output_format, expected_output", [ From 4351b6daab5d435f62a2d2f04b3f672a8193120c Mon Sep 17 00:00:00 2001 From: Mark Bell Date: Sat, 17 Jul 2021 13:17:05 +0100 Subject: [PATCH 4/6] Linted and re-added None check of score. --- pylint/lint/run.py | 2 +- tests/test_self.py | 50 ++++++++++++++++++++++++++-------------------- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/pylint/lint/run.py b/pylint/lint/run.py index 1f6d6b71c7..08806e6c83 100644 --- a/pylint/lint/run.py +++ b/pylint/lint/run.py @@ -398,7 +398,7 @@ def __init__( # We need to make sure we return a failing exit code in this case. # So we use self.linter.msg_status if that is non-zero, otherwise we just return 1. sys.exit(self.linter.msg_status or 1) - elif score_value >= linter.config.fail_under: + elif score_value is not None and score_value >= linter.config.fail_under: sys.exit(0) else: sys.exit(self.linter.msg_status) diff --git a/tests/test_self.py b/tests/test_self.py index ef70cdf333..160ff59b38 100644 --- a/tests/test_self.py +++ b/tests/test_self.py @@ -1136,34 +1136,40 @@ def test_output_file_invalid_path_exits_with_code_32(self): output_file = "thisdirectorydoesnotexit/output.txt" self._runtest([path, f"--output={output_file}"], code=32) - @pytest.mark.parametrize("args, expected", [ - ([], 0), - (["--enable=C"], 0), - (["--fail-on=superfluous-parens"], 0), - (["--fail-on=import-error"], 6), - (["--fail-on=unused-import"], 6), - (["--fail-on=unused-import", "--enable=C"], 22), - (["--fail-on=missing-function-docstring"], 22), - (["--fail-on=useless-suppression"], 6), - (["--fail-on=useless-suppression", "--enable=C"], 22), - ]) + @pytest.mark.parametrize( + "args, expected", + [ + ([], 0), + (["--enable=C"], 0), + (["--fail-on=superfluous-parens"], 0), + (["--fail-on=import-error"], 6), + (["--fail-on=unused-import"], 6), + (["--fail-on=unused-import", "--enable=C"], 22), + (["--fail-on=missing-function-docstring"], 22), + (["--fail-on=useless-suppression"], 6), + (["--fail-on=useless-suppression", "--enable=C"], 22), + ], + ) def test_fail_on_exit_code(self, args, expected): path = join(HERE, "regrtest_data", "fail_on.py") # We set fail-under to be something very low so that even with the warnings # and errors that are generated they don't affect the exit code. self._runtest([path, "--fail-under=-10"] + args, code=expected) - @pytest.mark.parametrize("args, expected", [ - ([], 0), - (["--enable=C"], 0), - (["--fail-on=superfluous-parens"], 0), - (["--fail-on=import-error"], 0), - (["--fail-on=unused-import"], 0), - (["--fail-on=unused-import", "--enable=C"], 0), - (["--fail-on=missing-function-docstring"], 0), - (["--fail-on=useless-suppression"], 1), - (["--fail-on=useless-suppression", "--enable=C"], 1), - ]) + @pytest.mark.parametrize( + "args, expected", + [ + ([], 0), + (["--enable=C"], 0), + (["--fail-on=superfluous-parens"], 0), + (["--fail-on=import-error"], 0), + (["--fail-on=unused-import"], 0), + (["--fail-on=unused-import", "--enable=C"], 0), + (["--fail-on=missing-function-docstring"], 0), + (["--fail-on=useless-suppression"], 1), + (["--fail-on=useless-suppression", "--enable=C"], 1), + ], + ) def test_fail_on_info_only_exit_code(self, args, expected): path = join(HERE, "regrtest_data", "fail_on_info_only.py") self._runtest([path] + args, code=expected) From c586f6c3e575061226dc4c662ce495f8703af74c Mon Sep 17 00:00:00 2001 From: Mark Bell Date: Sat, 17 Jul 2021 14:27:15 +0100 Subject: [PATCH 5/6] Update ChangeLog Co-authored-by: Pierre Sassoulas --- ChangeLog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ChangeLog b/ChangeLog index c03da1f968..254f339087 100644 --- a/ChangeLog +++ b/ChangeLog @@ -19,6 +19,9 @@ Release date: TBA * Fix bug in which --fail-on can return a zero exit code even when the specified issue is present + Closes #4296 + Closes #3363 + * Fix hard failure when handling missing attribute in a class with duplicated bases Closes #4687 From b750b8bd6db412322414c34b41873ac6cfab31bc Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 17 Jul 2021 13:27:57 +0000 Subject: [PATCH 6/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- ChangeLog | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 254f339087..ab42165663 100644 --- a/ChangeLog +++ b/ChangeLog @@ -19,8 +19,8 @@ Release date: TBA * Fix bug in which --fail-on can return a zero exit code even when the specified issue is present - Closes #4296 - Closes #3363 + Closes #4296 + Closes #3363 * Fix hard failure when handling missing attribute in a class with duplicated bases