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/ChangeLog b/ChangeLog index ba24ace79a..ab42165663 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,6 +17,11 @@ 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 + + Closes #4296 + Closes #3363 + * Fix hard failure when handling missing attribute in a class with duplicated bases Closes #4687 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..08806e6c83 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 is not None 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, _, __): 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..160ff59b38 100644 --- a/tests/test_self.py +++ b/tests/test_self.py @@ -1136,6 +1136,44 @@ 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", [