-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Codemod fix-assert-tuple
#217
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #217 +/- ##
==========================================
+ Coverage 96.44% 96.47% +0.02%
==========================================
Files 97 98 +1
Lines 4169 4196 +27
==========================================
+ Hits 4021 4048 +27
Misses 148 148
|
50a61fb
to
17d0b13
Compare
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel those empty lines shouldn't be here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice overall but some suggestions for metadata
class FixAssertTuple(SimpleCodemod, NameResolutionMixin): | ||
metadata = Metadata( | ||
name="fix-assert-tuple", | ||
summary="Fix Assert on Populated Tuple", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion:
summary="Fix Assert on Populated Tuple", | |
summary="Fix `assert` on Non-Empty Tuple Literal", |
references=[], | ||
) | ||
change_description = ( | ||
"Separate assertion on a populated tuple into multiple assert statements." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Separate assertion on a populated tuple into multiple assert statements." | |
"Separate assertion on a non-empty tuple literal into multiple assert statements." |
@@ -0,0 +1,11 @@ | |||
An assertion on a non-empty tuple will always evaluate to `True` so it's likely to be written unintentionally. This codemod will write one assert statement for each item in the tuple. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An assertion on a non-empty tuple will always evaluate to `True` so it's likely to be written unintentionally. This codemod will write one assert statement for each item in the tuple. | |
An assertion on a non-empty tuple will always evaluate to `True`. This means that `assert` statements involving non-empty tuple literals are likely unintentional and should be rewritten. This codemod rewrites the original `assert` statement by creating a new `assert` for each item in the original tuple. |
metadata = Metadata( | ||
name="fix-assert-tuple", | ||
summary="Fix Assert on Populated Tuple", | ||
review_guidance=ReviewGuidance.MERGE_WITHOUT_REVIEW, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be MERGE_AFTER_CURSORY_REVIEW
because there's a decent change we break some tests.
17d0b13
to
5953722
Compare
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
Overview
Codemod to convert
assert (1, 2)
intoNotice the same sonarcloud rule this PR triggered and this is the same pylint rule