[flake8-simplify] Fix raw string handling in SIM905 for embedded quotes#19591
[flake8-simplify] Fix raw string handling in SIM905 for embedded quotes#19591dylwil3 merged 4 commits intoastral-sh:mainfrom
flake8-simplify] Fix raw string handling in SIM905 for embedded quotes#19591Conversation
|
dylwil3
left a comment
There was a problem hiding this comment.
This looks good, thank you! A couple of nits and then a design point:
I think we should gate this fix behavior under preview, and when not in preview just skip the check in this situation.
The reason is that it's unclear to me if the best thing to do here is to offer your fix (which would change part of a raw string into a string with escapes), to skip the check here, or to offer the fix with the "ugly" triple quotes for the relevant list items (or some compromise - like offer your fix but make it Unsafe). On the other hand, we have to do something to fix the syntax error, and I think this arrangements allows us to consider the other options in the meantime without making a breaking change.
(But I could be overruled on this)
crates/ruff_linter/src/rules/flake8_simplify/rules/split_static_string.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/flake8_simplify/rules/split_static_string.rs
Show resolved
Hide resolved
At least in the example from #19577, there are no embedded single quote characters, so one should be able to cleanly convert it to a raw single quoted string without needing to add any additional escaping: test_email_addresses = [
r"simple@example.com",
r"very.common@example.com",
r"FirstName.LastName@EasierReading.org",
r"x@example.com",
r"long.email-address-with-hyphens@and.subdomains.example.com",
r"user.name+tag+sorting@example.com",
r"name/surname@example.com",
r"xample@s.example",
r'" "@example.org',
r'"john..doe"@example.org',
r"mailhost!username@example.org",
r'"very.(),:;<>[]\".VERY.\"very@\\ \"very\".unusual"@strange.example.com',
r"user%example.com@example.org",
r"user-@example.org",
r"I❤️CHOCOLATE@example.com",
r"this\ still\"not\\allowed@example.com",
r"stellyamburrr985@example.com",
r"Abc.123@example.com",
r"user+mailbox/department=shipping@example.com",
r"!#$%&'*+-/=?^_`.{|}~@example.com",
r'"Abc@def"@example.com',
r'"Fred\ Bloggs"@example.com',
r'"Joe.\\Blow"@example.com',
]In (rare) cases where a triple quoted string does include both single and double quote characters, I would argue that the keeping the triple quotes for those items would be better than adding escaping, which would make the resulting items very difficult to read, especially if they already include backslash characters. |
|
That's a good point @tdulcet , thank you! I think at that point it's maybe unobtrusive enough to fix the bug this way without gating behind preview. Let me know if the new snapshots look objectionable - otherwise I'll merge this in shortly |
flake8_simplify] Fix raw string handling in SIM905 for embedded quotesflake8-simplify] Fix raw string handling in SIM905 for embedded quotes
Summary
Fixes #19577