Skip to content

Commit e60ec85

Browse files
committed
Allow suppression of objc prefix checks.
- Accept "-" as the expected prefixes file and turn off all validations of prefixes (even the Apple conventions). - Reflow some of the prefix checks to hopefully make things a little easier to follow. - Don't print warnings about updates to the expected prefix file until there is a file.
1 parent 9087da2 commit e60ec85

File tree

1 file changed

+64
-54
lines changed

1 file changed

+64
-54
lines changed

src/google/protobuf/compiler/objectivec/objectivec_helpers.cc

Lines changed: 64 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1234,6 +1234,8 @@ bool ValidateObjCClassPrefix(
12341234
// without any prefix at all (for legacy reasons).
12351235

12361236
bool has_prefix = file->options().has_objc_class_prefix();
1237+
bool have_expected_prefix_file = !expected_prefixes_path.empty();
1238+
12371239
const std::string prefix = file->options().objc_class_prefix();
12381240
const std::string package = file->package();
12391241

@@ -1267,6 +1269,61 @@ bool ValidateObjCClassPrefix(
12671269
return true;
12681270
}
12691271

1272+
// When the prefix is non empty, check it against the expected entries.
1273+
if (!prefix.empty() && have_expected_prefix_file) {
1274+
// For a non empty prefix, look for any other package that uses the prefix.
1275+
std::string other_package_for_prefix;
1276+
for (std::map<std::string, std::string>::const_iterator i =
1277+
expected_package_prefixes.begin();
1278+
i != expected_package_prefixes.end(); ++i) {
1279+
if (i->second == prefix) {
1280+
other_package_for_prefix = i->first;
1281+
break;
1282+
}
1283+
}
1284+
1285+
// Check: Warning - If the file does not have a package, check whether the
1286+
// prefix was declared is being used by another package or not. This is
1287+
// a special case for empty packages.
1288+
if (package.empty()) {
1289+
// The file does not have a package and ...
1290+
if (other_package_for_prefix.empty()) {
1291+
// ... no other package has declared that prefix.
1292+
std::cerr
1293+
<< "protoc:0: warning: File '" << file->name() << "' has no "
1294+
<< "package. Consider adding a new package to the proto and adding '"
1295+
<< "new.package = " << prefix << "' to the expected prefixes file ("
1296+
<< expected_prefixes_path << ")." << std::endl;
1297+
std::cerr.flush();
1298+
} else {
1299+
// ... another package has declared the same prefix.
1300+
std::cerr
1301+
<< "protoc:0: warning: File '" << file->name() << "' has no package "
1302+
<< "and package '" << other_package_for_prefix << "' already uses '"
1303+
<< prefix << "' as its prefix. Consider either adding a new package "
1304+
<< "to the proto, or reusing one of the packages already using this "
1305+
<< "prefix in the expected prefixes file ("
1306+
<< expected_prefixes_path << ")." << std::endl;
1307+
std::cerr.flush();
1308+
}
1309+
return true;
1310+
}
1311+
1312+
// Check: Error - Make sure the prefix wasn't expected for a different
1313+
// package (overlap is allowed, but it has to be listed as an expected
1314+
// overlap).
1315+
if (!other_package_for_prefix.empty()) {
1316+
*out_error =
1317+
"error: Found 'option objc_class_prefix = \"" + prefix +
1318+
"\";' in '" + file->name() +
1319+
"'; that prefix is already used for 'package " +
1320+
other_package_for_prefix + ";'. It can only be reused by listing " +
1321+
"it in the expected file (" +
1322+
expected_prefixes_path + ").";
1323+
return false; // Only report first usage of the prefix.
1324+
}
1325+
} // !prefix.empty()
1326+
12701327
// Check: Warning - Make sure the prefix is is a reasonable value according
12711328
// to Apple's rules (the checks above implicitly whitelist anything that
12721329
// doesn't meet these rules).
@@ -1288,62 +1345,9 @@ bool ValidateObjCClassPrefix(
12881345
std::cerr.flush();
12891346
}
12901347

1291-
// Look for any other package that uses the same (non empty) prefix.
1292-
std::string other_package_for_prefix;
1293-
if (!prefix.empty()) {
1294-
for (std::map<std::string, std::string>::const_iterator i =
1295-
expected_package_prefixes.begin();
1296-
i != expected_package_prefixes.end(); ++i) {
1297-
if (i->second == prefix) {
1298-
other_package_for_prefix = i->first;
1299-
break;
1300-
}
1301-
}
1302-
}
1303-
1304-
// Check: Warning - If the file does not have a package, check whether the non
1305-
// empty prefix declared is being used by another package or not.
1306-
if (package.empty() && !prefix.empty()) {
1307-
// The file does not have a package and ...
1308-
if (other_package_for_prefix.empty()) {
1309-
// ... no other package has declared that prefix.
1310-
std::cerr
1311-
<< "protoc:0: warning: File '" << file->name() << "' has no "
1312-
<< "package. Consider adding a new package to the proto and adding '"
1313-
<< "new.package = " << prefix << "' to the expected prefixes file ("
1314-
<< expected_prefixes_path << ")." << std::endl;
1315-
std::cerr.flush();
1316-
} else {
1317-
// ... another package has declared the same prefix.
1318-
std::cerr
1319-
<< "protoc:0: warning: File '" << file->name() << "' has no package "
1320-
<< "and package '" << other_package_for_prefix << "' already uses '"
1321-
<< prefix << "' as its prefix. Consider either adding a new package "
1322-
<< "to the proto, or reusing one of the packages already using this "
1323-
<< "prefix in the expected prefixes file ("
1324-
<< expected_prefixes_path << ")." << std::endl;
1325-
std::cerr.flush();
1326-
}
1327-
return true;
1328-
}
1329-
1330-
// Check: Error - Make sure the prefix wasn't expected for a different
1331-
// package (overlap is allowed, but it has to be listed as an expected
1332-
// overlap).
1333-
if (!other_package_for_prefix.empty()) {
1334-
*out_error =
1335-
"error: Found 'option objc_class_prefix = \"" + prefix +
1336-
"\";' in '" + file->name() +
1337-
"'; that prefix is already used for 'package " +
1338-
other_package_for_prefix + ";'. It can only be reused by listing " +
1339-
"it in the expected file (" +
1340-
expected_prefixes_path + ").";
1341-
return false; // Only report first usage of the prefix.
1342-
}
1343-
13441348
// Check: Warning - If the given package/prefix pair wasn't expected, issue a
13451349
// warning suggesting it gets added to the file.
1346-
if (!expected_package_prefixes.empty()) {
1350+
if (have_expected_prefix_file) {
13471351
std::cerr
13481352
<< "protoc:0: warning: Found unexpected 'option objc_class_prefix = \""
13491353
<< prefix << "\";' in '" << file->name() << "';"
@@ -1360,6 +1364,12 @@ bool ValidateObjCClassPrefix(
13601364
bool ValidateObjCClassPrefixes(const std::vector<const FileDescriptor*>& files,
13611365
const Options& generation_options,
13621366
std::string* out_error) {
1367+
// Allow a '-' as the path for the expected prefixes to completely disable
1368+
// even the most basic of checks.
1369+
if (generation_options.expected_prefixes_path == "-") {
1370+
return true;
1371+
}
1372+
13631373
// Load the expected package prefixes, if available, to validate against.
13641374
std::map<std::string, std::string> expected_package_prefixes;
13651375
if (!LoadExpectedPackagePrefixes(generation_options,

0 commit comments

Comments
 (0)