alias import statements to fix issues with name collisions

Signed-off-by: Aidan Jensen <aidandj.github@gmail.com>

Author: aidandj

.github/workflows/main.yml

@@ -108,6 +108,8 @@ jobs:
         with:
           # By default, shellcheck tries to make sure any external files referenced actually exist
           shellcheck_flags: -e SC1091
+          exclude: |
+            */third_party/*
 
   sanity_check_windows:
     name: Sanity Check Windows Executable

CHANGELOG.md

@@ -13,6 +13,9 @@
 - Add `_WhichOneofArgType_<oneof_name>` and `_WhichOneofReturnType_<oneof_name>` type aliases
 - Use `__new__` overloads for async stubs instead of `TypeVar` based `__init__` overloads.
   - https://github.com/nipunn1313/mypy-protobuf/issues/707
+- Use `builtins.property` to handle conflicts with fields named `property`
+- Mangle all non provided message type imports, this prevents conflicts with field names like `collections`, `builtins`, etc.
+  - Do not mangle message imports, as that would be a breaking change.
 
 ## 3.7.0
 

README.md

@@ -350,7 +350,7 @@ protoc --python_out=output/location --mypy_grpc_out=generate_concrete_servicer_s
 
 ### `sync_only/async_only`
 
-By default, generated GRPC stubs are compatible with both sync and async variants. If you only 
+By default, generated GRPC stubs are compatible with both sync and async variants. If you only
 want sync or async GRPC stubs, use this option:
 
 ```
@@ -445,6 +445,26 @@ protoc --python_out=output/location --mypy_out=output/location
 mypy --target-version=2.7 {files}
 ```
 
+## 3rd Party Tests
+
+3rd Party proto files can be added to `third_party` using git subtree. These can be used for large scale testing of changes.
+
+### Adding
+
+```bash
+git subtree add --prefix=third_party/googleapis https://github.com/googleapis/googleapis.git master --squash
+```
+
+### Updating
+
+```bash
+git subtree pull --prefix=third_party/googleapis https://github.com/googleapis/googleapis.git master --squash
+```
+
+## 4.0 Breaking change proposals
+
+* De-dot all import statements
+
 ## Contributing
 
 Contributions to the implementation are welcome. Please run tests using `./run_test.sh`.

mypy_protobuf/extensions_pb2.pyi

@@ -3,53 +3,53 @@
 isort:skip_file
 """
 
-import builtins
-import google.protobuf.descriptor
+import builtins as _builtins
+import google.protobuf.descriptor as _google_protobuf_descriptor
 import google.protobuf.descriptor_pb2
-import google.protobuf.internal.extension_dict
-import google.protobuf.message
+import google.protobuf.internal.extension_dict as _google_protobuf_internal_extension_dict
+import google.protobuf.message as _google_protobuf_message
 import sys
-import typing
+import typing as _typing
 
 if sys.version_info >= (3, 10):
-    import typing as typing_extensions
+    import typing as _typing_extensions
 else:
-    import typing_extensions
+    import typing_extensions as _typing_extensions
 
-DESCRIPTOR: google.protobuf.descriptor.FileDescriptor
+DESCRIPTOR: _google_protobuf_descriptor.FileDescriptor
 
-@typing.final
-class FieldOptions(google.protobuf.message.Message):
-    DESCRIPTOR: google.protobuf.descriptor.Descriptor
+@_typing.final
+class FieldOptions(_google_protobuf_message.Message):
+    DESCRIPTOR: _google_protobuf_descriptor.Descriptor
 
-    CASTTYPE_FIELD_NUMBER: builtins.int
-    KEYTYPE_FIELD_NUMBER: builtins.int
-    VALUETYPE_FIELD_NUMBER: builtins.int
-    casttype: builtins.str
+    CASTTYPE_FIELD_NUMBER: _builtins.int
+    KEYTYPE_FIELD_NUMBER: _builtins.int
+    VALUETYPE_FIELD_NUMBER: _builtins.int
+    casttype: _builtins.str
     """Tells mypy-protobuf to use a specific newtype rather than the normal type for this field."""
-    keytype: builtins.str
+    keytype: _builtins.str
     """Tells mypy-protobuf to use a specific type for keys; only makes sense on map fields"""
-    valuetype: builtins.str
+    valuetype: _builtins.str
     """Tells mypy-protobuf to use a specific type for values; only makes sense on map fields"""
     def __init__(
         self,
         *,
-        casttype: builtins.str = ...,
-        keytype: builtins.str = ...,
-        valuetype: builtins.str = ...,
+        casttype: _builtins.str = ...,
+        keytype: _builtins.str = ...,
+        valuetype: _builtins.str = ...,
     ) -> None: ...
-    _ClearFieldArgType: typing_extensions.TypeAlias = typing.Literal["casttype", b"casttype", "keytype", b"keytype", "valuetype", b"valuetype"]
+    _ClearFieldArgType: _typing_extensions.TypeAlias = _typing.Literal["casttype", b"casttype", "keytype", b"keytype", "valuetype", b"valuetype"]  # noqa: Y015
     def ClearField(self, field_name: _ClearFieldArgType) -> None: ...
 
-Global___FieldOptions: typing_extensions.TypeAlias = FieldOptions
+Global___FieldOptions: _typing_extensions.TypeAlias = FieldOptions  # noqa: Y015
 
-OPTIONS_FIELD_NUMBER: builtins.int
-CASTTYPE_FIELD_NUMBER: builtins.int
-KEYTYPE_FIELD_NUMBER: builtins.int
-VALUETYPE_FIELD_NUMBER: builtins.int
-options: google.protobuf.internal.extension_dict._ExtensionFieldDescriptor[google.protobuf.descriptor_pb2.FieldOptions, Global___FieldOptions]
+OPTIONS_FIELD_NUMBER: _builtins.int
+CASTTYPE_FIELD_NUMBER: _builtins.int
+KEYTYPE_FIELD_NUMBER: _builtins.int
+VALUETYPE_FIELD_NUMBER: _builtins.int
+options: _google_protobuf_internal_extension_dict._ExtensionFieldDescriptor[google.protobuf.descriptor_pb2.FieldOptions, Global___FieldOptions]
 """Custom field options from mypy-protobuf"""
-casttype: google.protobuf.internal.extension_dict._ExtensionFieldDescriptor[google.protobuf.descriptor_pb2.FieldOptions, builtins.str]
+casttype: _google_protobuf_internal_extension_dict._ExtensionFieldDescriptor[google.protobuf.descriptor_pb2.FieldOptions, _builtins.str]
 """Legacy fields. Prefer to use ones within `options` instead."""
-keytype: google.protobuf.internal.extension_dict._ExtensionFieldDescriptor[google.protobuf.descriptor_pb2.FieldOptions, builtins.str]
-valuetype: google.protobuf.internal.extension_dict._ExtensionFieldDescriptor[google.protobuf.descriptor_pb2.FieldOptions, builtins.str]
+keytype: _google_protobuf_internal_extension_dict._ExtensionFieldDescriptor[google.protobuf.descriptor_pb2.FieldOptions, _builtins.str]
+valuetype: _google_protobuf_internal_extension_dict._ExtensionFieldDescriptor[google.protobuf.descriptor_pb2.FieldOptions, _builtins.str]

mypy_protobuf/main.py

@@ -191,7 +191,7 @@ def __init__(
         self.indent = ""
 
         # Set of {x}, where {x} corresponds to to `import {x}`
-        self.imports: Set[str] = set()
+        self.imports: Dict[str, bool] = {}
         # dictionary of x->(y,z) for `from {x} import {y} as {z}`
         # if {z} is None, then it shortens to `from {x} import {y}`
         self.from_imports: Dict[str, Set[Tuple[str, str | None]]] = defaultdict(set)
@@ -201,29 +201,46 @@ def __init__(
         # Comments
         self.source_code_info_by_scl = {tuple(location.path): location for location in fd.source_code_info.location}
 
-    def _import(self, path: str, name: str) -> str:
+    @property
+    def _deprecated_name(self) -> str:
+        return "_deprecated"
+
+    @property
+    def _typing_extensions_name(self) -> str:
+        return "_typing_extensions"
+
+    def _import_alias(self, path: str) -> str:
+        """import as prefixed with underscore to avoid conflicts with message/enum names"""
+        return f"_{path.replace('.', '_')}"
+
+    def _import(self, path: str, name: str, *, alias: bool = True) -> str:
         """Imports a stdlib path and returns a handle to it
         eg. self._import("typing", "Literal") -> "Literal"
+
+        If alias is true, then it will prefix the import with an underscore to prevent conflicts with builtin names
         """
         if path == "typing_extensions":
-            stabilization = {"TypeAlias": (3, 10), "TypeVar": (3, 13), "type_check_only": (3, 12)}
+            stabilization = {"TypeAlias": (3, 10), "TypeVar": (3, 13), "type_check_only": (3, 12), "Self": (3, 11)}
             assert name in stabilization
             if not self.typing_extensions_min or self.typing_extensions_min < stabilization[name]:
                 self.typing_extensions_min = stabilization[name]
-            return "typing_extensions." + name
+            return self._typing_extensions_name + "." + name
 
         if path == "warnings" and name == "deprecated":
             if not self.deprecated_min or self.deprecated_min < (3, 11):
                 self.deprecated_min = (3, 13)
-            return name
+            return self._deprecated_name
 
         imp = path.replace("/", ".")
         if self.readable_stubs:
             self.from_imports[imp].add((name, None))
             return name
         else:
-            self.imports.add(imp)
-            return imp + "." + name
+            self.imports[imp] = alias
+            return (self._import_alias(imp) if alias else imp) + "." + name
+
+    def _property(self) -> str:
+        return f"@{self._import('builtins', 'property')}"
 
     def _import_message(self, name: str) -> str:
         """Import a referenced message and return a handle"""
@@ -252,7 +269,7 @@ def _import_message(self, name: str) -> str:
         # 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.
-        import_name = self._import(message_fd.name[:-6].replace("-", "_") + "_pb2", split[0])
+        import_name = self._import(message_fd.name[:-6].replace("-", "_") + "_pb2", split[0], alias=False)
 
         remains = ".".join(split[1:])
         if not remains:
@@ -427,7 +444,7 @@ def write_enums(
                     self._builtin("int"),
                 )
                 # Alias to the classic shorter definition "V"
-                wl("V: {} = ValueType", self._import("typing_extensions", "TypeAlias"))
+                wl("V: {} = ValueType  # noqa: Y015", self._import("typing_extensions", "TypeAlias"))
             wl("")
             wl(
                 "class {}({}[{}], {}):",
@@ -467,7 +484,7 @@ def write_enums(
                 scl + [d.EnumDescriptorProto.VALUE_FIELD_NUMBER],
             )
             if prefix == "" and not self.readable_stubs:
-                wl(f"{_mangle_global_identifier(class_name)}: {self._import('typing_extensions', 'TypeAlias')} = {class_name}")
+                wl(f"{_mangle_global_identifier(class_name)}: {self._import('typing_extensions', 'TypeAlias')} = {class_name}  # noqa: Y015")
             wl("")
 
     def write_messages(
@@ -544,7 +561,7 @@ def write_messages(
                     if not (is_scalar(field) and field.label != d.FieldDescriptorProto.LABEL_REPEATED):
                         # r/o Getters for non-scalar fields and scalar-repeated fields
                         scl_field = scl + [d.DescriptorProto.FIELD_FIELD_NUMBER, idx]
-                        wl("@property")
+                        wl(self._property())
                         body = " ..." if not self._has_comments(scl_field) else ""
                         wl(f"def {field.name}(self) -> {field_type}:{body}")
                         if self._has_comments(scl_field):
@@ -580,7 +597,7 @@ def write_messages(
 
             if prefix == "" and not self.readable_stubs:
                 wl("")
-                wl(f"{_mangle_global_identifier(class_name)}: {self._import('typing_extensions', 'TypeAlias')} = {class_name}")
+                wl(f"{_mangle_global_identifier(class_name)}: {self._import('typing_extensions', 'TypeAlias')} = {class_name}  # noqa: Y015")
             wl("")
 
     def write_stringly_typed_fields(self, desc: d.DescriptorProto) -> None:
@@ -604,29 +621,29 @@ def write_stringly_typed_fields(self, desc: d.DescriptorProto) -> None:
             return
 
         if hf_fields:
-            wl("_HasFieldArgType: {} = {}[{}]", self._import("typing_extensions", "TypeAlias"), self._import("typing", "Literal"), hf_fields_text)
+            wl("_HasFieldArgType: {} = {}[{}]  # noqa: Y015", self._import("typing_extensions", "TypeAlias"), self._import("typing", "Literal"), hf_fields_text)
             wl(
                 "def HasField(self, field_name: _HasFieldArgType) -> {}: ...",
                 self._builtin("bool"),
             )
         if cf_fields:
-            wl("_ClearFieldArgType: {} = {}[{}]", self._import("typing_extensions", "TypeAlias"), self._import("typing", "Literal"), cf_fields_text)
+            wl("_ClearFieldArgType: {} = {}[{}]  # noqa: Y015", self._import("typing_extensions", "TypeAlias"), self._import("typing", "Literal"), cf_fields_text)
             wl(
                 "def ClearField(self, field_name: _ClearFieldArgType) -> None: ...",
             )
 
         # Write type aliases first so overloads are not interrupted
         for wo_field, members in sorted(wo_fields.items()):
             wl(
-                "_WhichOneofReturnType_{}: {} = {}[{}]",
+                "_WhichOneofReturnType_{}: {} = {}[{}]  # noqa: Y015",
                 wo_field,
                 self._import("typing_extensions", "TypeAlias"),
                 self._import("typing", "Literal"),
                 # Returns `str`
                 ", ".join(f'"{m}"' for m in members),
             )
             wl(
-                "_WhichOneofArgType_{}: {} = {}[{}]",
+                "_WhichOneofArgType_{}: {} = {}[{}]  # noqa: Y015",
                 wo_field,
                 self._import("typing_extensions", "TypeAlias"),
                 self._import("typing", "Literal"),
@@ -751,7 +768,7 @@ def _import_casttype(self, casttype: str) -> str:
         split = casttype.split(".")
         assert len(split) == 2, "mypy_protobuf.[casttype,keytype,valuetype] is expected to be of format path/to/file.TypeInFile"
         pkg = split[0].replace("/", ".")
-        return self._import(pkg, split[1])
+        return self._import(pkg, split[1], alias=False)
 
     def _map_key_value_types(
         self,
@@ -983,7 +1000,7 @@ def write_grpc_services(
                         wl(
                             "def __new__(cls, channel: {}) -> {}: ...",
                             self._import("grpc", "Channel"),
-                            class_name,
+                            self._import("typing_extensions", "Self"),
                         )
 
                         # Write async overload
@@ -1168,21 +1185,25 @@ def write(self) -> str:
                 self.from_imports[reexport_imp].update((n, n) for n in names)
 
         if self.typing_extensions_min or self.deprecated_min:
-            self.imports.add("sys")
-        for pkg in sorted(self.imports):
-            self._write_line(f"import {pkg}")
+            # Special case for `sys` as it is needed for version checks
+            self.imports["sys"] = False
+        for pkg, dedot in sorted(self.imports.items()):
+            if dedot:
+                self._write_line(f"import {pkg} as {self._import_alias(pkg)}")
+            else:
+                self._write_line(f"import {pkg}")
         if self.typing_extensions_min:
             self._write_line("")
             self._write_line(f"if sys.version_info >= {self.typing_extensions_min}:")
-            self._write_line("    import typing as typing_extensions")
+            self._write_line(f"    import typing as {self._typing_extensions_name}")
             self._write_line("else:")
-            self._write_line("    import typing_extensions")
+            self._write_line(f"    import typing_extensions as {self._typing_extensions_name}")
         if self.deprecated_min:
             self._write_line("")
             self._write_line(f"if sys.version_info >= {self.deprecated_min}:")
-            self._write_line("    from warnings import deprecated")
+            self._write_line(f"    from warnings import deprecated as {self._deprecated_name}")
             self._write_line("else:")
-            self._write_line("    from typing_extensions import deprecated")
+            self._write_line(f"    from typing_extensions import deprecated as {self._deprecated_name}")
 
         for pkg, items in sorted(self.from_imports.items()):
             self._write_line(f"from {pkg} import (")

proto/testproto/test.proto

@@ -74,6 +74,11 @@ message Simple1 {
     (mypy_protobuf.valuetype)="test/test_generated_mypy.Email"
   ];
 
+  optional string property = 24;
+  optional string collections = 25;
+  // This only works when all imports are de-dotted. Consider for a breaking release
+  // optional string testproto = 26;
+
   extensions 1000 to max;
 }
 

pyproject.toml

@@ -33,6 +33,7 @@ exclude = [
     "**/*_pb2.py",
     "**/*_pb2_grpc.py",
     "test/test_concrete.py",
+    "test/**/*unittest*_pb2.pyi"
 ]
 
 executionEnvironments = [

run_test.sh

@@ -149,6 +149,15 @@ MYPY_PROTOBUF_VENV=venv_$PY_VER_MYPY_PROTOBUF
         echo -e "${RED}Some .pyi files did not match. Please commit those files${NC}"
         exit 1
     fi
+
+    # Generate 3rd party protos
+    mkdir -p third_party/out/generated_googleapis
+    # Known conflict with extensions proto in googleapis - skip that one
+    find third_party/googleapis -name "*.proto" \
+        ! -path "third_party/googleapis/google/cloud/compute/v1/compute.proto" \
+        ! -path "third_party/googleapis/google/cloud/compute/v1beta/compute.proto" \
+        ! -path "third_party/googleapis/google/cloud/compute/v1small/compute_small.proto" \
+        -print0 | xargs -0 "$PROTOC" --proto_path=third_party/googleapis --mypy_out=third_party/out/generated_googleapis --mypy_grpc_out=third_party/out/generated_googleapis --python_out=third_party/out/generated_googleapis
 )
 
 ERROR_FILE=$(mktemp)
@@ -181,6 +190,14 @@ for PY_VER in $PY_VER_UNIT_TESTS; do
         ASYNC_ONLY_MODULES=( -m test.async_only.test_async_only )
         MYPYPATH=$MYPYPATH:test/generated_async_only mypy ${CUSTOM_TYPESHED_DIR_ARG:+"$CUSTOM_TYPESHED_DIR_ARG"} --report-deprecated-as-note --python-executable="$UNIT_TESTS_VENV"/bin/python --python-version="$PY_VER_MYPY_TARGET" "${ASYNC_ONLY_MODULES[@]}"
 
+        # Run google/protobuf mypy
+        GOOGLE_PROTOBUF=( test/generated/google/protobuf )
+        MYPYPATH=$MYPYPATH:test/generated PYTHONPATH=test/generated mypy --explicit-package-bases ${CUSTOM_TYPESHED_DIR_ARG:+"$CUSTOM_TYPESHED_DIR_ARG"} --report-deprecated-as-note --python-executable="$UNIT_TESTS_VENV"/bin/python --python-version="$PY_VER_MYPY_TARGET" "${GOOGLE_PROTOBUF[@]}"
+
+        # Run googleapis mypy
+        GOOGLEAPIS=( third_party/out/generated_googleapis )
+        MYPYPATH=$MYPYPATH:third_party/out/generated_googleapis mypy --explicit-package-bases ${CUSTOM_TYPESHED_DIR_ARG:+"$CUSTOM_TYPESHED_DIR_ARG"} --report-deprecated-as-note --python-executable="$UNIT_TESTS_VENV"/bin/python --python-version="$PY_VER_MYPY_TARGET" "${GOOGLEAPIS[@]}"
+
         export MYPYPATH=$MYPYPATH:test/generated
 
         # Run mypy
@@ -233,6 +250,10 @@ for PY_VER in $PY_VER_UNIT_TESTS; do
     )
 done
 
+
+# Clean up googleapis
+rm -rf third_party/out/generated_googleapis
+
 # Report all errors at the end
 if [ -s "$ERROR_FILE" ]; then
     echo -e "\n${RED}===============================================${NC}"