Commit ff2a6600 authored by Sergio Campamá's avatar Sergio Campamá Committed by Thomas Van Lenten

Adds better support for protos without packages (#1979)

Adds better support for protos without packages and more warnings on possible improvements
parent 549dde19
......@@ -1021,27 +1021,11 @@ bool ValidateObjCClassPrefix(const FileDescriptor* file,
}
// If there was no prefix option, we're done at this point.
if (prefix.length() == 0) {
if (prefix.empty()) {
// No prefix, nothing left to check.
return true;
}
// Check: Error - Make sure the prefix wasn't expected for a different
// package (overlap is allowed, but it has to be listed as an expected
// overlap).
for (map<string, string>::iterator i = expected_package_prefixes.begin();
i != expected_package_prefixes.end(); ++i) {
if (i->second == prefix) {
*out_error =
"error: Found 'option objc_class_prefix = \"" + prefix +
"\";' in '" + file->name() +
"'; that prefix is already used for 'package " + i->first +
";'. It can only be reused by listing it in the expected file (" +
generation_options.expected_prefixes_path + ").";
return false; // Only report first usage of the prefix.
}
}
// 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).
......@@ -1063,6 +1047,56 @@ bool ValidateObjCClassPrefix(const FileDescriptor* file,
cerr.flush();
}
// Look for any other package that uses the same prefix.
string other_package_for_prefix;
for (map<string, string>::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()) {
// The file does not have a package and ...
if (other_package_for_prefix.empty()) {
// ... no other package has declared that prefix.
cerr << endl
<< "protoc:0: warning: File '" << file->name() << "' has no "
<< "package. Consider adding a new package to the proto and adding '"
<< "new.package = " << prefix << "' to the expected prefixes file ("
<< generation_options.expected_prefixes_path << ")." << endl;
cerr.flush();
} else {
// ... another package has declared the same prefix.
cerr << endl
<< "protoc:0: warning: File '" << file->name() << "' has no package "
<< "and package '" << other_package_for_prefix << "' already uses '"
<< prefix << "' as its prefix. Consider either adding a new package "
<< "to the proto, or reusing one of the packages already using this "
<< "prefix in the expected prefixes file ("
<< generation_options.expected_prefixes_path << ")." << endl;
cerr.flush();
}
return true;
}
// Check: Error - Make sure the prefix wasn't expected for a different
// package (overlap is allowed, but it has to be listed as an expected
// overlap).
if (!other_package_for_prefix.empty()) {
*out_error =
"error: Found 'option objc_class_prefix = \"" + prefix +
"\";' in '" + file->name() +
"'; that prefix is already used for 'package " +
other_package_for_prefix + ";'. It can only be reused by listing " +
"it in the expected file (" +
generation_options.expected_prefixes_path + ").";
return false; // Only report first usage of the prefix.
}
// Check: Warning - If the given package/prefix pair wasn't expected, issue a
// warning issue a warning suggesting it gets added to the file.
if (!expected_package_prefixes.empty()) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment