Annotation for target attribute in ast.Assign and similar nodes is too broad

[Domlenart]

Currently, the targets attribute for ast.Assign, and other nodes where assignment happens is likely too broad - it is ast.expr.
I have a suspicion that, this is likely too generic and should be narrowed down, to correct set of types that can truly serve the role of an assignment target.

From a quick look at the official docs for the assignment statement, looks like the correct type should roughly be the union of the following types: ast.Name, ast.Attribute, ast.Subscript.

If I get a confirmation that indeed this could be improved, then I'd love to work on this change. In general in the PR I would take a look at all nodes that cause assignment of value to a target (Assign, AnnAsign, AugAssign, withitem, NamedExpr) and introduce a new type called ast.assign_target.

[AlexWaygood]

I believe you are correct. I'd certainly be interested to see a PR proposing this change, so that we can see what the mypy_primer report looks like.

Note that the restrictions on the walrus operator (NamedExpr nodes) are much more severe than for the other nodes with target attributes:

C:\Users\alexw\coding>python
Python 3.10.7 (tags/v3.10.7:6cc6b13, Sep  5 2022, 14:08:36) [MSC v.1933 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> x = [1, 2, 3]
>>> x[0] := 4
  File "<stdin>", line 1
    x[0] := 4
    ^^^^
SyntaxError: cannot use assignment expressions with subscript
>>> class Foo: ...
...
>>> Foo.bar := 42
  File "<stdin>", line 1
    Foo.bar := 42
    ^^^^^^^
SyntaxError: cannot use assignment expressions with attribute

There may be other differences between the nodes, so we need to be careful.

[Domlenart]

Thanks Alex! I will start working on it then :)

[JelleZijlstra]

The LHS of the walrus operator can only be ast.Name I believe. The LHS of Assign (but not AnnAssign or AugAssign) can also be ast.Tuple or ast.List. Another AST node that can be similarly changed is ast.Delete.

Your original message suggest adding something called ast.assign_target, but that's not something we can do in typeshed: it would have to be a CPython change. And I'm not sure it would work well anyway, since different assignment operators allow different expressions.

I'm a little hesitant to narrow the typeshed types more than the types in the official AST definition (https://docs.python.org/3.11/library/ast.html), but it's probably harmless.

[Domlenart]

You are correct, ast.assign_target idea is discarded.

For ast.Assign:

This now indeed looks like a change that would benefit from a change on the CPython side and therefore in the official docs that you linked. Especially since they are inconsistent with the docs I linked in the issue desciption. I think doing what I suggested directly in typeshed might be a bandaid fix that would occlude an underlying issue.

Especially, since now that you mentioned it, if we include the ast.Tuple as a valid target in ast.Assign, then we are almost back to square one, since this is how this node is currently typed:

class Tuple(expr):
    elts: list[expr]
    ctx: expr_context

Then, at current stage, best we could do for ast.Assign would be:

class Assign(stmt):
    targets: list[Union[ast.Tuple, ast.Name, ast.Attribute, ast.Subscript]]
    value: expr

Therefore, we would probably need a new node called ast.AssignTargetGroup (since it holds variables from one group of targets separated by the "=" symbol. I.e a, b = c, d = 1, 2 currently generates an ast.Assign which under target has a list with two ast.Tuple) which could look like this:

class AssignTargetGroup(expr):
    elts: list[ast.Name, ast.Attribute, ast.Subscript]

Am I making sense and should I file a ticket under CPython with a request to change how ast.Assign works? :)

Remaining assign-like nodes:

As for other nodes (ast.AnnAssign, ast.AugAssign, ast.withitem, ast.NamedExper). Those indeed look fixable on typeshed side, since they all take one argument, which can be typed easily. Therefore I propose we limit the PR I will work on to those nodes for now.

[AlexWaygood]

Especially since they are inconsistent with the docs I linked in the issue desciption.

Hmm, are they? Before the description of the semantics when assigning an object to a single target, the docs state:

Assignment of an object to a target list, optionally enclosed in parentheses or square brackets, is recursively defined as follows.

If the target list is a single target with no trailing comma, optionally in parentheses, the object is assigned to that target.

Else:

If the target list contains one target prefixed with an asterisk, called a “starred” target: The object must be an iterable with at least as many items as there are targets in the target list, minus one. The first items of the iterable are assigned, from left to right, to the targets before the starred target. The final items of the iterable are assigned to the targets after the starred target. A list of the remaining items in the iterable is then assigned to the starred target (the list can be empty).

Else: The object must be an iterable with the same number of items as there are targets in the target list, and the items are assigned, from left to right, to the corresponding targets.

That seems to cover assigning to a tuple or list to me?

Am I making sense and should I file a ticket under CPython with a request to change how ast.Assign works? :)

I'm not sure I fully understand what you're proposing, but CPython would need carefully about backwards-compatibility considerations, if there are any. (Jelle and I are both also CPython core devs BTW, but I for one don't consider myself an AST expert.) Anyway, I agree that there seems to be much less value in changing ast.Assign compared to the other nodes, at least for now :)

[JelleZijlstra]

The problem you see with ast.Tuple is that it doesn't encode that its elements must also be valid assignment targets, right?

It's unlikely we'll change the actual types in the ast module unless you can make a very compelling case. There are many places where the language is more restrictive than the ASDL types (for example, ast.Starred is illegal in many contexts; await is an expression but is legal only in coroutines; yield from expressions are illegal in coroutines; FormattedValue can appear only in JoinedStr). It's not feasible to resolve all these discrepancies as it would make the AST far more complicated and harder to work with.