Antrl4 generated files (target python3) are implicit optional for typing

[DagSonntag]

I have noticed that the Antrl4 generated files (target python3) are using implicit optional on almost all its method declarations. An example is: def variableModifier(self, i:int=None):

While this is allowed in python typing, it has been discouraged since pep484 and is causing issues with for typecheckers like mypy. So I was just wondering if it would be a good idea to add Optional everywhere. i.e. replacing the declarations with: def variableModifier(self, i:Optional[int]=None): with an extra import from typing import Optional in the beginning?

This is a decomposed issue from #4183.

[wuzzo-mike]

I think there's a deeper typing issue with some of the functions that take an optional index like this:

The type-checker (pyright in my case) has problems at the call-site determining whether the return type is a list or an element of the list, so in cases where you call the function without an index and then try to iterate over the result, you get an error.

You end up needing to assert that the type is a list before you iterate the members to appease the type-checker, even though it's clear when looking under the hood that you get a single element when you provide and index and the whole list when you don't

It's the code generated by this little snippet that is troubling me:

ContextTokenListIndexedGetterDecl(t)  ::= <<
def <t.escapedName>(self, i:int=None):
    if i is None:
        return self.getTokens(<parser.name>.<t.name>)
    else:
        return self.getToken(<parser.name>.<t.name>, i)
>>

that's from python3.stg at line 608

(and likewise the very similar ContextRuleListIndexedGetterDecl below that)

This generated code ends up in the generated subclasses of ParserRuleContext

I think, rather than being clever with the optional index, it would be more straightforward to typecheck this if it generated two different functions with different signatures (one that takes an index and another than does not). They would probably have to have different names, since i don't think you can have different overloads of the same member function name in Python. But that is OK... one function can be named singular and the other plural, which mirrors the underlying getTypedRuleContext vs getTypedRuleContexts -- member functions of the base ParserRuleContext which are already split up in to different getters for plural vs singular.

[ericvergnaud]

I agree with the proposal to split but bear in mind that we would need to be backwards compatible for some time, thus keeping the existing method and adding a new one. As a result, this would not immediately improve the situation with mypy.