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

add async120, await-in-except #265

Merged
merged 8 commits into from
Jun 12, 2024
Merged

Conversation

jakkdl
Copy link
Member

@jakkdl jakkdl commented May 29, 2024

First part of #262

It's maybe not great to have the same error messages for both 102 and 120, and docs could maybe be improved as well.

docs/rules.rst Outdated Show resolved Hide resolved
flake8_async/visitors/visitor102.py Outdated Show resolved Hide resolved
flake8_async/visitors/visitor102.py Outdated Show resolved Hide resolved
@jakkdl
Copy link
Member Author

jakkdl commented Jun 3, 2024

I noticed the "exclude x.aclose()" just now, I don't remember the reasoning behind that, so idk if 120 should respect it as well.

@jakkdl jakkdl requested a review from Zac-HD June 3, 2024 13:40
@jakkdl
Copy link
Member Author

jakkdl commented Jun 3, 2024

I just reread python-trio/trio#455 (comment) and noticed

I guess we should add any other except-that-raises here.

and realized that this check does not filter on whether-the-except-would-otherwise-raise, so this will currently give an async120 error:

try:
  ...
except ValueError:
  # we don't give a shit about ValueError
  await doSomeOtherThing()

can fix it by saving the potential async120 errors in a container and only report them upon reaching a raise

…arm on nested function definitions for 102 & 120
@Zac-HD
Copy link
Member

Zac-HD commented Jun 7, 2024

(I haven't forgotten this, I just need to think it through carefully and haven't had a block of time free to do that lately)

Copy link
Member

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for your patience! Merge at your discretion.

try:
...
except:
x = lambda: await foo()
Copy link
Member

Choose a reason for hiding this comment

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

SyntaxError: 'await' outside async function - there's no such thing as an async lambda.

We might want to check that our test files are syntactically valid to avoid any subtle issues in future?

Copy link
Member Author

Choose a reason for hiding this comment

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

After testing, it appears that ast.parse does not pick up on it - and neither mypy nor ruff has any checks for it. Calling compile on it does surface the error, as it does additional scoping checks that ast.parse does not do.

Copy link
Member Author

Choose a reason for hiding this comment

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

(more obvious errors have been caught in the past by mypy/ruff). Opened #268

flake8_async/__init__.py Outdated Show resolved Hide resolved
jakkdl and others added 2 commits June 12, 2024 11:04
Co-authored-by: Zac Hatfield-Dodds <zac.hatfield.dodds@gmail.com>
@jakkdl jakkdl merged commit e198655 into python-trio:main Jun 12, 2024
10 checks passed
@jakkdl jakkdl deleted the await_in_except branch June 12, 2024 10:32
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.

2 participants