refactor(generator): prepare for upcoming string_view return type change (#14997)

scotthart · web-flow · commit ad247a4fd5e8 · 2025-02-25T10:53:51.000-05:00
diff --git a/generator/internal/client_generator.cc b/generator/internal/client_generator.cc
@@ -231,8 +231,8 @@ R"""(  std::unique_ptr<::google::cloud::AsyncStreamingReadWriteRpc<
       if (get_iam_policy_extension_ && set_iam_policy_extension_ == &method) {
         auto response_type = ProtoNameToCppName(
             set_iam_policy_extension_->output_type()->full_name());
-        auto set_method_name = set_iam_policy_extension_->name();
-        auto get_method_name = get_iam_policy_extension_->name();
+        std::string set_method_name{set_iam_policy_extension_->name()};
+        std::string get_method_name{get_iam_policy_extension_->name()};
         HeaderPrint({
             PredicatedFragment<void>(""),
             {R"""(
@@ -533,8 +533,8 @@ std::unique_ptr<::google::cloud::AsyncStreamingReadWriteRpc<
       if (get_iam_policy_extension_ && set_iam_policy_extension_ == &method) {
         auto response_type = ProtoNameToCppName(
             set_iam_policy_extension_->output_type()->full_name());
-        auto set_method_name = set_iam_policy_extension_->name();
-        auto get_method_name = get_iam_policy_extension_->name();
+        std::string set_method_name{set_iam_policy_extension_->name()};
+        std::string get_method_name{get_iam_policy_extension_->name()};
         CcPrint({
             PredicatedFragment<void>(""),
             {"\nStatusOr<" + response_type + ">\n"},
diff --git a/generator/internal/descriptor_utils.cc b/generator/internal/descriptor_utils.cc
@@ -572,7 +572,7 @@ std::string CppTypeToString(FieldDescriptor const* field) {
   GCP_LOG(FATAL) << "FieldDescriptor " << field->cpp_type_name()
                  << " not handled";
   /*NOTREACHED*/
-  return field->cpp_type_name();
+  return std::string{field->cpp_type_name()};
 }
 
 std::string FormatMethodCommentsMethodSignature(
@@ -880,13 +880,13 @@ std::map<std::string, VarsDictionary> CreateMethodVars(
     if (method.options().HasExtension(google::api::http)) {
       http_rule = method.options().GetExtension(google::api::http);
     }
-    service_methods_vars[method.full_name()] =
+    service_methods_vars[std::string{method.full_name()}] =
         GetMethodVars(service, service_config, method, http_rule, "grpc_stub",
                       idempotency_overrides, omitted_rpcs);
   }
   for (auto const& mixin_method : mixin_methods) {
     auto const& method = mixin_method.method.get();
-    service_methods_vars[method.full_name()] = GetMethodVars(
+    service_methods_vars[std::string{method.full_name()}] = GetMethodVars(
         service, service_config, method, mixin_method.http_override,
         mixin_method.grpc_stub_name, idempotency_overrides, omitted_rpcs);
   }
diff --git a/generator/internal/discovery_type_vertex.cc b/generator/internal/discovery_type_vertex.cc
@@ -495,9 +495,9 @@ StatusOr<int> DiscoveryTypeVertex::GetFieldNumber(
 
   auto qualified_type_name = [](google::protobuf::FieldDescriptor const& f) {
     if (f.type() == google::protobuf::FieldDescriptor::TYPE_MESSAGE) {
-      return f.message_type()->full_name();
+      return std::string{f.message_type()->full_name()};
     }
-    return std::string(f.type_name());
+    return std::string{f.type_name()};
   };
 
   // Keep the field number the same for existing fields.
diff --git a/generator/internal/doxygen.cc b/generator/internal/doxygen.cc
@@ -24,7 +24,7 @@ std::string FormatDoxygenLink(
     google::protobuf::Descriptor const& message_type) {
   google::protobuf::SourceLocation loc;
   message_type.GetSourceLocation(&loc);
-  std::string output_type_proto_file_name = message_type.file()->name();
+  std::string output_type_proto_file_name{message_type.file()->name()};
   return absl::StrCat(
       "@googleapis_link{", ProtoNameToCppName(message_type.full_name()), ",",
       output_type_proto_file_name, "#L", loc.start_line + 1, "}");
diff --git a/generator/internal/format_class_comments_test.cc b/generator/internal/format_class_comments_test.cc
@@ -53,7 +53,7 @@ service Service {
   ASSERT_THAT(service, NotNull());
 
   auto const actual = FormatClassCommentsFromServiceComments(
-      *service, service->name(), absl::nullopt);
+      *service, std::string{service->name()}, absl::nullopt);
 
   // Verify it has exactly one trailing `///` comment.
   EXPECT_THAT(actual, AllOf(EndsWith("\n///"), Not(EndsWith("\n///\n///"))));
@@ -82,7 +82,7 @@ service Service {
   ASSERT_THAT(service, NotNull());
 
   auto const actual = FormatClassCommentsFromServiceComments(
-      *service, service->name(), absl::nullopt);
+      *service, std::string{service->name()}, absl::nullopt);
 
   auto const lines = std::vector<std::string>{absl::StrSplit(actual, '\n')};
   EXPECT_THAT(
@@ -119,7 +119,7 @@ message Resource {
   ASSERT_THAT(service, NotNull());
 
   auto const actual = FormatClassCommentsFromServiceComments(
-      *service, service->name(), absl::nullopt);
+      *service, std::string{service->name()}, absl::nullopt);
 
   // Verify the first reference is separated by an empty line and that it ends
   // with a single `///` comment.
@@ -165,7 +165,7 @@ service Service {
   ASSERT_THAT(service, NotNull());
 
   auto const actual = FormatClassCommentsFromServiceComments(
-      *service, service->name(), absl::nullopt);
+      *service, std::string{service->name()}, absl::nullopt);
   // Verify the relative link is converted to an absolute link.
   EXPECT_THAT(actual,
               AllOf(HasSubstr("[groups][google.monitoring.v3.Group]"),
@@ -223,7 +223,7 @@ service Service {
   ASSERT_THAT(service, NotNull());
 
   auto const actual = FormatClassCommentsFromServiceComments(
-      *service, service->name(), "New Service comments");
+      *service, std::string{service->name()}, "New Service comments");
   EXPECT_THAT(actual, AllOf(HasSubstr("New Service comments")));
 }
 
diff --git a/generator/internal/format_method_comments.cc b/generator/internal/format_method_comments.cc
@@ -54,7 +54,8 @@ auto constexpr kDeprecationComment = R"""( @deprecated This RPC is deprecated.
 ProtoDefinitionLocation Location(google::protobuf::Descriptor const& d) {
   google::protobuf::SourceLocation loc;
   d.GetSourceLocation(&loc);
-  return ProtoDefinitionLocation{d.file()->name(), loc.start_line + 1};
+  return ProtoDefinitionLocation{std::string{d.file()->name()},
+                                 loc.start_line + 1};
 }
 
 std::string ReturnCommentString(
diff --git a/generator/internal/http_option_utils.cc b/generator/internal/http_option_utils.cc
@@ -221,7 +221,7 @@ absl::optional<QueryParameterInfo> DetermineQueryParameterInfo(
     } else {
       // But also consider protobuf well known types that wrap simple types.
       auto iter = kSupportedWellKnownValueTypes->find(
-          field.message_type()->full_name());
+          std::string{field.message_type()->full_name()});
       if (iter != kSupportedWellKnownValueTypes->end()) {
         param_info = QueryParameterInfo{
             iter->second, absl::StrCat("request.", field.name(), "().value()"),
diff --git a/generator/internal/longrunning.cc b/generator/internal/longrunning.cc
@@ -48,7 +48,9 @@ absl::variant<std::string, Descriptor const*> FullyQualifyMessageType(
 
 struct FullyQualifiedMessageTypeVisitor {
   std::string operator()(std::string const& s) const { return s; }
-  std::string operator()(Descriptor const* d) const { return d->full_name(); }
+  std::string operator()(Descriptor const* d) const {
+    return std::string{d->full_name()};
+  }
 };
 
 struct FormatDoxygenLinkVisitor {
@@ -118,7 +120,8 @@ void SetLongrunningOperationMethodVars(
   if (IsHttpLongrunningOperation(method)) {
     method_vars["longrunning_response_type"] = ProtoNameToCppName(absl::visit(
         FullyQualifiedMessageTypeVisitor(),
-        FullyQualifyMessageType(method, method.output_type()->full_name())));
+        FullyQualifyMessageType(
+            method, std::string{method.output_type()->full_name()})));
     absl::variant<std::string, google::protobuf::Descriptor const*>
         deduced_response_type = method.output_type();
     method_vars["longrunning_deduced_response_message_type"] =
@@ -166,10 +169,11 @@ void SetLongrunningOperationServiceVars(
       return;
     }
     if (IsHttpLongrunningOperation(*method)) {
-      service_vars["longrunning_response_type"] = ProtoNameToCppName(
-          absl::visit(FullyQualifiedMessageTypeVisitor(),
-                      FullyQualifyMessageType(
-                          *method, method->output_type()->full_name())));
+      service_vars["longrunning_response_type"] =
+          ProtoNameToCppName(absl::visit(
+              FullyQualifiedMessageTypeVisitor(),
+              FullyQualifyMessageType(
+                  *method, std::string{method->output_type()->full_name()})));
       auto operation_service_extension =
           method->options().GetExtension(google::cloud::operation_service);
 
diff --git a/generator/internal/mixin_utils.cc b/generator/internal/mixin_utils.cc
@@ -133,7 +133,7 @@ std::unordered_set<std::string> GetMethodNames(
   std::unordered_set<std::string> method_names;
   for (int i = 0; i < service.method_count(); ++i) {
     auto const* method = service.method(i);
-    method_names.insert(method->name());
+    method_names.insert(std::string{method->name()});
   }
   return method_names;
 }
@@ -188,14 +188,15 @@ std::vector<MixinMethod> GetMixinMethods(YAML::Node const& service_config,
       ServiceDescriptor const* mixin_service = mixin_file->service(i);
       for (int j = 0; j < mixin_service->method_count(); ++j) {
         MethodDescriptor const* mixin_method = mixin_service->method(j);
-        auto mixin_method_full_name = mixin_method->full_name();
+        std::string mixin_method_full_name{mixin_method->full_name()};
         // Add the mixin method only if it appears in the http field of YAML
         auto const it = mixin_http_overrides.find(mixin_method_full_name);
         if (it == mixin_http_overrides.end()) continue;
 
         // If the mixin method name required from YAML appears in the original
         // service proto, ignore the mixin.
-        if (method_names.find(mixin_method->name()) != method_names.end())
+        if (method_names.find(std::string{mixin_method->name()}) !=
+            method_names.end())
           continue;
 
         mixin_methods.push_back(
diff --git a/generator/internal/pagination.cc b/generator/internal/pagination.cc
@@ -133,11 +133,11 @@ google::cloud::optional<PaginationInfo> DetermineAlternatePagination(
   if (!items->is_repeated()) return {};
 
   if (items->is_map()) {
-    return PaginationInfo{items->name(),
+    return PaginationInfo{std::string{items->name()},
                           items->message_type()->map_value()->message_type(),
                           items->message_type()->map_key()};
   }
-  return PaginationInfo{items->name(), items->message_type(), {}};
+  return PaginationInfo{std::string{items->name()}, items->message_type(), {}};
 }
 
 // For the BigQuery v2 proto definitions, the paging conventions
@@ -180,7 +180,8 @@ google::cloud::optional<PaginationInfo> DetermineBigQueryPagination(
       FieldDescriptor const* items =
           response_message->FindFieldByName(field_name);
       if (!items->is_repeated()) return {};
-      return PaginationInfo{items->name(), items->message_type(), {}};
+      return PaginationInfo{
+          std::string{items->name()}, items->message_type(), {}};
     }
   }
 
diff --git a/generator/internal/request_id.cc b/generator/internal/request_id.cc
@@ -58,7 +58,7 @@ std::string RequestIdFieldName(
       if (f.Type() != YAML::NodeType::Scalar) continue;
       auto const* fd = request_descriptor.FindFieldByName(f.as<std::string>());
       if (fd == nullptr) continue;
-      if (MeetsRequestIdRequirements(*fd)) return fd->name();
+      if (MeetsRequestIdRequirements(*fd)) return std::string{fd->name()};
     }
   }
   return {};
diff --git a/generator/internal/resolve_comment_references.cc b/generator/internal/resolve_comment_references.cc
@@ -29,9 +29,9 @@ absl::optional<std::pair<std::string, ProtoDefinitionLocation>> GetLocation(
   if (d == nullptr) return absl::nullopt;
   google::protobuf::SourceLocation loc;
   d->GetSourceLocation(&loc);
-  return std::make_pair(
-      d->full_name(),
-      ProtoDefinitionLocation{d->file()->name(), loc.start_line + 1});
+  return std::make_pair(std::string{d->full_name()},
+                        ProtoDefinitionLocation{std::string{d->file()->name()},
+                                                loc.start_line + 1});
 }
 
 absl::optional<std::pair<std::string, ProtoDefinitionLocation>>
diff --git a/generator/internal/resolve_method_return.cc b/generator/internal/resolve_method_return.cc
@@ -28,7 +28,8 @@ namespace {
 ProtoDefinitionLocation Location(google::protobuf::Descriptor const& d) {
   google::protobuf::SourceLocation loc;
   d.GetSourceLocation(&loc);
-  return ProtoDefinitionLocation{d.file()->name(), loc.start_line + 1};
+  return ProtoDefinitionLocation{std::string{d.file()->name()},
+                                 loc.start_line + 1};
 }
 
 }  // namespace
@@ -46,7 +47,7 @@ ResolveMethodReturn(google::protobuf::MethodDescriptor const& method) {
     // For string pagination we return nothing, there is no need to link the
     // definition of the `std::string` type.
     if (!info->range_output_type) return absl::nullopt;
-    return std::make_pair(info->range_output_type->full_name(),
+    return std::make_pair(std::string{info->range_output_type->full_name()},
                           Location(*info->range_output_type));
   }
 
@@ -59,14 +60,15 @@ ResolveMethodReturn(google::protobuf::MethodDescriptor const& method) {
     message = method.file()->pool()->FindMessageTypeByName(name);
     if (!message) {
       // Qualify the name and try again
-      auto const fqname = method.file()->package() + "." + name;
+      auto const fqname = std::string{method.file()->package()} + "." + name;
       message = method.file()->pool()->FindMessageTypeByName(fqname);
       if (!message) return absl::nullopt;
     }
-    return std::make_pair(message->full_name(), Location(*message));
+    return std::make_pair(std::string{message->full_name()},
+                          Location(*message));
   }
 
-  return std::make_pair(message->full_name(), Location(*message));
+  return std::make_pair(std::string{message->full_name()}, Location(*message));
 }
 
 }  // namespace generator_internal
diff --git a/generator/internal/service_code_generator.cc b/generator/internal/service_code_generator.cc
@@ -44,7 +44,8 @@ absl::optional<std::string> IncludePathForWellKnownProtobufType(
       new std::unordered_map<std::string, std::string>(
           {{"google.protobuf.Duration", "google/protobuf/duration.pb.h"}});
   if (parameter.type() == google::protobuf::FieldDescriptor::TYPE_MESSAGE) {
-    auto iter = kTypeIncludeMap->find(parameter.message_type()->full_name());
+    auto iter = kTypeIncludeMap->find(
+        std::string{parameter.message_type()->full_name()});
     if (iter != kTypeIncludeMap->end()) {
       return iter->second;
     }
@@ -216,7 +217,7 @@ bool ServiceCodeGenerator::HasRequestId() const {
 
 bool ServiceCodeGenerator::HasRequestId(
     google::protobuf::MethodDescriptor const& method) const {
-  auto mv = service_method_vars_.find(method.full_name());
+  auto mv = service_method_vars_.find(std::string{method.full_name()});
   if (mv == service_method_vars_.end()) return false;
   auto const& method_vars = mv->second;
   return method_vars.find("request_id_field_name") != method_vars.end();
@@ -261,7 +262,7 @@ bool ServiceCodeGenerator::MethodSignatureUsesDeprecatedField() const {
 bool ServiceCodeGenerator::OmitMethodSignature(
     google::protobuf::MethodDescriptor const& method,
     int method_signature_number) const {
-  auto method_vars = service_method_vars_.find(method.full_name());
+  auto method_vars = service_method_vars_.find(std::string{method.full_name()});
   if (method_vars == service_method_vars_.end()) {
     GCP_LOG(FATAL) << method.full_name()
                    << " not found in service_method_vars_\n";
@@ -286,8 +287,10 @@ std::string ServiceCodeGenerator::vars(std::string const& key) const {
 VarsDictionary ServiceCodeGenerator::MergeServiceAndMethodVars(
     google::protobuf::MethodDescriptor const& method) const {
   auto vars = service_vars_;
-  vars.insert(service_method_vars_.at(method.full_name()).begin(),
-              service_method_vars_.at(method.full_name()).end());
+  // NOLINTNEXTLINE(performance-unnecessary-copy-initialization)
+  std::string const method_full_name{method.full_name()};
+  vars.insert(service_method_vars_.at(method_full_name).begin(),
+              service_method_vars_.at(method_full_name).end());
   return vars;
 }
 

Original file line number	Diff line number	Diff line change
`@@ -572,7 +572,7 @@ std::string CppTypeToString(FieldDescriptor const* field) {`
`572`	`572`	`GCP_LOG(FATAL) << "FieldDescriptor " << field->cpp_type_name()`
`573`	`573`	`<< " not handled";`
`574`	`574`	`/NOTREACHED/`
`575`		`- return field->cpp_type_name();`
	`575`	`+ return std::string{field->cpp_type_name()};`
`576`	`576`	`}`
`577`	`577`
`578`	`578`	`std::string FormatMethodCommentsMethodSignature(`
`@@ -880,13 +880,13 @@ std::map<std::string, VarsDictionary> CreateMethodVars(`
`880`	`880`	`if (method.options().HasExtension(google::api::http)) {`
`881`	`881`	`http_rule = method.options().GetExtension(google::api::http);`
`882`	`882`	`}`
`883`		`- service_methods_vars[method.full_name()] =`
	`883`	`+ service_methods_vars[std::string{method.full_name()}] =`
`884`	`884`	`GetMethodVars(service, service_config, method, http_rule, "grpc_stub",`
`885`	`885`	`idempotency_overrides, omitted_rpcs);`
`886`	`886`	`}`
`887`	`887`	`for (auto const& mixin_method : mixin_methods) {`
`888`	`888`	`auto const& method = mixin_method.method.get();`
`889`		`- service_methods_vars[method.full_name()] = GetMethodVars(`
	`889`	`+ service_methods_vars[std::string{method.full_name()}] = GetMethodVars(`
`890`	`890`	`service, service_config, method, mixin_method.http_override,`
`891`	`891`	`mixin_method.grpc_stub_name, idempotency_overrides, omitted_rpcs);`
`892`	`892`	`}`
Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,8 @@ auto constexpr kDeprecationComment = R"""( @deprecated This RPC is deprecated.`
`54`	`54`	`ProtoDefinitionLocation Location(google::protobuf::Descriptor const& d) {`
`55`	`55`	`google::protobuf::SourceLocation loc;`
`56`	`56`	`d.GetSourceLocation(&loc);`
`57`		`- return ProtoDefinitionLocation{d.file()->name(), loc.start_line + 1};`
	`57`	`+ return ProtoDefinitionLocation{std::string{d.file()->name()},`
	`58`	`+ loc.start_line + 1};`
`58`	`59`	`}`
`59`	`60`
`60`	`61`	`std::string ReturnCommentString(`
Original file line number	Diff line number	Diff line change
`@@ -133,11 +133,11 @@ google::cloud::optional<PaginationInfo> DetermineAlternatePagination(`
`133`	`133`	`if (!items->is_repeated()) return {};`
`134`	`134`
`135`	`135`	`if (items->is_map()) {`
`136`		`- return PaginationInfo{items->name(),`
	`136`	`+ return PaginationInfo{std::string{items->name()},`
`137`	`137`	`items->message_type()->map_value()->message_type(),`
`138`	`138`	`items->message_type()->map_key()};`
`139`	`139`	`}`
`140`		`- return PaginationInfo{items->name(), items->message_type(), {}};`
	`140`	`+ return PaginationInfo{std::string{items->name()}, items->message_type(), {}};`
`141`	`141`	`}`
`142`	`142`
`143`	`143`	`// For the BigQuery v2 proto definitions, the paging conventions`
`@@ -180,7 +180,8 @@ google::cloud::optional<PaginationInfo> DetermineBigQueryPagination(`
`180`	`180`	`FieldDescriptor const* items =`
`181`	`181`	`response_message->FindFieldByName(field_name);`
`182`	`182`	`if (!items->is_repeated()) return {};`
`183`		`- return PaginationInfo{items->name(), items->message_type(), {}};`
	`183`	`+ return PaginationInfo{`
	`184`	`+ std::string{items->name()}, items->message_type(), {}};`
`184`	`185`	`}`
`185`	`186`	`}`
`186`	`187`