classification
Title: change representation of match as / capture as `Name(..., ctx=Store())`
Type: Stage:
Components: Versions: Python 3.10
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Anthony Sottile, brandtbucher, freundTech, gvanrossum, ncoghlan
Priority: high Keywords:

Created on 2021-04-30 22:49 by Anthony Sottile, last changed 2021-05-01 17:43 by gvanrossum.

Messages (10)
msg392531 - (view) Author: Anthony Sottile (Anthony Sottile) * Date: 2021-04-30 22:49
I'm looking at adding support to `match` for pyflakes, and the first impression I have is that `MatchAs` is unnecessarily different from `Name` with `ctx=Store()`

if it were represented as the latter pyflakes would not require special handling of `match`, it would work the same as the current code

I suspect other static analysis tools would benefit from a change as well
msg392547 - (view) Author: Brandt Bucher (brandtbucher) * (Python committer) Date: 2021-05-01 00:04
Do you mind providing a bit more context? I'm sort of confused what exactly is being proposed here (as far as I can tell, it's not really possible to represent "<pattern> as <name>" with a single Name node).

Also, I'm not sure if you're aware, but we just completed a huge overhaul of the ASTs generated for match statements. This work was specifically done for clients of the AST like mypy. See the discussion at: https://github.com/python/mypy/pull/10191 and in issue 43892.

The new nodes are documented here: https://docs.python.org/3.10/library/ast.html?highlight=matchas#pattern-matching
msg392548 - (view) Author: Brandt Bucher (brandtbucher) * (Python committer) Date: 2021-05-01 00:13
FWIW, I'm never used pyflakes, but I'm not really sure how it would be able to provide useful linting by just treating patterns as expressions (which is what I assume is desired here).

I assume that these are the three lines you're trying to get rid of?

https://github.com/asottile/pyflakes/blob/silly_match_statement/pyflakes/checker.py#L2390-L2392
msg392552 - (view) Author: Anthony Sottile (Anthony Sottile) * Date: 2021-05-01 00:37
I'm suggesting instead of:

MatchAs(pattern=None, name='foo')

to have

MatchAs(pattern=None, name=Name('foo', ctx=Store()))
msg392562 - (view) Author: Anthony Sottile (Anthony Sottile) * Date: 2021-05-01 02:08
and actually, now that I look close it would be useful for `MatchStar` and `MatchMapping` to also use a `Name(..., ctx=Store())` for their respective parameters as well
msg392565 - (view) Author: Brandt Bucher (brandtbucher) * (Python committer) Date: 2021-05-01 02:25
+ Nick and Guido

The only benefit I see on our side is that it leaves the door open for complex assignment targets in the future, like (a, b), a[b], etc.

(If I recall correctly, this is also why NamedExpr uses an expr target rather than just an identifier.)

I guess I'm neutral on this. The usability argument seems weak, but I can see the separate case for forward-compatibility with possible syntax extensions in the future. Thoughts?

Marking as high priority since we need to make a decision on this before the beta.
msg392566 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-05-01 03:02
I'm -0.5 on reusing Name(ctx=Store). The reason we're using that in assignments is that in the past (before the PEG parser) the parser literally used the same grammar for the LHS and RHS of assignments, and a second pass was used to rule out invalid targets (like "1 = e" or "f() = a") and mark the Name nodes representing targets as stores.

We don't reuse Name nodes for other binding targets like parameters or imports.

PS. To reach Mark it's best to email him directly, he's not very responsive to GitHub notifications (there are too many).

PPSS. I'm on vacation until May 8 and am trying not to check my email much. But I already failed on the first day. :-)
msg392568 - (view) Author: Anthony Sottile (Anthony Sottile) * Date: 2021-05-01 03:25
at least for static analysis of other python constructs it's very convenient to know from a `Name` node alone whether it's being used in a read or write context -- without this information an ast traversal needs to maintain more information about whether it's a read or write context

for pyflakes this is especially important as it needs to know what names are defined in scope (and referenced in scope) to produce diagnostic messages

for other tools like `dead` / `vulture` it's useful to identify introduced and referenced names similarly

the `as` in `with` does and the target for assignment expressions so I would expect the similar constructs in `match` to do so as well

`Name` nodes are also useful for better diagnostic messages as they contain positioning information, which isn't easily extracted from `MatchAs`, etc. -- if I recall correctly, the `asname` for imports was recently extended to add this information for the same purpose
msg392571 - (view) Author: Adrian Freund (freundTech) * Date: 2021-05-01 05:40
I already brought this up on the main pattern matching issue some time ago (https://bugs.python.org/issue42128#msg388554), where the consensus was that not using a Name is consistent with other parts of the ast, such as `import ... as identifier`, `except ... as identifier` and others.

For mypy having a Name node there would slightly simplify the code (I'm currently inserting a dummy NameExpr at AST-Conversion.

+0 from me.
msg392612 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2021-05-01 17:43
Honestly if someone manages to get a PR in I won’t be a spoilsport. So make
me +0.--
--Guido (mobile)
History
Date User Action Args
2021-05-01 17:43:55gvanrossumsetmessages: + msg392612
2021-05-01 05:40:51freundTechsetmessages: + msg392571
2021-05-01 03:25:43Anthony Sottilesetmessages: + msg392568
2021-05-01 03:02:30gvanrossumsetmessages: + msg392566
2021-05-01 02:28:03brandtbuchersetnosy: + freundTech
2021-05-01 02:25:56brandtbuchersetpriority: normal -> high
nosy: + gvanrossum, ncoghlan
messages: + msg392565

2021-05-01 02:08:32Anthony Sottilesetmessages: + msg392562
2021-05-01 00:37:31Anthony Sottilesetmessages: + msg392552
2021-05-01 00:13:38brandtbuchersetmessages: + msg392548
2021-05-01 00:04:47brandtbuchersetnosy: + brandtbucher
messages: + msg392547
2021-04-30 22:49:51Anthony Sottilecreate