Skip to content

Commit

Permalink
Implement the Edition 2024 naming style enforcement feature.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 724001536
  • Loading branch information
protobuf-github-bot authored and copybara-github committed Feb 6, 2025
1 parent 92db7e2 commit 7804f69
Show file tree
Hide file tree
Showing 6 changed files with 439 additions and 4 deletions.
1 change: 1 addition & 0 deletions src/google/protobuf/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,7 @@ cc_library(
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/strings:charset",
"@com_google_absl//absl/strings:cord",
"@com_google_absl//absl/strings:internal",
"@com_google_absl//absl/strings:str_format",
Expand Down
1 change: 1 addition & 0 deletions src/google/protobuf/compiler/command_line_interface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1247,6 +1247,7 @@ int CommandLineInterface::Run(int argc, const char* const argv[]) {
}

descriptor_pool->EnforceWeakDependencies(true);
descriptor_pool->EnforceNamingStyle(true);

if (!SetupFeatureResolution(*descriptor_pool)) {
return EXIT_FAILURE;
Expand Down
10 changes: 10 additions & 0 deletions src/google/protobuf/compiler/command_line_interface_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1520,6 +1520,16 @@ TEST_F(CommandLineInterfaceTest, InvalidFeatureExtensionError) {
"generator --test_out specifies an unknown feature extension");
}

TEST_F(CommandLineInterfaceTest, NamingStyleEnforced) {
CreateTempFile("foo.proto",
R"schema(
edition = "2024";
package badPackage;
)schema");
Run("protocol_compiler --proto_path=$tmpdir --test_out=$tmpdir foo.proto");
ExpectErrorSubstring("Package name badPackage should be lower_snake_case");
}

TEST_F(CommandLineInterfaceTest, Plugin_InvalidFeatureExtensionError) {
CreateTempFile("foo.proto", R"schema(
edition = "2023";
Expand Down
224 changes: 221 additions & 3 deletions src/google/protobuf/descriptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/strings/ascii.h"
#include "absl/strings/charset.h"
#include "absl/strings/escaping.h"
#include "absl/strings/match.h"
#include "absl/strings/str_cat.h"
Expand Down Expand Up @@ -2123,7 +2124,8 @@ DescriptorPool::DescriptorPool()
enforce_weak_(false),
enforce_extension_declarations_(ExtDeclEnforcementLevel::kNoEnforcement),
disallow_enforce_utf8_(false),
deprecated_legacy_json_field_conflicts_(false) {}
deprecated_legacy_json_field_conflicts_(false),
enforce_naming_style_(false) {}

DescriptorPool::DescriptorPool(DescriptorDatabase* fallback_database,
ErrorCollector* error_collector)
Expand All @@ -2138,7 +2140,8 @@ DescriptorPool::DescriptorPool(DescriptorDatabase* fallback_database,
enforce_weak_(false),
enforce_extension_declarations_(ExtDeclEnforcementLevel::kNoEnforcement),
disallow_enforce_utf8_(false),
deprecated_legacy_json_field_conflicts_(false) {}
deprecated_legacy_json_field_conflicts_(false),
enforce_naming_style_(false) {}

DescriptorPool::DescriptorPool(const DescriptorPool* underlay)
: mutex_(nullptr),
Expand All @@ -2152,7 +2155,8 @@ DescriptorPool::DescriptorPool(const DescriptorPool* underlay)
enforce_weak_(false),
enforce_extension_declarations_(ExtDeclEnforcementLevel::kNoEnforcement),
disallow_enforce_utf8_(false),
deprecated_legacy_json_field_conflicts_(false) {}
deprecated_legacy_json_field_conflicts_(false),
enforce_naming_style_(false) {}

DescriptorPool::~DescriptorPool() {
if (mutex_ != nullptr) delete mutex_;
Expand Down Expand Up @@ -4729,6 +4733,28 @@ class DescriptorBuilder {

void ValidateJSType(const FieldDescriptor* field,
const FieldDescriptorProto& proto);

void ValidateNamingStyle(const FileDescriptor* file,
const FileDescriptorProto& proto);
void ValidateNamingStyle(const Descriptor* message,
const DescriptorProto& proto);
void ValidateNamingStyle(const OneofDescriptor* oneof,
const OneofDescriptorProto& proto);
void ValidateNamingStyle(const FieldDescriptor* field,
const FieldDescriptorProto& proto);
void ValidateNamingStyle(const EnumDescriptor* enum_descriptor,
const EnumDescriptorProto& proto);
void ValidateNamingStyle(const EnumValueDescriptor* enum_value,
const EnumValueDescriptorProto& proto);
void ValidateNamingStyle(const ServiceDescriptor* service,
const ServiceDescriptorProto& proto);
void ValidateNamingStyle(const MethodDescriptor* method,
const MethodDescriptorProto& proto);

// Nothing to validate for extension ranges. This overload only exists
// so that VisitDescriptors can be exhaustive.
void ValidateNamingStyle(const Descriptor::ExtensionRange* ext_range,
const DescriptorProto::ExtensionRange& proto) {}
};

const FileDescriptor* DescriptorPool::BuildFile(
Expand Down Expand Up @@ -6274,6 +6300,16 @@ FileDescriptor* DescriptorBuilder::BuildFileImpl(
});
}

if (!had_errors_ && pool_->enforce_naming_style_) {
internal::VisitDescriptors(
*result, proto, [&](const auto& descriptor, const auto& desc_proto) {
if (internal::InternalFeatureHelper::GetFeatures(descriptor)
.enforce_naming_style() == FeatureSet::STYLE2024) {
ValidateNamingStyle(&descriptor, desc_proto);
}
});
}

if (had_errors_) {
return nullptr;
} else {
Expand Down Expand Up @@ -8616,6 +8652,188 @@ void DescriptorBuilder::ValidateJSType(const FieldDescriptor* field,
}
}

namespace {

// Whether the name contains underscores that violate the naming style guide (
// a leading or trailing underscore, or an underscore which is not followed by
// a letter)
bool ContainsBadUnderscores(absl::string_view name) {
if (name.empty()) {
return false;
}
if (name[0] == '_' || name[name.size() - 1] == '_') {
return true;
}
for (size_t i = 1; i < name.size(); ++i) {
if (name[i - 1] == '_' && !absl::ascii_isalpha(name[i])) {
return true;
}
}
return false;
}

bool IsValidTitleCaseName(absl::string_view name, std::string* error) {
ABSL_CHECK(!name.empty());
for (char c : name) {
if (!absl::ascii_isalnum(c)) {
*error = "should be TitleCase";
return false;
}
}
if (!absl::ascii_isupper(name[0])) {
*error = "should begin with a capital letter";
return false;
}
return true;
}

bool IsValidLowerSnakeCaseName(absl::string_view name, std::string* error) {
ABSL_CHECK(!name.empty());

constexpr absl::CharSet kLowerSnakeCaseChars =
absl::CharSet::Range('a', 'z') | absl::CharSet::Range('0', '9') |
absl::CharSet::Char('_') | absl::CharSet::Char('.');
for (char c : name) {
if (!kLowerSnakeCaseChars.contains(c)) {
*error = "should be lower_snake_case";
return false;
}
}
if (!absl::ascii_islower(name[0])) {
*error = "should begin with a lower case letter";
return false;
}
if (ContainsBadUnderscores(name)) {
*error = "contains style violating underscores";
return false;
}
return true;
}

bool IsValidUpperSnakeCaseName(absl::string_view name, std::string* error) {
ABSL_CHECK(!name.empty());

constexpr absl::CharSet kUpperSnakeCaseChars =
absl::CharSet::Range('A', 'Z') | absl::CharSet::Range('0', '9') |
absl::CharSet::Char('_');
for (char c : name) {
if (!kUpperSnakeCaseChars.contains(c)) {
*error = "should be UPPER_SNAKE_CASE";
return false;
}
}
if (!absl::ascii_isupper(name[0])) {
*error = "should begin with an upper case letter";
return false;
}
if (ContainsBadUnderscores(name)) {
*error = "contains style violating underscores";
return false;
}
return true;
}

constexpr absl::string_view kNamingStyleOptOutMessage =
" (feature.enforce_naming_style = STYLE_LEGACY can be used to opt out of "
"this check)";

} // namespace

void DescriptorBuilder::ValidateNamingStyle(const FileDescriptor* file,
const FileDescriptorProto& proto) {
// Ignore empty packages for style checks.
if (file->package().empty()) {
return;
}
std::string error;
if (!IsValidLowerSnakeCaseName(file->package(), &error)) {
AddError(file->name(), proto, DescriptorPool::ErrorCollector::NAME, [&] {
return absl::StrCat("Package name ", file->package(), " ", error,
kNamingStyleOptOutMessage);
});
}
}

void DescriptorBuilder::ValidateNamingStyle(const Descriptor* message,
const DescriptorProto& proto) {
std::string error;
if (!IsValidTitleCaseName(message->name(), &error)) {
AddError(message->name(), proto, DescriptorPool::ErrorCollector::NAME, [&] {
return absl::StrCat("Message name ", message->name(), " ", error,
kNamingStyleOptOutMessage);
});
}
}

void DescriptorBuilder::ValidateNamingStyle(const OneofDescriptor* oneof,
const OneofDescriptorProto& proto) {
std::string error;
if (!IsValidLowerSnakeCaseName(oneof->name(), &error)) {
AddError(oneof->name(), proto, DescriptorPool::ErrorCollector::NAME, [&] {
return absl::StrCat("Oneof name ", oneof->name(), " ", error,
kNamingStyleOptOutMessage);
});
}
}

void DescriptorBuilder::ValidateNamingStyle(const FieldDescriptor* field,
const FieldDescriptorProto& proto) {
std::string error;
if (!IsValidLowerSnakeCaseName(field->name(), &error)) {
AddError(field->name(), proto, DescriptorPool::ErrorCollector::NAME, [&] {
return absl::StrCat("Field name ", field->name(), " ", error,
kNamingStyleOptOutMessage);
});
}
}

void DescriptorBuilder::ValidateNamingStyle(
const EnumDescriptor* enum_descriptor, const EnumDescriptorProto& proto) {
std::string error;
if (!IsValidTitleCaseName(enum_descriptor->name(), &error)) {
AddError(enum_descriptor->name(), proto,
DescriptorPool::ErrorCollector::NAME, [&] {
return absl::StrCat("Enum name ", enum_descriptor->name(), " ",
error, kNamingStyleOptOutMessage);
});
}
}

void DescriptorBuilder::ValidateNamingStyle(
const EnumValueDescriptor* enum_value,
const EnumValueDescriptorProto& proto) {
std::string error;
if (!IsValidUpperSnakeCaseName(enum_value->name(), &error)) {
AddError(enum_value->name(), proto, DescriptorPool::ErrorCollector::NAME,
[&] {
return absl::StrCat("Enum value name ", enum_value->name(), " ",
error, kNamingStyleOptOutMessage);
});
}
}

void DescriptorBuilder::ValidateNamingStyle(
const ServiceDescriptor* service, const ServiceDescriptorProto& proto) {
std::string error;
if (!IsValidTitleCaseName(service->name(), &error)) {
AddError(service->name(), proto, DescriptorPool::ErrorCollector::NAME, [&] {
return absl::StrCat("Service name ", service->name(), " ", error,
kNamingStyleOptOutMessage);
});
}
}

void DescriptorBuilder::ValidateNamingStyle(
const MethodDescriptor* method, const MethodDescriptorProto& proto) {
std::string error;
if (!IsValidTitleCaseName(method->name(), &error)) {
AddError(method->name(), proto, DescriptorPool::ErrorCollector::NAME, [&] {
return absl::StrCat("Method name ", method->name(), " ", error,
kNamingStyleOptOutMessage);
});
}
}

// -------------------------------------------------------------------

DescriptorBuilder::OptionInterpreter::OptionInterpreter(
Expand Down
10 changes: 10 additions & 0 deletions src/google/protobuf/descriptor.h
Original file line number Diff line number Diff line change
Expand Up @@ -2276,6 +2276,15 @@ class PROTOBUF_EXPORT DescriptorPool {
// DescriptorPool will report a import not found error.
void EnforceWeakDependencies(bool enforce) { enforce_weak_ = enforce; }

// Enforce the naming style rules. This applies the intended style rules
// corresponding to the edition of the file, and was first introduced
// with Edition 2024. Proto2, Proto3 and Edition 2023 are never considered
// to be in violation and so are unaffected by this.
//
// In Edition 2024+, the 'enforce_naming_style` feature can be used to opt out
// of this enforcement.
void EnforceNamingStyle(bool enforce) { enforce_naming_style_ = enforce; }

// Sets the default feature mappings used during the build. If this function
// isn't called, the C++ feature set defaults are used. If this function is
// called, these defaults will be used instead.
Expand Down Expand Up @@ -2576,6 +2585,7 @@ class PROTOBUF_EXPORT DescriptorPool {
ExtDeclEnforcementLevel enforce_extension_declarations_;
bool disallow_enforce_utf8_;
bool deprecated_legacy_json_field_conflicts_;
bool enforce_naming_style_;
mutable bool build_started_ = false;

// Set of files to track for additional validation. The bool value when true
Expand Down
Loading

0 comments on commit 7804f69

Please sign in to comment.