fix: refactor sem analysis of the await-not-async on generators and list comprehension

[josetapadas]

Fixes #18124

This PR proposes the introduction of two extra checks in order to error or any await or async within list comprehensions but not when inside a GeneratorExpression.

To achieve this we propose:

• when visiting a ListComprehension we should also check for the existing of an await expression and error outside a courotine
• error on the same scenario when visiting a DictionaryComprehension
• when visiting a AwaitExpression we add an extra check to verify if it is inside a GeneratorExpression to not fail

[sterliakov]

More explicit tests would be really helpful here. Consider at least checking all of the following in different scopes (module level, non-async function body and async function body at least; the former two should produce the same errors and async function should allow all of those):

(await x for x in xs)  # OK
[await x for x in xs]  # E
{await x for x in xs}  # E
{0: await x for x in xs}  # E
{await x: 0 for x in xs}  # E

(x async for x in xs)  # OK
[x async for x in xs]  # E
{x async for x in xs}  # E
{x: 0 async for x in xs}  # E

(x for x in await xs)  # E
[x for x in await xs]  # E
{x for x in await xs}  # E
{x: 0 for x in await xs}  # E

There's already testAsyncForOutsideCoroutine for the 2nd block, the 3rd one is probably not interesting (follows regular rules), but the 1st one isn't exercised yet.

[josetapadas]

Thank you for the review @sterliakov 🙏 I have now pushed the suggested changes

[sterliakov]

I don't really like the use of codes.SYNTAX vs codes.AWAIT_NOT_ASYNC here. Prior to this PR AWAIT_NOT_ASYNC was only used once.

As far as I understand, the purpose of using a special code was to support "specific" environments: e.g. IPython allows top-level awaits, so it should be easy to disable such a check when checking IPython snippets.

I see two possible resolutions here:

1. Declare that we only type-check true python code. Then codes.AWAIT_NOT_ASYNC can be safely removed altogether and replaced with codes.SYNTAX
2. Declare that we want to support such environments. Then all these checks become more complicated: when at module scope (note it isn't same as not self.is_func_scope()), produce codes.AWAIT_NOT_ASYNC, otherwise produce codes.SYNTAX. This can also be extracted into a helper like def require_async_scope(self, message). This logic is close to what visit_await_expr was doing before.

I somewhat prefer 2 as that isn't that much work to do, but can live with 1 either.

Disclaimer: please note I'm not a mypy maintainer and just try to help sometimes. You'd better ask for a maintainer's opinion here: @brianschubert do you have any preference?

[sterliakov]

OTOH 2 can also be misleading and deserves an explicit mention in the docs.

[josetapadas]

Thank you for the review and explanation, I understand your point so thank you for clarifying it. Will then wait for any guidance on how to proceed on any of these two options.

[jakkdl]

I don't think this is an important enough problem to change our stance on allowing top-level awaits, so you're probably safer to go ahead with option 2 for now. A bare await outside async has always been a syntax error in python, so whatever reason the original PR had for not making that use codes.SYNTAX should apply here as well.

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