Chiebot-Mirror
/
protobuf
mirror of https://github.com/protocolbuffers/protobuf.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Improve .NET debugging of Protobuf messages (#13838)

Browse Source 
Generated .NET protobuf messages override `ToString` and return JSON. By default, the .NET debugger displays the result from `ToString` and wraps it in curly braces, e.g. `{{ "Message": "Hello world" }}`. The double curly braces is an ugly result. People expecting JSON here are confused.

This PR adds a [DebuggerDisplayAttribute](https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.debuggerdisplayattribute?view=net-7.0) to generated messages. The attribute tells the debugger to not quote the ToString value with `nq`.

Before:
![image](https://github.com/protocolbuffers/protobuf/assets/303201/b1cc5f45-4064-4ba0-b266-8894ca0f35c8)

After:
![image](https://github.com/protocolbuffers/protobuf/assets/303201/70f6ffc0-9421-428d-bc22-ead7aa82c37f)

I haven't touched the protoc compiler in a long time. I'm guessing I need to build it and then run it to regenerate the checked-in code. I don't suppose someone else could do that? It would save me a lot of time 😬

Closes #13838

COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/13838 from JamesNK:jamesnk/csharp-message-debuggerdisplay 5222b82cf9
PiperOrigin-RevId: 567659327
pull/14186/head
James Newton-King 1 year ago committed by Copybara-Service
parent a30d71d873
commit e519c62f82
1 changed files with 220 additions and 216 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 222
      src/google/protobuf/compiler/csharp/csharp_message.cc

222
src/google/protobuf/compiler/csharp/csharp_message.cc
Unescape View File

@ -64,18 +64,16 @@ MessageGenerator::MessageGenerator(const Descriptor* descriptor,
}
}
MessageGenerator::~MessageGenerator() {
}
MessageGenerator::~MessageGenerator() {}
std::string MessageGenerator::class_name() {
return descriptor_->name();
}
std::string MessageGenerator::class_name() { return descriptor_->name(); }
std::string MessageGenerator::full_class_name() {
return GetClassName(descriptor_);
}
const std::vector<const FieldDescriptor*>& MessageGenerator::fields_by_number() {
const std::vector<const FieldDescriptor*>&
MessageGenerator::fields_by_number() {
return fields_by_number_;
}
@ -101,13 +99,13 @@ void MessageGenerator::Generate(io::Printer* printer) {
AddSerializableAttribute(printer);
printer->Print(
vars,
"$access_level$ sealed partial class $class_name$ : ");
"[global::System.Diagnostics.DebuggerDisplayAttribute(\"{ToString(),nq}"
"\")]\n");
printer->Print(vars, "$access_level$ sealed partial class $class_name$ : ");
if (has_extension_ranges_) {
printer->Print(vars, "pb::IExtendableMessage<$class_name$>\n");
}
else {
} else {
printer->Print(vars, "pb::IMessage<$class_name$>\n");
}
printer->Print("#if !GOOGLE_PROTOBUF_REFSTRUCT_COMPATIBILITY_MODE\n");
@ -119,16 +117,19 @@ void MessageGenerator::Generate(io::Printer* printer) {
// All static fields and properties
printer->Print(
vars,
"private static readonly pb::MessageParser<$class_name$> _parser = new pb::MessageParser<$class_name$>(() => new $class_name$());\n");
"private static readonly pb::MessageParser<$class_name$> _parser = new "
"pb::MessageParser<$class_name$>(() => new $class_name$());\n");
printer->Print(
"private pb::UnknownFieldSet _unknownFields;\n");
printer->Print("private pb::UnknownFieldSet _unknownFields;\n");
if (has_extension_ranges_) {
if (IsDescriptorProto(descriptor_->file())) {
printer->Print(vars, "internal pb::ExtensionSet<$class_name$> _extensions;\n"); // CustomOptions compatibility
printer->Print(vars,
// CustomOptions compatibility
"internal pb::ExtensionSet<$class_name$> _extensions;\n");
} else {
printer->Print(vars, "private pb::ExtensionSet<$class_name$> _extensions;\n");
printer->Print(vars,
"private pb::ExtensionSet<$class_name$> _extensions;\n");
}
// a read-only property for fast
@ -140,17 +141,19 @@ void MessageGenerator::Generate(io::Printer* printer) {
for (int i = 0; i < has_bit_field_count_; i++) {
// don't use arrays since all arrays are heap allocated, saving allocations
// use ints instead of bytes since bytes lack bitwise operators, saving casts
// use ints instead of bytes since bytes lack bitwise operators, saving
// casts
printer->Print("private int _hasBits$i$;\n", "i", absl::StrCat(i));
}
WriteGeneratedCodeAttributes(printer);
printer->Print(
vars,
"public static pb::MessageParser<$class_name$> Parser { get { return _parser; } }\n\n");
printer->Print(vars,
"public static pb::MessageParser<$class_name$> Parser { get { "
"return _parser; } }\n\n");
// Access the message descriptor via the relevant file descriptor or containing message descriptor.
// Access the message descriptor via the relevant file descriptor or
// containing message descriptor.
if (!descriptor_->containing_type()) {
vars["descriptor_accessor"] =
absl::StrCat(GetReflectionClassName(descriptor_->file()),
@ -162,15 +165,13 @@ void MessageGenerator::Generate(io::Printer* printer) {
}
WriteGeneratedCodeAttributes(printer);
printer->Print(
vars,
printer->Print(vars,
"public static pbr::MessageDescriptor Descriptor {\n"
" get { return $descriptor_accessor$; }\n"
"}\n"
"\n");
WriteGeneratedCodeAttributes(printer);
printer->Print(
vars,
printer->Print(vars,
"pbr::MessageDescriptor pb::IMessage.Descriptor {\n"
" get { return Descriptor; }\n"
"}\n"
@ -178,8 +179,7 @@ void MessageGenerator::Generate(io::Printer* printer) {
// Parameterless constructor and partial OnConstruction method.
WriteGeneratedCodeAttributes(printer);
printer->Print(
vars,
printer->Print(vars,
"public $class_name$() {\n"
" OnConstruction();\n"
"}\n\n"
@ -196,9 +196,9 @@ void MessageGenerator::Generate(io::Printer* printer) {
printer->Print(
"/// <summary>Field number for the \"$field_name$\" field.</summary>\n"
"public const int $field_constant_name$ = $index$;\n",
"field_name", fieldDescriptor->name(),
"field_constant_name", GetFieldConstantName(fieldDescriptor),
"index", absl::StrCat(fieldDescriptor->number()));
"field_name", fieldDescriptor->name(), "field_constant_name",
GetFieldConstantName(fieldDescriptor), "index",
absl::StrCat(fieldDescriptor->number()));
std::unique_ptr<FieldGeneratorBase> generator(
CreateFieldGeneratorInternal(fieldDescriptor));
generator->GenerateMembers(printer);
@ -211,35 +211,33 @@ void MessageGenerator::Generate(io::Printer* printer) {
vars["name"] = UnderscoresToCamelCase(oneof->name(), false);
vars["property_name"] = UnderscoresToCamelCase(oneof->name(), true);
vars["original_name"] = oneof->name();
printer->Print(
vars,
printer->Print(vars,
"private object $name$_;\n"
"/// <summary>Enum of possible cases for the \"$original_name$\" oneof.</summary>\n"
"/// <summary>Enum of possible cases for the "
"\"$original_name$\" oneof.</summary>\n"
"public enum $property_name$OneofCase {\n");
printer->Indent();
printer->Print("None = 0,\n");
for (int j = 0; j < oneof->field_count(); j++) {
const FieldDescriptor* field = oneof->field(j);
printer->Print("$oneof_case_name$ = $index$,\n",
"oneof_case_name", GetOneofCaseName(field),
"index", absl::StrCat(field->number()));
printer->Print("$oneof_case_name$ = $index$,\n", "oneof_case_name",
GetOneofCaseName(field), "index",
absl::StrCat(field->number()));
}
printer->Outdent();
printer->Print("}\n");
// TODO: Should we put the oneof .proto comments here?
// It's unclear exactly where they should go.
printer->Print(
vars,
"private $property_name$OneofCase $name$Case_ = $property_name$OneofCase.None;\n");
printer->Print(vars,
"private $property_name$OneofCase $name$Case_ = "
"$property_name$OneofCase.None;\n");
WriteGeneratedCodeAttributes(printer);
printer->Print(
vars,
printer->Print(vars,
"public $property_name$OneofCase $property_name$Case {\n"
" get { return $name$Case_; }\n"
"}\n\n");
WriteGeneratedCodeAttributes(printer);
printer->Print(
vars,
printer->Print(vars,
"public void Clear$property_name$() {\n"
" $name$Case_ = $property_name$OneofCase.None;\n"
" $name$_ = null;\n"
@ -290,10 +288,10 @@ void MessageGenerator::Generate(io::Printer* printer) {
// Nested messages and enums
if (HasNestedGeneratedTypes()) {
printer->Print(
vars,
printer->Print(vars,
"#region Nested types\n"
"/// <summary>Container for nested types declared in the $class_name$ message type.</summary>\n");
"/// <summary>Container for nested types declared in the "
"$class_name$ message type.</summary>\n");
WriteGeneratedCodeAttributes(printer);
printer->Print("public static partial class Types {\n");
printer->Indent();
@ -304,22 +302,23 @@ void MessageGenerator::Generate(io::Printer* printer) {
for (int i = 0; i < descriptor_->nested_type_count(); i++) {
// Don't generate nested types for maps...
if (!IsMapEntryMessage(descriptor_->nested_type(i))) {
MessageGenerator messageGenerator(
descriptor_->nested_type(i), this->options());
MessageGenerator messageGenerator(descriptor_->nested_type(i),
this->options());
messageGenerator.Generate(printer);
}
}
printer->Outdent();
printer->Print("}\n"
printer->Print(
"}\n"
"#endregion\n"
"\n");
}
if (descriptor_->extension_count() > 0) {
printer->Print(
vars,
printer->Print(vars,
"#region Extensions\n"
"/// <summary>Container for extensions for other messages declared in the $class_name$ message type.</summary>\n");
"/// <summary>Container for extensions for other messages "
"declared in the $class_name$ message type.</summary>\n");
WriteGeneratedCodeAttributes(printer);
printer->Print("public static partial class Extensions {\n");
printer->Indent();
@ -340,10 +339,9 @@ void MessageGenerator::Generate(io::Printer* printer) {
printer->Print("\n");
}
// Helper to work out whether we need to generate a class to hold nested types/enums.
// Only tricky because we don't want to generate map entry types.
bool MessageGenerator::HasNestedGeneratedTypes()
{
// Helper to work out whether we need to generate a class to hold nested
// types/enums. Only tricky because we don't want to generate map entry types.
bool MessageGenerator::HasNestedGeneratedTypes() {
if (descriptor_->enum_type_count() > 0) {
return true;
}
@ -359,9 +357,7 @@ void MessageGenerator::GenerateCloningCode(io::Printer* printer) {
absl::flat_hash_map<absl::string_view, std::string> vars;
WriteGeneratedCodeAttributes(printer);
vars["class_name"] = class_name();
printer->Print(
vars,
"public $class_name$($class_name$ other) : this() {\n");
printer->Print(vars, "public $class_name$($class_name$ other) : this() {\n");
printer->Indent();
for (int i = 0; i < has_bit_field_count_; i++) {
printer->Print("_hasBits$i$ = other._hasBits$i$;\n", "i", absl::StrCat(i));
@ -372,7 +368,8 @@ void MessageGenerator::GenerateCloningCode(io::Printer* printer) {
if (field->real_containing_oneof()) {
continue;
}
std::unique_ptr<FieldGeneratorBase> generator(CreateFieldGeneratorInternal(field));
std::unique_ptr<FieldGeneratorBase> generator(
CreateFieldGeneratorInternal(field));
generator->GenerateCloningCode(printer);
}
// Clone just the right field for each real oneof
@ -384,10 +381,10 @@ void MessageGenerator::GenerateCloningCode(io::Printer* printer) {
printer->Indent();
for (int j = 0; j < oneof->field_count(); j++) {
const FieldDescriptor* field = oneof->field(j);
std::unique_ptr<FieldGeneratorBase> generator(CreateFieldGeneratorInternal(field));
std::unique_ptr<FieldGeneratorBase> generator(
CreateFieldGeneratorInternal(field));
vars["oneof_case_name"] = GetOneofCaseName(field);
printer->Print(
vars,
printer->Print(vars,
"case $property_name$OneofCase.$oneof_case_name$:\n");
printer->Indent();
generator->GenerateCloningCode(printer);
@ -409,15 +406,13 @@ void MessageGenerator::GenerateCloningCode(io::Printer* printer) {
printer->Print("}\n\n");
WriteGeneratedCodeAttributes(printer);
printer->Print(
vars,
printer->Print(vars,
"public $class_name$ Clone() {\n"
" return new $class_name$(this);\n"
"}\n\n");
}
void MessageGenerator::GenerateFreezingCode(io::Printer* printer) {
}
void MessageGenerator::GenerateFreezingCode(io::Printer* printer) {}
void MessageGenerator::GenerateFrameworkMethods(io::Printer* printer) {
absl::flat_hash_map<absl::string_view, std::string> vars;
@ -445,8 +440,10 @@ void MessageGenerator::GenerateFrameworkMethods(io::Printer* printer) {
generator->WriteEquals(printer);
}
for (int i = 0; i < descriptor_->real_oneof_decl_count(); i++) {
printer->Print("if ($property_name$Case != other.$property_name$Case) return false;\n",
"property_name", UnderscoresToCamelCase(descriptor_->oneof_decl(i)->name(), true));
printer->Print(
"if ($property_name$Case != other.$property_name$Case) return false;\n",
"property_name",
UnderscoresToCamelCase(descriptor_->oneof_decl(i)->name(), true));
}
if (has_extension_ranges_) {
printer->Print(
@ -460,7 +457,8 @@ void MessageGenerator::GenerateFrameworkMethods(io::Printer* printer) {
"}\n\n");
// GetHashCode
// Start with a non-zero value to easily distinguish between null and "empty" messages.
// Start with a non-zero value to easily distinguish between null and "empty"
// messages.
WriteGeneratedCodeAttributes(printer);
printer->Print(
"public override int GetHashCode() {\n"
@ -472,8 +470,9 @@ void MessageGenerator::GenerateFrameworkMethods(io::Printer* printer) {
generator->WriteHash(printer);
}
for (int i = 0; i < descriptor_->real_oneof_decl_count(); i++) {
printer->Print("hash ^= (int) $name$Case_;\n",
"name", UnderscoresToCamelCase(descriptor_->oneof_decl(i)->name(), false));
printer->Print(
"hash ^= (int) $name$Case_;\n", "name",
UnderscoresToCamelCase(descriptor_->oneof_decl(i)->name(), false));
}
if (has_extension_ranges_) {
printer->Print(
@ -496,10 +495,10 @@ void MessageGenerator::GenerateFrameworkMethods(io::Printer* printer) {
"}\n\n");
}
void MessageGenerator::GenerateMessageSerializationMethods(io::Printer* printer) {
void MessageGenerator::GenerateMessageSerializationMethods(
io::Printer* printer) {
WriteGeneratedCodeAttributes(printer);
printer->Print(
"public void WriteTo(pb::CodedOutputStream output) {\n");
printer->Print("public void WriteTo(pb::CodedOutputStream output) {\n");
printer->Print("#if !GOOGLE_PROTOBUF_REFSTRUCT_COMPATIBILITY_MODE\n");
printer->Indent();
printer->Print("output.WriteRawMessage(this);\n");
@ -513,7 +512,9 @@ void MessageGenerator::GenerateMessageSerializationMethods(io::Printer* printer)
printer->Print("#if !GOOGLE_PROTOBUF_REFSTRUCT_COMPATIBILITY_MODE\n");
WriteGeneratedCodeAttributes(printer);
printer->Print("void pb::IBufferMessage.InternalWriteTo(ref pb::WriteContext output) {\n");
printer->Print(
"void pb::IBufferMessage.InternalWriteTo(ref pb::WriteContext output) "
"{\n");
printer->Indent();
GenerateWriteToBody(printer, true);
printer->Outdent();
@ -521,8 +522,7 @@ void MessageGenerator::GenerateMessageSerializationMethods(io::Printer* printer)
printer->Print("#endif\n\n");
WriteGeneratedCodeAttributes(printer);
printer->Print(
"public int CalculateSize() {\n");
printer->Print("public int CalculateSize() {\n");
printer->Indent();
printer->Print("int size = 0;\n");
for (int i = 0; i < descriptor_->field_count(); i++) {
@ -548,7 +548,8 @@ void MessageGenerator::GenerateMessageSerializationMethods(io::Printer* printer)
printer->Print("}\n\n");
}
void MessageGenerator::GenerateWriteToBody(io::Printer* printer, bool use_write_context) {
void MessageGenerator::GenerateWriteToBody(io::Printer* printer,
bool use_write_context) {
// Serialize all the fields
for (int i = 0; i < fields_by_number().size(); i++) {
std::unique_ptr<FieldGeneratorBase> generator(
@ -558,9 +559,7 @@ void MessageGenerator::GenerateWriteToBody(io::Printer* printer, bool use_write_
if (has_extension_ranges_) {
// Serialize extensions
printer->Print(
use_write_context
? "if (_extensions != null) {\n"
printer->Print(use_write_context ? "if (_extensions != null) {\n"
" _extensions.WriteTo(ref output);\n"
"}\n"
: "if (_extensions != null) {\n"
@ -569,9 +568,7 @@ void MessageGenerator::GenerateWriteToBody(io::Printer* printer, bool use_write_
}
// Serialize unknown fields
printer->Print(
use_write_context
? "if (_unknownFields != null) {\n"
printer->Print(use_write_context ? "if (_unknownFields != null) {\n"
" _unknownFields.WriteTo(ref output);\n"
"}\n"
: "if (_unknownFields != null) {\n"
@ -589,9 +586,7 @@ void MessageGenerator::GenerateMergingMethods(io::Printer* printer) {
vars["class_name"] = class_name();
WriteGeneratedCodeAttributes(printer);
printer->Print(
vars,
"public void MergeFrom($class_name$ other) {\n");
printer->Print(vars, "public void MergeFrom($class_name$ other) {\n");
printer->Indent();
printer->Print(
"if (other == null) {\n"
@ -603,7 +598,8 @@ void MessageGenerator::GenerateMergingMethods(io::Printer* printer) {
if (field->real_containing_oneof()) {
continue;
}
std::unique_ptr<FieldGeneratorBase> generator(CreateFieldGeneratorInternal(field));
std::unique_ptr<FieldGeneratorBase> generator(
CreateFieldGeneratorInternal(field));
generator->GenerateMergingCode(printer);
}
// Merge oneof fields (for non-synthetic oneofs)
@ -616,11 +612,11 @@ void MessageGenerator::GenerateMergingMethods(io::Printer* printer) {
for (int j = 0; j < oneof->field_count(); j++) {
const FieldDescriptor* field = oneof->field(j);
vars["oneof_case_name"] = GetOneofCaseName(field);
printer->Print(
vars,
printer->Print(vars,
"case $property_name$OneofCase.$oneof_case_name$:\n");
printer->Indent();
std::unique_ptr<FieldGeneratorBase> generator(CreateFieldGeneratorInternal(field));
std::unique_ptr<FieldGeneratorBase> generator(
CreateFieldGeneratorInternal(field));
generator->GenerateMergingCode(printer);
printer->Print("break;\n");
printer->Outdent();
@ -630,12 +626,14 @@ void MessageGenerator::GenerateMergingMethods(io::Printer* printer) {
}
// Merge extensions
if (has_extension_ranges_) {
printer->Print("pb::ExtensionSet.MergeFrom(ref _extensions, other._extensions);\n");
printer->Print(
"pb::ExtensionSet.MergeFrom(ref _extensions, other._extensions);\n");
}
// Merge unknown fields.
printer->Print(
"_unknownFields = pb::UnknownFieldSet.MergeFrom(_unknownFields, other._unknownFields);\n");
"_unknownFields = pb::UnknownFieldSet.MergeFrom(_unknownFields, "
"other._unknownFields);\n");
printer->Outdent();
printer->Print("}\n\n");
@ -655,16 +653,18 @@ void MessageGenerator::GenerateMergingMethods(io::Printer* printer) {
printer->Print("#if !GOOGLE_PROTOBUF_REFSTRUCT_COMPATIBILITY_MODE\n");
WriteGeneratedCodeAttributes(printer);
printer->Print("void pb::IBufferMessage.InternalMergeFrom(ref pb::ParseContext input) {\n");
printer->Print(
"void pb::IBufferMessage.InternalMergeFrom(ref pb::ParseContext input) "
"{\n");
printer->Indent();
GenerateMainParseLoop(printer, true);
printer->Outdent();
printer->Print("}\n"); // method
printer->Print("#endif\n\n");
}
void MessageGenerator::GenerateMainParseLoop(io::Printer* printer, bool use_parse_context) {
void MessageGenerator::GenerateMainParseLoop(io::Printer* printer,
bool use_parse_context) {
absl::flat_hash_map<absl::string_view, std::string> vars;
vars["maybe_ref_input"] = use_parse_context ? "ref input" : "input";
@ -683,14 +683,19 @@ void MessageGenerator::GenerateMainParseLoop(io::Printer* printer, bool use_pars
if (has_extension_ranges_) {
printer->Print(vars,
"default:\n"
" if (!pb::ExtensionSet.TryMergeFieldFrom(ref _extensions, $maybe_ref_input$)) {\n"
" _unknownFields = pb::UnknownFieldSet.MergeFieldFrom(_unknownFields, $maybe_ref_input$);\n"
" if (!pb::ExtensionSet.TryMergeFieldFrom(ref _extensions, "
"$maybe_ref_input$)) {\n"
" _unknownFields = "
"pb::UnknownFieldSet.MergeFieldFrom(_unknownFields, "
"$maybe_ref_input$);\n"
" }\n"
" break;\n");
} else {
printer->Print(vars,
printer->Print(
vars,
"default:\n"
" _unknownFields = pb::UnknownFieldSet.MergeFieldFrom(_unknownFields, $maybe_ref_input$);\n"
" _unknownFields = pb::UnknownFieldSet.MergeFieldFrom(_unknownFields, "
"$maybe_ref_input$);\n"
" break;\n");
}
for (int i = 0; i < fields_by_number().size(); i++) {
@ -698,17 +703,14 @@ void MessageGenerator::GenerateMainParseLoop(io::Printer* printer, bool use_pars
internal::WireFormatLite::WireType wt =
internal::WireFormat::WireTypeForFieldType(field->type());
uint32_t tag = internal::WireFormatLite::MakeTag(field->number(), wt);
// Handle both packed and unpacked repeated fields with the same Read*Array call;
// the two generated cases are the packed and unpacked tags.
// Handle both packed and unpacked repeated fields with the same Read*Array
// call; the two generated cases are the packed and unpacked tags.
// TODO: Check that is_packable is equivalent to
// is_repeated && wt in { VARINT, FIXED32, FIXED64 }.
// It looks like it is...
if (field->is_packable()) {
printer->Print(
"case $packed_tag$:\n",
"packed_tag",
absl::StrCat(
internal::WireFormatLite::MakeTag(
printer->Print("case $packed_tag$:\n", "packed_tag",
absl::StrCat(internal::WireFormatLite::MakeTag(
field->number(),
internal::WireFormatLite::WIRETYPE_LENGTH_DELIMITED)));
}
@ -728,7 +730,8 @@ void MessageGenerator::GenerateMainParseLoop(io::Printer* printer, bool use_pars
printer->Print("}\n"); // while
}
// it's a waste of space to track presence for all values, so we only track them if they're not nullable
// it's a waste of space to track presence for all values, so we only track them
// if they're not nullable
int MessageGenerator::GetPresenceIndex(const FieldDescriptor* descriptor) {
if (!RequiresPresenceBit(descriptor)) {
return -1;
@ -751,7 +754,8 @@ int MessageGenerator::GetPresenceIndex(const FieldDescriptor* descriptor) {
FieldGeneratorBase* MessageGenerator::CreateFieldGeneratorInternal(
const FieldDescriptor* descriptor) {
return CreateFieldGenerator(descriptor, GetPresenceIndex(descriptor), this->options());
return CreateFieldGenerator(descriptor, GetPresenceIndex(descriptor),
this->options());
}
} // namespace csharp

Write Preview
Loading…
Cancel
Save