Skip to content

Commit

Permalink
Breaking change: Remove deprecated syntax accessor.
Browse files Browse the repository at this point in the history
This has been replaced by edition and feature getters.  Any code depending on syntax will likely be broken by editions files, and should be migrated to the finer-grained feature helpers.

PiperOrigin-RevId: 589866745
  • Loading branch information
mkruskal-google authored and Copybara-Service committed Dec 11, 2023
1 parent a303ccb commit fd40c87
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 77 deletions.
41 changes: 4 additions & 37 deletions python/descriptor.c
Expand Up @@ -648,18 +648,6 @@ static PyObject* PyUpb_Descriptor_GetOneofsByName(PyObject* _self,
return PyUpb_ByNameMap_New(&funcs, self->def, self->pool);
}

static PyObject* PyUpb_Descriptor_GetSyntax(PyObject* self, void* closure) {
PyErr_WarnEx(NULL,
"descriptor.syntax is deprecated. It will be removed soon. "
"Most usages are checking field descriptors. Consider to use "
"has_presence, is_packed on field descriptors.",
1);
const upb_MessageDef* msgdef = PyUpb_Descriptor_GetDef(self);
const char* syntax =
upb_MessageDef_Syntax(msgdef) == kUpb_Syntax_Proto2 ? "proto2" : "proto3";
return PyUnicode_InternFromString(syntax);
}

static PyGetSetDef PyUpb_Descriptor_Getters[] = {
{"name", PyUpb_Descriptor_GetName, NULL, "Last name"},
{"full_name", PyUpb_Descriptor_GetFullName, NULL, "Full name"},
Expand Down Expand Up @@ -694,13 +682,6 @@ static PyGetSetDef PyUpb_Descriptor_Getters[] = {
"Containing type"},
{"is_extendable", PyUpb_Descriptor_GetIsExtendable, NULL},
{"has_options", PyUpb_Descriptor_GetHasOptions, NULL, "Has Options"},
// begin:github_only
{"syntax", &PyUpb_Descriptor_GetSyntax, NULL, "Syntax"},
// end:github_only
// begin:google_only
// // TODO Use this to open-source syntax deprecation.
// {"deprecated_syntax", &PyUpb_Descriptor_GetSyntax, NULL, "Syntax"},
// end:google_only
{NULL}};

