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

Respect py-version for inconsistent-quotes inside f-strings #9152

Merged
merged 4 commits into from Oct 16, 2023

Conversation

theirix
Copy link
Contributor

@theirix theirix commented Oct 15, 2023

Type of Changes

Type
βœ“ πŸ› Bug fix
✨ New feature
πŸ”¨ Refactoring
βœ“ πŸ“œ Docs

Description

When running pre-3.12 interpeter, f-strings are opaque and are represented as a STRING token (like 'f'{dictionary["0"]}' for the test case in the ticket). So they are not used in analysis.

With 3.12 PEG interpreter, f-string is a proper Python expression, even if we target pre-3.12. So this PR enables inconsistent-quotes checks for string literals only if a target version is 3.12 or later.

Closes #9113

@codecov
Copy link

codecov bot commented Oct 15, 2023

Codecov Report

Merging #9152 (720de79) into main (0796dfa) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #9152   +/-   ##
=======================================
  Coverage   95.76%   95.76%           
=======================================
  Files         173      173           
  Lines       18680    18690   +10     
=======================================
+ Hits        17889    17899   +10     
  Misses        791      791           
Files Coverage Ξ”
pylint/checkers/strings.py 94.27% <100.00%> (+0.13%) ⬆️

@github-actions

This comment has been minimized.

Despite ``sys.version`` check, pylint raises the warning
with Python 3.11
@Pierre-Sassoulas Pierre-Sassoulas added the False Positive 🦟 A message is emitted but nothing is wrong with the code label Oct 15, 2023
@github-actions
Copy link
Contributor

πŸ€– According to the primer, this change has no effect on the checked open source code. πŸ€–πŸŽ‰

This comment was generated for commit 720de79

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.

Great contribution. Maybe the following could be added in the message definition: {"maxversion": (3, 12)}, (see

{"minversion": (3, 5)},
and
if self.minversion is not None and self.minversion > py_version:
). This should simplify the sys.version_info check too?

@theirix
Copy link
Contributor Author

theirix commented Oct 16, 2023

Great contribution. Maybe the following could be added in the message definition: {"maxversion": (3, 12)}, (see

{"minversion": (3, 5)},

Thank you!

I am not sure if we can add a version limit to the message "W1405 inconsistent-quotes" definition itself because this PR addresses only differences with f-strings handling. Other problems with inconsistent quotes are not related to Python version so they cannot be dispatched from message definition.

Maybe if we have inconsistent-quotes-f-strings, it could be easily added to message definition.

@Pierre-Sassoulas Pierre-Sassoulas added this to the 3.0.2 milestone Oct 16, 2023
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.

Hmm you're right, probably not worth the very small perf/maintance improvment then.

@Pierre-Sassoulas Pierre-Sassoulas merged commit 1c0bc70 into pylint-dev:main Oct 16, 2023
46 checks passed
github-actions bot pushed a commit that referenced this pull request Oct 16, 2023
* [inconsistent-quotes] Emit for f-strings for 3.12 only

* Add docs for inconsistent-quotes with f-strings

Despite ``sys.version`` check, pylint raises the warning
with Python 3.11

(cherry picked from commit 1c0bc70)
Pierre-Sassoulas pushed a commit that referenced this pull request Oct 16, 2023
… (#9155)

* [inconsistent-quotes] Emit for f-strings for 3.12 only

* Add docs for inconsistent-quotes with f-strings

Despite ``sys.version`` check, pylint raises the warning
with Python 3.11

(cherry picked from commit 1c0bc70)

Co-authored-by: theirix <theirix@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backported False Positive 🦟 A message is emitted but nothing is wrong with the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

W1405: inconsistent-quotes does not respect py-version
2 participants