Improve nullability support for generated parsers #142
Conversation
|
@ftomassetti let me know what you think about this. |
This can't be done at all, as it's then impossible to check if a |
| } | ||
| is ErrorNode -> return t.toString() | ||
| is TerminalNode -> return t.symbol.text!! |
There was a problem hiding this comment.
Should we write something along the lines of:
t.symbol.text ?: throw IllegalStateException("Token symbol text should not be null")
Wouldn't it be null for EOF?
There was a problem hiding this comment.
Yes that's better, I'll commit the change.
|
|
||
| if (symbol != null) { | ||
| return symbol.text!! | ||
| when (t) { |
There was a problem hiding this comment.
Should we factor out the return?
There was a problem hiding this comment.
We cannot as that's a non-local return (when inside of an if). Will be supported starting from Kotlin 2.0.
There was a problem hiding this comment.
Sorry, wrong answer. We cannot, as it requires an else branch to return something. We don't want that, we want it to continue over the rest of the method, as it was doing with if-elseif.
|
It looks good, but it is difficult to assess if the code work. I assume tests guarantee that to some extent. I just added minor comments |
|
@ftomassetti haven't changed much so everything still work as before. The "issue" here is the change in generated method names. This is a breaking change basically. |
|
Thank you for your answers. Merging |
Improves type nullability support in generated parsers, and solves a part of #84.
In this case we are checking for the
optionalproperty - injected by the ANTLR code generator - on theContextTokenGetterDeclandContextRuleGetterDecltemplate parts.Note that the generated methods now start with
get*instead offind*.The missing part is for
TokenDecl. We would have to use thelateinitmodifier, but I'm still unsure if it's appropriate.