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

Pylint 2.8.2 broke pylint-quotes #4420

Closed
mkielar opened this issue Apr 28, 2021 · 4 comments · Fixed by #4421
Closed

Pylint 2.8.2 broke pylint-quotes #4420

mkielar opened this issue Apr 28, 2021 · 4 comments · Fixed by #4421
Milestone

Comments

@mkielar
Copy link

mkielar commented Apr 28, 2021

Steps to reproduce

See edaniszewski/pylint-quotes#24
See pylintrc in attached pylint_bug.zip

python -m venv venv
. venv/bin/activate
pip install -r requirements.txt
pylint --rcfile pylintrc demo.py

Current behavior

Running this configuration on a file containing:

foo = "bar"

results in an exception:

Traceback (most recent call last):
  File "/c/tmp/pylint_bug/venv/bin/pylint", line 10, in <module>
    sys.exit(run_pylint())
  File "/c/tmp/pylint_bug/venv/lib64/python3.8/site-packages/pylint/__init__.py", line 24, in run_pylint
    PylintRun(sys.argv[1:])
  File "/c/tmp/pylint_bug/venv/lib64/python3.8/site-packages/pylint/lint/run.py", line 381, in __init__
    linter.check(args)
  File "/c/tmp/pylint_bug/venv/lib64/python3.8/site-packages/pylint/lint/pylinter.py", line 873, in check
    self._check_files(
  File "/c/tmp/pylint_bug/venv/lib64/python3.8/site-packages/pylint/lint/pylinter.py", line 907, in _check_files
    self._check_file(get_ast, check_astroid_module, name, filepath, modname)
  File "/c/tmp/pylint_bug/venv/lib64/python3.8/site-packages/pylint/lint/pylinter.py", line 933, in _check_file
    check_astroid_module(ast_node)
  File "/c/tmp/pylint_bug/venv/lib64/python3.8/site-packages/pylint/lint/pylinter.py", line 1067, in check_astroid_module
    retval = self._check_astroid_module(
  File "/c/tmp/pylint_bug/venv/lib64/python3.8/site-packages/pylint/lint/pylinter.py", line 1110, in _check_astroid_module
    checker.process_tokens(tokens)
  File "/c/tmp/pylint_bug/venv/lib64/python3.8/site-packages/pylint_quotes/checker.py", line 259, in process_tokens
    self._process_string_token(token, start_row, start_col)
  File "/c/tmp/pylint_bug/venv/lib64/python3.8/site-packages/pylint_quotes/checker.py", line 295, in _process_string_token
    self._invalid_string_quote(
  File "/c/tmp/pylint_bug/venv/lib64/python3.8/site-packages/pylint_quotes/checker.py", line 341, in _invalid_string_quote
    **self.get_offset(col)
  File "/c/tmp/pylint_bug/venv/lib64/python3.8/site-packages/pylint_quotes/checker.py", line 360, in get_offset
    if (2, 2, 2) < pylint_version:
TypeError: '<' not supported between instances of 'int' and 'str'

Expected behavior

pylint should not throw exceptions, and instead generate report complaining about the use of double quotes ("") and illegal variable name foo.

pylint --version output

Result of pylint --version output:

→ pylint --version
pylint 2.8.2
astroid 2.5.6
Python 3.8.3 (default, Feb 26 2020, 00:00:00)
[GCC 9.3.1 20200408 (Red Hat 9.3.1-2)]

Additional dependencies:

pylint-quotes==0.2.1

Probable cause:

  1. pylint seems to have just changed versioning scheme: pylint-2.8.1...v2.8.2
  2. Which broke this line: https://github.com/edaniszewski/pylint-quotes/blob/master/pylint_quotes/checker.py#L360
@Pierre-Sassoulas
Copy link
Member

Hello, thank you for the report. We changed the tuple from int to string because we can't be sure that we're capable of getting the version from importlib_metadata so there is a default 2.8.2+ value that would not be castable to an int.

@mkielar
Copy link
Author

mkielar commented Apr 28, 2021

Okay, but that was a breaking change for plugin developers, and as such I'm not entirely sure why this was released as PATCH version, and not at least MINOR. See: https://semver.org/#what-if-i-inadvertently-alter-the-public-api-in-a-way-that-is-not-compliant-with-the-version-number-change-ie-the-code-incorrectly-introduces-a-major-breaking-change-in-a-patch-release

The pylint-quotes plugin seems to be dead (no update for 2 years, opened issues) so I'm wondering if you plan on doing anything to keep it working, or should we rather start looking for an alternative. I've seen the check-quote-consistency setting which we may want to try. Can you confirm it's is this the same as what pylint-quotes did?

Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Apr 28, 2021
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Apr 28, 2021
@Pierre-Sassoulas
Copy link
Member

Personally I would advise to just use black or another auto-formatter. Flake8 had a lot of check to not use " in its pre-commit configuration at some point (you could check in their git history if you're interested) but even them have black now. Regardless numversion will be fixed in 2.8.3.

@Pierre-Sassoulas
Copy link
Member

Btw 2.8 was the minor that removed it, we added it back in a patch.

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

Successfully merging a pull request may close this issue.

2 participants