This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: ast.unparse produces: 'FunctionDef' object has no attribute 'lineno' for valid module
Type: crash Stage: resolved
Components: Library (Lib) Versions: Python 3.11, Python 3.10, Python 3.9
process
Status: closed Resolution: duplicate
Dependencies: Superseder: AttributeError in ast.unparse
View: 44896
Assigned To: Nosy List: BTaskaya, TheRobotCarlson, sobolevn
Priority: normal Keywords:

Created on 2021-12-14 16:38 by TheRobotCarlson, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
test.py TheRobotCarlson, 2021-12-14 16:38 test file showing the ast failure
Messages (7)
msg408546 - (view) Author: Brian Carlson (TheRobotCarlson) * Date: 2021-12-14 16:38
Test file linked. When unparsing the output from ast.parse on a simple class, unparse throws an error: 'FunctionDef' object has no attribute 'lineno' for a valid class and valid AST. It fails when programmatically building the module AST as well.

It seems to be from this function: https://github.com/python/cpython/blob/1cbb88736c32ac30fd530371adf53fe7554be0a5/Lib/ast.py#L790
msg408561 - (view) Author: Nikita Sobolev (sobolevn) * (Python triager) Date: 2021-12-14 20:57
Looks like this little patch can solve this problem:

```
   def unparse(ast_obj):
       unparser = _Unparser()
---    return unparser.visit(ast_obj)
+++    return unparser.visit(fix_missing_locations(ast_obj))
```

But, I am not sure we should call this helper here.

Another approach is to refactor `get_type_comment` function into something like this:

```
    def get_type_comment(self, node):
        comment = None
        if hasattr(node, 'lineno'):
            comment = self._type_ignores.get(node.lineno)
        comment = comment or node.type_comment
        if comment is not None:
            return f" # type: {comment}"
```

I feel like this is more suitable for this case. Even `ast.pyi` has this line:

```
    # TODO: Not all nodes have all of the following attributes
    lineno: int
    col_offset: int
```

So, I am going to propose a PR with the second option. Please, send your feedback!
msg408563 - (view) Author: Nikita Sobolev (sobolevn) * (Python triager) Date: 2021-12-14 21:05
Moreover, there's always an option to pass `lineno` and `column` explicitly:

```
from ast import *

# if we build the module manually and try it directly
value = Module(
    body=[
        FunctionDef(
            name="items_needed",
            args=arguments(
                posonlyargs=[],
                args=[],
                kwonlyargs=[],
                kw_defaults=[],
                defaults=[],
            ),
            body=[Return(value=Constant(value="test"))],
            decorator_list=[],
            lineno=1,
            column=1,
        )
    ],
    type_ignores=[],
)

unparse(value)  # ok
```
msg408565 - (view) Author: Brian Carlson (TheRobotCarlson) * Date: 2021-12-14 21:14
The second solution seems more optimal, in my opinion. I monkey patched the function like this in my own code:
```
def get_type_comment(self, node):
    comment = self._type_ignores.get(node.lineno) if hasattr(node, "lineno") else node.type_comment
    if comment is not None:
        return f" # type: {comment}"
```

Thanks!
msg408566 - (view) Author: Brian Carlson (TheRobotCarlson) * Date: 2021-12-14 21:16
I don't think passing `lineno` and `column` is preferred. It makes code generation harder because `lineno` and `column` are hard to know ahead of when code is being unparsed.
msg408567 - (view) Author: Nikita Sobolev (sobolevn) * (Python triager) Date: 2021-12-14 21:32
Or, maybe this is a correct thing to do. Consider this case. Let's say we now check that `node` has `lineno` and exclude ones that don't.

Then, we write this test (in `Lib/test/test_unparse`):

```
    def test_unparse_nodes_without_lineno(self):
        # https://bugs.python.org/issue46073
        def create_module(ignores):
            return ast.Module(
                body=[
                    ast.FunctionDef(
                        name="items_needed",
                        args=ast.arguments(
                            posonlyargs=[], args=[],
                            kwonlyargs=[], kw_defaults=[], defaults=[],
                        ),
                        body=[ast.Pass()], decorator_list=[],
                    ),
                ],
                type_ignores=ignores,
            )

        ast1 = create_module([])
        self.assertASTEqual(ast1, ast.parse(ast.unparse(ast1)))

        ast2 = create_module([ast.TypeIgnore(lineno=1, tag='')])
        # This fill fail without `ast2 = ast.fix_missing_locations(ast2)`
        self.assertASTEqual(ast2, ast.parse(ast.unparse(ast2), type_comments=True))
```

In this case the second `assert` will fail, because the resulting codes would be different:

```
# "original"
def items_needed():  # type: ignore
    pass

# unparsed
def items_needed():
    pass
```

So, basically users will get different code. Which is not a good thing.

So, maybe raising an error that some `node` does not have `lineno` is good after all?
msg408574 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python committer) Date: 2021-12-14 22:58
From the first glance, this seems like a duplicate of bpo-44896.
History
Date User Action Args
2022-04-11 14:59:53adminsetgithub: 90231
2022-03-19 15:01:30iritkatrielsetstatus: open -> closed
superseder: AttributeError in ast.unparse
resolution: duplicate
stage: resolved
2021-12-14 22:58:20BTaskayasetnosy: + BTaskaya
messages: + msg408574
2021-12-14 21:32:52sobolevnsetmessages: + msg408567
2021-12-14 21:16:19TheRobotCarlsonsetmessages: + msg408566
2021-12-14 21:14:03TheRobotCarlsonsetmessages: + msg408565
2021-12-14 21:06:00sobolevnsetmessages: + msg408563
2021-12-14 20:57:42sobolevnsetnosy: + sobolevn
messages: + msg408561
2021-12-14 16:38:50TheRobotCarlsoncreate