Skip to content
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

Forbid to have too many raise statements #1160

Closed
sobolevn opened this issue Feb 18, 2020 · 5 comments · Fixed by #1198
Closed

Forbid to have too many raise statements #1160

sobolevn opened this issue Feb 18, 2020 · 5 comments · Fixed by #1198
Assignees
Labels
help wanted Extra attention is needed level:starter Good for newcomers pr-available rule request Adding a new rule

Comments

@sobolevn
Copy link
Member

sobolevn commented Feb 18, 2020

Rule request

Thesis

Currently we check for a lot of function internals.
But, one can write any number of raise MyError statements. It is not checked at all.

We need to create new TooManyRaisesViolation with configuration to be able to find these problems. I can suggest to use 4 as maximum amount of raise uses. Or even 3.

This is a complexity violation.

@sobolevn sobolevn added help wanted Extra attention is needed level:starter Good for newcomers rule request Adding a new rule labels Feb 18, 2020
@sobolevn sobolevn added this to the Version 0.15 milestone Feb 18, 2020
@iZafiro
Copy link
Contributor

iZafiro commented Feb 23, 2020

Hi! Can I take this issue? Thanks!

@sobolevn
Copy link
Member Author

Sure! Thank you! Make sure to read our contributing guides

I am here to help: ask anything you need.

@iZafiro
Copy link
Contributor

iZafiro commented Feb 24, 2020

@sobolevn Hi! I've written the code by improving upon the _ComplexityCounter class in visitors\ast\complexity\function.py by adding an extra public attribute that counts the number of raise statements within a function. However, I get a lint error because the linter finds too many public attributes. If I change the MAX_ATTRIBUTES default (by one), the tests for TooManyPublicAttributesViolation fail. I was thinking that the best solution may be to make the linter ignore the class, so how can I do that? Thanks so much!

adrwes referenced this issue in adrwes/wemake-python-styleguide Feb 24, 2020
@sobolevn
Copy link
Member Author

@iZafiro no, complexity violations generally should not be ignored. Instead, we should refactor our class to be simplier. Here's our policy: https://sobolevn.me/2019/10/complexity-waterfall

I see several solutions:

  1. We can split _ComplexityCounter into several classes and refactor how it is used
  2. We can change how properties are defined. For example you can create @final @dataclass class _ComplexityMetrics(object) with fields that you need.

Does this sound fine to you? 🙂

@iZafiro
Copy link
Contributor

iZafiro commented Feb 24, 2020

@sobolevn Yeah, I'll refactor the class then. :)

iZafiro added a commit to adrwes/wemake-python-styleguide that referenced this issue Feb 25, 2020
iZafiro added a commit to adrwes/wemake-python-styleguide that referenced this issue Feb 27, 2020
iZafiro added a commit to adrwes/wemake-python-styleguide that referenced this issue Feb 27, 2020
@iZafiro iZafiro mentioned this issue Feb 27, 2020
4 tasks
@helpr helpr bot added the pr-available label Feb 27, 2020
Mema5 pushed a commit to adrwes/wemake-python-styleguide that referenced this issue Feb 28, 2020
Mema5 pushed a commit to adrwes/wemake-python-styleguide that referenced this issue Feb 28, 2020
iZafiro added a commit to adrwes/wemake-python-styleguide that referenced this issue Feb 28, 2020
* Refactor: Refactored function.py to include a @DataClass.

* Fix: Added missing comma to function.py.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.
iZafiro added a commit to adrwes/wemake-python-styleguide that referenced this issue Mar 1, 2020
* Refactor issue wemake-services#1160 (#25)

* Refactor: Refactored function.py to include a @DataClass.

* Fix: Added missing comma to function.py.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Changed wrong frozen=False tag.

* Fix: Changed default to factory.

* Fix: Fixed lint errors.

* Fix: Added defaultDict variable to dataclass.

* Fix: Lint fix.

* Refactor: Finish refactoring dataclass.

* Fix: Fixed lint errors.

* Fix: Fixed lint errors.

* Fix: Fixed test errors.
@sobolevn sobolevn modified the milestones: Version 0.16, Version 0.15 aka New runtime Oct 20, 2020
sobolevn pushed a commit to adrwes/wemake-python-styleguide that referenced this issue Oct 22, 2020
Test: Adds tests for TooManyRaisesViolation which for now are disabled

Test:
* Adds false positivity test with 3 raises in method
* Adds false positivity test with 4 raises in module
* Fixes wrong number of raises in tests
* Fixes string literals in tests to be more pythonic

Fix: fixes test for python 3.8

Docs: Document TooManyRaisesViolation rule. Issue #16 (#20)

Merge branches

Issue #14 (#21)

* Feat: Implement TooManyRaisesViolation rule and config. Issue #14

* Fix: A priori fix for I001 and WPS230 lint failures. Issue #14

* Fix: A priori fix for AssertionError related to tests. Issue #14

* Fix: Modify test_public_attrs_count tests to ignore class _ComplexityCounter. Issue #14

* Fix: Sort imports in test_public_attrs_count. Issue #14

* Fix: Try to avoid lint failures in test_public_attrs_count. Issue #14

* Fix: Undo a few commits and restore defaults. Issue #14

* Refactor: Create class _ComplexityExitMetrics(object). Issue #14

