[ObjC] Add support for using the proto package to prefix symbols.

This likely should have been the default from the start, as without it is way to
common to get symbol collisions between different proto files. It would be nice
to support a "migration" mode where both names are created to aid it moving code
to this model, but with ObjC `@class` decls being very common to avoid header
imports to control rebuilds/etc., it doesn't work as an `@class` usage will
error if one also uses `@compatibility_alias`. Falling back to `#define` the two
together also doesn't work as the header with the `@class` will cause methods to
get defined with one interface, but when methods taking those types are define
will likely #import the generate header and thus get the define and end up with
a different signature. So for now, there is no migration support and code has to
be updated in one shot with enable the new prefixing.

- Add a generation option to enable this change in generation.
- Add a second generation option to provide a list of proto package that are
  exceptions from using the proto package. This allows easier
  migration/updating of code one package at a time.
This commit is contained in:
Thomas Van Lenten 2021-06-11 16:58:51 -04:00
parent 2ea5c9951c
commit cf12bffd9d
5 changed files with 234 additions and 27 deletions

View File

@ -131,10 +131,16 @@ Objective C Generator Proto File Options
**objc_class_prefix=\<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
------------

View File

@ -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_;

View File

@ -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;

View File

@ -71,6 +71,102 @@ using ::open;
#endif
} // namespace port
namespace {
class SimpleLineCollector : public LineConsumer {
public:
SimpleLineCollector(std::unordered_set<std::string>* 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<std::string>* 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<std::string> 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("<not a real package>");
}
}
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<std::string> 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<std::string, std::string>& 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<std::string, std::string>::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<std::string, std::string>::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 = \""

View File

@ -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