Skip to content

Expose generated per-file Rust proto modules as public. #29496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
copybara-service wants to merge 1 commit into from
Closed

Expose generated per-file Rust proto modules as public. #29496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 11 additions & 14 deletions src/google/protobuf/compiler/rust/generator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,23 +132,20 @@ void EmitEntryPointRsFile(GeneratorContext* generator_context,
std::string non_primary_file_path = GetRsFile(ctx, *file);
std::string relative_mod_path =
primary_relpath.Relative(RelativePath(non_primary_file_path));
// Expose each generated .proto file as a public module named after its
// (flattened) file path, providing a fully-qualified path to every type
// (e.g. `my_crate::google_network_api_proto::Config`). See
// RustModuleName for how the module name is derived from the path.
//
// The flat `pub use` re-export into the crate root is also emitted so that
// existing consumers relying on the crate-root namespace continue to work.
ctx.Emit(
{{"file_path", relative_mod_path}, {"mod_name", RustModuleName(*file)}},
R"rs(
// Temporarily emit these re-exported mods as pub to avoid issues with
// Crubit. In a future change we should change these back to be private
// mods.
ctx.Emit({{"file_path", relative_mod_path},
{"mod_name", RustInternalModuleName(*file)}},
R"rs(
#[path="$file_path$"]
#[allow(nonstandard_style, unused)]
pub mod $mod_name$;
#[allow(nonstandard_style, unused, unreachable_pub)]
#[doc(hidden)]
mod internal_do_not_use_$mod_name$;

#[allow(nonstandard_style, unused)]
#[doc(inline)]
pub use $mod_name$::*;
pub use internal_do_not_use_$mod_name$::*;
)rs");
}

Expand Down Expand Up @@ -217,7 +214,7 @@ bool RustGenerator::Generate(const FileDescriptor* file,
&*import_path_to_crate_name);

std::vector<std::string> modules;
modules.emplace_back(RustModuleName(*file));
modules.emplace_back(RustInternalModuleName(*file));
Context ctx_without_printer(&*opts, &rust_generator_context, nullptr,
std::move(modules));

Expand Down
90 changes: 0 additions & 90 deletions src/google/protobuf/compiler/rust/generator_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,96 +204,6 @@ TEST_F(RustGeneratorTest, EmitsEnumMetadata) {
"Alias1", GeneratedCodeInfo::Annotation::NONE, 1);
}

TEST_F(RustGeneratorTest, EmitsPublicFileModuleInEntryPoint) {
constexpr absl::string_view kFooProto = R"schema(
syntax = "proto2";
package foo;
message Message {
})schema";
CreateTempFile("foo.proto", kFooProto);
RunProtoc(
"protocol_compiler --proto_path=$tmpdir "
"--rust_out=$tmpdir "
"--rust_opt=experimental-codegen=enabled,kernel=cpp "
"foo.proto");
ExpectNoErrors();

std::string entry_point = FileContents("generated.rs");
EXPECT_THAT(entry_point, HasSubstr("pub mod foo_proto;"));
EXPECT_THAT(entry_point, HasSubstr("pub use foo_proto::*;"));
EXPECT_THAT(entry_point, Not(HasSubstr("internal_do_not_use_")));
EXPECT_THAT(entry_point, Not(HasSubstr("#[doc(hidden)]")));
}

TEST_F(RustGeneratorTest, EmitsPublicFileModulesForMultiFileCrate) {
constexpr absl::string_view kFooProto = R"schema(
syntax = "proto2";
package foo;
message Config {
})schema";
constexpr absl::string_view kBarProto = R"schema(
syntax = "proto2";
package bar;
message Config {
})schema";
CreateTempFile("foo.proto", kFooProto);
CreateTempFile("bar.proto", kBarProto);
RunProtoc(
"protocol_compiler --proto_path=$tmpdir "
"--rust_out=$tmpdir "
"--rust_opt=experimental-codegen=enabled,kernel=cpp "
"foo.proto bar.proto");
ExpectNoErrors();

std::string entry_point = FileContents("generated.rs");
EXPECT_THAT(entry_point, HasSubstr("pub mod foo_proto;"));
EXPECT_THAT(entry_point, HasSubstr("pub mod bar_proto;"));
EXPECT_THAT(entry_point, Not(HasSubstr("internal_do_not_use_")));
}

TEST_F(RustGeneratorTest, EmitsValidIdentifierForLeadingDigitFile) {
constexpr absl::string_view kProto = R"schema(
syntax = "proto2";
package sample;
message Config {}
)schema";
CreateTempFile("0.1.proto", kProto);
RunProtoc(
"protocol_compiler --proto_path=$tmpdir "
"--rust_out=$tmpdir "
"--rust_opt=experimental-codegen=enabled,kernel=cpp "
"0.1.proto");
ExpectNoErrors();

std::string entry_point = FileContents("generated.rs");
// A leading non-letter gets a `pb_` prefix so the module name is a valid
// identifier (`0.1.proto` must not produce `pub mod 0_1_proto;`).
EXPECT_THAT(entry_point, HasSubstr("pub mod pb_0_1_proto;"));
EXPECT_THAT(entry_point, HasSubstr("pub use pb_0_1_proto::*;"));
EXPECT_THAT(entry_point, Not(HasSubstr("pub mod 0_1_proto;")));
}

TEST_F(RustGeneratorTest, EmitsReadableModuleNameForHyphenatedFile) {
constexpr absl::string_view kProto = R"schema(
syntax = "proto2";
package sample;
message A {}
)schema";
CreateTempFile("my-service.proto", kProto);
RunProtoc(
"protocol_compiler --proto_path=$tmpdir "
"--rust_out=$tmpdir "
"--rust_opt=experimental-codegen=enabled,kernel=cpp "
"my-service.proto");
ExpectNoErrors();

std::string entry_point = FileContents("generated.rs");
// Separators collapse to `_` and the `.proto` extension becomes `_proto`;
// no `_2d_` hex escaping is used for a plain hyphen.
EXPECT_THAT(entry_point, HasSubstr("pub mod my_service_proto;"));
EXPECT_THAT(entry_point, Not(HasSubstr("_2d_")));
}

TEST_F(RustGeneratorTest, EmitsQualifiedPathForSameFileMessageReference) {
// Foo uses Bar from the same file.
constexpr absl::string_view kFooProto = R"schema(
Expand Down
30 changes: 10 additions & 20 deletions src/google/protobuf/compiler/rust/naming.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,32 +244,22 @@ std::string RustModule(Context& ctx, const OneofDescriptor& oneof) {
*oneof.file());
}

