Skip to content

Commit

Permalink
Add support for no package declaration (tokio-rs#343)
Browse files Browse the repository at this point in the history
Co-authored-by: Jacob Kiesel <[email protected]>
  • Loading branch information
tylerhawkes and Xaeroxe authored Sep 6, 2021
1 parent a4be973 commit 15296b8
Show file tree
Hide file tree
Showing 11 changed files with 179 additions and 26 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 20 additions & 3 deletions prost-build/src/code_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();
Expand Down
24 changes: 19 additions & 5 deletions prost-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ pub struct Config {
strip_enum_prefix: bool,
out_dir: Option<PathBuf>,
extern_paths: Vec<(String, String)>,
default_package_filename: String,
protoc_args: Vec<OsString>,
disable_comments: PathMap<()>,
}
Expand Down Expand Up @@ -658,6 +659,15 @@ impl Config {
self
}

/// Configures what filename protobufs with no package definition are written to.
pub fn default_package_filename<S>(&mut self, filename: S) -> &mut Self
where
S: Into<String>,
{
self.default_package_filename = filename.into();
self
}

/// Add an argument to the `protoc` protobuf compilation invocation.
///
/// # Example `build.rs`
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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(),
}
Expand All @@ -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)
Expand All @@ -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()
Expand Down
12 changes: 3 additions & 9 deletions prost-build/src/message_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
43 changes: 36 additions & 7 deletions tests/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ cfg_if! {
}

pub mod extern_paths;
pub mod no_root_packages;
pub mod packages;
pub mod unittest;

Expand Down
8 changes: 8 additions & 0 deletions tests/src/no_root_packages/gizmo.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
syntax = "proto3";

package gizmo;

message Gizmo {
message Inner {
}
}
46 changes: 46 additions & 0 deletions tests/src/no_root_packages/mod.rs
Original file line number Diff line number Diff line change
@@ -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());
}
6 changes: 6 additions & 0 deletions tests/src/no_root_packages/root.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
syntax = "proto3";

message Root {
message Inner {
}
}
14 changes: 14 additions & 0 deletions tests/src/no_root_packages/widget.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
syntax = "proto3";

package widget;

message Widget {
message Inner {
}

enum Type {
A = 0;
B = 1;
C = 2;
}
}
24 changes: 24 additions & 0 deletions tests/src/no_root_packages/widget_factory.proto
Original file line number Diff line number Diff line change
@@ -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;
}

0 comments on commit 15296b8

Please sign in to comment.