Detects single-crate cross-file symbol collisions and conditionally emits crate-root re-exports.

src/google/protobuf/compiler/rust/extension.cc

@@ -9,7 +9,6 @@
 
 #include <string>
 
-#include "absl/strings/ascii.h"
 #include "absl/strings/str_cat.h"
 #include "google/protobuf/compiler/cpp/names.h"
 #include "google/protobuf/compiler/rust/accessors/default_value.h"
@@ -55,7 +54,7 @@ void GenerateRs(Context& ctx, const FieldDescriptor& extension,
 
   // The extension symbol defined by both backends is the same.
   ctx.Emit({{"extendee", RsTypePath(ctx, *extension.containing_type())},
-            {"extension", absl::AsciiStrToUpper(extension.name())},
+            {"extension", ExtensionRsName(extension)},
             {"type", extension.is_repeated()
                          ? absl::StrCat("::protobuf::Repeated<",
                                         RsTypePath(ctx, extension), ">")

src/google/protobuf/compiler/rust/generator.cc

@@ -112,6 +112,59 @@ void EmitPublicImports(const RustGeneratorContext& rust_generator_context,
   }
 }
 
+// Checks whether any two files in the crate export symbols that would collide
+// at the crate root when re-exported via `pub use <file_mod>::*;`.
+//
+// Return true if there is at least one collision in this crate.
+bool CrateHasSymbolCollision(const std::vector<const FileDescriptor*>& files) {
+  absl::flat_hash_set<std::string> type_names;
+  absl::flat_hash_set<std::string> value_names;
+
+  // Returns true if `symbol` was already contributed by an earlier symbol.
+  auto collides = [](absl::flat_hash_set<std::string>& names,
+                     const std::string& symbol) {
+    return !names.insert(symbol).second;
+  };
+
+  for (const FileDescriptor* file : files) {
+    for (int i = 0; i < file->message_type_count(); ++i) {
+      const Descriptor* msg = file->message_type(i);
+      std::string name = MessageRsName(*msg);
+      // A top-level message is emitted as 'Msg, MsgView, MsgMut'
+      if (collides(type_names, name) ||
+          collides(type_names, absl::StrCat(name, "View")) ||
+          collides(type_names, absl::StrCat(name, "Mut"))) {
+        return true;
+      }
+
+      // A submodule is emitted if the message has nested messages, enums,
+      // extensions, or oneofs.
+      if (msg->nested_type_count() > 0 || msg->enum_type_count() > 0 ||
+          msg->extension_count() > 0 || msg->real_oneof_decl_count() > 0) {
+        if (collides(type_names, RsSafeName(CamelToSnakeCase(msg->name())))) {
+          return true;
+        }
+      }
+    }
+
+    // Enums
+    for (int i = 0; i < file->enum_type_count(); ++i) {
+      if (collides(type_names, EnumRsName(*file->enum_type(i)))) {
+        return true;
+      }
+    }
+
+    // Extensions are emitted as `pub const <ext>`.
+    for (int i = 0; i < file->extension_count(); ++i) {
+      if (collides(value_names, ExtensionRsName(*file->extension(i)))) {
+        return true;
+      }
+    }
+  }
+
+  return false;
+}
+
 void EmitEntryPointRsFile(GeneratorContext* generator_context,
                           Context& ctx_without_printer,
                           const std::vector<const FileDescriptor*>& files) {
@@ -125,31 +178,50 @@ void EmitEntryPointRsFile(GeneratorContext* generator_context,
   io::Printer printer(outfile.get());
   Context ctx = ctx_without_printer.WithPrinter(&printer);
 
-  // Declare the submodules for all of the the generated code and pub re-export
-  // all of them into a flat namespace.
+  // Declare the submodules for all of the generated code and, where safe,
+  // pub re-export all of them into a flat namespace.
   RelativePath primary_relpath(entry_point_rs_file_path);
+  const bool has_collision = CrateHasSymbolCollision(files);
+
   for (const FileDescriptor* file : files) {
     std::string non_primary_file_path = GetRsFile(ctx, *file);
     std::string relative_mod_path =
         primary_relpath.Relative(RelativePath(non_primary_file_path));
+    std::string mod_name = RustModuleName(*file);
+
     // 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(
+    // The flat `pub use` re-export into the crate root is conditionally emitted
+    ctx.Emit({{"file_path", relative_mod_path}, {"mod_name", mod_name}},
+             R"rs(
               #[path="$file_path$"]
               #[allow(nonstandard_style, unused)]
               pub mod $mod_name$;
-
-              #[allow(nonstandard_style, unused)]
-              #[doc(inline)]
-              pub use $mod_name$::*;
             )rs");
+
+    if (!has_collision) {
+      ctx.Emit({{"mod_name", mod_name}},
+               R"rs(
+                #[allow(nonstandard_style, unused)]
+                #[doc(inline)]
+                pub use $mod_name$::*;
+              )rs");
+    }
+  }
+
+  // When the crate has cross-file symbol collisions, the flat `pub use`
+  // re-exports above are omitted for every file. Emit a single breadcrumb (not
+  // one per file) explaining why, so consumers know to use fully-qualified
+  // paths.
+  if (has_collision) {
+    ctx.Emit(R"rs(
+            // Crate-root re-exports (`pub use <mod>::*`) are disabled because
+            // this crate contains symbol name collisions across file modules.
+            // Use fully-qualified paths, e.g. `<crate>::<module>::YourType`.
+          )rs");
   }
 
   auto v = ctx.printer().WithVars({

src/google/protobuf/compiler/rust/generator_test.cc

@@ -434,6 +434,211 @@ TEST_F(RustGeneratorTest, EmitsQualifiedExtendeePathForExtensions) {
               HasSubstr("ExtensionId<super::super::foo_proto::Target, i32>"));
 }
 
+TEST_F(RustGeneratorTest, DropsReexportForEntireCrate) {
+  CreateTempFile("a.proto", R"schema(
+    syntax = "proto2";
+    package pkg_a;
+    message Foo { optional int32 x = 1; })schema");
+  CreateTempFile("b.proto", R"schema(
+    syntax = "proto2";
+    package pkg_b;
+    message Foo { optional int32 y = 1; })schema");
+  CreateTempFile("c.proto", R"schema(
+    syntax = "proto2";
+    package pkg_c;
+    message Unique { optional int32 z = 1; })schema");
+  RunProtoc(
+      "protocol_compiler --proto_path=$tmpdir "
+      "--rust_out=$tmpdir "
+      "--rust_opt=experimental-codegen=enabled,kernel=cpp "
+      "a.proto b.proto c.proto");
+  ExpectNoErrors();
+
+  std::string entry_point = FileContents("generated.rs");
+  EXPECT_THAT(entry_point, HasSubstr("pub mod a_proto;"));
+  EXPECT_THAT(entry_point, HasSubstr("pub mod b_proto;"));
+  EXPECT_THAT(entry_point, HasSubstr("pub mod c_proto;"));
+  EXPECT_THAT(entry_point, Not(HasSubstr("pub use a_proto::*;")));
+  EXPECT_THAT(entry_point, Not(HasSubstr("pub use b_proto::*;")));
+  EXPECT_THAT(entry_point, Not(HasSubstr("pub use c_proto::*;")));
+}
+
+TEST_F(RustGeneratorTest, DropsReexportOnEnumCollision) {
+  CreateTempFile("a.proto", R"schema(
+    syntax = "proto2";
+    package pkg_a;
+    enum Status { UNKNOWN = 0; OK = 1; })schema");
+  CreateTempFile("b.proto", R"schema(
+    syntax = "proto2";
+    package pkg_b;
+    enum Status { PENDING = 0; DONE = 1; })schema");
+  RunProtoc(
+      "protocol_compiler --proto_path=$tmpdir "
+      "--rust_out=$tmpdir "
+      "--rust_opt=experimental-codegen=enabled,kernel=cpp "
+      "a.proto b.proto");
+  ExpectNoErrors();
+
+  std::string entry_point = FileContents("generated.rs");
+  EXPECT_THAT(entry_point, HasSubstr("pub mod a_proto;"));
+  EXPECT_THAT(entry_point, HasSubstr("pub mod b_proto;"));
+  EXPECT_THAT(entry_point, Not(HasSubstr("pub use a_proto::*;")));
+  EXPECT_THAT(entry_point, Not(HasSubstr("pub use b_proto::*;")));
+}
+
+TEST_F(RustGeneratorTest, KeepsReexportForNonCollidingFiles) {
+  CreateTempFile("a.proto", R"schema(
+    syntax = "proto2";
+    package pkg_a;
+    message Alpha {
+      optional int32 x = 1;
+    })schema");
+  CreateTempFile("b.proto", R"schema(
+    syntax = "proto2";
+    package pkg_b;
+    message Beta {
+      optional int32 y = 1;
+    })schema");
+  RunProtoc(
+      "protocol_compiler --proto_path=$tmpdir "
+      "--rust_out=$tmpdir "
+      "--rust_opt=experimental-codegen=enabled,kernel=cpp "
+      "a.proto b.proto");
+  ExpectNoErrors();
+
+  std::string entry_point = FileContents("generated.rs");
+  EXPECT_THAT(entry_point, HasSubstr("pub use a_proto::*;"));
+  EXPECT_THAT(entry_point, HasSubstr("pub use b_proto::*;"));
+}
+
+TEST_F(RustGeneratorTest, DropsReexportOnViewCollision) {
+  CreateTempFile("a.proto", R"schema(
+    syntax = "proto2";
+    package pkg_a;
+    message BarView { optional int32 x = 1; })schema");
+  CreateTempFile("b.proto", R"schema(
+    syntax = "proto2";
+    package pkg_b;
+    message Bar { optional int32 y = 1; })schema");
+  RunProtoc(
+      "protocol_compiler --proto_path=$tmpdir "
+      "--rust_out=$tmpdir "
+      "--rust_opt=experimental-codegen=enabled,kernel=cpp "
+      "a.proto b.proto");
+  ExpectNoErrors();
+
+  std::string entry_point = FileContents("generated.rs");
+  EXPECT_THAT(entry_point, Not(HasSubstr("pub use a_proto::*;")));
+  EXPECT_THAT(entry_point, Not(HasSubstr("pub use b_proto::*;")));
+}
+
+TEST_F(RustGeneratorTest, DropsReexportOnMutCollision) {
+  CreateTempFile("a.proto", R"schema(
+    syntax = "proto2";
+    package pkg_a;
+    message QuxMut { optional int32 x = 1; })schema");
+  CreateTempFile("b.proto", R"schema(
+    syntax = "proto2";
+    package pkg_b;
+    message Qux { optional int32 y = 1; })schema");
+  RunProtoc(
+      "protocol_compiler --proto_path=$tmpdir "
+      "--rust_out=$tmpdir "
+      "--rust_opt=experimental-codegen=enabled,kernel=cpp "
+      "a.proto b.proto");
+  ExpectNoErrors();
+
+  std::string entry_point = FileContents("generated.rs");
+  EXPECT_THAT(entry_point, Not(HasSubstr("pub use a_proto::*;")));
+  EXPECT_THAT(entry_point, Not(HasSubstr("pub use b_proto::*;")));
+}
+
+TEST_F(RustGeneratorTest, DropsReexportOnSubmoduleCollision) {
+  CreateTempFile("a.proto", R"schema(
+    syntax = "proto2";
+    package pkg_a;
+    message data { optional int32 x = 1; })schema");
+  CreateTempFile("b.proto", R"schema(
+    syntax = "proto2";
+    package pkg_b;
+    message Data {
+      message Row { optional int32 v = 1; }
+      optional Row row = 1;
+    })schema");
+  RunProtoc(
+      "protocol_compiler --proto_path=$tmpdir "
+      "--rust_out=$tmpdir "
+      "--rust_opt=experimental-codegen=enabled,kernel=cpp "
+      "a.proto b.proto");
+  ExpectNoErrors();
+
+  std::string entry_point = FileContents("generated.rs");
+  EXPECT_THAT(entry_point, Not(HasSubstr("pub use a_proto::*;")));
+  EXPECT_THAT(entry_point, Not(HasSubstr("pub use b_proto::*;")));
+}
+
+TEST_F(RustGeneratorTest, DropsReexportOnExtensionCollision) {
+  CreateTempFile("a.proto", R"schema(
+    syntax = "proto2";
+    package pkg_a;
+    message TargetA { extensions 100 to 200; }
+    extend TargetA { optional int32 shared = 100; })schema");
+  CreateTempFile("b.proto", R"schema(
+    syntax = "proto2";
+    package pkg_b;
+    message TargetB { extensions 100 to 200; }
+    extend TargetB { optional int32 shared = 100; })schema");
+  RunProtoc(
+      "protocol_compiler --proto_path=$tmpdir "
+      "--rust_out=$tmpdir "
+      "--rust_opt=experimental-codegen=enabled,kernel=cpp "
+      "a.proto b.proto");
+  ExpectNoErrors();
+
+  std::string entry_point = FileContents("generated.rs");
+  EXPECT_THAT(entry_point, Not(HasSubstr("pub use a_proto::*;")));
+  EXPECT_THAT(entry_point, Not(HasSubstr("pub use b_proto::*;")));
+}
+
+TEST_F(RustGeneratorTest, KeepsReexportForCrateWithFeaturesButNoCollision) {
+  CreateTempFile("a.proto", R"schema(
+    syntax = "proto2";
+    package pkg_a;
+    message AlphaMsg {
+      message AlphaInner { optional int32 v = 1; }
+      extensions 100 to 200;
+      oneof alpha_choice {
+        int32 a = 1;
+        int32 b = 2;
+      }
+    }
+    enum AlphaEnum { AE0 = 0; AE1 = 1; }
+    extend AlphaMsg { optional int32 alpha_ext = 100; })schema");
+  CreateTempFile("b.proto", R"schema(
+    syntax = "proto2";
+    package pkg_b;
+    message BetaMsg {
+      message BetaInner { optional int32 v = 1; }
+      extensions 100 to 200;
+      oneof beta_choice {
+        int32 a = 1;
+        int32 b = 2;
+      }
+    }
+    enum BetaEnum { BE0 = 0; BE1 = 1; }
+    extend BetaMsg { optional int32 beta_ext = 100; })schema");
+  RunProtoc(
+      "protocol_compiler --proto_path=$tmpdir "
+      "--rust_out=$tmpdir "
+      "--rust_opt=experimental-codegen=enabled,kernel=cpp "
+      "a.proto b.proto");
+  ExpectNoErrors();
+
+  std::string entry_point = FileContents("generated.rs");
+  EXPECT_THAT(entry_point, HasSubstr("pub use a_proto::*;"));
+  EXPECT_THAT(entry_point, HasSubstr("pub use b_proto::*;"));
+}
+
 }  // namespace
 }  // namespace rust
 }  // namespace compiler

src/google/protobuf/compiler/rust/naming.cc

@@ -391,6 +391,10 @@ std::string EnumRsName(const EnumDescriptor& desc) {
   return name;
 }
 
+std::string ExtensionRsName(const FieldDescriptor& extension) {
+  return absl::AsciiStrToUpper(extension.name());
+}
+
 std::string EnumValueRsName(const EnumValueDescriptor& value) {
   MultiCasePrefixStripper stripper(value.type()->name());
   return EnumValueRsName(stripper, value.name());

src/google/protobuf/compiler/rust/naming.h

@@ -64,6 +64,12 @@ std::string MessageRsName(const Descriptor& desc);
 std::string EnumRsName(const EnumDescriptor& desc);
 std::string EnumValueRsName(const EnumValueDescriptor& value);
 
+// Returns the Rust identifier for a message extension, emitted as a
+// `pub const <NAME>: ExtensionId<...>`. Centralizing this here keeps the name
+// used by the code generator in sync with any future mangling (e.g. for
+// extension names that are not valid Rust identifiers).
+std::string ExtensionRsName(const FieldDescriptor& extension);
+
 std::string OneofViewEnumRsName(const OneofDescriptor& oneof);
 std::string OneofCaseEnumRsName(const OneofDescriptor& oneof);