From d9048081b25ee4a48b40ce687d60a5228cb182b9 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Mon, 28 Feb 2022 17:01:23 -0500 Subject: [PATCH] Remove the non prefix validation options from Options. The other generation options never were in the Options structure and ImportWriter was the only thing that needed some the non validation options and it already required directly passing the options. So refine Options to just be validation options and bundle all the File related generation options into a new FileGenerator specific GenerationOptions. --- .../compiler/objectivec/objectivec_file.cc | 22 ++++++++++--------- .../compiler/objectivec/objectivec_file.h | 14 ++++++++---- .../objectivec/objectivec_generator.cc | 13 ++++++----- .../compiler/objectivec/objectivec_helpers.cc | 8 +++---- .../compiler/objectivec/objectivec_helpers.h | 6 ++--- 5 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/google/protobuf/compiler/objectivec/objectivec_file.cc b/src/google/protobuf/compiler/objectivec/objectivec_file.cc index 0b4f4fa291..e11ca0e245 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_file.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_file.cc @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -185,11 +186,12 @@ bool IsDirectDependency(const FileDescriptor* dep, const FileDescriptor* file) { } // namespace -FileGenerator::FileGenerator(const FileDescriptor* file, const Options& options) +FileGenerator::FileGenerator(const FileDescriptor* file, + const GenerationOptions& generation_options) : file_(file), + generation_options_(generation_options), root_class_name_(FileClassName(file)), - is_bundled_proto_(IsProtobufLibraryBundledProtoFile(file)), - options_(options) { + is_bundled_proto_(IsProtobufLibraryBundledProtoFile(file)) { for (int i = 0; i < file_->enum_type_count(); i++) { EnumGenerator* generator = new EnumGenerator(file_->enum_type(i)); enum_generators_.emplace_back(generator); @@ -240,9 +242,9 @@ void FileGenerator::GenerateHeader(io::Printer* printer) { // #import any headers for "public imports" in the proto file. { ImportWriter import_writer( - options_.generate_for_named_framework, - options_.named_framework_to_proto_path_mappings_path, - options_.runtime_import_prefix, + generation_options_.generate_for_named_framework, + generation_options_.named_framework_to_proto_path_mappings_path, + generation_options_.runtime_import_prefix, is_bundled_proto_); const std::string header_extension(kHeaderExtension); for (int i = 0; i < file_->public_dependency_count(); i++) { @@ -354,9 +356,9 @@ void FileGenerator::GenerateSource(io::Printer* printer) { { ImportWriter import_writer( - options_.generate_for_named_framework, - options_.named_framework_to_proto_path_mappings_path, - options_.runtime_import_prefix, + generation_options_.generate_for_named_framework, + generation_options_.named_framework_to_proto_path_mappings_path, + generation_options_.runtime_import_prefix, is_bundled_proto_); const std::string header_extension(kHeaderExtension); @@ -600,7 +602,7 @@ void FileGenerator::PrintFileRuntimePreamble( "\n", "filename", file_->name()); ImportWriter::PrintRuntimeImports( - printer, headers_to_import, options_.runtime_import_prefix, true); + printer, headers_to_import, generation_options_.runtime_import_prefix, true); printer->Print("\n"); } diff --git a/src/google/protobuf/compiler/objectivec/objectivec_file.h b/src/google/protobuf/compiler/objectivec/objectivec_file.h index 87258a39fe..fc71c70c9d 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_file.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_file.h @@ -34,7 +34,6 @@ #include #include #include -#include #include #include @@ -49,7 +48,15 @@ class MessageGenerator; class FileGenerator { public: - FileGenerator(const FileDescriptor* file, const Options& options); + struct GenerationOptions { + GenerationOptions() {} + std::string generate_for_named_framework; + std::string named_framework_to_proto_path_mappings_path; + std::string runtime_import_prefix; + }; + + FileGenerator(const FileDescriptor* file, + const GenerationOptions& generation_options); ~FileGenerator(); FileGenerator(const FileGenerator&) = delete; @@ -60,6 +67,7 @@ class FileGenerator { private: const FileDescriptor* file_; + const GenerationOptions& generation_options_; std::string root_class_name_; bool is_bundled_proto_; @@ -67,8 +75,6 @@ class FileGenerator { std::vector> message_generators_; std::vector> extension_generators_; - const Options options_; - void PrintFileRuntimePreamble( io::Printer* printer, const std::vector& headers_to_import) const; diff --git a/src/google/protobuf/compiler/objectivec/objectivec_generator.cc b/src/google/protobuf/compiler/objectivec/objectivec_generator.cc index 9e9bbfd4f9..e0279a1d2e 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_generator.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_generator.cc @@ -94,7 +94,8 @@ bool ObjectiveCGenerator::GenerateAll( // // e.g. protoc ... --objc_opt=expected_prefixes=file.txt,generate_for_named_framework=MyFramework - Options generation_options; + Options validation_options; + FileGenerator::GenerationOptions generation_options; std::vector > options; ParseGeneratorParameter(parameter, &options); @@ -116,14 +117,14 @@ bool ObjectiveCGenerator::GenerateAll( // // There is no validation that the prefixes are good prefixes, it is // assumed that they are when you create the file. - generation_options.expected_prefixes_path = options[i].second; + validation_options.expected_prefixes_path = options[i].second; } else if (options[i].first == "expected_prefixes_suppressions") { // A semicolon delimited string that lists the paths of .proto files to // exclude from the package prefix validations (expected_prefixes_path). // This is provided as an "out", to skip some files being checked. for (StringPiece split_piece : Split( options[i].second, ";", true)) { - generation_options.expected_prefixes_suppressions.push_back( + validation_options.expected_prefixes_suppressions.push_back( std::string(split_piece)); } } else if (options[i].first == "prefixes_must_be_registered") { @@ -135,7 +136,7 @@ bool ObjectiveCGenerator::GenerateAll( // tried to use a prefix that isn't registered. // Default is "no". if (!StringToBool(options[i].second, - &generation_options.prefixes_must_be_registered)) { + &validation_options.prefixes_must_be_registered)) { *error = "error: Unknown value for prefixes_must_be_registered: " + options[i].second; return false; } @@ -147,7 +148,7 @@ bool ObjectiveCGenerator::GenerateAll( // raised if a files doesn't have one. // Default is "no". if (!StringToBool(options[i].second, - &generation_options.require_prefixes)) { + &validation_options.require_prefixes)) { *error = "error: Unknown value for require_prefixes: " + options[i].second; return false; } @@ -258,7 +259,7 @@ bool ObjectiveCGenerator::GenerateAll( // ----------------------------------------------------------------- // Validate the objc prefix/package pairings. - if (!ValidateObjCClassPrefixes(files, generation_options, error)) { + if (!ValidateObjCClassPrefixes(files, validation_options, error)) { // *error will have been filled in. return false; } diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc index b085d0283c..696ea2cc6d 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc @@ -1298,16 +1298,16 @@ bool PackageToPrefixesCollector::ConsumeLine( return true; } -bool LoadExpectedPackagePrefixes(const Options& generation_options, +bool LoadExpectedPackagePrefixes(const std::string& expected_prefixes_path, std::map* prefix_map, std::string* out_error) { - if (generation_options.expected_prefixes_path.empty()) { + if (expected_prefixes_path.empty()) { return true; } PackageToPrefixesCollector collector("Expected prefixes", prefix_map); return ParseSimpleFile( - generation_options.expected_prefixes_path, &collector, out_error); + expected_prefixes_path, &collector, out_error); } bool ValidateObjCClassPrefix( @@ -1465,7 +1465,7 @@ bool ValidateObjCClassPrefixes(const std::vector& files, // Load the expected package prefixes, if available, to validate against. std::map expected_package_prefixes; - if (!LoadExpectedPackagePrefixes(generation_options, + if (!LoadExpectedPackagePrefixes(generation_options.expected_prefixes_path, &expected_package_prefixes, out_error)) { return false; diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h index d67ec04eba..490cb02b9e 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h @@ -64,14 +64,12 @@ std::string PROTOC_EXPORT GetProtoPackagePrefixExceptionList(); void PROTOC_EXPORT SetProtoPackagePrefixExceptionList( const std::string& file_path); -// Generator options (see objectivec_generator.cc for a description of each): +// Generator Prefix Validation Options (see objectivec_generator.cc for a +// description of each): struct Options { Options(); std::string expected_prefixes_path; std::vector expected_prefixes_suppressions; - std::string generate_for_named_framework; - std::string named_framework_to_proto_path_mappings_path; - std::string runtime_import_prefix; bool prefixes_must_be_registered; bool require_prefixes; };