diff --git a/objectivec/README.md b/objectivec/README.md index bbe5726d455b..485a7203d28e 100644 --- a/objectivec/README.md +++ b/objectivec/README.md @@ -131,10 +131,16 @@ Objective C Generator Proto File Options **objc_class_prefix=\** (no default) -Since Objective C uses a global namespace for all of its classes, there can -be collisions. This option provides a prefix that will be added to the Enums -and Objects (for messages) generated from the proto. Convention is to base -the prefix on the package the proto is in. +This options allow you to provide a custom prefix for all the symbols generated +from a proto file (classes (from message), enums, the Root for extension +support). + +If not set, the generation option `use_package_as_prefix` (documented below) +controls what is used instead. Since Objective C uses a global namespace for all +of its classes, there can be collisions. `use_package_as_prefix=yes` should +avoid collisions since proto package are used to scope/name things in other +languages, but this option can be used to get shorter names instead. Convention +is to base the explicit prefix on the proto package. Objective C Generator `protoc` Options -------------------------------------- @@ -178,6 +184,24 @@ supported keys are: having to add the runtime directory to the header search path since the generate `#import` will be more complete. + * `use_package_as_prefix` and `proto_package_prefix_exceptions_path`: The + `value` for `use_package_as_prefix` can be `yes` or `no`, and indicates + if a prefix should be derived from the proto package for all the symbols + for files that don't have the `objc_class_prefix` file option (mentioned + above). This helps ensure the symbols are more unique and means there is + less chance of ObjC class name collisions. + + To help in migrating code to using this support, + `proto_package_prefix_exceptions_path` can be used to provide the path + to a file that contains proto package names (one per line, comments allowed + if prefixed with `#`). These package won't get the derived prefix, allowing + migrations to the behavior one proto package at a time across a code base. + + `use_package_as_prefix` currently defaults to `no` (existing behavior), but + in the future (as a breaking change), that is likely to change since it + helps prepare folks before they end up using a lot of protos and getting a + lot of collisions. + Contributing ------------ diff --git a/src/google/protobuf/compiler/objectivec/objectivec_file.h b/src/google/protobuf/compiler/objectivec/objectivec_file.h index cecbda2e336c..87258a39fe06 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_file.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_file.h @@ -58,8 +58,6 @@ class FileGenerator { void GenerateSource(io::Printer* printer); void GenerateHeader(io::Printer* printer); - const std::string& RootClassName() const { return root_class_name_; } - private: const FileDescriptor* file_; std::string root_class_name_; diff --git a/src/google/protobuf/compiler/objectivec/objectivec_generator.cc b/src/google/protobuf/compiler/objectivec/objectivec_generator.cc index 3002c68335ee..9aefa4a9843b 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_generator.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_generator.cc @@ -144,6 +144,34 @@ bool ObjectiveCGenerator::GenerateAll( // of not being able to iterate through the properties using the Objective-C // runtime. generation_options.elide_message_metadata = true; + } else if (options[i].first == "use_package_as_prefix") { + // Controls how the symbols should be prefixed to avoid symbols + // collisions. The objc_class_prefix file option is always honored, this + // is just what to do if that isn't set. The available options are: + // "no": Not prefixed (the existing mode). + // "yes": Make a prefix out of the proto package. + std::string lower_value(options[i].second); + LowerString(&lower_value); + if (lower_value == "no") { + SetUseProtoPackageAsDefaultPrefix(false); + } else if (lower_value == "yes") { + SetUseProtoPackageAsDefaultPrefix(true); + } else { + *error = "error: Unknown use_package_as_prefix: " + options[i].second; + return false; + } + } else if (options[i].first == "proto_package_prefix_exceptions_path") { + // Path to find a file containing the list of proto package names that are + // exceptions when use_package_as_prefix is enabled. This can be used to + // migrate packages one at a time to use_package_as_prefix since there + // are likely code updates needed with each one. + // + // The format of the file is: + // - An entry is a line of "proto.package.name". + // - Comments start with "#". + // - A comment can go on a line after a expected package/prefix pair. + // (i.e. - "some.proto.package # comment") + SetProtoPackagePrefixExceptionList(options[i].second); } else { *error = "error: Unknown generator option: " + options[i].first; return false; diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc index 4809b16707dd..9aeb0f6f2e37 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.cc @@ -71,6 +71,102 @@ using ::open; #endif } // namespace port +namespace { + +class SimpleLineCollector : public LineConsumer { + public: + SimpleLineCollector(std::unordered_set* inout_set) + : set_(inout_set) {} + + virtual bool ConsumeLine(const StringPiece& line, std::string* out_error) override { + set_->insert(std::string(line)); + return true; + } + + private: + std::unordered_set* set_; +}; + +class PrefixModeStorage { + public: + PrefixModeStorage(); + + bool use_package_name() const { return use_package_name_; } + void set_use_package_name(bool on_or_off) { use_package_name_ = on_or_off; } + + const std::string exception_path() const { return exception_path_; } + void set_exception_path(const std::string& path) { + exception_path_ = path; + exceptions_.clear(); + } + + bool is_package_exempted(const std::string& package); + + private: + bool use_package_name_; + std::string exception_path_; + std::unordered_set exceptions_; +}; + +PrefixModeStorage::PrefixModeStorage() { + // Even thought there are generation options, have an env back door since some + // of these helpers could be used in other plugins. + + const char* use_package_cstr = getenv("GPB_OBJC_USE_PACKAGE_AS_PREFIX"); + use_package_name_ = + (use_package_cstr && (std::string("YES") == ToUpper(use_package_cstr))); + + const char* exception_path = getenv("GPB_OBJC_PACKAGE_PREFIX_EXCEPTIONS_PATH"); + if (exception_path) { + exception_path_ = exception_path; + } +} + +bool PrefixModeStorage::is_package_exempted(const std::string& package) { + if (exceptions_.empty() && !exception_path_.empty()) { + std::string error_str; + SimpleLineCollector collector(&exceptions_); + if (ParseSimpleFile(exception_path_, &collector, &error_str)) { + if (error_str.empty()) { + error_str = std::string("protoc:0: warning: Failed to parse") + + std::string(" package prefix exceptions file: ") + + exception_path_; + } + std::cerr << error_str << std::endl; + std::cerr.flush(); + exceptions_.clear(); + } + + // If the file was empty put something in it so it doesn't get reloaded over + // and over. + if (exceptions_.empty()) { + exceptions_.insert(""); + } + } + + return exceptions_.count(package) != 0; +} + +PrefixModeStorage g_prefix_mode; + +} // namespace + +bool UseProtoPackageAsDefaultPrefix() { + return g_prefix_mode.use_package_name(); +} + +void SetUseProtoPackageAsDefaultPrefix(bool on_or_off) { + g_prefix_mode.set_use_package_name(on_or_off); +} + +std::string GetProtoPackagePrefixExceptionList() { + return g_prefix_mode.exception_path(); +} + +void SetProtoPackagePrefixExceptionList(const std::string& file_path) { + g_prefix_mode.set_exception_path(file_path); +} + Options::Options() { // Default is the value of the env for the package prefixes. const char* file_path = getenv("GPB_OBJC_EXPECTED_PACKAGE_PREFIXES"); @@ -361,6 +457,15 @@ std::string GetEnumNameForFlagType(const FlagType flag_type) { } } +void MaybeUnQuote(StringPiece* input) { + if ((input->length() >= 2) && + ((*input->data() == '\'' || *input->data() == '"')) && + ((*input)[input->length() - 1] == *input->data())) { + input->remove_prefix(1); + input->remove_suffix(1); + } +} + } // namespace // Escape C++ trigraphs by escaping question marks to \? @@ -399,8 +504,40 @@ std::string BaseFileName(const FileDescriptor* file) { } std::string FileClassPrefix(const FileDescriptor* file) { - // Default is empty string, no need to check has_objc_class_prefix. - std::string result = file->options().objc_class_prefix(); + // Always honor the file option. + if (file->options().has_objc_class_prefix()) { + return file->options().objc_class_prefix(); + } + + // If package prefix isn't enabled or no package, done. + if (!g_prefix_mode.use_package_name() || file->package().empty()) { + return ""; + } + + // If the package is in the exceptions list, done. + if (g_prefix_mode.is_package_exempted(file->package())) { + return ""; + } + + // Transform the package into a prefix: use the dot segments as part, + // camelcase each one and then join them with underscores, and add an + // underscore at the end. + std::string result; + const std::vector segments = + Split(file->package(), ".", true /* skip_empty */); + for (const auto& segment : segments) { + const std::string part = UnderscoresToCamelCase(segment, true); + if (part.empty()) { + continue; + } + if (!result.empty()) { + result.append("_"); + } + result.append(part); + } + if (!result.empty()) { + result.append("_"); + } return result; } @@ -1070,6 +1207,7 @@ bool ExpectedPrefixesCollector::ConsumeLine( StringPiece prefix = line.substr(offset + 1); TrimWhitespace(&package); TrimWhitespace(&prefix); + MaybeUnQuote(&prefix); // Don't really worry about error checking the package/prefix for // being valid. Assume the file is validated when it is created/edited. (*prefix_map_)[std::string(package)] = std::string(prefix); @@ -1092,6 +1230,11 @@ bool ValidateObjCClassPrefix( const FileDescriptor* file, const std::string& expected_prefixes_path, const std::map& expected_package_prefixes, std::string* out_error) { + // Reminder: An explicit prefix option of "" is valid in case the default + // prefixing is set to use the proto package and a file needs to be generated + // without any prefix at all (for legacy reasons). + + bool has_prefix = file->options().has_objc_class_prefix(); const std::string prefix = file->options().objc_class_prefix(); const std::string package = file->package(); @@ -1104,7 +1247,7 @@ bool ValidateObjCClassPrefix( expected_package_prefixes.find(package); if (package_match != expected_package_prefixes.end()) { // There was an entry, and... - if (package_match->second == prefix) { + if (has_prefix && package_match->second == prefix) { // ...it matches. All good, out of here! return true; } else { @@ -1112,7 +1255,7 @@ bool ValidateObjCClassPrefix( *out_error = "error: Expected 'option objc_class_prefix = \"" + package_match->second + "\";' for package '" + package + "' in '" + file->name() + "'"; - if (prefix.length()) { + if (has_prefix) { *out_error += "; but found '" + prefix + "' instead"; } *out_error += "."; @@ -1121,22 +1264,21 @@ bool ValidateObjCClassPrefix( } // If there was no prefix option, we're done at this point. - if (prefix.empty()) { - // No prefix, nothing left to check. + if (!has_prefix) { return true; } // Check: Warning - Make sure the prefix is is a reasonable value according // to Apple's rules (the checks above implicitly whitelist anything that // doesn't meet these rules). - if (!ascii_isupper(prefix[0])) { + if (!prefix.empty() && !ascii_isupper(prefix[0])) { std::cerr << "protoc:0: warning: Invalid 'option objc_class_prefix = \"" << prefix << "\";' in '" << file->name() << "';" << " it should start with a capital letter." << std::endl; std::cerr.flush(); } - if (prefix.length() < 3) { + if (!prefix.empty() && prefix.length() < 3) { // Apple reserves 2 character prefixes for themselves. They do use some // 3 character prefixes, but they haven't updated the rules/docs. std::cerr @@ -1147,20 +1289,22 @@ bool ValidateObjCClassPrefix( std::cerr.flush(); } - // Look for any other package that uses the same prefix. + // Look for any other package that uses the same (non empty) prefix. std::string other_package_for_prefix; - for (std::map::const_iterator i = - expected_package_prefixes.begin(); - i != expected_package_prefixes.end(); ++i) { - if (i->second == prefix) { - other_package_for_prefix = i->first; - break; + if (!prefix.empty()) { + for (std::map::const_iterator i = + expected_package_prefixes.begin(); + i != expected_package_prefixes.end(); ++i) { + if (i->second == prefix) { + other_package_for_prefix = i->first; + break; + } } } - // Check: Warning - If the file does not have a package, check whether - // the prefix declared is being used by another package or not. - if (package.empty()) { + // Check: Warning - If the file does not have a package, check whether the non + // empty prefix declared is being used by another package or not. + if (package.empty() && !prefix.empty()) { // The file does not have a package and ... if (other_package_for_prefix.empty()) { // ... no other package has declared that prefix. @@ -1199,7 +1343,7 @@ bool ValidateObjCClassPrefix( } // Check: Warning - If the given package/prefix pair wasn't expected, issue a - // warning issue a warning suggesting it gets added to the file. + // warning suggesting it gets added to the file. if (!expected_package_prefixes.empty()) { std::cerr << "protoc:0: warning: Found unexpected 'option objc_class_prefix = \"" diff --git a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h index 8b011422545c..7e30c13dd855 100644 --- a/src/google/protobuf/compiler/objectivec/objectivec_helpers.h +++ b/src/google/protobuf/compiler/objectivec/objectivec_helpers.h @@ -46,6 +46,19 @@ namespace protobuf { namespace compiler { namespace objectivec { +// Get/Set if the proto package should be used to make the default prefix for +// symbols. This will then impact most of the type naming apis below. It is done +// as a global to not break any other generator reusing the methods since they +// are exported. +bool PROTOC_EXPORT UseProtoPackageAsDefaultPrefix(); +void PROTOC_EXPORT SetUseProtoPackageAsDefaultPrefix(bool on_or_off); +// Get/Set the path to a file to load as exceptions when +// `UseProtoPackageAsDefaultPrefixUseProtoPackageAsDefaultPrefix()` is `true`. +// And empty string means there should be no exceptions loaded. +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): struct Options { Options(); @@ -71,7 +84,7 @@ bool PROTOC_EXPORT IsRetainedName(const std::string& name); // handling under ARC. bool PROTOC_EXPORT IsInitName(const std::string& name); -// Gets the objc_class_prefix. +// Gets the objc_class_prefix or the prefix made from the proto package. std::string PROTOC_EXPORT FileClassPrefix(const FileDescriptor* file); // Gets the path of the file we're going to generate (sans the .pb.h @@ -91,7 +104,7 @@ std::string PROTOC_EXPORT FileClassName(const FileDescriptor* file); // descriptor. std::string PROTOC_EXPORT ClassName(const Descriptor* descriptor); std::string PROTOC_EXPORT ClassName(const Descriptor* descriptor, - std::string* out_suffix_added); + std::string* out_suffix_added); std::string PROTOC_EXPORT EnumName(const EnumDescriptor* descriptor); // Returns the fully-qualified name of the enum value corresponding to the