AttributeError in ast.unparse

[b50ffb2f-f676-4837-a388-b749c27aee74]

BPO	44896
Nosy	@brettcannon, @terryjreedy, @benjaminp, @ambv, @markshannon, @1st1, @pablogsal, @isidentical, @xiaket
PRs	python/cpython#27770

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields: ```python assignee = None closed_at = created_at = labels = ['type-bug', 'library', '3.9', '3.10', '3.11'] title = 'AttributeError in ast.unparse' updated_at = user = 'https://github.com/xiaket' ``` bugs.python.org fields: ```python activity = actor = 'terry.reedy' assignee = 'none' closed = True closed_date = closer = 'BTaskaya' components = ['Library (Lib)'] creation = creator = 'xiaket' dependencies = [] files = [] hgrepos = [] issue_num = 44896 keywords = ['patch'] message_count = 4.0 messages = ['399427', '399593', '402868', '408824'] nosy_count = 9.0 nosy_names = ['brett.cannon', 'terry.reedy', 'benjamin.peterson', 'lukasz.langa', 'Mark.Shannon', 'yselivanov', 'pablogsal', 'BTaskaya', 'xiaket'] pr_nums = ['27770'] priority = 'normal' resolution = 'rejected' stage = 'resolved' status = 'closed' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue44896' versions = ['Python 3.9', 'Python 3.10', 'Python 3.11'] ```

[b50ffb2f-f676-4837-a388-b749c27aee74]

I was trying to construct an ast object dynamically and I think I can identify some potential issue.

With the following snippet:

#!/usr/bin/env python3
import ast
import sys

print(sys.version)

good = ast.Assign(
    targets=[ast.Name(id="hello", ctx=ast.Store())],
    value=ast.Constant(value="world"),
    lineno=1
)
print(ast.unparse(good))

bad = ast.Assign(
    targets=[ast.Name(id="hello", ctx=ast.Store())],
    value=ast.Constant(value="world"),
)
print(ast.unparse(bad))

On my box the output looks like:

3.9.6 (default, Jun 29 2021, 05:25:02)
[Clang 12.0.5 (clang-1205.0.22.9)]
hello = 'world'
Traceback (most recent call last):
  File "/Users/xiaket/py.py", line 19, in <module>
    print(ast.unparse(bad))
  File "/usr/local/Cellar/python@3.9/3.9.6/Frameworks/Python.framework/Versions/3.9/lib/python3.9/ast.py", line 1572, in unparse
    return unparser.visit(ast_obj)
  File "/usr/local/Cellar/python@3.9/3.9.6/Frameworks/Python.framework/Versions/3.9/lib/python3.9/ast.py", line 801, in visit
    self.traverse(node)
  File "/usr/local/Cellar/python@3.9/3.9.6/Frameworks/Python.framework/Versions/3.9/lib/python3.9/ast.py", line 795, in traverse
    super().visit(node)
  File "/usr/local/Cellar/python@3.9/3.9.6/Frameworks/Python.framework/Versions/3.9/lib/python3.9/ast.py", line 407, in visit
    return visitor(node)
  File "/usr/local/Cellar/python@3.9/3.9.6/Frameworks/Python.framework/Versions/3.9/lib/python3.9/ast.py", line 858, in visit_Assign
    if type_comment := self.get_type_comment(node):
  File "/usr/local/Cellar/python@3.9/3.9.6/Frameworks/Python.framework/Versions/3.9/lib/python3.9/ast.py", line 786, in get_type_comment
    comment = self._type_ignores.get(node.lineno) or node.type_comment
AttributeError: 'Assign' object has no attribute 'lineno'

As I can understand, when we need to construct the Assign object, we'll need to provide two keyword arguments, targets and value. We don't need to provide the lineno as it should be an attribute of the statement node. Also, if we don't run unparse against the object, apparently it works fine.

I think in the get_type_comment method, we are making the assumption that the lineno is set automatically, this is true when we are parsing python source code as string. But when we are creating the object from scratch, we don't have that lineno attribute and it will fail.

[isidentical]

I don't think this is really an issue considering some other functionalities that consumes AST code (e.g compile) will expect it to be annotated with location metadata. This is why the ast module already comes bundled with a utility function called fix_missing_locations which in your example would work;

print(ast.unparse(bad)) print(ast.unparse(ast.fix_missing_locations(bad)))

Though I recall seeing this error once before, and since there also some counter examples (e.g ast.increment_lineno) that assume an AST node might lack of these attributes even though the nodes declare them makes it worth to fix.

[ambv]

Sorry, I think we shouldn't be changing ast.py this way. To reiterate my review comment from the PR:

I think we should just let this go and instead advertise that if you put new synthetic nodes in a tree, you have to either:

• provide lineno and col_offset yourself; or
• use fix_missing_locations() to fill out the information for you.

The problem with the logic that I commented on twice now (on the PR) is one reason for my opinion. Another is bigger though:

Suppose user code is replacing nodes or just inserting new ones. This operation not only creates nodes without location attributes. It also in all likelihood moves locations of type comments and those aren't updated (for example, _type_ignores[123] should now really be _type_ignores[125] to still point at the same tokens). So user code shouldn't be naive when it comes to missing locations, and we definitely shouldn't paper over missing locations.

[terryjreedy]

bpo-46073 may be a duplicate of this and perhaps should have the same resolution.