Function is not valid as a type

[forestbelton]

Hi,

Thank you for this wonderful project. I'm integrating mypy with a codebase that uses protobufs and feel that this will be quite helpful in doing so!

I'm able to generate the *.pyi type stub files along with the other definitions by invoking the following command:

$ python -m grpc_tools.protoc -I=github.com/gogo/protobuf/gogoproto -I=. \
    --python_out=generated/python \
    --grpc_python_out=generated/python \
    --mypy_out=generated/python

The generated *.pyi files look correct, however when I try to run mypy over modules that import these definitions, I receive error messages like the following (names/line #s scrubbed):

generated/python/foo_pb2.pyi:123: error: Function "Foo" is not valid as a type
generated/python/foo_pb2.pyi:123: note: Perhaps you need "Callable[...]" or a callback protocol?

Inside of foo_pb2.py, Foo is defined like follows:

Foo = _reflection.GeneratedProtocolMessageType('Foo', (_message.Message,), {
  'DESCRIPTOR' : _FOO,
  '__module__' : 'path.to.foo'
  # @@protoc_insertion_point(class_scope:Foo)
  })
_sym_db.RegisterMessage(Foo)

Is there something obvious I'm doing wrong? I'm using the following versions for my dependencies:

• mypy 0.761
• mypy-protobuf 1.18
• Python 3.6.5
• python-protobuf 3.11.3

$ python -m grpc_tools.protoc --version
libprotoc 3.8.0

Thanks!

[nipunn1313]

Interesting. Thanks for the report.

Do you know what line of your source file you see this error at? What is going on in your source leading to this? Is it at the time of import?

The .py code you pasted seems to indicate that Foo is a Message here. It would also be helpful to have the equivalent .pyi code pasted to debug.

These errors:

generated/python/foo_pb2.pyi:123: error: Function "Foo" is not valid as a type
generated/python/foo_pb2.pyi:123: note: Perhaps you need "Callable[...]" or a callback protocol?

seem to indicate that something in your code is using Foo as a function rather than as a message?

If you have a repro of the situation that you are willing to share, that'll help (ideally minimal).

Hopefully this is enough information to help your debugging!

[forestbelton]

I believe I've tracked down the issue. It appears there are two different errors that arise as a result of the same underlying problem — when an instance member/method has the same name as an existing type.

Here is one way to trigger an error by using an enum:

enum AThing {
    Unknown = 0;
    A = 1;
    B = 2;
}

message Foo {
    AThing AThing = 1;
}

This results in the following generated .pyi file:

# definition of AThing
# ...

class Foo(google___protobuf___message___Message):
    DESCRIPTOR: google___protobuf___descriptor___Descriptor = ...
    AThing = ... # type: AThing

    def __init__(self,
        *,
        AThing : typing___Optional[AThing] = None,
        ) -> None: ...
    if sys.version_info >= (3,):
        @classmethod
        def FromString(cls, s: builtin___bytes) -> Foo: ...
    else:
        @classmethod
        def FromString(cls, s: typing___Union[builtin___bytes, builtin___buffer, builtin___unicode]) -> Foo: ...
    def MergeFrom(self, other_msg: google___protobuf___message___Message) -> None: ...
    def CopyFrom(self, other_msg: google___protobuf___message___Message) -> None: ...
    if sys.version_info >= (3,):
        def ClearField(self, field_name: typing_extensions___Literal[u"AThing"]) -> None: ...
    else:
        def ClearField(self, field_name: typing_extensions___Literal[u"AThing",b"AThing"]) -> None: ...

And attempts to use Foo result in the following error:

from foo_pb2 import AThing, Foo

x: Foo = Foo(AThing=AThing.A)

test/foo_pb2.pyi:60: error: Variable "foo_pb2.Foo.AThing" is not valid as a type
Found 1 error in 1 file (checked 1 source file)

Per python/mypy#1775, this can be fixed by adding an alias for AThing and referring to it in Foo like so:

# definition of AThing
# ...

_AThing = AThing

class Foo(google___protobuf___message___Message):
    DESCRIPTOR: google___protobuf___descriptor___Descriptor = ...
    AThing = ... # type: _AThing

    def __init__(self,
        *,
        AThing : typing___Optional[_AThing] = None,
        ) -> None: ...
    if sys.version_info >= (3,):
        @classmethod
        def FromString(cls, s: builtin___bytes) -> Foo: ...
    else:
        @classmethod
        def FromString(cls, s: typing___Union[builtin___bytes, builtin___buffer, builtin___unicode]) -> Foo: ...
    def MergeFrom(self, other_msg: google___protobuf___message___Message) -> None: ...
    def CopyFrom(self, other_msg: google___protobuf___message___Message) -> None: ...
    if sys.version_info >= (3,):
        def ClearField(self, field_name: typing_extensions___Literal[u"AThing"]) -> None: ...
    else:
        def ClearField(self, field_name: typing_extensions___Literal[u"AThing",b"AThing"]) -> None: ...

The second way is similar, using a nested Message instead of an enum:

message Bar {
    int64 X = 1;
}

message Foo {
    Bar Bar = 1;
}

which results in the following .pyi:

# definition of Bar
# ...

class Foo(google___protobuf___message___Message):
    DESCRIPTOR: google___protobuf___descriptor___Descriptor = ...

    @property
    def Bar(self) -> Bar: ...

    def __init__(self,
        *,
        Bar : typing___Optional[Bar] = None,
        ) -> None: ...
    if sys.version_info >= (3,):
        @classmethod
        def FromString(cls, s: builtin___bytes) -> Foo: ...
    else:
        @classmethod
        def FromString(cls, s: typing___Union[builtin___bytes, builtin___buffer, builtin___unicode]) -> Foo: ...
    def MergeFrom(self, other_msg: google___protobuf___message___Message) -> None: ...
    def CopyFrom(self, other_msg: google___protobuf___message___Message) -> None: ...
    if sys.version_info >= (3,):
        def HasField(self, field_name: typing_extensions___Literal[u"Bar"]) -> builtin___bool: ...
        def ClearField(self, field_name: typing_extensions___Literal[u"Bar"]) -> None: ...
    else:
        def HasField(self, field_name: typing_extensions___Literal[u"Bar",b"Bar"]) -> builtin___bool: ...
        def ClearField(self, field_name: typing_extensions___Literal[u"Bar",b"Bar"]) -> None: ...

and the following test case reproduces the error message from my original comment:

from foo_pb2 import Bar, Foo

x: Foo = Foo(Bar=Bar(X=123))

This can also be fixed by defining an alias like _Bar = Bar and referring to _Bar in the definition of Foo.

---

While using a field that has the same name as a type is admittedly not a great pattern, unfortunately we are stuck with it since we are already using these types in production.

Is there any workaround we could do here?

[nipunn1313]

yeah - this seems like a totally reasonable concern - that may be avoidable by autogenerating an alias. We already generate some aliases like typing_extensions__Literal for mangling - so this seems like a reasonable strategy. Poking around with it to see what we can get working.

Great bugreport and analysis

[nipunn1313]

@forestbelton - mind taking a look at #112 to see if it handles the case you've brought up here (There are tests which should help illustrate).

[forestbelton]

@nipunn1313, #112 looks like it will cover these above cases. Thanks for the quick turnaround!