Skip to content

Commit

Permalink
Remove enum type ignore by pulling out V into a separate parent class
Browse files Browse the repository at this point in the history
Fixes #239
  • Loading branch information
nipunn1313 committed Jul 22, 2021
1 parent 26b7df6 commit eb93c6e
Show file tree
Hide file tree
Showing 11 changed files with 117 additions and 101 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

- Propagate comments from .proto files to .pyi files
- Add protobuf type stubs to the setup requirements
- Fix [#239](https://github.com/dropbox/mypy-protobuf/issues/239) Remove type: ignore used in enum by pulling `V` into a separate class.
- Use pytest 6.2.4 for internal test suites on python3

## 2.7
Expand Down
36 changes: 24 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,27 +89,39 @@ This allows mypy to catch bugs where the wrong enum value is being used.
mypy-protobuf autogenerates an instance of the EnumTypeWrapper as follows.

```
class MyEnum(metaclass=_MyEnum):
class MyEnum(_MyEnum, metaclass=_MyEnumEnumTypeWrapper):
pass
class _MyEnum:
V = typing.NewType('V', builtins.int)
FOO = MyEnum.V(1)
BAR = MyEnum.V(2)
class _MyEnum(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[MyEnum.V], builtins.type):
class _MyEnumEnumTypeWrapper(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[_MyEnum.V], builtins.type):
DESCRIPTOR: google.protobuf.descriptor.EnumDescriptor = ...
FOO = MyEnum.V(1)
BAR = MyEnum.V(2)
FOO = MyEnum.V(0)
BAR = MyEnum.V(1)
FOO = MyEnum.V(0)
BAR = MyEnum.V(1)
```

Calling code may be typed as follows. Note that the type of `x` must be quoted
until [upstream protobuf](https://github.com/protocolbuffers/protobuf/pull/8182) supports `V`
`_MyEnumEnumTypeWrapper` extends the EnumTypeWrapper to take/return MyEnum.V rather than int
`MyEnum` is an instance of the `EnumTypeWrapper`.
- Use `_MyEnum` and of metaclass is an implementation detail to make MyEnum.V a valid type w/o a circular dependency

Calling code may be typed as follows.

In python >= 3.7
```
def f(x: 'MyEnum.V'):
# Need [PEP 563](https://www.python.org/dev/peps/pep-0563/) to postpone evaluation of annotations
from __future__ import annotations # Not needed with python>=3.10
def f(x: MyEnum.V):
print(x)
f(MyEnum.Value("FOO"))
```

Note that for usages of cast, the type of `x` must be quoted
until [upstream protobuf](https://github.com/protocolbuffers/protobuf/pull/8182) includes `V`
```
cast('MyEnum.V', x)
```

### Supports generating type wrappers for fields and maps

M.proto
Expand Down
3 changes: 0 additions & 3 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,3 @@ no_implicit_optional = True

[mypy-pytest.*]
ignore_missing_imports = True

[mypy-testproto.*]
warn_unused_ignores = False
36 changes: 17 additions & 19 deletions mypy_protobuf/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,35 +295,23 @@ def write_enums(
)
value_type_fq = prefix + class_name + ".V"

l("class {}(metaclass={}):", class_name, "_" + enum.name)
l("class {}({}, metaclass={}):", class_name, "_" + enum.name, "_" + enum.name + "EnumTypeWrapper")
with self._indent():
l("pass")
l("class {}:", "_" + enum.name)
with self._indent():
l(
"V = {}('V', {})",
self._import("typing", "NewType"),
self._builtin("int"),
)
l("")
if prefix == "" and not self.readable_stubs:
l("{} = {}", _mangle_global_identifier(class_name), class_name)
l("")

self.write_enum_values(
enumerate(enum.value),
value_type_fq,
scl + [d.EnumDescriptorProto.VALUE_FIELD_NUMBER],
)
l("")

# do a type-ignore to avoid the circular dependency. It's ugly.
# See https://github.com/dropbox/mypy-protobuf/issues/214 for discussion.
# Hopefully we can improve this in the future
l(
"class {}({}[{}], {}): # type: ignore",
"_" + enum.name,
"class {}({}[{}], {}):",
"_" + enum.name + "EnumTypeWrapper",
self._import(
"google.protobuf.internal.enum_type_wrapper", "_EnumTypeWrapper"
),
class_name + ".V",
"_" + enum.name + ".V",
self._builtin("type"),
)
with self._indent():
Expand All @@ -342,6 +330,16 @@ def write_enums(
)
l("")

self.write_enum_values(
enumerate(enum.value),
value_type_fq,
scl + [d.EnumDescriptorProto.VALUE_FIELD_NUMBER],
)
if prefix == "" and not self.readable_stubs:
l("{} = {}", _mangle_global_identifier(class_name), class_name)
l("")
l("")

def write_messages(
self,
messages: Iterable[d.DescriptorProto],
Expand Down
30 changes: 16 additions & 14 deletions test/generated/testproto/nested/nested_pb2.pyi.expected
Original file line number Diff line number Diff line change
Expand Up @@ -25,34 +25,36 @@ global___Nested = Nested

class AnotherNested(google.protobuf.message.Message):
DESCRIPTOR: google.protobuf.descriptor.Descriptor = ...
class NestedEnum(metaclass=_NestedEnum):
class NestedEnum(_NestedEnum, metaclass=_NestedEnumEnumTypeWrapper):
pass
class _NestedEnum:
V = typing.NewType('V', builtins.int)

INVALID = AnotherNested.NestedEnum.V(0)
ONE = AnotherNested.NestedEnum.V(1)
TWO = AnotherNested.NestedEnum.V(2)

class _NestedEnum(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[NestedEnum.V], builtins.type): # type: ignore
class _NestedEnumEnumTypeWrapper(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[_NestedEnum.V], builtins.type):
DESCRIPTOR: google.protobuf.descriptor.EnumDescriptor = ...
INVALID = AnotherNested.NestedEnum.V(0)
ONE = AnotherNested.NestedEnum.V(1)
TWO = AnotherNested.NestedEnum.V(2)

INVALID = AnotherNested.NestedEnum.V(0)
ONE = AnotherNested.NestedEnum.V(1)
TWO = AnotherNested.NestedEnum.V(2)

class NestedMessage(google.protobuf.message.Message):
DESCRIPTOR: google.protobuf.descriptor.Descriptor = ...
class NestedEnum2(metaclass=_NestedEnum2):
class NestedEnum2(_NestedEnum2, metaclass=_NestedEnum2EnumTypeWrapper):
pass
class _NestedEnum2:
V = typing.NewType('V', builtins.int)

UNDEFINED = AnotherNested.NestedMessage.NestedEnum2.V(0)
NESTED_ENUM1 = AnotherNested.NestedMessage.NestedEnum2.V(1)
NESTED_ENUM2 = AnotherNested.NestedMessage.NestedEnum2.V(2)

class _NestedEnum2(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[NestedEnum2.V], builtins.type): # type: ignore
class _NestedEnum2EnumTypeWrapper(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[_NestedEnum2.V], builtins.type):
DESCRIPTOR: google.protobuf.descriptor.EnumDescriptor = ...
UNDEFINED = AnotherNested.NestedMessage.NestedEnum2.V(0)
NESTED_ENUM1 = AnotherNested.NestedMessage.NestedEnum2.V(1)
NESTED_ENUM2 = AnotherNested.NestedMessage.NestedEnum2.V(2)

UNDEFINED = AnotherNested.NestedMessage.NestedEnum2.V(0)
NESTED_ENUM1 = AnotherNested.NestedMessage.NestedEnum2.V(1)
NESTED_ENUM2 = AnotherNested.NestedMessage.NestedEnum2.V(2)

S_FIELD_NUMBER: builtins.int
B_FIELD_NUMBER: builtins.int
NE_FIELD_NUMBER: builtins.int
Expand Down
30 changes: 16 additions & 14 deletions test/generated/testproto/test3_pb2.pyi.expected
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,21 @@ import typing_extensions

DESCRIPTOR: google.protobuf.descriptor.FileDescriptor = ...

class OuterEnum(metaclass=_OuterEnum):
class OuterEnum(_OuterEnum, metaclass=_OuterEnumEnumTypeWrapper):
pass
class _OuterEnum:
V = typing.NewType('V', builtins.int)

global___OuterEnum = OuterEnum
class _OuterEnumEnumTypeWrapper(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[_OuterEnum.V], builtins.type):
DESCRIPTOR: google.protobuf.descriptor.EnumDescriptor = ...
UNKNOWN = OuterEnum.V(0)
FOO3 = OuterEnum.V(1)
BAR3 = OuterEnum.V(2)

UNKNOWN = OuterEnum.V(0)
FOO3 = OuterEnum.V(1)
BAR3 = OuterEnum.V(2)
global___OuterEnum = OuterEnum

class _OuterEnum(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[OuterEnum.V], builtins.type): # type: ignore
DESCRIPTOR: google.protobuf.descriptor.EnumDescriptor = ...
UNKNOWN = OuterEnum.V(0)
FOO3 = OuterEnum.V(1)
BAR3 = OuterEnum.V(2)

class OuterMessage3(google.protobuf.message.Message):
DESCRIPTOR: google.protobuf.descriptor.Descriptor = ...
Expand All @@ -40,17 +41,18 @@ global___OuterMessage3 = OuterMessage3

class SimpleProto3(google.protobuf.message.Message):
DESCRIPTOR: google.protobuf.descriptor.Descriptor = ...
class InnerEnum(metaclass=_InnerEnum):
class InnerEnum(_InnerEnum, metaclass=_InnerEnumEnumTypeWrapper):
pass
class _InnerEnum:
V = typing.NewType('V', builtins.int)

INNER1 = SimpleProto3.InnerEnum.V(0)
INNER2 = SimpleProto3.InnerEnum.V(1)

class _InnerEnum(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[InnerEnum.V], builtins.type): # type: ignore
class _InnerEnumEnumTypeWrapper(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[_InnerEnum.V], builtins.type):
DESCRIPTOR: google.protobuf.descriptor.EnumDescriptor = ...
INNER1 = SimpleProto3.InnerEnum.V(0)
INNER2 = SimpleProto3.InnerEnum.V(1)

INNER1 = SimpleProto3.InnerEnum.V(0)
INNER2 = SimpleProto3.InnerEnum.V(1)

class MapScalarEntry(google.protobuf.message.Message):
DESCRIPTOR: google.protobuf.descriptor.Descriptor = ...
KEY_FIELD_NUMBER: builtins.int
Expand Down
58 changes: 31 additions & 27 deletions test/generated/testproto/test_pb2.pyi.expected
Original file line number Diff line number Diff line change
Expand Up @@ -22,27 +22,30 @@ import typing_extensions
DESCRIPTOR: google.protobuf.descriptor.FileDescriptor = ...

# Outer Enum
class OuterEnum(metaclass=_OuterEnum):
class OuterEnum(_OuterEnum, metaclass=_OuterEnumEnumTypeWrapper):
pass
class _OuterEnum:
V = typing.NewType('V', builtins.int)

global___OuterEnum = OuterEnum
class _OuterEnumEnumTypeWrapper(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[_OuterEnum.V], builtins.type):
DESCRIPTOR: google.protobuf.descriptor.EnumDescriptor = ...
# FOO
FOO = OuterEnum.V(1)
# BAR
BAR = OuterEnum.V(2)

# FOO
FOO = OuterEnum.V(1)
# BAR
BAR = OuterEnum.V(2)
global___OuterEnum = OuterEnum

class _OuterEnum(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[OuterEnum.V], builtins.type): # type: ignore
DESCRIPTOR: google.protobuf.descriptor.EnumDescriptor = ...
# FOO
FOO = OuterEnum.V(1)
# BAR
BAR = OuterEnum.V(2)

class NamingConflicts(metaclass=_NamingConflicts):
class NamingConflicts(_NamingConflicts, metaclass=_NamingConflictsEnumTypeWrapper):
pass
class _NamingConflicts:
V = typing.NewType('V', builtins.int)

global___NamingConflicts = NamingConflicts
class _NamingConflictsEnumTypeWrapper(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[_NamingConflicts.V], builtins.type):
DESCRIPTOR: google.protobuf.descriptor.EnumDescriptor = ...

# Naming conflicts!
Name = NamingConflicts.V(1)
Expand All @@ -53,28 +56,28 @@ values = NamingConflicts.V(4)
# proto itself generates broken code when DESCRIPTOR is there
# DESCRIPTOR = 8;
items = NamingConflicts.V(5)
global___NamingConflicts = NamingConflicts

class _NamingConflicts(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[NamingConflicts.V], builtins.type): # type: ignore
DESCRIPTOR: google.protobuf.descriptor.EnumDescriptor = ...

class Simple1(google.protobuf.message.Message):
DESCRIPTOR: google.protobuf.descriptor.Descriptor = ...
# Inner Enum
class InnerEnum(metaclass=_InnerEnum):
class InnerEnum(_InnerEnum, metaclass=_InnerEnumEnumTypeWrapper):
pass
class _InnerEnum:
V = typing.NewType('V', builtins.int)

# INNER1
INNER1 = Simple1.InnerEnum.V(1)
# INNER2
INNER2 = Simple1.InnerEnum.V(2)

class _InnerEnum(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[InnerEnum.V], builtins.type): # type: ignore
class _InnerEnumEnumTypeWrapper(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[_InnerEnum.V], builtins.type):
DESCRIPTOR: google.protobuf.descriptor.EnumDescriptor = ...
# INNER1
INNER1 = Simple1.InnerEnum.V(1)
# INNER2
INNER2 = Simple1.InnerEnum.V(2)

# INNER1
INNER1 = Simple1.InnerEnum.V(1)
# INNER2
INNER2 = Simple1.InnerEnum.V(2)

class InnerMessage(google.protobuf.message.Message):
DESCRIPTOR: google.protobuf.descriptor.Descriptor = ...
def __init__(self,
Expand Down Expand Up @@ -237,15 +240,16 @@ global_____None = __None

class PythonReservedKeywords(google.protobuf.message.Message):
DESCRIPTOR: google.protobuf.descriptor.Descriptor = ...
class __finally(metaclass=_finally):
class __finally(_finally, metaclass=_finallyEnumTypeWrapper):
pass
class _finally:
V = typing.NewType('V', builtins.int)

valid_in_finally = PythonReservedKeywords.__finally.V(2)

class _finally(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[__finally.V], builtins.type): # type: ignore
class _finallyEnumTypeWrapper(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[_finally.V], builtins.type):
DESCRIPTOR: google.protobuf.descriptor.EnumDescriptor = ...
valid_in_finally = PythonReservedKeywords.__finally.V(2)

valid_in_finally = PythonReservedKeywords.__finally.V(2)

class __lambda(google.protobuf.message.Message):
DESCRIPTOR: google.protobuf.descriptor.Descriptor = ...
CONTINUE_FIELD_NUMBER: builtins.int
Expand Down
6 changes: 3 additions & 3 deletions test_negative/output.expected.2.7

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions test_negative/output.expected.2.7.omit_linenos
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ test_negative/negative.py: error: "EnumDescriptor" has no attribute "Garbage"
test_negative/negative.py: error: "FileDescriptor" has no attribute "Garbage"
test_negative/negative.py: error: "V" has no attribute "FOO"
test_negative/negative.py: error: Argument 1 to "Name" of "_EnumTypeWrapper" has incompatible type "int"; expected "V"
test_negative/negative.py: error: Argument 1 to "Name" of "_EnumTypeWrapper" has incompatible type "testproto.test3_pb2.SimpleProto3.InnerEnum.V"; expected "testproto.test3_pb2.OuterEnum.V"
test_negative/negative.py: error: Argument 1 to "Name" of "_EnumTypeWrapper" has incompatible type "testproto.test3_pb2.SimpleProto3.InnerEnum.V"; expected "testproto.test3_pb2.OuterEnum.V"
test_negative/negative.py: error: Argument 1 to "Name" of "_EnumTypeWrapper" has incompatible type "testproto.test3_pb2.SimpleProto3.InnerEnum.V"; expected "testproto.test3_pb2.OuterEnum.V"
test_negative/negative.py: error: Argument 1 to "Name" of "_EnumTypeWrapper" has incompatible type "testproto.test3_pb2.SimpleProto3._InnerEnum.V"; expected "testproto.test3_pb2._OuterEnum.V"
test_negative/negative.py: error: Argument 1 to "Name" of "_EnumTypeWrapper" has incompatible type "testproto.test3_pb2.SimpleProto3._InnerEnum.V"; expected "testproto.test3_pb2._OuterEnum.V"
test_negative/negative.py: error: Argument 1 to "Name" of "_EnumTypeWrapper" has incompatible type "testproto.test3_pb2.SimpleProto3._InnerEnum.V"; expected "testproto.test3_pb2._OuterEnum.V"
test_negative/negative.py: error: "ScalarMap[int, unicode]" has no attribute "get_or_create"
test_negative/negative.py: error: No overload variant of "get" of "Mapping" matches argument type "str"
test_negative/negative.py: note: Possible overload variant:
Expand Down
6 changes: 3 additions & 3 deletions test_negative/output.expected.3.8

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit eb93c6e

Please sign in to comment.