Updates to build with the latest SDK #179
Conversation
| "Type": UASTType(uast.FunctionType{}, Fields{ | ||
| {Name: "Arguments", Op: Var("arguments")}, | ||
| {Name: "Returns", Optional: "ret_opt", Op: Cases("ret_case", | ||
| Is(nil), |
There was a problem hiding this comment.
Python functions always have an implicit return, so should be a single empty Argument with Init = None when there are no type annotations. There is a similar piece of code in JS).
There was a problem hiding this comment.
I implemented this and it worked beautifully... but then I noticed that this annotations only has a value IF the function has a Python 3.4+ type annotation BUT if it doesn't have it, it doesn't mean that the function returns None! Just that it's not annotated. So I think is better to left it with the current state (nil if there is no annotation, the annotation if there is one) because otherwise the UAST user could be confused into thinking that some functions return None while in fact they return other types.
Edit: will add a comment to clarify this.
There was a problem hiding this comment.
I haven't meant that the function returns None as type, only that if there is no return, all Python2 and Python3 functions returns None implicitly. So it's Returns.Argument.Init: None, same way as we do it in JS with undefined.
There was a problem hiding this comment.
Yes, but to know if the function returns None implicitly we should parse the function and find the return statement in this case.
Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech>
Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech> Remove some commented code, add some comments
Co-Authored-By: juanjux <juanjo@juanjoalvarez.net>
Co-Authored-By: juanjux <juanjo@juanjoalvarez.net>
… that the function returns None Signed-off-by: Juanjo Alvarez <juanjo@sourced.tech>
|
Ready for another review round! |
There was a problem hiding this comment.
LGTM
but the arguments/ask for adding more information to commits with such changes in drivers (example of code + link to language speck + XPath to highlight the change) from elsewhere still holds.
Co-Authored-By: juanjux <juanjo@juanjoalvarez.net>
This fix some of the fields detected by the new check and explicitly ignores others, with comments and link to #178 in those cases.