* Fix: (Temporarily) remove decorator @DataClass from class _ComplexityExitMetrics(object). Issue #14

* Fix: Added _ at the beginning of a public attribute in class _ComplexityCounter(object). Issue #14

* Fix: Added = to define a public attribute in class _ComplexityCounter(object). Issue #14

* Fix: Fix various lint errors. Issue #14

* Docs: Changed docs for class _ComplexityExitMetrics(object). Issue #14

* Test: Added Gwin73's fixes for his tests. Issue #14

* Fix: fixes incorrect annotation for test

* Fix for python 3.8

Co-authored-by: Adrian Westerberg <adrianwesterberg@hotmail.com>

Test: Removes integration tests and add unit tests which right now fail

Fix: Fixed a few lint errrors in test_raises_count.py. Issue #15

Issue #15 (#23)

* Test: Created new tests for issue 3. Issue #15

* Test: Modified WPS235 in test_noqa.py. Issue #15

Co-authored-by: iZafiro <60047972+iZafiro@users.noreply.github.com>

Docs: Added WPS235 to CHANGELOG.md. Issue wemake-services#1160

Docs: Changed weird comment in test_raises.py. Issue wemake-services#1160

Doc: Fixes doc

Test: Simplifies integration test

Test: Adds more tests

Fix: Changes code of violation to make merge easier

Refac: changes imports and formatting after rebasing.

Refact issue wemake-services#1160 (#26)

* Refactor issue wemake-services#1160 (#25)

* Refactor: Refactored function.py to include a @DataClass.

* Fix: Added missing comma to function.py.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Changed wrong frozen=False tag.

* Fix: Changed default to factory.

* Fix: Fixed lint errors.

* Fix: Added defaultDict variable to dataclass.

* Fix: Lint fix.

* Refactor: Finish refactoring dataclass.

* Fix: Fixed lint errors.

* Fix: Fixed lint errors.

* Fix: Fixed test errors.
sobolevn added a commit that referenced this issue Oct 22, 2020
* Feat: adds stub of the new violation TooManyRaisesViolation

Test: Adds tests for TooManyRaisesViolation which for now are disabled

Test:
* Adds false positivity test with 3 raises in method
* Adds false positivity test with 4 raises in module
* Fixes wrong number of raises in tests
* Fixes string literals in tests to be more pythonic

Fix: fixes test for python 3.8

Docs: Document TooManyRaisesViolation rule. Issue #16 (#20)

Merge branches

Issue #14 (#21)

* Feat: Implement TooManyRaisesViolation rule and config. Issue #14

* Fix: A priori fix for I001 and WPS230 lint failures. Issue #14

* Fix: A priori fix for AssertionError related to tests. Issue #14

* Fix: Modify test_public_attrs_count tests to ignore class _ComplexityCounter. Issue #14

* Fix: Sort imports in test_public_attrs_count. Issue #14

* Fix: Try to avoid lint failures in test_public_attrs_count. Issue #14

* Fix: Undo a few commits and restore defaults. Issue #14

* Refactor: Create class _ComplexityExitMetrics(object). Issue #14

* Fix: (Temporarily) remove decorator @DataClass from class _ComplexityExitMetrics(object). Issue #14

* Fix: Added _ at the beginning of a public attribute in class _ComplexityCounter(object). Issue #14

* Fix: Added = to define a public attribute in class _ComplexityCounter(object). Issue #14

* Fix: Fix various lint errors. Issue #14

* Docs: Changed docs for class _ComplexityExitMetrics(object). Issue #14

* Test: Added Gwin73's fixes for his tests. Issue #14

* Fix: fixes incorrect annotation for test

* Fix for python 3.8

Co-authored-by: Adrian Westerberg <adrianwesterberg@hotmail.com>

Test: Removes integration tests and add unit tests which right now fail

Fix: Fixed a few lint errrors in test_raises_count.py. Issue #15

Issue #15 (#23)

* Test: Created new tests for issue 3. Issue #15

* Test: Modified WPS235 in test_noqa.py. Issue #15

Co-authored-by: iZafiro <60047972+iZafiro@users.noreply.github.com>

Docs: Added WPS235 to CHANGELOG.md. Issue #1160

Docs: Changed weird comment in test_raises.py. Issue #1160

Doc: Fixes doc

Test: Simplifies integration test

Test: Adds more tests

Fix: Changes code of violation to make merge easier

Refac: changes imports and formatting after rebasing.

Refact issue #1160 (#26)

* Refactor issue #1160 (#25)

* Refactor: Refactored function.py to include a @DataClass.

* Fix: Added missing comma to function.py.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix syntax errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix lint errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Tried to fix test errors.

* Fix: Changed wrong frozen=False tag.

* Fix: Changed default to factory.

* Fix: Fixed lint errors.

* Fix: Added defaultDict variable to dataclass.

* Fix: Lint fix.

* Refactor: Finish refactoring dataclass.

* Fix: Fixed lint errors.

* Fix: Fixed lint errors.

* Fix: Fixed test errors.

* Fixes merge

* Fixes merge

* Fixes merge

Co-authored-by: Gwin73 <adrianwesterberg@hotmail.com>
Co-authored-by: sobolevn <mail@sobolevn.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed level:starter Good for newcomers pr-available rule request Adding a new rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants