Skip to content

Fix #5451: Produce a score of 0 for fatal errors#5521

Merged
DanielNoord merged 6 commits into
pylint-dev:mainfrom
jacobtylerwalls:exit-code
Dec 14, 2021
Merged

Fix #5451: Produce a score of 0 for fatal errors#5521
DanielNoord merged 6 commits into
pylint-dev:mainfrom
jacobtylerwalls:exit-code

Conversation

@jacobtylerwalls
Copy link
Copy Markdown
Member

@jacobtylerwalls jacobtylerwalls commented Dec 13, 2021

Type of Changes

Type
🐛 Bug fix

Description

Before
Modules without statements (such as typo'd paths) don't generate scores. When a linter is run over both valid and invalid paths, this can lead to a perfect score. Scores meeting the --fail-under threshold exit with a 0 code (passing), which is counterintuitive for the case just described.

Now
As long as --fail-under is 10.0, this PR fails runs with perfect scores if they had any non-zero exit codes.

Of course, the other approach would be to start emitting non-perfect scores for modules without statements, but this seemed less invasive.

Update: new approach is to simply produce a score of 0 for fatal errors.

Closes #5451

@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.13.0 milestone Dec 13, 2021
@DanielNoord
Copy link
Copy Markdown
Collaborator

I'm not sure if emitting non-perfect scores for all statement-less modules is correct, but I think we should emit a score for this error.
A fatal message is emitted and the user has taken extra care to setup pylint how they want it to behave (ie., change from the default 10 value). Fixing this in _report_evaluation makes more sense to me.

@Pierre-Sassoulas
Copy link
Copy Markdown
Member

Pierre-Sassoulas commented Dec 13, 2021

the other approach would be to start emitting non-perfect scores for modules without statements, but this seemed less invasive.

I think this makes more sense. We won't have to add a special case and the corresponding tests this way. The score should be 10 if there isn't any error or 0 if there is at least an error/warning/convention/info.

@jacobtylerwalls
Copy link
Copy Markdown
Member Author

Sure, happy to go in the other direction.

the user has taken extra care to setup pylint how they want it to behave (ie., change from the default 10 value).

Just to be clear, the case was when the user has not changed from the default 10. We were emitting 10 anyway.

@jacobtylerwalls jacobtylerwalls changed the title Fix #5451: Fail linter runs having both valid and invalid paths Fix #5451: Produce a score of 0 for fatal errors Dec 13, 2021
@coveralls
Copy link
Copy Markdown

coveralls commented Dec 13, 2021

Pull Request Test Coverage Report for Build 1579479507

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 73 unchanged lines in 5 files lost coverage.
  • Overall coverage increased (+0.002%) to 93.656%

Files with Coverage Reduction New Missed Lines %
pylint/interfaces.py 1 96.77%
pylint/testutils/checker_test_case.py 3 93.48%
pylint/checkers/utils.py 9 95.46%
pylint/checkers/variables.py 28 95.63%
pylint/checkers/typecheck.py 32 94.98%
Totals Coverage Status
Change from base Build 1573223459: 0.002%
Covered Lines: 14217
Relevant Lines: 15180

💛 - Coveralls

Comment thread pylint/lint/pylinter.py Outdated
Copy link
Copy Markdown
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering where the actual code change was, then I realized that the changed default value is valid python code. That's pretty clever !

Copy link
Copy Markdown
Collaborator

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small changes, mostly because I forgot to do stuff in #5318 😅

Note to self: copy pylintrc to examples/pylintrc in a follow-up PR.

Comment thread ChangeLog Outdated
Comment thread doc/whatsnew/2.13.rst Outdated
Comment thread examples/pylintrc Outdated
Comment thread pylintrc Outdated
Comment thread tests/test_self.py
Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>
Copy link
Copy Markdown
Collaborator

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick work @jacobtylerwalls!

I'll try and think of the pylintrc move tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

wrong exit code when linting a mix of existing and non-existing modules

4 participants