Skip to content

Commit a260db2

Browse files
devbwwdevbww
authored
feat(generator): handle proto fields named with C++ keywords (#8063)
Motivated by a new service with a proto containing a "namespace" field. The C++ proto compiler adds a trailing underscore to such fields and their associated accessors, so we should do the same. And we should also do it in the parameters to "method_signature" overloads, and in generated @param annotations.
1 parent a9fd87c commit a260db2

File tree

2 files changed

+64
-23
lines changed

2 files changed

+64
-23
lines changed

‎generator/internal/descriptor_utils.cc‎

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,14 @@
3535
#include "generator/internal/stub_generator.h"
3636
#include <google/longrunning/operations.pb.h>
3737
#include <google/protobuf/compiler/code_generator.h>
38+
#include <google/protobuf/compiler/cpp/cpp_names.h>
3839
#include <regex>
3940
#include <string>
4041

4142
using ::google::protobuf::FieldDescriptor;
4243
using ::google::protobuf::MethodDescriptor;
4344
using ::google::protobuf::ServiceDescriptor;
45+
using ::google::protobuf::compiler::cpp::FieldName;
4446

4547
namespace google {
4648
namespace cloud {
@@ -186,41 +188,43 @@ void SetMethodSignatureMethodVars(
186188
absl::StripAsciiWhitespace(&parameter);
187189
google::protobuf::FieldDescriptor const* parameter_descriptor =
188190
input_type->FindFieldByName(parameter);
191+
auto const parameter_name = FieldName(parameter_descriptor);
189192
if (parameter_descriptor->is_map()) {
190193
method_signature += absl::StrFormat(
191194
"std::map<%s, %s> const& %s",
192195
CppTypeToString(parameter_descriptor->message_type()->map_key()),
193196
CppTypeToString(parameter_descriptor->message_type()->map_value()),
194-
parameter);
197+
parameter_name);
195198
method_request_setters += absl::StrFormat(
196-
" *request.mutable_%s() = {%s.begin(), %s.end()};\n", parameter,
197-
parameter, parameter);
199+
" *request.mutable_%s() = {%s.begin(), %s.end()};\n",
200+
parameter_name, parameter_name, parameter_name);
198201
} else if (parameter_descriptor->is_repeated()) {
199-
method_signature +=
200-
absl::StrFormat("std::vector<%s> const& %s",
201-
CppTypeToString(parameter_descriptor), parameter);
202+
method_signature += absl::StrFormat(
203+
"std::vector<%s> const& %s", CppTypeToString(parameter_descriptor),
204+
parameter_name);
202205
method_request_setters += absl::StrFormat(
203-
" *request.mutable_%s() = {%s.begin(), %s.end()};\n", parameter,
204-
parameter, parameter);
206+
" *request.mutable_%s() = {%s.begin(), %s.end()};\n",
207+
parameter_name, parameter_name, parameter_name);
205208
} else if (parameter_descriptor->type() ==
206209
FieldDescriptor::TYPE_MESSAGE) {
207210
method_signature += absl::StrFormat(
208-
"%s const& %s", CppTypeToString(parameter_descriptor), parameter);
211+
"%s const& %s", CppTypeToString(parameter_descriptor),
212+
parameter_name);
209213
method_request_setters += absl::StrFormat(
210-
" *request.mutable_%s() = %s;\n", parameter, parameter);
214+
" *request.mutable_%s() = %s;\n", parameter_name, parameter_name);
211215
} else {
212216
switch (parameter_descriptor->cpp_type()) {
213217
case FieldDescriptor::CPPTYPE_STRING:
214218
method_signature += absl::StrFormat(
215219
"%s const& %s", CppTypeToString(parameter_descriptor),
216-
parameter);
220+
parameter_name);
217221
break;
218222
default:
219223
method_signature += absl::StrFormat(
220-
"%s %s", CppTypeToString(parameter_descriptor), parameter);
224+
"%s %s", CppTypeToString(parameter_descriptor), parameter_name);
221225
}
222-
method_request_setters +=
223-
absl::StrFormat(" request.set_%s(%s);\n", parameter, parameter);
226+
method_request_setters += absl::StrFormat(
227+
" request.set_%s(%s);\n", parameter_name, parameter_name);
224228
}
225229
method_signature += ", ";
226230
}
@@ -240,7 +244,14 @@ void SetResourceRoutingMethodVars(
240244
method_vars["method_request_url_substitution"] = result->url_substitution;
241245
std::string param = result->param_key;
242246
method_vars["method_request_param_key"] = param;
243-
std::vector<std::string> chunks = absl::StrSplit(param, '.');
247+
std::vector<std::string> chunks;
248+
auto const* input_type = method.input_type();
249+
for (auto const sv : absl::StrSplit(param, '.')) {
250+
auto const chunk = std::string(sv);
251+
auto const* chunk_descriptor = input_type->FindFieldByName(chunk);
252+
chunks.push_back(FieldName(chunk_descriptor));
253+
input_type = chunk_descriptor->message_type();
254+
}
244255
method_vars["method_request_param_value"] =
245256
absl::StrJoin(chunks, "().") + "()";
246257
method_vars["method_request_body"] = result->body;
@@ -334,7 +345,7 @@ std::string FormatApiMethodSignatureParameters(
334345
EscapePrinterDelimiter(ChompByValue(loc.leading_comments)),
335346
{{"\n\n", "\n /// "}, {"\n", "\n /// "}});
336347
absl::StrAppendFormat(&parameter_comments, " /// @param %s %s\n",
337-
parameter, std::move(comment));
348+
FieldName(parameter_descriptor), std::move(comment));
338349
}
339350
return parameter_comments;
340351
}

