-
-
Notifications
You must be signed in to change notification settings - Fork 410
Create Untrivial Logic in Finally Block Rule #3461
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
Conversation
flake.nix
Outdated
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.
Please, remove this file.
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.
Please, revert this change.
.gitignore
Outdated
@@ -1,4 +1,7 @@ | |||
#### joe made this: http://goel.io/joe | |||
#### NIX #### | |||
flake.lock |
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.
We don't need this :)
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.
Please, add a test case with a custom setting.
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.
Forbids "fat" finally block. | |
Forbids complex finally block. |
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.
class UntrivialLogicInFinallyViolation(ASTViolation): | |
class ComplexFinallyViolation(ASTViolation): |
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.
error_template = 'Found untrivial logic in `finally` block: {0}' | |
error_template = 'Found complex `finally` block: {0}' |
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.
don't forget to add this check to test_checker.py
Default: | ||
:int:`wemake_python_styleguide.options.defaults.MAX_LINES_IN_FINALLY. | ||
..versionadded:: 1.1.0 |
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.
..versionadded:: 1.1.0 | |
..versionadded:: 1.2.0 |
For now some test checks doesn't work correctly. |
i cant install dependencies error log. I use poetry install |
Please, attach error log as a text, not a file. I don't want to download any files. |
Co-authored-by: sobolevn <mail@sobolevn.me>
|
@alexeev-prog you are using 3.14 which is not supported, please use 3.13 :) |
My local testing logs: https://pastebin.com/qJp9nkLp |
Please help, i am not understand test errors... |
Please, fix WPS errors first. Tests are not even executed :) |
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.
Please, add ComplexFinallyBlocksVisitor
to a preset
tests/fixtures/noqa/noqa.py
Outdated
my_print(first_argpm, second_argpm) | ||
|
||
|
||
Я |
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.
Я |
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.
Why is it named complex_continue
? When it checks complex try finally blocks?
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.
So sorry! I renamed it.
In my local environment, i got tests error:
Whats is solution of fix it? |
|
||
# Wrong examples: | ||
|
||
untrivial_logic_example1 = """ |
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.
It is the same as trivial_logic_custom_example1
|
||
# Correct examples: | ||
|
||
trivial_logic_example1 = """ |
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.
Please, add two test cases with try/finally
without except
: both correct and incorrect ones.
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 add this, by i get Total coverage: 99.91%
in local tests.
|
||
total_lines = 0 | ||
|
||
for stmt in node.finalbody: |
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.
You don't need any of this. Here's the code you need:
if not node.finalbody:
return
first_line = node.finalbody[0].lineno
last_line = node.finalbody[-1].lineno
total_lines = last_line - first_line
if total_lines > self.options.max_lines_in_finally:
self.add_violation(
complexity.ComplexFinallyViolation(
node,
text=str(total_lines),
baseline=self.options.max_lines_in_finally,
),
)
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 add this
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3461 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 362 364 +2
Lines 11987 12040 +53
Branches 821 823 +2
=========================================
+ Hits 11987 12040 +53 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Its ready? |
Create Untrivial Logic in Finally Block Rule
Checklist
CHANGELOG.md
Related issues
Closes #3458