Skip to content

Add B028 - suggest !r for quote-wrapped variables in f-strings#328

Merged
Zac-HD merged 1 commit intoPyCQA:mainfrom
jakkdl:suggest_r_format_specifier
Jan 11, 2023
Merged

Add B028 - suggest !r for quote-wrapped variables in f-strings#328
Zac-HD merged 1 commit intoPyCQA:mainfrom
jakkdl:suggest_r_format_specifier

Conversation

@jakkdl
Copy link
Copy Markdown
Contributor

@jakkdl jakkdl commented Jan 9, 2023

fixes #319

I decided not to implement 3.7 support at all, expecting it to be dropped soon enough / be rare enough that it's not worth the added code complexity to support it.
3.8 is somewhat of a pain with not having ast.unparse, but this implementation should be decent-ish even when there's complex contents of an f-string variable.

This should likely not be merged until edge cases in #319 has been sorted out.

@Zac-HD

@Zac-HD Zac-HD self-requested a review January 10, 2023 04:02
Copy link
Copy Markdown
Collaborator

@cooperlees cooperlees left a comment

Choose a reason for hiding this comment

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

Sounds good to do >= 3.8 - Lets just state that in the README.rst description of this check please (that is missing here).

3.7 still has another ~6 months of life, we've kind of decided to follow python supported versions and follow flake8 version dropping. So I would expect 3.7 to hang around for little while longer ...

@jakkdl jakkdl force-pushed the suggest_r_format_specifier branch 2 times, most recently from a320fdb to 73aa5ce Compare January 10, 2023 14:19
@jakkdl
Copy link
Copy Markdown
Contributor Author

jakkdl commented Jan 10, 2023

Added Readme and Change Log entry.

Now won't warn if there's any format specifiers only valid for numerics - or width specified (which will misplace quotes relative to the padding).

And you said you wanted lots of tests, so there's now >90k checks 😅 The format spec permutation test takes a few seconds to run on my machine, but not much slower than the existing fuzz tests. I can scale it back if you think it's overkill though

@jakkdl jakkdl force-pushed the suggest_r_format_specifier branch from 73aa5ce to ef39e9a Compare January 10, 2023 14:26
Copy link
Copy Markdown
Collaborator

@cooperlees cooperlees left a comment

Choose a reason for hiding this comment

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

Thanks John! All looks good to me. Will see if @Zac-HD pops past at all to double check :)

Copy link
Copy Markdown
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

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

Looks fantastic - thanks John!

@Zac-HD Zac-HD merged commit 48a61e4 into PyCQA:main Jan 11, 2023
@jakkdl jakkdl deleted the suggest_r_format_specifier branch January 12, 2023 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Suggest !r format specifier for strings like f"foo is '{self.foo}'"

3 participants