Python: don't emit ClassVar for a file's top-level extensions (#29222)
Fixes #29221.
### Problem
`PyiGenerator::PrintExtensions` is a template instantiated for two different scopes, and emits `_ClassVar[int]` for both:
| call site | descriptor | output scope | `ClassVar` valid |
| --- | --- | --- | --- |
| `pyi_generator.cc:481` `PrintExtensions(message_descriptor)` | `Descriptor` | class body | yes |
| `pyi_generator.cc:656` `PrintExtensions(*public_dep)` | `FileDescriptor` | module | **no** |
| `pyi_generator.cc:669` `PrintExtensions(*file_)` | `FileDescriptor` | module | **no** |
PEP 526 restricts `ClassVar` to class bodies, so a proto declaring a file-level extension generates a `.pyi` that does not type-check.
Given:
```proto
syntax = "proto2";
package probe;
import "google/protobuf/descriptor.proto";
extend google.protobuf.FileOptions {
optional bool top_level_ext = 50001;
}
message Holder {
extend google.protobuf.MessageOptions {
optional bool nested_ext = 50002;
}
}
```
before:
```python
DESCRIPTOR: _descriptor.FileDescriptor
TOP_LEVEL_EXT_FIELD_NUMBER: _ClassVar[int] # module scope
top_level_ext: _descriptor.FieldDescriptor
class Holder(_message.Message):
__slots__ = ()
NESTED_EXT_FIELD_NUMBER: _ClassVar[int] # class body
nested_ext: _descriptor.FieldDescriptor
def __init__(self) -> None: ...
```
```
ext_pb2.pyi:7:1 - error: "ClassVar" is not allowed in this context (reportInvalidTypeForm)
```
Only the module-scope constant is flagged; the nested one is correct as-is.
### Change
`PrintEnumValues` in the same file already carries a `bool is_classvar = false` parameter for exactly this distinction. This gives `PrintExtensions` the same parameter and passes `true` only from the message call site, so the two `FileDescriptor` instantiations fall through to a plain `int` annotation. No template specialisation needed.
After:
```python
TOP_LEVEL_EXT_FIELD_NUMBER: int
```
The nested case is unchanged.
### Notes
- Affects any proto with a top-level `extend`, not only extension-only files — the repro above also contains a message.
- `.pyi` golden/expected outputs containing top-level extensions will need regenerating.
- I have not built or run the test suite locally; relying on CI for that.
Closes #29222
COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/29222 from folded:pyi-no-classvar-for-toplevel-extensions 53f1b7bb99829727d012b0cf5567ff7217d5aaef
FUTURE_COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/29222 from folded:pyi-no-classvar-for-toplevel-extensions 53f1b7bb99829727d012b0cf5567ff7217d5aaef
PiperOrigin-RevId: 970646537
diff --git a/src/google/protobuf/compiler/python/plugin_unittest.cc b/src/google/protobuf/compiler/python/plugin_unittest.cc
index e792566..0b8ae69 100644
--- a/src/google/protobuf/compiler/python/plugin_unittest.cc
+++ b/src/google/protobuf/compiler/python/plugin_unittest.cc
@@ -15,6 +15,7 @@
#include "google/protobuf/testing/file.h"
#include <gtest/gtest.h>
#include "absl/log/absl_check.h"
+#include "absl/strings/match.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/str_split.h"
#include "absl/strings/substitute.h"
@@ -22,6 +23,7 @@
#include "google/protobuf/compiler/command_line_interface_tester.h"
#include "google/protobuf/compiler/cpp/generator.h"
#include "google/protobuf/compiler/python/generator.h"
+#include "google/protobuf/compiler/python/pyi_generator.h"
#include "google/protobuf/cpp_features.pb.h"
#include "google/protobuf/io/printer.h"
#include "google/protobuf/io/zero_copy_stream.h"
@@ -105,6 +107,63 @@
EXPECT_TRUE(found_expected_import);
}
+// `_ClassVar` is only a valid annotation inside a class body: at module level a
+// type checker rejects the whole stub. Nothing in the generator's structure
+// enforces that, so assert it over the generated file rather than pinning the
+// annotation each declaration happens to carry today.
+TEST(PythonPyiTest, ClassVarIsOnlyUsedInsideAClassBody) {
+ ABSL_CHECK_OK(
+ File::SetContents(absl::StrCat(::testing::TempDir(), "/extensions.proto"),
+ "syntax = \"proto2\";\n"
+ "package foo;\n"
+ "message Extendable {\n"
+ " extensions 1000 to max;\n"
+ "}\n"
+ "extend Extendable {\n"
+ " optional int32 top_level_ext = 1000;\n"
+ "}\n"
+ "message Holder {\n"
+ " extend Extendable {\n"
+ " optional int32 nested_ext = 1001;\n"
+ " }\n"
+ "}\n",
+ true));
+
+ compiler::CommandLineInterface cli;
+ cli.SetInputsAreProtoPathRelative(true);
+ python::PyiGenerator pyi_generator;
+ cli.RegisterGenerator("--pyi_out", &pyi_generator, "");
+ std::string proto_path = absl::StrCat("-I", ::testing::TempDir());
+ std::string pyi_out = absl::StrCat("--pyi_out=", ::testing::TempDir());
+ const char* argv[] = {"protoc", proto_path.c_str(), "-I.", pyi_out.c_str(),
+ "extensions.proto"};
+ ASSERT_EQ(0, cli.Run(5, argv));
+
+ std::string output;
+ ABSL_CHECK_OK(File::GetContents(
+ absl::StrCat(::testing::TempDir(), "/extensions_pb2.pyi"), &output,
+ true));
+
+ bool annotates_a_top_level_extension = false;
+ bool annotates_a_nested_extension = false;
+ for (absl::string_view line : absl::StrSplit(output, '\n')) {
+ if (absl::StartsWith(line, "TOP_LEVEL_EXT_FIELD_NUMBER")) {
+ annotates_a_top_level_extension = true;
+ }
+ if (absl::StrContains(line, "NESTED_EXT_FIELD_NUMBER")) {
+ annotates_a_nested_extension = true;
+ }
+ if (absl::StrContains(line, "_ClassVar[")) {
+ EXPECT_TRUE(absl::StartsWith(line, " "))
+ << "module-level _ClassVar annotation: " << line;
+ }
+ }
+ // Both extensions reach the stub, so neither branch of the assertion above is
+ // vacuous.
+ EXPECT_TRUE(annotates_a_top_level_extension);
+ EXPECT_TRUE(annotates_a_nested_extension);
+}
+
class PythonGeneratorTest : public CommandLineInterfaceTester,
public testing::WithParamInterface<bool> {
protected:
diff --git a/src/google/protobuf/compiler/python/pyi_generator.cc b/src/google/protobuf/compiler/python/pyi_generator.cc
index 541bec0..dfa4bc4 100644
--- a/src/google/protobuf/compiler/python/pyi_generator.cc
+++ b/src/google/protobuf/compiler/python/pyi_generator.cc
@@ -361,14 +361,21 @@
}
template <typename DescriptorT>
-void PyiGenerator::PrintExtensions(const DescriptorT& descriptor) const {
+void PyiGenerator::PrintExtensions(const DescriptorT& descriptor,
+ bool is_classvar) const {
for (int i = 0; i < descriptor.extension_count(); ++i) {
const FieldDescriptor* extension_field = descriptor.extension(i);
std::string constant_name =
absl::StrCat(extension_field->name(), "_FIELD_NUMBER");
absl::AsciiStrToUpper(&constant_name);
- printer_->Print("$constant_name$: _ClassVar[int]\n",
- "constant_name", constant_name);
+ // ClassVar is only a valid annotation inside a class body, so it is used
+ // for a message's nested extensions and not for a file's top-level ones.
+ if (is_classvar) {
+ printer_->Print("$constant_name$: _ClassVar[int]\n", "constant_name",
+ constant_name);
+ } else {
+ printer_->Print("$constant_name$: int\n", "constant_name", constant_name);
+ }
Annotate("constant_name", extension_field);
printer_->Print("$name$: _descriptor.FieldDescriptor\n",
"name", extension_field->name());
@@ -478,7 +485,7 @@
PrintMessage(*message_descriptor.nested_type(i), true);
}
- PrintExtensions(message_descriptor);
+ PrintExtensions(message_descriptor, /* is_classvar = */ true);
// Prints field number
for (int i = 0; i < message_descriptor.field_count(); ++i) {
diff --git a/src/google/protobuf/compiler/python/pyi_generator.h b/src/google/protobuf/compiler/python/pyi_generator.h
index 1beb4d1..adf752c 100644
--- a/src/google/protobuf/compiler/python/pyi_generator.h
+++ b/src/google/protobuf/compiler/python/pyi_generator.h
@@ -73,7 +73,8 @@
void PrintEnumValues(const EnumDescriptor& enum_descriptor,
bool is_classvar = false) const;
template <typename DescriptorT>
- void PrintExtensions(const DescriptorT& descriptor) const;
+ void PrintExtensions(const DescriptorT& descriptor,
+ bool is_classvar = false) const;
void PrintMessages() const;
void PrintMessage(const Descriptor& message_descriptor, bool is_nested) const;
void PrintServices() const;
