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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix magic method yields (__next__, __anext__, __aiter__) #2400

Merged
merged 9 commits into from Sep 25, 2022

Conversation

Sxderp
Copy link
Contributor

@Sxderp Sxderp commented Apr 21, 2022

I have made things!

Checklist

  • I have double checked that there are no unrelated changes in this pull request (old patches, accidental config files, etc)
  • I have created at least one test case for the changes I have made
  • I have updated the documentation for the changes I have made
  • I have added my changes to the CHANGELOG.md

Related issues

Closes #2399

馃檹 Please, if you or your company is finding wemake-python-styleguide valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/wemake-python-styleguide. As a thank you, your profile/company logo will be added to our main README which receives hundreds of unique visitors per day.

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

We totally need:

  • A CHANGELOG entry for this
  • A test case for this, since it is rather complex

oop.AsyncMagicMethodViolation(node, text=node.name),
)
can_async = node.name in constants.ASYNC_IF_YIELDS_MAGIC_METHODS
if not (can_async and walk.is_contained(node, (ast.Yield))):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if not (can_async and walk.is_contained(node, (ast.Yield))):
if not (can_async and walk.is_contained(node, ast.Yield)):

@Sxderp
Copy link
Contributor Author

Sxderp commented Apr 25, 2022

I'll work on figuring out how to do the tests and hopefully have something by the end of the week.

@Sxderp
Copy link
Contributor Author

Sxderp commented Apr 26, 2022

  1. I'm not sure how to run the tests.. So letting GitHub do that, hopefully they work.
  2. __aiter__ could have been added to the some of the existing test files, but that's only because those test happen to not be async / sync or yield / not yield. I figured it was better to be explicit about these relationships and created new tests for everything.
  3. Changelog also needs updating. But I'm not sure about where to put it. 16.1 is already released so obviously not there. Do I just create a 16.2 section?

Sxderp and others added 8 commits September 25, 2022 12:30
__anext__:
  This is a runtime constraint. __anext__ cannot be a generator. This
will cause the runtime to throw a TypeError when used in an async for
loop.

__next__:
  The usefulness of having __next__ be a generator is practically nil.
Doing so will cause an infinite loop with a new generator object passed to
each iteration.
__aiter__ can only contain yield if it is an async function (async
generator). Otherwise it must be a sync function that does not contain
yield and returns an object that implements __anext__.
@codecov
Copy link

codecov bot commented Sep 25, 2022

Codecov Report

Merging #2400 (fc2e861) into master (e8c5485) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #2400   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          119       119           
  Lines         6383      6390    +7     
  Branches      1446      1449    +3     
=========================================
+ Hits          6383      6390    +7     
Impacted Files Coverage 螖
wemake_python_styleguide/violations/oop.py 100.00% <100.00%> (酶)
wemake_python_styleguide/visitors/ast/classes.py 100.00% <100.00%> (酶)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@sobolevn sobolevn merged commit 5bfdbdd into wemake-services:master Sep 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WPS610 (async magic) false positive on __aiter__
2 participants