diff --git a/python/google/protobuf/internal/proto_test.py b/python/google/protobuf/internal/proto_test.py index b3351a498f..6740730e2d 100644 --- a/python/google/protobuf/internal/proto_test.py +++ b/python/google/protobuf/internal/proto_test.py @@ -13,6 +13,7 @@ import unittest from google.protobuf import proto from google.protobuf.internal import encoder +from google.protobuf.internal import test_proto2_pb2 from google.protobuf.internal import test_util from google.protobuf.internal import testing_refleaks @@ -21,8 +22,10 @@ from google.protobuf import unittest_pb2 from google.protobuf import unittest_proto3_arena_pb2 -@_parameterized.named_parameters(('_proto2', unittest_pb2), - ('_proto3', unittest_proto3_arena_pb2)) +@_parameterized.named_parameters( + ('_proto2', unittest_pb2), + ('_proto3', unittest_proto3_arena_pb2), +) @testing_refleaks.TestCase class ProtoTest(unittest.TestCase): @@ -75,6 +78,22 @@ class ProtoTest(unittest.TestCase): ) +class SelfFieldTest(unittest.TestCase): + + def test_pytype_allows_unset_self_field(self): + self.assertEqual( + test_proto2_pb2.MessageWithSelfField(something=123).something, 123 + ) + + def test_pytype_allows_unset_self_and_self_underscore_field(self): + self.assertEqual( + test_proto2_pb2.MessageWithSelfAndSelfUnderscoreField( + something=123 + ).something, + 123, + ) + + _EXPECTED_PROTO3 = b'\x04r\x02hi\x06\x08\x01r\x02hi\x06\x08\x02r\x02hi' _EXPECTED_PROTO2 = b'\x06\x08\x00r\x02hi\x06\x08\x01r\x02hi\x06\x08\x02r\x02hi' diff --git a/python/google/protobuf/internal/test_proto2.proto b/python/google/protobuf/internal/test_proto2.proto index 9c1022eacb..94fd50c8ca 100644 --- a/python/google/protobuf/internal/test_proto2.proto +++ b/python/google/protobuf/internal/test_proto2.proto @@ -37,3 +37,14 @@ message TestProto2 { repeated int32 repeated_int32 = 22; repeated NestedMessage repeated_nested_message = 23; } + +message MessageWithSelfField { + optional int32 something = 1; + optional int32 self = 2; +} + +message MessageWithSelfAndSelfUnderscoreField { + optional int32 something = 1; + optional int32 self = 2; + optional int32 self_ = 3 [json_name = "self_underscore"]; +} diff --git a/src/google/protobuf/compiler/python/pyi_generator.cc b/src/google/protobuf/compiler/python/pyi_generator.cc index f86777c2c2..c8975b5737 100644 --- a/src/google/protobuf/compiler/python/pyi_generator.cc +++ b/src/google/protobuf/compiler/python/pyi_generator.cc @@ -475,23 +475,25 @@ void PyiGenerator::PrintMessage( } // Prints __init__ - printer_->Print("def __init__(self"); - bool has_key_words = false; - bool is_first = true; + printer_->Print("def __init__("); + // If the message has a field named "self" (see b/144146793), it can still be + // passed to the initializer, which takes those as **kwargs. To avoid name + // collision, we rename the self parameter by appending underscores until it + // no longer collides. The self-parameter is in fact positional-only, so the + // name in the pyi doesn't matter with regard to what runtime usage is valid. + std::string self_arg_name = "self"; + while (message_descriptor.FindFieldByName(self_arg_name) != nullptr) { + self_arg_name.append("_"); + } + printer_->Print(self_arg_name); + bool has_python_keywords = false; for (int i = 0; i < message_descriptor.field_count(); ++i) { const FieldDescriptor* field_des = message_descriptor.field(i); if (IsPythonKeyword(field_des->name())) { - has_key_words = true; + has_python_keywords = true; continue; } std::string field_name = std::string(field_des->name()); - if (is_first && field_name == "self") { - // See b/144146793 for an example of real code that generates a (self, - // self) method signature. Since repeating a parameter name is illegal in - // Python, we rename the duplicate self. - field_name = "self_"; - } - is_first = false; printer_->Print(", $field_name$: ", "field_name", field_name); Annotate("field_name", field_des); if (field_des->is_repeated() || @@ -533,7 +535,7 @@ void PyiGenerator::PrintMessage( } printer_->Print(" = ..."); } - if (has_key_words) { + if (has_python_keywords) { printer_->Print(", **kwargs"); } printer_->Print(") -> None: ...\n");