Skip to content

Commit

Permalink
Support reserved names for message types
Browse files Browse the repository at this point in the history
Unfortunately, our tests missed this, because all our tests with
reserved names for message types also had reserved names for the
identifiers. Added a couple tests for the new scenario, reproed,
and fixed by replacing such instances with `Any` (best we can do)

Fixes #221
  • Loading branch information
nipunn1313 committed Jul 7, 2021
1 parent 02378ee commit 814ddd6
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 5 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,13 @@

- Bump protoc support to 3.17.3
- Use latest python versions in tests (3.6.14 3.7.11 3.8.11 3.9.6)
- Support reserved names for message types. Previously generated invalid mypy.
```
message M {
message None {}
None none = 1;
}
```

## 2.5

Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ black mypy_protobuf/main.py test/
- [@Evgenus](https://github.com/Evgenus)
- [@MHDante](https://github.com/MHDante)
- [@nelfin](https://github.com/nelfin)
- [@alkasm](https://github.com/alkasm)

Licence etc.
------------
Expand Down
8 changes: 7 additions & 1 deletion mypy_protobuf/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,14 +174,20 @@ def _import_message(self, name: str) -> str:
assert name.startswith(".")
name = name[1:]

split = name.split(".")

# Can't represent identifiers that are reserved, so mark as Any
# and don't bother importing
if any(s in PYTHON_RESERVED for s in split):
return self._import("typing", "Any")

# Message defined in this file.
if message_fd.name == self.fd.name:
return name if self.readable_stubs else _mangle_global_identifier(name)

# Not in file. Must import
# Python generated code ignores proto packages, so the only relevant factor is
# whether it is in the file or not.
split = name.split(".")
import_name = self._import(
message_fd.name[:-6].replace("-", "_") + "_pb2", split[0]
)
Expand Down
7 changes: 6 additions & 1 deletion proto/testproto/test.proto
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ message Extensions2 {
optional bool flag = 1;
}

message None {}

message PythonReservedKeywords {
enum finally {
continue = 1;
Expand Down Expand Up @@ -110,9 +112,12 @@ message PythonReservedKeywords {
optional int64 except = 25;
optional int64 raise = 26;
optional int64 False = 27;
optional int64 None = 28;
optional int64 True = 29;
optional int64 class = 30;

// Test unreserved identifiers w/ reserved message names
optional None none = 28;
optional finally valid = 31;
}

// Do one with just one arg - to make sure it's syntactically correct
Expand Down
14 changes: 11 additions & 3 deletions test/generated/testproto/test_pb2.pyi.expected
Original file line number Diff line number Diff line change
Expand Up @@ -237,14 +237,22 @@ class PythonReservedKeywords(google.protobuf.message.Message):
EXCEPT_FIELD_NUMBER: builtins.int
RAISE_FIELD_NUMBER: builtins.int
FALSE_FIELD_NUMBER: builtins.int
NONE_FIELD_NUMBER: builtins.int
TRUE_FIELD_NUMBER: builtins.int
CLASS_FIELD_NUMBER: builtins.int
NONE_FIELD_NUMBER: builtins.int
VALID_FIELD_NUMBER: builtins.int
valid: typing.Any = ...

@property
def none(self) -> typing.Any: ...

def __init__(self,
*,
none : typing.Optional[typing.Any] = ...,
valid : typing.Optional[typing.Any] = ...,
) -> None: ...
def HasField(self, field_name: typing_extensions.Literal[u"False",b"False",u"None",b"None",u"True",b"True",u"and",b"and",u"as",b"as",u"assert",b"assert",u"break",b"break",u"class",b"class",u"def",b"def",u"del",b"del",u"elif",b"elif",u"else",b"else",u"except",b"except",u"for",b"for",u"from",b"from",u"global",b"global",u"if",b"if",u"import",b"import",u"in",b"in",u"is",b"is",u"nonlocal",b"nonlocal",u"not",b"not",u"or",b"or",u"pass",b"pass",u"raise",b"raise",u"try",b"try",u"while",b"while",u"with",b"with",u"yield",b"yield"]) -> builtins.bool: ...
def ClearField(self, field_name: typing_extensions.Literal[u"False",b"False",u"None",b"None",u"True",b"True",u"and",b"and",u"as",b"as",u"assert",b"assert",u"break",b"break",u"class",b"class",u"def",b"def",u"del",b"del",u"elif",b"elif",u"else",b"else",u"except",b"except",u"for",b"for",u"from",b"from",u"global",b"global",u"if",b"if",u"import",b"import",u"in",b"in",u"is",b"is",u"nonlocal",b"nonlocal",u"not",b"not",u"or",b"or",u"pass",b"pass",u"raise",b"raise",u"try",b"try",u"while",b"while",u"with",b"with",u"yield",b"yield"]) -> None: ...
def HasField(self, field_name: typing_extensions.Literal[u"False",b"False",u"True",b"True",u"and",b"and",u"as",b"as",u"assert",b"assert",u"break",b"break",u"class",b"class",u"def",b"def",u"del",b"del",u"elif",b"elif",u"else",b"else",u"except",b"except",u"for",b"for",u"from",b"from",u"global",b"global",u"if",b"if",u"import",b"import",u"in",b"in",u"is",b"is",u"none",b"none",u"nonlocal",b"nonlocal",u"not",b"not",u"or",b"or",u"pass",b"pass",u"raise",b"raise",u"try",b"try",u"valid",b"valid",u"while",b"while",u"with",b"with",u"yield",b"yield"]) -> builtins.bool: ...
def ClearField(self, field_name: typing_extensions.Literal[u"False",b"False",u"True",b"True",u"and",b"and",u"as",b"as",u"assert",b"assert",u"break",b"break",u"class",b"class",u"def",b"def",u"del",b"del",u"elif",b"elif",u"else",b"else",u"except",b"except",u"for",b"for",u"from",b"from",u"global",b"global",u"if",b"if",u"import",b"import",u"in",b"in",u"is",b"is",u"none",b"none",u"nonlocal",b"nonlocal",u"not",b"not",u"or",b"or",u"pass",b"pass",u"raise",b"raise",u"try",b"try",u"valid",b"valid",u"while",b"while",u"with",b"with",u"yield",b"yield"]) -> None: ...
global___PythonReservedKeywords = PythonReservedKeywords

class PythonReservedKeywordsSmall(google.protobuf.message.Message):
Expand Down
9 changes: 9 additions & 0 deletions test/test_generated_mypy.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import six

from google.protobuf.descriptor import FieldDescriptor
from google.protobuf.message import Message

import testproto.test_pb2 as test_pb2
from testproto.reexport_pb2 import (
Expand Down Expand Up @@ -476,3 +477,11 @@ def test_casttype():
s.email = Email("[email protected]")
assert s.email == "[email protected]"
s.email_by_uid[UserId(33)] = Email("[email protected]")

def test_access_message_with_python_keyword_name():
# type: () -> None
with pytest.raises(AttributeError, match="module 'testproto.test_pb2' has no attribute 'asdf'"):
getattr(test_pb2, "asdf")
msg_cls = getattr(test_pb2, "None")
none = msg_cls()
assert isinstance(none, Message)

0 comments on commit 814ddd6

Please sign in to comment.