search

berkerpeksag / astor

Python AST read/write
https://pypi.org/project/astor/
BSD 3-Clause "New" or "Revised" License
810 stars 102 forks source link

Fix to_source() when optional attributes are missing #189

Closed tsundvoll closed 4 years ago

tsundvoll commented 4 years ago

The grammar in the ast library is defined with optional attributes for some of the nodes [1]. However, astor.to_source(node) currently requires that all (or most of, anyways) the attributes are set. Therefore, for example the code

astor.to_source(ast.arg(arg='a'))

, missing the 'annotation' attribute, will fail with an AttributeError.

This problem occur for us when we create our own AST and only specify the mandatory attributes.

This PR starts fixing this problem by checking for the existence of the optional attribute before it is used to generate source code. Currently, the fix is only performed for the nodes required for our use, but if desired, I can fix this for the remaining optional attributes as well.

This PR possibly resolves #185.

isidentical commented 4 years ago

optional attributes for some of the nodes

What 'optional' means in that context, is 'nullable' (can be None).

Currently, the fix is only performed for the nodes required for our use

IMHO fixing this partially for some nodes in 'astor', is not the right way, especially if you are not fixing all of the cases which would then result with a big inconsistency. astor.to_source is not the only API that expects all attributes to set, it is same for the ast.unparse and all other utilities actually.

I believe the right action is either fixing these in your own code (just passing None's when the attribute is missing) or proposing to build some kind of annotator inside of astor to add these automatically (a nodevisitor of sort) before processing the tree. The latter would bring the responsibility of adding these None's to all places in all versions of ASTs (might be autogenerated from the ASDL files via pyasdl) but personally I wouldn't go for that route.

Hopefully, in 3.10 this problem will be fixed in CPython. There is an awaiting PR which adds node initalizators like Dict() would give you Dict([],[]) and Constant('test') would give you Constant('test', None), basically defaults for some attributes (python/cpython#21417)

tsundvoll commented 4 years ago

Ah, I see I misunderstood 'optional'. Thanks for your suggestions, this solves my problem.