From b2e046a64e584a60491ead8357cf6875702053c6 Mon Sep 17 00:00:00 2001 From: Marcus Griep Date: Thu, 24 Feb 2022 16:46:06 -0500 Subject: [PATCH 1/5] Minimum viable protoc-gen interface --- prost-build/src/lib.rs | 47 ++++++++++++++++++++------------ prost-build/src/message_graph.rs | 2 +- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/prost-build/src/lib.rs b/prost-build/src/lib.rs index 678ea7b1e..38b8b22e6 100644 --- a/prost-build/src/lib.rs +++ b/prost-build/src/lib.rs @@ -847,17 +847,26 @@ impl Config { ) })?; - let modules = self.generate(file_descriptor_set.file)?; - for (module, content) in &modules { - let mut filename = if module.is_empty() { + let requests: Vec<_> = file_descriptor_set.file.into_iter().map(|descriptor| { + (self.module(&descriptor), descriptor) + }).collect(); + + let file_names: HashMap = requests.iter().map(|req| { + let mut file_name = if req.0.is_empty() { self.default_package_filename.clone() } else { - module.join(".") + req.0.join(".") }; - filename.push_str(".rs"); + file_name.push_str(".rs"); + + (req.0.clone(), file_name) + }).collect(); - let output_path = target.join(&filename); + let modules = self.generate(requests)?; + for (module, content) in &modules { + let file_name = file_names.get(module).expect("every module should have a filename"); + let output_path = target.join(file_name); let previous_content = fs::read(&output_path); @@ -865,9 +874,9 @@ impl Config { .map(|previous_content| previous_content == content.as_bytes()) .unwrap_or(false) { - trace!("unchanged: {:?}", filename); + trace!("unchanged: {:?}", file_name); } else { - trace!("writing: {:?}", filename); + trace!("writing: {:?}", file_name); fs::write(output_path, content)?; } } @@ -949,25 +958,29 @@ impl Config { outfile.write_all(format!("{}{}\n", (" ").to_owned().repeat(depth), line).as_bytes()) } - fn generate(&mut self, files: Vec) -> Result> { + /// Processes a set of modules and file descriptors, returning a map of modules to generated + /// code contents. + /// + /// This is generally used when control over the output should not be managed by Prost, + /// such as in a flow for a `protoc` code generating plugin. When compiling as part of a + /// `build.rs` file, instead use [`compile_protos()`]. + pub fn generate(&mut self, requests: Vec<(Module, FileDescriptorProto)>) -> Result> { let mut modules = HashMap::new(); let mut packages = HashMap::new(); - let message_graph = MessageGraph::new(&files) + let message_graph = MessageGraph::new(requests.iter().map(|x| &x.1)) .map_err(|error| Error::new(ErrorKind::InvalidInput, error))?; let extern_paths = ExternPaths::new(&self.extern_paths, self.prost_types) .map_err(|error| Error::new(ErrorKind::InvalidInput, error))?; - for file in files { - let module = self.module(&file); - + for request in requests { // Only record packages that have services - if !file.service.is_empty() { - packages.insert(module.clone(), file.package().to_string()); + if !request.1.service.is_empty() { + packages.insert(request.0.clone(), request.1.package().to_string()); } - let buf = modules.entry(module).or_insert_with(String::new); - CodeGenerator::generate(self, &message_graph, &extern_paths, file, buf); + let buf = modules.entry(request.0).or_insert_with(String::new); + CodeGenerator::generate(self, &message_graph, &extern_paths, request.1, buf); } if let Some(ref mut service_generator) = self.service_generator { diff --git a/prost-build/src/message_graph.rs b/prost-build/src/message_graph.rs index fb4a3187e..5812912ee 100644 --- a/prost-build/src/message_graph.rs +++ b/prost-build/src/message_graph.rs @@ -15,7 +15,7 @@ pub struct MessageGraph { } impl MessageGraph { - pub fn new(files: &[FileDescriptorProto]) -> Result { + pub fn new<'a>(files: impl Iterator) -> Result { let mut msg_graph = MessageGraph { index: HashMap::new(), graph: Graph::new(), From a1616f70f49a86e1d06db0538313dc9740cd439a Mon Sep 17 00:00:00 2001 From: Marcus Griep Date: Tue, 1 Mar 2022 14:25:25 -0500 Subject: [PATCH 2/5] chore: rustfmt --- prost-build/src/lib.rs | 40 ++++++++++++++++++++------------ prost-build/src/message_graph.rs | 4 +++- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/prost-build/src/lib.rs b/prost-build/src/lib.rs index 38b8b22e6..fe6b994e2 100644 --- a/prost-build/src/lib.rs +++ b/prost-build/src/lib.rs @@ -847,25 +847,32 @@ impl Config { ) })?; - let requests: Vec<_> = file_descriptor_set.file.into_iter().map(|descriptor| { - (self.module(&descriptor), descriptor) - }).collect(); - - let file_names: HashMap = requests.iter().map(|req| { - let mut file_name = if req.0.is_empty() { - self.default_package_filename.clone() - } else { - req.0.join(".") - }; + let requests: Vec<_> = file_descriptor_set + .file + .into_iter() + .map(|descriptor| (self.module(&descriptor), descriptor)) + .collect(); + + let file_names: HashMap = requests + .iter() + .map(|req| { + let mut file_name = if req.0.is_empty() { + self.default_package_filename.clone() + } else { + req.0.join(".") + }; - file_name.push_str(".rs"); + file_name.push_str(".rs"); - (req.0.clone(), file_name) - }).collect(); + (req.0.clone(), file_name) + }) + .collect(); let modules = self.generate(requests)?; for (module, content) in &modules { - let file_name = file_names.get(module).expect("every module should have a filename"); + let file_name = file_names + .get(module) + .expect("every module should have a filename"); let output_path = target.join(file_name); let previous_content = fs::read(&output_path); @@ -964,7 +971,10 @@ impl Config { /// This is generally used when control over the output should not be managed by Prost, /// such as in a flow for a `protoc` code generating plugin. When compiling as part of a /// `build.rs` file, instead use [`compile_protos()`]. - pub fn generate(&mut self, requests: Vec<(Module, FileDescriptorProto)>) -> Result> { + pub fn generate( + &mut self, + requests: Vec<(Module, FileDescriptorProto)>, + ) -> Result> { let mut modules = HashMap::new(); let mut packages = HashMap::new(); diff --git a/prost-build/src/message_graph.rs b/prost-build/src/message_graph.rs index 5812912ee..ac0ad1523 100644 --- a/prost-build/src/message_graph.rs +++ b/prost-build/src/message_graph.rs @@ -15,7 +15,9 @@ pub struct MessageGraph { } impl MessageGraph { - pub fn new<'a>(files: impl Iterator) -> Result { + pub fn new<'a>( + files: impl Iterator, + ) -> Result { let mut msg_graph = MessageGraph { index: HashMap::new(), graph: Graph::new(), From 7574a1b0e2b4204854d5410499b7ed4deea5b469 Mon Sep 17 00:00:00 2001 From: Marcus Griep Date: Wed, 2 Mar 2022 08:48:36 -0500 Subject: [PATCH 3/5] chore: prefer to use turbofish operator --- prost-build/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/prost-build/src/lib.rs b/prost-build/src/lib.rs index fe6b994e2..f6c80e32d 100644 --- a/prost-build/src/lib.rs +++ b/prost-build/src/lib.rs @@ -847,13 +847,13 @@ impl Config { ) })?; - let requests: Vec<_> = file_descriptor_set + let requests = file_descriptor_set .file .into_iter() .map(|descriptor| (self.module(&descriptor), descriptor)) - .collect(); + .collect::>(); - let file_names: HashMap = requests + let file_names = requests .iter() .map(|req| { let mut file_name = if req.0.is_empty() { @@ -866,7 +866,7 @@ impl Config { (req.0.clone(), file_name) }) - .collect(); + .collect::>(); let modules = self.generate(requests)?; for (module, content) in &modules { From 4d05b6a86dadc70ec45f40aeba156fec5b188e4e Mon Sep 17 00:00:00 2001 From: Marcus Griep Date: Thu, 3 Mar 2022 12:37:57 -0500 Subject: [PATCH 4/5] feat(prost-build): introduce Module type --- prost-build/src/lib.rs | 101 +++++++++++++++++++++++++++++++---------- 1 file changed, 77 insertions(+), 24 deletions(-) diff --git a/prost-build/src/lib.rs b/prost-build/src/lib.rs index f6c80e32d..cd658e01c 100644 --- a/prost-build/src/lib.rs +++ b/prost-build/src/lib.rs @@ -123,6 +123,7 @@ use std::ffi::{OsStr, OsString}; use std::fmt; use std::fs; use std::io::{Error, ErrorKind, Result, Write}; +use std::ops::RangeToInclusive; use std::path::{Path, PathBuf}; use std::process::Command; @@ -137,8 +138,6 @@ use crate::ident::to_snake; use crate::message_graph::MessageGraph; use crate::path::PathMap; -type Module = Vec; - /// A service generator takes a service descriptor and generates Rust code. /// /// `ServiceGenerator` can be used to generate application-specific interfaces @@ -850,21 +849,21 @@ impl Config { let requests = file_descriptor_set .file .into_iter() - .map(|descriptor| (self.module(&descriptor), descriptor)) + .map(|descriptor| { + ( + Module::from_protobuf_package_name(descriptor.package()), + descriptor, + ) + }) .collect::>(); let file_names = requests .iter() .map(|req| { - let mut file_name = if req.0.is_empty() { - self.default_package_filename.clone() - } else { - req.0.join(".") - }; - - file_name.push_str(".rs"); - - (req.0.clone(), file_name) + ( + req.0.clone(), + req.0.to_file_name_or(&self.default_package_filename), + ) }) .collect::>(); @@ -912,17 +911,17 @@ impl Config { ) -> Result { let mut written = 0; while !entries.is_empty() { - let modident = &entries[0][depth]; + let modident = entries[0].part(depth); let matching: Vec<&Module> = entries .iter() - .filter(|&v| &v[depth] == modident) + .filter(|&v| v.part(depth) == modident) .copied() .collect(); { // Will NLL sort this mess out? let _temp = entries .drain(..) - .filter(|&v| &v[depth] != modident) + .filter(|&v| v.part(depth) != modident) .collect(); entries = _temp; } @@ -939,7 +938,7 @@ impl Config { )?; written += subwritten; if subwritten != matching.len() { - let modname = matching[0][..=depth].join("."); + let modname = matching[0].to_partial_file_name(..=depth); if basepath.is_some() { self.write_line( outfile, @@ -1002,14 +1001,6 @@ impl Config { Ok(modules) } - - fn module(&self, file: &FileDescriptorProto) -> Module { - file.package() - .split('.') - .filter(|s| !s.is_empty()) - .map(to_snake) - .collect() - } } impl default::Default for Config { @@ -1054,6 +1045,68 @@ impl fmt::Debug for Config { } } +/// A Rust module path for a Protobuf package. +#[derive(Clone, PartialEq, Eq, Hash)] +pub struct Module { + components: Vec, +} + +impl Module { + /// Construct a module path from an iterator of parts. + pub fn from_parts, S: Into>(parts: I) -> Self { + Self { + components: parts.into_iter().map(|s| s.into()).collect(), + } + } + + /// Construct a module path from a Protobuf package name. + /// + /// Constituent parts are automatically converted to snake case in order to follow + /// Rust module naming conventions. + pub fn from_protobuf_package_name(name: &str) -> Self { + Self { + components: name + .split('.') + .filter(|s| !s.is_empty()) + .map(to_snake) + .collect(), + } + } + + /// An iterator over the parts of the path + pub fn parts(&self) -> impl Iterator { + self.components.iter().map(|s| s.as_str()) + } + + /// Format the module path into a filename for generated Rust code. + /// + /// If the module path is empty, `default` is used to provide the root of the filename. + pub fn to_file_name_or(&self, default: &str) -> String { + let mut root = if self.components.is_empty() { + default.to_owned() + } else { + self.components.join(".") + }; + + root.push_str(".rs"); + + root + } + + /// The number of parts in the module's path. + pub fn len(&self) -> usize { + self.components.len() + } + + fn to_partial_file_name(&self, range: RangeToInclusive) -> String { + self.components[range].join(".") + } + + fn part(&self, idx: usize) -> &str { + self.components[idx].as_str() + } +} + /// Compile `.proto` files into Rust files during a Cargo build. /// /// The generated `.rs` files are written to the Cargo `OUT_DIR` directory, suitable for use with From fa391b21059cac21186e4c0f9d558a9a1fe6b16a Mon Sep 17 00:00:00 2001 From: Marcus Griep Date: Fri, 4 Mar 2022 08:05:01 -0500 Subject: [PATCH 5/5] chore: move trait bounds to where --- prost-build/src/lib.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/prost-build/src/lib.rs b/prost-build/src/lib.rs index cd658e01c..edf91a6a8 100644 --- a/prost-build/src/lib.rs +++ b/prost-build/src/lib.rs @@ -1053,7 +1053,11 @@ pub struct Module { impl Module { /// Construct a module path from an iterator of parts. - pub fn from_parts, S: Into>(parts: I) -> Self { + pub fn from_parts(parts: I) -> Self + where + I: IntoIterator, + I::Item: Into, + { Self { components: parts.into_iter().map(|s| s.into()).collect(), }