‎generator/internal/descriptor_utils_test.cc‎

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -352,8 +352,8 @@ char const* const kServiceProto =
352352
"import \"google/longrunning/operation.proto\";\n"
353353
"// Leading comments about message Foo.\n"
354354
"message Foo {\n"
355-
" // baz field$ comment.\n"
356-
" string baz = 1;\n"
355+
" // name field$ comment.\n"
356+
" string name = 1;\n"
357357
" // labels $field comment.\n"
358358
" map<string, string> labels = 2;\n"
359359
"}\n"
@@ -370,19 +370,28 @@ char const* const kServiceProto =
370370
" bool toggle = 4;\n"
371371
" string title = 5;\n"
372372
" repeated SwallowType swallow_types = 6;\n"
373+
" string parent = 7;\n"
373374
"}\n"
374375
"// Leading comments about message Empty.\n"
375376
"message Empty {}\n"
376377
"// Leading comments about message PaginatedInput.\n"
377378
"message PaginatedInput {\n"
378379
" int32 page_size = 1;\n"
379380
" string page_token = 2;\n"
381+
" string name = 3;\n"
380382
"}\n"
381383
"// Leading comments about message PaginatedOutput.\n"
382384
"message PaginatedOutput {\n"
383385
" string next_page_token = 1;\n"
384386
" repeated Bar repeated_field = 2;\n"
385387
"}\n"
388+
"message Namespace {\n"
389+
" string name = 1;\n"
390+
"}\n"
391+
"message NamespaceRequest {\n"
392+
" // namespace $field comment.\n"
393+
" Namespace namespace = 1;\n"
394+
"}\n"
386395
"// Leading comments about service Service.\n"
387396
"service Service {\n"
388397
" // Leading comments about rpc Method0$.\n"
@@ -440,7 +449,7 @@ char const* const kServiceProto =
440449
" get: \"/v1/{name=projects/*/instances/*/databases/*}\"\n"
441450
" };\n"
442451
" option (google.api.method_signature) = \"labels\";\n"
443-
" option (google.api.method_signature) = \"baz,labels\";\n"
452+
" option (google.api.method_signature) = \"name,labels\";\n"
444453
" }\n"
445454
" // Leading comments about rpc Method7.\n"
446455
" rpc Method7(Bar) returns (google.longrunning.Operation) {\n"
@@ -453,6 +462,15 @@ char const* const kServiceProto =
453462
" metadata_type: \"google.protobuf.Method2Metadata\"\n"
454463
" };\n"
455464
" }\n"
465+
" // Leading comments about rpc Method8.\n"
466+
" rpc Method8(NamespaceRequest) returns (Empty) {\n"
467+
" option (google.api.http) = {\n"
468+
" patch: "
469+
"\"/v1/{namespace.name=projects/*/locations/*/namespaces/*}\"\n"
470+
" body: \"namespace\"\n"
471+
" };\n"
472+
" option (google.api.method_signature) = \"namespace\";\n"
473+
" }\n"
456474
"}\n";
457475

458476
struct MethodVarsTestValues {
@@ -539,11 +557,11 @@ TEST_F(CreateMethodVarsTest, FormatMethodCommentsMethodSignature) {
539557
)"""));
540558
EXPECT_THAT(
541559
FormatMethodCommentsMethodSignature(
542-
*service_file_descriptor->service(0)->method(6), "baz,labels"),
560+
*service_file_descriptor->service(0)->method(6), "name,labels"),
543561
HasSubstr(R"""( ///
544562
/// Leading comments about rpc $$Method6.
545563
///
546-
/// @param baz baz field$$ comment.
564+
/// @param name name field$$ comment.
547565
/// @param labels labels $$field comment.
548566
/// @param options Optional. Operation options.
549567
///
@@ -579,7 +597,7 @@ INSTANTIATE_TEST_SUITE_P(
579597
MethodVarsTestValues("google.protobuf.Service.Method0",
580598
"method_return_doxygen_link",
581599
"@googleapis_link{google::protobuf::Empty,google/"
582-
"foo/v1/service.proto#L29}"),
600+
"foo/v1/service.proto#L30}"),
583601
// Method1
584602
MethodVarsTestValues("google.protobuf.Service.Method1", "method_name",
585603
"Method1"),
@@ -740,7 +758,19 @@ INSTANTIATE_TEST_SUITE_P(
740758
MethodVarsTestValues("google.protobuf.Service.Method7",
741759
"method_longrunning_deduced_return_doxygen_link",
742760
"@googleapis_link{google::protobuf::Bar,google/"
743-
"foo/v1/service.proto#L15}")),
761+
"foo/v1/service.proto#L15}"),
762+
// Method8
763+
MethodVarsTestValues("google.protobuf.Service.Method8",
764+
"method_signature0",
765+
"google::protobuf::Namespace const& namespace_, "),
766+
MethodVarsTestValues("google.protobuf.Service.Method8",
767+
"method_request_setters0",
768+
" *request.mutable_namespace_() = namespace_;\n"),
769+
MethodVarsTestValues("google.protobuf.Service.Method8",
770+
"method_request_param_key", "namespace.name"),
771+
MethodVarsTestValues("google.protobuf.Service.Method8",
772+
"method_request_param_value",
773+
"namespace_().name()")),
744774
[](testing::TestParamInfo<CreateMethodVarsTest::ParamType> const& info) {
745775
std::vector<std::string> pieces = absl::StrSplit(info.param.method, '.');
746776
return pieces.back() + "_" + info.param.vars_key;

0 commit comments

Comments
 (0)