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

Remove redundant parentheses around awaited coroutines/tasks #2991

Merged
merged 8 commits into from Apr 9, 2022

Conversation

jpy-git
Copy link
Contributor

@jpy-git jpy-git commented Apr 4, 2022

Description

Closes #275. Remove redundant parentheses around awaited coroutines/tasks.

This is a tricky one as await is technically an expression and therefore in certain situations requires brackets for operator precedence.
However, the vast majority of await usage is just await some_coroutine(...) and similar in format to return statements. Therefore this PR removes redundant parens around these await expressions.

Let's see what diff shades shades has to say...

Checklist - did you ...

  • Add a CHANGELOG entry if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation?

@github-actions
Copy link

github-actions bot commented Apr 4, 2022

diff-shades results comparing this PR (ee470d6) to main (98fccce). The full diff is available in the logs under the "Generate HTML diff report" step.

╭─────────────────────── Summary ────────────────────────╮
│ 1 projects & 1 files changed / 2 changes [+1/-1]       │
│                                                        │
│ ... out of 2 178 976 lines, 10 492 files & 23 projects │
╰────────────────────────────────────────────────────────╯

Differences found.

What is this? | Workflow run | diff-shades documentation

@ichard26
Copy link
Collaborator

ichard26 commented Apr 4, 2022

I should definitely color the "(allowed)" text next to the sqlalchemy:test/orm/test_relationship_criteria.py failure (since that one is known and unrelated to this PR) as it's almost impossible to notice. The first two failures are new though.

Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Thank you!

tests/data/remove_await_parens.py Show resolved Hide resolved
CHANGES.md Outdated Show resolved Hide resolved
src/black/linegen.py Outdated Show resolved Hide resolved
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@jpy-git
Copy link
Contributor Author

jpy-git commented Apr 4, 2022

The first two failures are new though.

1. test/ext/asyncio/test_engine_py3k.py: InvalidInput - Cannot parse: 985:31:                 result = await     
  await conn.stream_scalars(select(users)).all()                                                                   
  2. test/ext/asyncio/test_session_py3k.py: InvalidInput - Cannot parse: 1[13](https://github.com/psf/black/runs/5817065194?check_suite_focus=true#step:17:13):27:             result = await await  
  async_session.stream_scalars(stmt).all()

I see what's going on here, in the Python grammar await is an optional prefix for power so it is possible to call await(await something(...)). I'll just need to not remove the brackets in this case and add a unit test for this! 😄

@jpy-git
Copy link
Contributor Author

jpy-git commented Apr 4, 2022

I've added the extra unit tests and handled the nested awaits, note that I also recurse down the nested awaits to remove any redundant parens within those 👍

Also not sure why that lint check is failing, it's fine locally for me 🤷

@JelleZijlstra
Copy link
Collaborator

JelleZijlstra commented Apr 4, 2022

Also not sure why that lint check is failing, it's fine locally for me 🤷

Looks like more click 8.1 fallout, I can look into it later. We need to either update our type ignores or tell the mypy check to use a different version of click.

I've added the extra unit tests and handled the nested awaits, note that I also recurse down the nested awaits to remove any redundant parens within those 👍

Thanks!

@JelleZijlstra
Copy link
Collaborator

JelleZijlstra commented Apr 5, 2022

@ichard26 actually fixed the Click issue, thanks! I merged main in and hopefully CI will be green now.

@jpy-git
Copy link
Contributor Author

jpy-git commented Apr 5, 2022

@ichard26 actually fixed the Click issue, thanks!

Thanks @ichard26 😄

@ichard26 ichard26 self-requested a review Apr 6, 2022
Copy link
Collaborator

@ichard26 ichard26 left a comment

Beautiful work! I think I'm also getting better at reading code involving blib2to3's CST.

Thank you once again!

@ichard26 ichard26 merged commit 75f99bd into psf:main Apr 9, 2022
40 checks passed
@jpy-git jpy-git deleted the async_brackets branch Apr 9, 2022
felix-hilden added a commit to felix-hilden/black that referenced this pull request Jun 24, 2022
ichard26 added a commit that referenced this pull request Jun 27, 2022
Covers GH-2926, GH-2990, GH-2991, and GH-3035.

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Richard Si <63936253+ichard26@users.noreply.github.com>
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.

3 participants