search

MichaelKim0407 / flake8-use-fstring

MIT License
51 stars 7 forks source link

Remove false positive FS003 for pure regex-strings containing curly brackets #9

Closed teonat closed 3 years ago

teonat commented 3 years ago

ref #6, #7
Fixes false positives on pure regex strings prefixed with only a single r, containing curly brackets as repetition quantifiers, by utilizing a negative lookbehind.

MichaelKim0407 commented 3 years ago

Thanks for the PR - can you please add a test case for it? Thanks!

JonasKs commented 3 years ago

Can you provide steps on how to run your test suite? Tests seem to be failing for me even without the changes.

MichaelKim0407 commented 3 years ago

Can you provide steps on how to run your test suite? Tests seem to be failing for me even without the changes.

Very straightforward pytest: https://github.com/MichaelKim0407/flake8-use-fstring/blob/develop/.travis.yml

The tests were passing when it was last released, so I don't know if anything broke because of newer versions of dependencies.

MichaelKim0407 commented 3 years ago

I apologize for the lack of activity from my side. I don't really use this package and it was created for fun quite a while ago, so I'm not really actively maintaining or developing it. But I will make releases once changes are good. Everyone is welcome to submit changes.

tekdj7 commented 3 years ago

@teonat @MichaelKim0407 this is still incorrectly flagging regex patterns as not being an f-string. See example below.

Any updates on a fix for this?

JonasKs commented 3 years ago

Not really, no. Tests didn't run properly for me, and I haven't looked at it since.

I might take a look again next week, but feel free to contribute with tests.

MichaelKim0407 commented 3 years ago

Thanks for the update. If I understand the regex correctly, it means that FS003 will report an error when there are {} s in a string, except when f or r modifiers are present. Is that correct?

teonat commented 3 years ago

The intention of this change is to allow strings containing curly brackets and prefixed with only an r to pass the check for FS003, yeah. Strings prefixed with an f should also be allowed. I haven't done anything to that pattern.
This change can still trigger false positives on regex-strings though, if the string prefix contains other letters in addition to the r, except if an f is in the prefix as well. Hence the pure regex strings in the description :)

MichaelKim0407 commented 3 years ago

Thank you!