Skip to content
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

Fix _regexp_paths_csv_validator for already validated values #5439

Merged
merged 1 commit into from Nov 29, 2021

Conversation

DanielNoord
Copy link
Collaborator

  • Add yourself to CONTRIBUTORS if you are a new contributor.
  • Add a ChangeLog entry describing what your PR does.
  • If it's a new feature, or an important bug fix, add a What's New entry in
    doc/whatsnew/<current release.rst>.
  • Write a good description on what the PR does.

Type of Changes

Type
βœ“ πŸ› Bug fix

Description

Closes #5437

I have not done anything to the error. The error is actually based on a pattern we have made, but should never be encountered by users. Since this is also related to optparse I don't think it is worth the effort to improve the error code and handling of it for now.

@DanielNoord DanielNoord added Regression Crash πŸ’₯ A bug that makes pylint crash labels Nov 29, 2021
@DanielNoord DanielNoord added this to the 2.12.2 milestone Nov 29, 2021
@coveralls
Copy link

coveralls commented Nov 29, 2021

Pull Request Test Coverage Report for Build 1517860490

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.0009%) to 93.512%

Totals Coverage Status
Change from base Build 1517753174: 0.0009%
Covered Lines: 13996
Relevant Lines: 14967

πŸ’› - Coveralls

Copy link
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.

The fix is so elegant that I don't know why it works πŸ˜„ Why are we encountering already validated value ? The option parsing code is messed up and the problem will disappear when we move to something else in #5392 ?

@@ -27,7 +27,11 @@ def _regexp_csv_validator(_, name, value):
return [_regexp_validator(_, name, val) for val in _csv_validator(_, name, value)]


def _regexp_paths_csv_validator(_, name: str, value: str) -> List[Pattern[str]]:
def _regexp_paths_csv_validator(
_, name: str, value: Union[str, List[Pattern[str]]]
Copy link
Member

Choose a reason for hiding this comment

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

πŸ‘

@DanielNoord
Copy link
Collaborator Author

The fix is so elegant that I don't know why it works πŸ˜„ Why are we encountering already validated value ? The option parsing code is messed up and the problem will disappear when we move to something else in #5392 ?

It has something to do with:
https://github.com/PyCQA/pylint/blob/07ea5b8687b50828538633f8e7520a0d2c2bbece/pylint/config/option.py#L186-L198

Both self.convert_value and self.take_action call the validator. self is a class of optparse.Option so let's just say "it works"... πŸ˜…

@DanielNoord DanielNoord merged commit 6fa8218 into pylint-dev:main Nov 29, 2021
@DanielNoord DanielNoord deleted the regexp-paths-validator branch November 29, 2021 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Crash πŸ’₯ A bug that makes pylint crash Regression
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option --ignore-paths broken in 2.12.0
3 participants