Fix pyi generation for messages with a field named "self"

This should rename the self parameter name to "self_" if any field is named "self", not rename the corresponding keyword parameter name to "self_" if the first field is named "self".

PiperOrigin-RevId: 681872250

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'
 

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"];
+}

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");