static PyMethodDef PyUpb_Descriptor_Methods[] = {
Expand Down Expand Up @@ -1384,17 +1365,10 @@ static PyObject* PyUpb_FileDescriptor_GetPublicDependencies(PyObject* _self,
return PyUpb_GenericSequence_New(&funcs, self->def, self->pool);
}

static PyObject* PyUpb_FileDescriptor_GetSyntax(PyObject* _self,
void* closure) {
PyErr_WarnEx(NULL,
"descriptor.syntax is deprecated. It will be removed soon. "
"Most usages are checking field descriptors. Consider to use "
"has_presence, is_packed on field descriptors.",
1);
static PyObject* PyUpb_FileDescriptor_GetEdition(PyObject* _self,
void* closure) {
PyUpb_DescriptorBase* self = (void*)_self;
const char* syntax =
upb_FileDef_Syntax(self->def) == kUpb_Syntax_Proto2 ? "proto2" : "proto3";
return PyUnicode_FromString(syntax);
return PyLong_FromLong(upb_FileDef_Edition(self->def));
}

static PyObject* PyUpb_FileDescriptor_GetHasOptions(PyObject* _self,
Expand Down Expand Up @@ -1445,14 +1419,7 @@ static PyGetSetDef PyUpb_FileDescriptor_Getters[] = {
{"public_dependencies", PyUpb_FileDescriptor_GetPublicDependencies, NULL,
"Dependencies"},
{"has_options", PyUpb_FileDescriptor_GetHasOptions, NULL, "Has Options"},
// begin:github_only
{"syntax", PyUpb_FileDescriptor_GetSyntax, (setter)NULL, "Syntax"},
// end:github_only
// begin:google_only
// // TODO Use this to open-source syntax deprecation.
// {"deprecated_syntax", PyUpb_FileDescriptor_GetSyntax, (setter)NULL,
// "Syntax"},
// end:google_only
{"edition", PyUpb_FileDescriptor_GetEdition, (setter)NULL, "Edition"},
{NULL},
};

Expand Down
28 changes: 12 additions & 16 deletions python/google/protobuf/descriptor.py
Expand Up @@ -1179,24 +1179,23 @@ class FileDescriptor(DescriptorBase):
Attributes:
name (str): Name of file, relative to root of source tree.
package (str): Name of the package
syntax (str): string indicating syntax of the file (can be "proto2" or
"proto3")
edition (Edition): Enum value indicating edition of the file
serialized_pb (bytes): Byte string of serialized
:class:`descriptor_pb2.FileDescriptorProto`.
dependencies (list[FileDescriptor]): List of other :class:`FileDescriptor`
objects this :class:`FileDescriptor` depends on.
public_dependencies (list[FileDescriptor]): A subset of
:attr:`dependencies`, which were declared as "public".
message_types_by_name (dict(str, Descriptor)): Mapping from message names
to their :class:`Descriptor`.
message_types_by_name (dict(str, Descriptor)): Mapping from message names to
their :class:`Descriptor`.
enum_types_by_name (dict(str, EnumDescriptor)): Mapping from enum names to
their :class:`EnumDescriptor`.
extensions_by_name (dict(str, FieldDescriptor)): Mapping from extension
names declared at file scope to their :class:`FieldDescriptor`.
services_by_name (dict(str, ServiceDescriptor)): Mapping from services'
names to their :class:`ServiceDescriptor`.
pool (DescriptorPool): The pool this descriptor belongs to. When not
passed to the constructor, the global default pool is used.
pool (DescriptorPool): The pool this descriptor belongs to. When not passed
to the constructor, the global default pool is used.
"""

if _USE_C_DESCRIPTORS:
Expand Down Expand Up @@ -1260,7 +1259,6 @@ def __init__(
self.message_types_by_name = {}
self.name = name
self.package = package
self._deprecated_syntax = syntax or "proto2"
self.serialized_pb = serialized_pb

self.enum_types_by_name = {}
Expand All @@ -1269,15 +1267,6 @@ def __init__(
self.dependencies = (dependencies or [])
self.public_dependencies = (public_dependencies or [])

@property
def syntax(self):
warnings.warn(
'descriptor.syntax is deprecated. It will be removed'
' soon. Most usages are checking field descriptors. Consider to use'
' has_presence, is_packed on field descriptors.'
)
return self._deprecated_syntax

def CopyToProto(self, proto):
"""Copies this to a descriptor_pb2.FileDescriptorProto.
Expand All @@ -1290,6 +1279,13 @@ def CopyToProto(self, proto):
def _parent(self):
return None

@property
def edition(self):
# pylint: disable=g-import-not-at-top
from google.protobuf import descriptor_pb2

return descriptor_pb2.Edition.Value(self._edition)


def _ParseOptions(message, string):
"""Parses serialized options.
Expand Down
4 changes: 3 additions & 1 deletion python/google/protobuf/internal/descriptor_test.py
Expand Up @@ -539,7 +539,9 @@ def testFileDescriptor(self):
self.assertEqual(self.my_file.package, 'protobuf_unittest')
self.assertEqual(self.my_file.pool, self.pool)
self.assertFalse(self.my_file.has_options)
self.assertEqual(self.my_file.syntax, 'proto2')
self.assertEqual(
self.my_file.edition, descriptor_pb2.Edition.EDITION_PROTO2
)
file_proto = descriptor_pb2.FileDescriptorProto()
self.my_file.CopyToProto(file_proto)
self.assertEqual(self.my_file.serialized_pb,
Expand Down
26 changes: 3 additions & 23 deletions python/google/protobuf/pyext/descriptor.cc
Expand Up @@ -22,7 +22,6 @@
#include "absl/log/absl_check.h"
#include "absl/strings/string_view.h"
#include "google/protobuf/descriptor.h"
#include "google/protobuf/descriptor_legacy.h"
#include "google/protobuf/dynamic_message.h"
#include "google/protobuf/io/coded_stream.h"
#include "google/protobuf/pyext/descriptor_containers.h"
Expand Down Expand Up @@ -696,17 +695,6 @@ static PyObject* EnumValueName(PyBaseDescriptor *self, PyObject *args) {
return PyString_FromCppString(enum_value->name());
}

static PyObject* GetSyntax(PyBaseDescriptor *self, void *closure) {
PyErr_WarnEx(nullptr,
"descriptor.syntax is deprecated. It will be removed soon. "
"Most usages are checking field descriptors. Consider to use "
"has_presence, is_packed on field descriptors.",
1);
std::string syntax(FileDescriptorLegacy::SyntaxName(
FileDescriptorLegacy(_GetDescriptor(self)->file()).syntax()));
return PyUnicode_InternFromString(syntax.c_str());
}

static PyGetSetDef Getters[] = {
{"name", (getter)GetName, nullptr, "Last name"},
{"full_name", (getter)GetFullName, nullptr, "Full name"},
Expand Down Expand Up @@ -743,7 +731,6 @@ static PyGetSetDef Getters[] = {
{"_options", (getter) nullptr, (setter)SetOptions, "Options"},
{"_serialized_options", (getter) nullptr, (setter)SetSerializedOptions,
"Serialized Options"},
{"syntax", (getter)GetSyntax, (setter) nullptr, "Syntax"},
{nullptr},
};

Expand Down Expand Up @@ -1542,15 +1529,8 @@ static int SetSerializedOptions(PyFileDescriptor *self, PyObject *value,
return CheckCalledFromGeneratedFile("_serialized_options");
}

static PyObject* GetSyntax(PyFileDescriptor *self, void *closure) {
PyErr_WarnEx(nullptr,
"descriptor.syntax is deprecated. It will be removed soon. "
"Most usages are checking field descriptors. Consider to use "
"has_presence, is_packed on field descriptors.",
1);
std::string syntax(FileDescriptorLegacy::SyntaxName(
FileDescriptorLegacy(_GetDescriptor(self)).syntax()));
return PyUnicode_InternFromString(syntax.c_str());
static PyObject* GetEdition(PyFileDescriptor* self, void* closure) {
return PyLong_FromLong(_GetDescriptor(self)->edition());
}

static PyObject* CopyToProto(PyFileDescriptor *self, PyObject *target) {
Expand Down Expand Up @@ -1579,7 +1559,7 @@ static PyGetSetDef Getters[] = {
{"_options", (getter) nullptr, (setter)SetOptions, "Options"},
{"_serialized_options", (getter) nullptr, (setter)SetSerializedOptions,
"Serialized Options"},
{"syntax", (getter)GetSyntax, (setter) nullptr, "Syntax"},
{"edition", (getter)GetEdition, (setter) nullptr, "Edition"},
{nullptr},
};

Expand Down

0 comments on commit fd40c87

Please sign in to comment.