std::string RustModuleName(const FileDescriptor& file) {
// Derive a readable and (mostly) unique Rust module name from the full
// proto file path, e.g. `foo/bar/baz.proto` becomes `foo_bar_baz_proto`.
absl::string_view name = file.name();
absl::string_view prefix = "pb_";

std::string RustInternalModuleName(const FileDescriptor& file) {
std::string result;
result.reserve(name.size() + prefix.size());

// Rust identifiers must start with a letter or underscore. If the path begins
// with anything else (e.g. a digit), prepend `pb_` so the result is valid.
if (name.empty() || !absl::ascii_isalpha(name[0])) {
result += prefix;
}

for (char c : name) {
// Common path/file separators (`/`, `-`, `.`, and `_`) all collapse to a
// single underscore for better readability.
if (c == '/' || c == '-' || c == '.' || c == '_') {
result += '_';
result.reserve(file.name().size());
for (char c : StripProto(file.name())) {
if (c == '_') {
result += "__";
} else if (c == '-') {
result += "__";
} else if (c == '/') {
result += "_s";
} else if (absl::ascii_isalnum(c)) {
result += c;
} else {
// Escape any other characters that aren't valid in Rust identifiers
// by substituting them with an underscore followed by their hex value
// and another underscore.
// and another underscore (e.g. '.' becomes '_2e_').
absl::StrAppendFormat(&result, "_%02x_", static_cast<unsigned char>(c));
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/google/protobuf/compiler/rust/naming.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ std::string RustModule(Context& ctx, const Descriptor& msg);
std::string RustModule(Context& ctx, const EnumDescriptor& enum_);
std::string RustModule(Context& ctx, const OneofDescriptor& oneof);

std::string RustModuleName(const FileDescriptor& file);
std::string RustInternalModuleName(const FileDescriptor& file);

template <typename Desc>
std::string GetUnderscoreDelimitedFullName(Context& ctx, const Desc& desc);
Expand Down
37 changes: 14 additions & 23 deletions src/google/protobuf/compiler/rust/naming_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,41 +9,32 @@
#include "google/protobuf/io/zero_copy_stream_impl_lite.h"

using google::protobuf::compiler::rust::CamelToSnakeCase;
using google::protobuf::compiler::rust::RustModuleName;
using google::protobuf::compiler::rust::RustInternalModuleName;
using google::protobuf::compiler::rust::ScreamingSnakeToUpperCamelCase;

namespace {
TEST(RustProtoNaming, RustModuleName) {
TEST(RustProtoNaming, RustInternalModuleName) {
auto get_internal_module_name = [](const std::string& name) {
google::protobuf::FileDescriptorProto file_proto;
file_proto.set_name(name);
google::protobuf::DescriptorPool pool;
return RustModuleName(*pool.BuildFile(file_proto));
return RustInternalModuleName(*pool.BuildFile(file_proto));
};

EXPECT_EQ(get_internal_module_name("strong_bad/lol.proto"),
"strong_bad_lol_proto");
EXPECT_EQ(get_internal_module_name("0.1.proto"), "pb_0_1_proto");
EXPECT_EQ(get_internal_module_name("2fa.proto"), "pb_2fa_proto");
EXPECT_EQ(get_internal_module_name("_.proto"), "pb___proto");
EXPECT_EQ(get_internal_module_name("abc .proto"), "abc_20__20__20__proto");
EXPECT_EQ(get_internal_module_name("hello (2).proto"),
"hello_20__28_2_29__proto");
EXPECT_EQ(get_internal_module_name("k8s.min.proto"), "k8s_min_proto");
EXPECT_EQ(get_internal_module_name("c++.proto"), "c_2b__2b__proto");
EXPECT_EQ(get_internal_module_name("hello,world.proto"),
"hello_2c_world_proto");
"strong__bad_slol");
EXPECT_EQ(get_internal_module_name("0.1.proto"), "0_2e_1");
EXPECT_EQ(get_internal_module_name("_.proto"), "__");
EXPECT_EQ(get_internal_module_name("abc .proto"), "abc_20__20__20_");
EXPECT_EQ(get_internal_module_name("hello (2).proto"), "hello_20__28_2_29_");
EXPECT_EQ(get_internal_module_name("k8s.min.proto"), "k8s_2e_min");
EXPECT_EQ(get_internal_module_name("c++.proto"), "c_2b__2b_");
EXPECT_EQ(get_internal_module_name("hello,world.proto"), "hello_2c_world");
EXPECT_EQ(get_internal_module_name("hello..world.proto"),
"hello__world_proto");
"hello_2e__2e_world");
EXPECT_EQ(get_internal_module_name("hello_你好.proto"),
"hello__e4__bd__a0__e5__a5__bd__proto");
// Common separators (`/`, `-`, `.`, `_`) all collapse to a single underscore,
// so files differing only by these separators intentionally map to the same
// module name (any collision surfaces as a rustc E0428 error at build time).
EXPECT_EQ(get_internal_module_name("my-message.proto"), "my_message_proto");
EXPECT_EQ(get_internal_module_name("my_message.proto"), "my_message_proto");
EXPECT_EQ(get_internal_module_name("foo-bar.proto"),
get_internal_module_name("foo_bar.proto"));
"hello___e4__bd__a0__e5__a5__bd_");
EXPECT_EQ(get_internal_module_name("my-message.proto"), "my__message");
}

TEST(RustProtoNaming, CamelToSnakeCase) {
Expand Down
Loading