From 15296b8f2f561f03f7c8a071d732c537c3a5f5f7 Mon Sep 17 00:00:00 2001 From: Tyler Hawkes Date: Mon, 6 Sep 2021 12:36:30 -0600 Subject: [PATCH] Add support for no package declaration (#343) Co-authored-by: Jacob Kiesel --- README.md | 4 +- prost-build/src/code_generator.rs | 23 ++++++++-- prost-build/src/lib.rs | 24 ++++++++-- prost-build/src/message_graph.rs | 12 ++--- tests/src/build.rs | 43 ++++++++++++++--- tests/src/lib.rs | 1 + tests/src/no_root_packages/gizmo.proto | 8 ++++ tests/src/no_root_packages/mod.rs | 46 +++++++++++++++++++ tests/src/no_root_packages/root.proto | 6 +++ tests/src/no_root_packages/widget.proto | 14 ++++++ .../src/no_root_packages/widget_factory.proto | 24 ++++++++++ 11 files changed, 179 insertions(+), 26 deletions(-) create mode 100644 tests/src/no_root_packages/gizmo.proto create mode 100644 tests/src/no_root_packages/mod.rs create mode 100644 tests/src/no_root_packages/root.proto create mode 100644 tests/src/no_root_packages/widget.proto create mode 100644 tests/src/no_root_packages/widget_factory.proto diff --git a/README.md b/README.md index 1db38c5cf..99fc9ab53 100644 --- a/README.md +++ b/README.md @@ -49,8 +49,8 @@ possible. ### Packages -All `.proto` files used with `prost` must contain a -[`package` specifier][package]. `prost` will translate the Protobuf package into +Prost can now generate code for `.proto` files that don't have a package spec. +`prost` will translate the Protobuf package into a Rust module. For example, given the `package` specifier: [package]: https://developers.google.com/protocol-buffers/docs/proto#packages diff --git a/prost-build/src/code_generator.rs b/prost-build/src/code_generator.rs index 36737a0be..24f8b1133 100644 --- a/prost-build/src/code_generator.rs +++ b/prost-build/src/code_generator.rs @@ -65,7 +65,7 @@ impl<'a> CodeGenerator<'a> { let mut code_gen = CodeGenerator { config, - package: file.package.unwrap(), + package: file.package.unwrap_or_else(String::new), source_info, syntax, message_graph, @@ -117,7 +117,12 @@ impl<'a> CodeGenerator<'a> { debug!(" message: {:?}", message.name()); let message_name = message.name().to_string(); - let fq_message_name = format!(".{}.{}", self.package, message.name()); + let fq_message_name = format!( + "{}{}.{}", + if self.package.is_empty() { "" } else { "." }, + self.package, + message.name() + ); // Skip external types. if self.extern_paths.resolve_ident(&fq_message_name).is_some() { @@ -581,7 +586,12 @@ impl<'a> CodeGenerator<'a> { // Skip external types. let enum_name = &desc.name(); let enum_values = &desc.value; - let fq_enum_name = format!(".{}.{}", self.package, enum_name); + let fq_enum_name = format!( + "{}{}.{}", + if self.package.is_empty() { "" } else { "." }, + self.package, + enum_name + ); if self.extern_paths.resolve_ident(&fq_enum_name).is_some() { return; } @@ -766,6 +776,13 @@ impl<'a> CodeGenerator<'a> { let mut local_path = self.package.split('.').peekable(); + // If no package is specified the start of the package name will be '.' + // and split will return an empty string ("") which breaks resolution + // The fix to this is to ignore the first item if it is empty. + if local_path.peek().map_or(false, |s| s.is_empty()) { + local_path.next(); + } + let mut ident_path = pb_ident[1..].split('.'); let ident_type = ident_path.next_back().unwrap(); let mut ident_path = ident_path.peekable(); diff --git a/prost-build/src/lib.rs b/prost-build/src/lib.rs index 816fd7351..8bb0cb429 100644 --- a/prost-build/src/lib.rs +++ b/prost-build/src/lib.rs @@ -230,6 +230,7 @@ pub struct Config { strip_enum_prefix: bool, out_dir: Option, extern_paths: Vec<(String, String)>, + default_package_filename: String, protoc_args: Vec, disable_comments: PathMap<()>, } @@ -658,6 +659,15 @@ impl Config { self } + /// Configures what filename protobufs with no package definition are written to. + pub fn default_package_filename(&mut self, filename: S) -> &mut Self + where + S: Into, + { + self.default_package_filename = filename.into(); + self + } + /// Add an argument to the `protoc` protobuf compilation invocation. /// /// # Example `build.rs` @@ -772,7 +782,12 @@ impl Config { let modules = self.generate(file_descriptor_set.file)?; for (module, content) in modules { - let mut filename = module.join("."); + let mut filename = if module.is_empty() { + self.default_package_filename.clone() + } else { + module.join(".") + }; + filename.push_str(".rs"); let output_path = target.join(&filename); @@ -846,6 +861,7 @@ impl default::Default for Config { strip_enum_prefix: true, out_dir: None, extern_paths: Vec::new(), + default_package_filename: "_".to_string(), protoc_args: Vec::new(), disable_comments: PathMap::default(), } @@ -856,10 +872,7 @@ impl fmt::Debug for Config { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { fmt.debug_struct("Config") .field("file_descriptor_set_path", &self.file_descriptor_set_path) - .field( - "service_generator", - &self.file_descriptor_set_path.is_some(), - ) + .field("service_generator", &self.service_generator.is_some()) .field("map_type", &self.map_type) .field("bytes_type", &self.bytes_type) .field("type_attributes", &self.type_attributes) @@ -868,6 +881,7 @@ impl fmt::Debug for Config { .field("strip_enum_prefix", &self.strip_enum_prefix) .field("out_dir", &self.out_dir) .field("extern_paths", &self.extern_paths) + .field("default_package_filename", &self.default_package_filename) .field("protoc_args", &self.protoc_args) .field("disable_comments", &self.disable_comments) .finish() diff --git a/prost-build/src/message_graph.rs b/prost-build/src/message_graph.rs index e66066c2b..fb4a3187e 100644 --- a/prost-build/src/message_graph.rs +++ b/prost-build/src/message_graph.rs @@ -23,15 +23,9 @@ impl MessageGraph { for file in files { let package = format!( - ".{}", - file.package.as_ref().ok_or_else(|| { - format!( - "prost requires a package specifier in all .proto files \ - (https://developers.google.com/protocol-buffers/docs/proto#packages); \ - file with missing package specifier: {}", - file.name.as_ref().map_or("(unknown)", String::as_ref), - ) - })?, + "{}{}", + if file.package.is_some() { "." } else { "" }, + file.package.as_ref().map(String::as_str).unwrap_or("") ); for msg in &file.message_type { msg_graph.add_message(&package, msg); diff --git a/tests/src/build.rs b/tests/src/build.rs index aa6fc100f..bd847d494 100644 --- a/tests/src/build.rs +++ b/tests/src/build.rs @@ -115,17 +115,46 @@ fn main() { ) .unwrap(); - // Check that attempting to compile a .proto without a package declaration results in an error. + // Check that attempting to compile a .proto without a package declaration does not result in an error. config .compile_protos(&[src.join("no_package.proto")], includes) - .err() .unwrap(); - let out_dir = - &PathBuf::from(env::var("OUT_DIR").expect("OUT_DIR environment variable not set")) - .join("extern_paths"); - fs::create_dir_all(out_dir).expect("failed to create prefix directory"); - config.out_dir(out_dir); + let out_dir = PathBuf::from(env::var("OUT_DIR").expect("OUT_DIR environment variable not set")); + + // Check that attempting to compile a .proto without a package declaration succeeds. + let no_root_packages = out_dir.as_path().join("no_root_packages"); + + fs::create_dir_all(&no_root_packages).expect("failed to create prefix directory"); + let mut no_root_packages_config = prost_build::Config::new(); + no_root_packages_config + .out_dir(&no_root_packages) + .default_package_filename("__.default") + .compile_protos( + &[src.join("no_root_packages/widget_factory.proto")], + &[src.join("no_root_packages")], + ) + .unwrap(); + + // Check that attempting to compile a .proto without a package declaration succeeds. + let no_root_packages_with_default = out_dir.as_path().join("no_root_packages_with_default"); + + fs::create_dir_all(&no_root_packages_with_default).expect("failed to create prefix directory"); + let mut no_root_packages_config = prost_build::Config::new(); + no_root_packages_config + .out_dir(&no_root_packages_with_default) + .compile_protos( + &[src.join("no_root_packages/widget_factory.proto")], + &[src.join("no_root_packages")], + ) + .unwrap(); + + assert!(no_root_packages_with_default.join("_.rs").exists()); + + let extern_paths = out_dir.as_path().join("extern_paths"); + fs::create_dir_all(&extern_paths).expect("failed to create prefix directory"); + + config.out_dir(&extern_paths); // Compile some of the module examples as an extern path. The extern path syntax is edition // specific, since the way crate-internal fully qualified paths has changed. diff --git a/tests/src/lib.rs b/tests/src/lib.rs index a2d3585ad..6012570f4 100644 --- a/tests/src/lib.rs +++ b/tests/src/lib.rs @@ -26,6 +26,7 @@ cfg_if! { } pub mod extern_paths; +pub mod no_root_packages; pub mod packages; pub mod unittest; diff --git a/tests/src/no_root_packages/gizmo.proto b/tests/src/no_root_packages/gizmo.proto new file mode 100644 index 000000000..df9fb19cc --- /dev/null +++ b/tests/src/no_root_packages/gizmo.proto @@ -0,0 +1,8 @@ +syntax = "proto3"; + +package gizmo; + +message Gizmo { + message Inner { + } +} diff --git a/tests/src/no_root_packages/mod.rs b/tests/src/no_root_packages/mod.rs new file mode 100644 index 000000000..2025eb983 --- /dev/null +++ b/tests/src/no_root_packages/mod.rs @@ -0,0 +1,46 @@ +//! Tests nested packages without a root package. + +include!(concat!(env!("OUT_DIR"), "/no_root_packages/__.default.rs")); + +pub mod gizmo { + include!(concat!(env!("OUT_DIR"), "/no_root_packages/gizmo.rs")); +} + +pub mod widget { + include!(concat!(env!("OUT_DIR"), "/no_root_packages/widget.rs")); + pub mod factory { + include!(concat!( + env!("OUT_DIR"), + "/no_root_packages/widget.factory.rs" + )); + } +} + +#[test] +fn test() { + use prost::Message; + + let mut widget_factory = widget::factory::WidgetFactory::default(); + assert_eq!(0, widget_factory.encoded_len()); + + widget_factory.inner = Some(widget::factory::widget_factory::Inner {}); + assert_eq!(2, widget_factory.encoded_len()); + + widget_factory.root = Some(Root {}); + assert_eq!(4, widget_factory.encoded_len()); + + widget_factory.root_inner = Some(root::Inner {}); + assert_eq!(6, widget_factory.encoded_len()); + + widget_factory.widget = Some(widget::Widget {}); + assert_eq!(8, widget_factory.encoded_len()); + + widget_factory.widget_inner = Some(widget::widget::Inner {}); + assert_eq!(10, widget_factory.encoded_len()); + + widget_factory.gizmo = Some(gizmo::Gizmo {}); + assert_eq!(12, widget_factory.encoded_len()); + + widget_factory.gizmo_inner = Some(gizmo::gizmo::Inner {}); + assert_eq!(14, widget_factory.encoded_len()); +} diff --git a/tests/src/no_root_packages/root.proto b/tests/src/no_root_packages/root.proto new file mode 100644 index 000000000..315a770b5 --- /dev/null +++ b/tests/src/no_root_packages/root.proto @@ -0,0 +1,6 @@ +syntax = "proto3"; + +message Root { + message Inner { + } +} diff --git a/tests/src/no_root_packages/widget.proto b/tests/src/no_root_packages/widget.proto new file mode 100644 index 000000000..17bf9ce84 --- /dev/null +++ b/tests/src/no_root_packages/widget.proto @@ -0,0 +1,14 @@ +syntax = "proto3"; + +package widget; + +message Widget { + message Inner { + } + + enum Type { + A = 0; + B = 1; + C = 2; + } +} diff --git a/tests/src/no_root_packages/widget_factory.proto b/tests/src/no_root_packages/widget_factory.proto new file mode 100644 index 000000000..3f0e688d8 --- /dev/null +++ b/tests/src/no_root_packages/widget_factory.proto @@ -0,0 +1,24 @@ +syntax = "proto3"; + +package widget.factory; + +import "root.proto"; +import "gizmo.proto"; +import "widget.proto"; + +message WidgetFactory { + message Inner { + } + + Inner inner = 1; + + Root root = 2; + Root.Inner root_inner = 3; + + Widget widget = 4; + Widget.Inner widget_inner = 5; + Widget.Type widget_type = 8; + + gizmo.Gizmo gizmo = 6; + gizmo.Gizmo.Inner gizmo_inner = 7; +}