From f032242b6583cc2895cfcb61e98b1aa15bb5df50 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Fri, 30 Jun 2023 15:44:47 +1000 Subject: [PATCH] Add must_use annotations to generated protobufs --- Cargo.lock | 7 ++++ cargo/licenses.json | 27 ++++++++++++++ rslib/io/Cargo.toml | 1 + rslib/io/src/error.rs | 2 ++ rslib/io/src/lib.rs | 38 ++++++++++++++++++-- rslib/proto/rust.rs | 8 +++++ rslib/proto_gen/Cargo.toml | 6 ++++ rslib/proto_gen/src/lib.rs | 73 ++++++++++++++++++++++++++++++++++++-- rslib/src/decks/tree.rs | 2 +- 9 files changed, 158 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f349de7d1..891e76fa1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -180,6 +180,7 @@ dependencies = [ name = "anki_io" version = "0.0.0" dependencies = [ + "camino", "snafu", "tempfile", ] @@ -214,10 +215,16 @@ dependencies = [ name = "anki_proto_gen" version = "0.0.0" dependencies = [ + "anki_io", + "anyhow", + "camino", "inflections", "itertools", + "once_cell", "prost-reflect", "prost-types", + "regex", + "walkdir", ] [[package]] diff --git a/cargo/licenses.json b/cargo/licenses.json index c922cbb3d..23d40a6a9 100644 --- a/cargo/licenses.json +++ b/cargo/licenses.json @@ -332,6 +332,15 @@ "license_file": null, "description": "Types and traits for working with bytes" }, + { + "name": "camino", + "version": "1.1.4", + "authors": "Without Boats |Ashley Williams |Steve Klabnik |Rain ", + "repository": "https://github.com/camino-rs/camino", + "license": "Apache-2.0 OR MIT", + "license_file": null, + "description": "UTF-8 paths" + }, { "name": "cc", "version": "1.0.79", @@ -2078,6 +2087,15 @@ "license_file": null, "description": "Fast floating point to string conversion" }, + { + "name": "same-file", + "version": "1.0.6", + "authors": "Andrew Gallant ", + "repository": "https://github.com/BurntSushi/same-file", + "license": "MIT OR Unlicense", + "license_file": null, + "description": "A simple crate for determining whether two file paths point to the same file." + }, { "name": "schannel", "version": "0.1.21", @@ -2915,6 +2933,15 @@ "license_file": null, "description": "Convert closures into wakers" }, + { + "name": "walkdir", + "version": "2.3.3", + "authors": "Andrew Gallant ", + "repository": "https://github.com/BurntSushi/walkdir", + "license": "MIT OR Unlicense", + "license_file": null, + "description": "Recursively walk a directory." + }, { "name": "want", "version": "0.3.0", diff --git a/rslib/io/Cargo.toml b/rslib/io/Cargo.toml index 37ba8eca4..e602260e8 100644 --- a/rslib/io/Cargo.toml +++ b/rslib/io/Cargo.toml @@ -9,5 +9,6 @@ rust-version.workspace = true description = "Utils for better I/O error reporting" [dependencies] +camino.workspace = true snafu.workspace = true tempfile.workspace = true diff --git a/rslib/io/src/error.rs b/rslib/io/src/error.rs index 656f994b7..1e05047b6 100644 --- a/rslib/io/src/error.rs +++ b/rslib/io/src/error.rs @@ -34,6 +34,7 @@ pub enum FileOp { Persist, Sync, Metadata, + DecodeUtf8Filename, /// For legacy errors without any context. Unknown, } @@ -59,6 +60,7 @@ impl FileIoError { FileOp::Persist => "persist".into(), FileOp::Sync => "sync".into(), FileOp::Metadata => "get metadata".into(), + FileOp::DecodeUtf8Filename => "decode utf8 filename".into(), }, self.path.to_string_lossy(), self.source diff --git a/rslib/io/src/lib.rs b/rslib/io/src/lib.rs index 0beea0cbc..7f1e68df5 100644 --- a/rslib/io/src/lib.rs +++ b/rslib/io/src/lib.rs @@ -8,7 +8,10 @@ use std::io::Read; use std::io::Seek; use std::path::Component; use std::path::Path; +use std::path::PathBuf; +use camino::Utf8Path; +use camino::Utf8PathBuf; use snafu::ResultExt; use tempfile::NamedTempFile; @@ -215,7 +218,8 @@ impl Iterator for ReadDirFiles { } } -pub fn write_file_if_changed(path: impl AsRef, contents: impl AsRef<[u8]>) -> Result<()> { +/// True if changed. +pub fn write_file_if_changed(path: impl AsRef, contents: impl AsRef<[u8]>) -> Result { let path = path.as_ref(); let contents = contents.as_ref(); let changed = { @@ -225,8 +229,38 @@ pub fn write_file_if_changed(path: impl AsRef, contents: impl AsRef<[u8]>) }; if changed { write_file(path, contents)?; + Ok(true) + } else { + Ok(false) + } +} + +pub trait ToUtf8PathBuf { + fn utf8(self) -> Result; +} + +impl ToUtf8PathBuf for PathBuf { + fn utf8(self) -> Result { + Utf8PathBuf::from_path_buf(self).map_err(|path| FileIoError { + path, + op: FileOp::DecodeUtf8Filename, + source: std::io::Error::from(std::io::ErrorKind::InvalidData), + }) + } +} + +pub trait ToUtf8Path { + fn utf8(&self) -> Result<&Utf8Path>; +} + +impl ToUtf8Path for Path { + fn utf8(&self) -> Result<&Utf8Path> { + Utf8Path::from_path(self).ok_or_else(|| FileIoError { + path: self.into(), + op: FileOp::DecodeUtf8Filename, + source: std::io::Error::from(std::io::ErrorKind::InvalidData), + }) } - Ok(()) } #[cfg(test)] diff --git a/rslib/proto/rust.rs b/rslib/proto/rust.rs index 6b08bca69..8a559ba8d 100644 --- a/rslib/proto/rust.rs +++ b/rslib/proto/rust.rs @@ -8,6 +8,8 @@ use std::path::PathBuf; use anki_io::create_dir_all; use anki_io::read_file; use anki_io::write_file_if_changed; +use anki_proto_gen::add_must_use_annotations; +use anki_proto_gen::determine_if_message_is_empty; use anyhow::Context; use anyhow::Result; use prost_reflect::DescriptorPool; @@ -55,7 +57,13 @@ pub fn write_rust_protos(descriptors_path: Option) -> Result Option { - msg_if_empty(self.proto.input()) + msg_if_not_empty(self.proto.input()) } /// The output type, if not empty. pub fn output(&self) -> Option { - msg_if_empty(self.proto.output()) + msg_if_not_empty(self.proto.output()) } } -fn msg_if_empty(msg: MessageDescriptor) -> Option { +fn msg_if_not_empty(msg: MessageDescriptor) -> Option { if msg.full_name() == "anki.generic.Empty" { None } else { @@ -199,3 +209,60 @@ impl<'a> MethodComments<'a> { .and_then(|s| if s.is_empty() { None } else { Some(s.into()) }) } } + +pub fn add_must_use_annotations( + out_dir: &PathBuf, + should_process_path: P, + is_empty: E, +) -> Result<()> +where + P: Fn(&Utf8Path) -> bool, + E: Fn(&Utf8Path, &str) -> bool, +{ + for file in WalkDir::new(out_dir).into_iter() { + let file = file?; + let path = file.path().utf8()?; + if path.file_name().unwrap().ends_with(".rs") && should_process_path(path) { + add_must_use_annotations_to_file(path, &is_empty)?; + } + } + Ok(()) +} + +pub fn add_must_use_annotations_to_file(path: &Utf8Path, is_empty: E) -> Result<()> +where + E: Fn(&Utf8Path, &str) -> bool, +{ + static MESSAGE_OR_ENUM_RE: Lazy = + Lazy::new(|| Regex::new(r#"pub (struct|enum) ([[:alnum:]]+?)\s"#).unwrap()); + let contents = read_to_string(path)?; + let contents = MESSAGE_OR_ENUM_RE.replace_all(&contents, |caps: &Captures| { + let is_enum = caps.get(1).unwrap().as_str() == "enum"; + let name = caps.get(2).unwrap().as_str(); + if is_enum || !is_empty(path, name) { + format!("#[must_use]\n{}", caps.get(0).unwrap().as_str()) + } else { + caps.get(0).unwrap().as_str().to_string() + } + }); + write_file_if_changed(path, contents.as_ref())?; + Ok(()) +} + +/// Given a generated prost filename and a struct name, try to determine whether +/// the message has 0 fields. +/// +/// This is unfortunately rather circuitous, as Prost doesn't allow us to easily +/// alter the code generation with access to the associated proto descriptor. So +/// we need to infer the full proto path based on the filename and the Rust type +/// name, which we can only do for top-level elements. For any nested messages +/// we can't find, we assume they must be used. +pub fn determine_if_message_is_empty(pool: &DescriptorPool, path: &Utf8Path, name: &str) -> bool { + let package = path.file_stem().unwrap(); + let full_name = format!("{package}.{name}"); + if let Some(msg) = pool.get_message_by_name(&full_name) { + msg.fields().count() == 0 + } else { + false + } +} diff --git a/rslib/src/decks/tree.rs b/rslib/src/decks/tree.rs index 54d59fa3b..bd2725ee4 100644 --- a/rslib/src/decks/tree.rs +++ b/rslib/src/decks/tree.rs @@ -243,7 +243,7 @@ fn hide_default_deck(node: &mut DeckTreeNode) { // can't remove if there are no other decks } else { // safe to remove - node.children.remove(idx); + _ = node.children.remove(idx); } return; }