zheller / flake8-quotes

Flake8 extension for checking quotes in python
MIT License
178 stars 39 forks source link

Q000 error on f-string for CPython 3.12 #117

Closed skirpichev closed 10 months ago

skirpichev commented 1 year ago

An example:

$ cat a.py
a = 'f'
b = [2, 3]
c = f'{a}({", ".join(map(str, b))})'
print(c)

No errors from the plugin on the CPython 3.11:

$ python -V
Python 3.11.3+
$ pip install -U flake518 flake8-quotes
[...]
$ flake518 a.py
$

While on the CPython 3.12 I got this error: a.py:3:12: Q000 Double quotes found but single quotes preferred.

mscuthbert commented 1 year ago

This has to do with the change in f-string behavior (adoption of a formal grammar) in Python 3.12 -- now each { ... } expression creates its own little scope, so within that scope single quotes are allowed again.

There are two issues that arise from this:

  1. The vast majority of us still maintain libraries that are compatible with 3.11 and earlier. Fixing the problem breaks the code on older versions of Python
  2. Even when Python 3.12 is the minimum version I support, I'm not sure I will want to switch to using single quotes within f-strings that are themselves opened and closed with single quotes -- I think it shows the hierarchy that we are within an f-string.

So an option to disable checking inside f-strings would be most helpful for either or both of these reasons. Thank you!

ThiefMaster commented 1 year ago

Please make this configurable, e.g. by using a separate error code for quotes inside fstrings.

Because even on a 3.12-only codebase one may prefer to NOT use the same type of quotes simple for the sake of readability and being nice towards editors whose syntax highlighting may not properly handle reusing the same quotes.

ThiefMaster commented 1 year ago

@JoeHitchen or @zheller (since you are the maintainers who can actually make a new release besides @twolfson who stepped down according to another issue): Any chance of this being fixed? Or at least a bugfix release being done if someone sends a PR to fix it?

JoeHitchen commented 1 year ago

Hi - Yes, if there is a pull request that fixes the problem and passes the tests then I can cast an eye over it and make a release.

arnimarj commented 1 year ago

Hi - Yes, if there is a pull request that fixes the problem and passes the tests then I can cast an eye over it and make a release.

I quickly hacked together a POC at https://github.com/zheller/flake8-quotes/pull/118, the state-machine tracking the nesting depth of f-strings is simple enough, I'll see how far I can take it this weekend

arnimarj commented 1 year ago

@JoeHitchen I have a preliminary version which passes (when run manually on my local machine) for Python 3.11 and 3.12. The fix is not backwards compatible though, there are positives in 3.12, which weren't there in 3.11, and guidance on that point would be appreciated.

JoeHitchen commented 1 year ago

We wouldn't want to break backwards compatibility, but is there a way to inspect the python version before deciding to apply the new rule or not? e.g. https://docs.python.org/3/library/sys.html#sys.version_info

arnimarj commented 1 year ago

@JoeHitchen I've built a simple test harness using tox to run the test-suite on the various Python versions. I'm wondering if you'd consider a PR to automate these using Github Actions?

(BTW: I've not managed to test this successfully on Python 3.4/3.5, perhaps its time to drop support for those, 3.5 was EOLed in 2020)

ThiefMaster commented 1 year ago

IMHO anything that's EOL, ie <=3.7 should be removed.

Someone using such an obsolete Python version can just use an old version of this package...

+1 on GH actions. Might also want to switch to pytest at some point since the current test runner is awful (in fact when I started working on this same thing yesterday before I saw you already being on it, it was quite easy to run even the existing tests using pytest)

JoeHitchen commented 12 months ago

I'd be happy to accept a PR to master which works for >=3.6 but drops 3.4 & 3.5. A release which formally drops 3.4 & 3.5 make take a bit longer as we probably need to tidy up some other bits (e.g. documentation) to reflect that change, but it will be installable from the repo for those that want it sooner.

Also agree that Github Actions would help test compatibility with different versions python versions.

JoeHitchen commented 10 months ago

Should be resolved following the merge of #118

Nudge nudge #109 if anyone on this thread is interested