astor
astor copied to clipboard
Validate input AST for user facing functions
I can take a stab at this when I get some time, but posting this now in case someone else is interested.
Sometimes I pass in an incorrect AST to an astor method, e.g. astor.to_source(ast)
(e.g. ast contains a raw int
instead of wrapping it an ast.Num
). This leads to tricky errors internally in astor that can be hard to understand for the user (this particular error resulted in a failed assertion in precedence_setter.set_precedence
).
I propose that astor validates the input AST for any user facing API functions. This would allow any internal logic to assume that the AST is valid and for users to be immediately notified that the input AST is wrong, rather than having to parse errors or exceptions from the internal logic.
Thanks for the report. I think that would be a good feature, but the tricky part is that we need to keep in mind different Python versions when doing validation. For example, wrapping int with ast.Num might be the correct solution for Python 3.x and 3.x+1. However, Python 3.x+2 might need ast.Number or ast.Guido in the future.
In general, I'm not a big fan of slowing down the general case for exceptions.
I would support your proposal turned inside out; e.g. we wrap the top functions with try/except Exception
, and if an exception occurs, we transfer control to a function that tries to do a better job of explaining to the caller what went wrong (e.g. re-raise a different exception).
@pmaupin Interesting point of view! Makes sense to me and does seem like a better approach.
Also, I don't have any time right now, but if someone is looking at this, we should probably merge my current PRs (or not) first...