Add B034: re.sub/subn/split must pass flags/count/maxsplit as keyword arguments #398
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JelleZijlstra
approved these changes
Jul 3, 2023
cooperlees
requested changes
Jul 3, 2023
README.rst
Outdated
| @@ -188,6 +188,8 @@ second usage. Save the result to a list if the result is needed multiple times. | |||
|
|
|||
| **B033**: Sets should not contain duplicate items. Duplicate items will be replaced with a single item at runtime. | |||
|
|
|||
| **B034**: Calls to `re.sub`, `re.subn` or `re.split` should pass `flags` or `output`/`maxsplit` as keyword arguments. Not doing so is a very common source of confusion. | |||
There was a problem hiding this comment.
I had to go read the issue to understand what the confusion was here. Let’s try make this clearer to save people hunting the issue down.
Suggested change
| **B034**: Calls to `re.sub`, `re.subn` or `re.split` should pass `flags` or `output`/`maxsplit` as keyword arguments. Not doing so is a very common source of confusion. | |
| **B034**: Calls to `re.sub`, `re.subn` or `re.split` should pass `flags` or `output`/`maxsplit` as keyword arguments. Not doing so is a very common source of confusion due to the positions of these arguments differing in the `re` module APIs. |
- Happy for better wording.
There was a problem hiding this comment.
Isn't it just as much, if not more, about people not being aware that count/maxsplit are arguments at all, and just being used to passing flags as the third argument to all the other functions in re?
I agree with making it more clear though
bugbear.py
Outdated
| @@ -1770,6 +1792,12 @@ def visit_Lambda(self, node): | |||
| ) | |||
| ) | |||
|
|
|||
| B034 = Error( | |||
| message=( | |||
| "B034 {} should pass `{}` and `flags` as keyword arguments to avoid confusion." | |||
There was a problem hiding this comment.
Suggested change
| "B034 {} should pass `{}` and `flags` as keyword arguments to avoid confusion." | |
| "B034 {} should pass `{}` and `flags` as keyword arguments to avoid confusion due to different argument positions." |
cooperlees
approved these changes
Jul 10, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #397