Support comments and deprecation for methods and enum values

* Express methods as properties, allows for deprecation
* Express enum values as properties, allows for deprecation

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

Author: aidandj

CHANGELOG.md

@@ -12,6 +12,8 @@
 - 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
+- Export stub methods as properties instead of attributes. This allows for deprecation
+- Export enum fields as properties on class level (not module level) enums. This allows for deprecation
 
 ## 3.7.0
 

mypy_protobuf/main.py

@@ -284,7 +284,7 @@ def _get_comments(self, scl: SourceCodeLocation) -> List[str]:
 
         return lines
 
-    def _write_deprecation_warning(self, scl: SourceCodeLocation, default_message: str) -> None:
+    def _get_deprecation_message(self, scl: SourceCodeLocation, default_message: str) -> str:
         msg = default_message
         if not self.use_default_depreaction_warnings and (comments := self._get_comments(scl)):
             # Make sure the comment string is a valid python string literal
@@ -296,6 +296,10 @@ def _write_deprecation_warning(self, scl: SourceCodeLocation, default_message: s
             except SyntaxError as e:
                 print(f"Warning: Deprecation comment {joined} could not be parsed as a python string literal. Using default deprecation message. {e}", file=sys.stderr)
                 pass
+        return msg
+
+    def _write_deprecation_warning(self, scl: SourceCodeLocation, default_message: str) -> None:
+        msg = self._get_deprecation_message(scl, default_message)
         self._write_line(
             '@{}("""{}""")',
             self._import("warnings", "deprecated"),
@@ -356,16 +360,33 @@ def write_enum_values(
         values: Iterable[Tuple[int, d.EnumValueDescriptorProto]],
         value_type: str,
         scl_prefix: SourceCodeLocation,
+        *,
+        as_properties: bool = False,
     ) -> None:
         for i, val in values:
             if val.name in PYTHON_RESERVED:
                 continue
 
             scl = scl_prefix + [i]
-            self._write_line(
-                f"{val.name}: {value_type}  # {val.number}",
-            )
-            self._write_comments(scl)
+            # Class level
+            if as_properties:
+                self._write_line("@property")
+                if val.options.deprecated:
+                    self._write_deprecation_warning(
+                        scl + [d.EnumValueDescriptorProto.OPTIONS_FIELD_NUMBER] + [d.EnumOptions.DEPRECATED_FIELD_NUMBER],
+                        "This enum value has been marked as deprecated using proto enum value options.",
+                    )
+                self._write_line(
+                    f"def {val.name}(self) -> {value_type}: {'' if self._has_comments(scl) else '...'}  # {val.number}",
+                )
+                with self._indent():
+                    self._write_comments(scl)
+            # Module level
+            else:
+                self._write_line(
+                    f"{val.name}: {value_type}  # {val.number}",
+                )
+                self._write_comments(scl)
 
     def write_module_attributes(self) -> None:
         wl = self._write_line
@@ -412,6 +433,7 @@ def write_enums(
                     [(i, v) for i, v in enumerate(enum.value) if v.name not in PROTO_ENUM_RESERVED],
                     value_type_helper_fq,
                     scl + [d.EnumDescriptorProto.VALUE_FIELD_NUMBER],
+                    as_properties=True,
                 )
             wl("")
 
@@ -430,6 +452,7 @@ def write_enums(
                 if prefix == "":
                     wl("")
 
+            # Write the module level constants for enum values
             self.write_enum_values(
                 enumerate(enum.value),
                 value_type_fq,
@@ -811,7 +834,7 @@ def write_grpc_async_hacks(self) -> None:
             wl("...")
         wl("")
 
-    def write_grpc_stub_methods(self, service: d.ServiceDescriptorProto, scl_prefix: SourceCodeLocation, *, is_async: bool, both: bool = False, ignore_assignment_errors: bool = False) -> None:
+    def write_grpc_stub_methods(self, service: d.ServiceDescriptorProto, scl_prefix: SourceCodeLocation, *, is_async: bool, both: bool = False, ignore_override_errors: bool = False) -> None:
         wl = self._write_line
         methods = [(i, m) for i, m in enumerate(service.method) if m.name not in PYTHON_RESERVED]
         if not methods:
@@ -822,17 +845,20 @@ def type_str(method: d.MethodDescriptorProto, is_async: bool) -> str:
 
         for i, method in methods:
             scl = scl_prefix + [d.ServiceDescriptorProto.METHOD_FIELD_NUMBER, i]
-            if both:
-                wl(
-                    "{}: {}[{}, {}]",
-                    method.name,
-                    self._import("typing", "Union"),
-                    type_str(method, is_async=False),
-                    type_str(method, is_async=True),
+            wl("@property")
+            if method.options.deprecated:
+                self._write_deprecation_warning(
+                    scl + [d.MethodDescriptorProto.OPTIONS_FIELD_NUMBER] + [d.MethodOptions.DEPRECATED_FIELD_NUMBER],
+                    "This method has been marked as deprecated using proto method options.",
                 )
+            if both:
+                wl("def {}(self) -> {}[{}, {}]:{}", method.name, self._import("typing", "Union"), type_str(method, is_async=False), type_str(method, is_async=True), " ..." if not self._has_comments(scl) else " ")
             else:
-                wl("{}: {}{}", method.name, type_str(method, is_async=is_async), "" if not ignore_assignment_errors else "  # type: ignore[assignment]")
-            self._write_comments(scl)
+                wl("def {}(self) -> {}:{}{}", method.name, type_str(method, is_async=is_async), " ..." if not self._has_comments(scl) else "", "" if not ignore_override_errors else "  # type: ignore[override]")
+            if self._has_comments(scl):
+                with self._indent():
+                    if not self._write_comments(scl):
+                        wl("...")
 
     def write_grpc_methods(self, service: d.ServiceDescriptorProto, scl_prefix: SourceCodeLocation) -> None:
         wl = self._write_line
@@ -924,7 +950,7 @@ def write_grpc_services(
                 if self._write_comments(scl):
                     wl("")
                 wl("def __init__(self, channel: {}) -> None: ...", self._import("grpc.aio", "Channel"))
-                self.write_grpc_stub_methods(service, scl, is_async=True, ignore_assignment_errors=True)
+                self.write_grpc_stub_methods(service, scl, is_async=True, ignore_override_errors=True)
             wl("")
 
             # The service definition interface

proto/testproto/grpc/dummy.proto

@@ -38,6 +38,7 @@ service DeprecatedService {
   }
   // DeprecatedMethodNotDeprecatedRequest
   rpc DeprecatedMethodNotDeprecatedRequest(DummyRequest) returns (DummyReply) {
+    // Method is deprecated, but request message is not
     option deprecated = true;
   }
 }

proto/testproto/test.proto

@@ -193,6 +193,10 @@ enum DeprecatedEnum {
     option deprecated = true;
     // Trailing comment
 
-    DEPRECATED_ONE = 1;
-    DEPRECATED_TWO = 2;
+    DEPRECATED_ONE = 1 [
+      // This enum value is deprecated but this message doesn't show
+      deprecated = true
+      ]; // Trailing comment for enum value
+    DEPRECATED_TWO = 2 [deprecated = true];
+    NOT_DEPRECATED = 3;
 }

stubtest_allowlist.txt

@@ -36,6 +36,7 @@ testproto.nested.nested_pb2.AnotherNested.NestedMessage.NESTED_ENUM2
 testproto.nested.nested_pb2.AnotherNested.INVALID
 testproto.test_pb2.DEPRECATED_ONE
 testproto.test_pb2.DEPRECATED_TWO
+testproto.test_pb2.NOT_DEPRECATED
 
 # Our enum types and helper types aren't there at runtime (Dynamic EnumTypeWrapper at runtime)
 # .*\..*EnumTypeWrapper$
@@ -246,9 +247,9 @@ testproto.test_pb2.Extensions1.ext
 .*_WhichOneofReturnType.*
 .*_WhichOneofArgType.*
 
-#
-testproto.grpc.dummy_pb2_grpc.DeprecatedServiceStub.__init__
-testproto.grpc.dummy_pb2_grpc.DummyServiceStub.__init__
-testproto.grpc.dummy_pb2_grpc.ManyRPCsServiceStub.__init__
-testproto.grpc.import_pb2_grpc.SimpleServiceStub.__init__
+# Because we represent methods as properties, they do not exist at runtime
+testproto.grpc.dummy_pb2_grpc.DeprecatedServiceStub.*
+testproto.grpc.dummy_pb2_grpc.DummyServiceStub.*
+testproto.grpc.dummy_pb2_grpc.ManyRPCsServiceStub.*
+testproto.grpc.import_pb2_grpc.SimpleServiceStub.*
 testproto.grpc.dummy_pb2_grpc.EmptyServiceStub.__init__