Skip to content

unknown-option-value should result in non-zero exit code #8457

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

Open
l0b0 opened this issue Mar 16, 2023 · 4 comments
Open

unknown-option-value should result in non-zero exit code #8457

l0b0 opened this issue Mar 16, 2023 · 4 comments
Labels
Breaking changes for 4.0 🦤 Enhancement ✨ Improvement to a component Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Milestone

Comments

@l0b0
Copy link

l0b0 commented Mar 16, 2023

Bug description

Unknown disable or enable values (on the command line, in pyproject.toml, or in .pylintrc) do not trigger a non-zero exit code when using the default configuration.

test.py which passes default pylint rules:

"""comment"""
pass

Configuration

No response

Command used

pylint --disable=foo test.py

Pylint output

************* Module Command line
Command line:1:0: W0012: Unknown option value for '--disable', expected a valid pylint message and got 'foo' (unknown-option-value)

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

Expected behavior

The exit code should be non-zero.

Pylint version

pylint 2.17.0
astroid 2.15.0
Python 3.10.9 (main, Dec  6 2022, 18:44:57) [GCC 11.3.0]

OS / Environment

NixOS 22.11

Additional dependencies

astroid==2.15.0
dill==0.3.6
isort==5.12.0
lazy-object-proxy==1.9.0
mccabe==0.7.0
platformdirs==3.1.1
pylint==2.17.0
tomli==2.0.1
tomlkit==0.11.6
typing_extensions==4.5.0
wrapt==1.15.0
@l0b0 l0b0 added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Mar 16, 2023
@Pierre-Sassoulas
Copy link
Member

If I remember correctly it was a conscious decision in #6824 (comment), we can reopen the discussion here.

@Pierre-Sassoulas Pierre-Sassoulas added Discussion 🤔 Needs decision 🔒 Needs a decision before implemention or rejection and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Mar 16, 2023
@l0b0
Copy link
Author

l0b0 commented Mar 16, 2023

The main issue is that it's surprising. I can't think of any tools worth using which treat unknown configuration items as anything other than an error.

Also, the warning message can easily be missed. At least in my case pylint is only one of several dozen tools I use every day, and combing the logs of each of them is unfeasible. This is made worse by the fact that some tools (like pre-commit) will hide the output of the programs it runs if the exit code is zero, to avoid swamping the user with irrelevant output. So it's really very important to make sure the exit code is non-zero in case of any issues.

@Pierre-Sassoulas
Copy link
Member

Right, it also sounds like a breaking change we can do with 3.0 approaching.

@ejfine
Copy link
Contributor

ejfine commented Aug 1, 2023

@jacobtylerwalls just pointed me to this issue. FWIW - I'm heavily supportive of having issues discovered while parsing a configuration throw a non-zero exit code. Glad it's going to be part of v3

@jacobtylerwalls jacobtylerwalls modified the milestones: 3.0.0, 3.0.0b1 Aug 1, 2023
@Pierre-Sassoulas Pierre-Sassoulas added Enhancement ✨ Improvement to a component Breaking changes for 3.0 🦤 Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Discussion 🤔 Needs decision 🔒 Needs a decision before implemention or rejection labels Aug 1, 2023
@jacobtylerwalls jacobtylerwalls self-assigned this Sep 24, 2023
@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 3.0.0b0, 4.0.0b0 Sep 25, 2023
@jacobtylerwalls jacobtylerwalls removed their assignment Dec 13, 2023
@jacobtylerwalls jacobtylerwalls modified the milestones: 4.0.0b0, 4.0.0 Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking changes for 4.0 🦤 Enhancement ✨ Improvement to a component Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests

4 participants