From 9c6843fd1efe33950484e9ba970784adbc360683 Mon Sep 17 00:00:00 2001 From: Manuel Fuchs Date: Fri, 19 Nov 2021 18:41:30 +0100 Subject: [PATCH] Add macros to construct `libcnb-data` newtypes from literal strings (#172) --- CHANGELOG.md | 2 + Cargo.toml | 1 + libcnb-data/Cargo.toml | 3 +- libcnb-data/src/buildpack.rs | 33 +++++- libcnb-data/src/internals.rs | 8 ++ libcnb-data/src/launch.rs | 30 +++-- libcnb-data/src/lib.rs | 4 + libcnb-data/src/newtypes.rs | 108 ++++++++++-------- libcnb-proc-macros/Cargo.toml | 12 ++ libcnb-proc-macros/src/lib.rs | 73 ++++++++++++ .../example-02-ruby-sample/src/main.rs | 9 +- libcnb/src/build.rs | 5 +- 12 files changed, 223 insertions(+), 65 deletions(-) create mode 100644 libcnb-data/src/internals.rs create mode 100644 libcnb-proc-macros/Cargo.toml create mode 100644 libcnb-proc-macros/src/lib.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 0139c5eb..d4c04c84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,8 @@ - Add `PartialEq` and `Eq` implementations for `LayerTypes`. - Add `LayerEnv::chainable_insert` - `LayerEnv` and `ModificationBehavior` now implement `Clone`. +- Add `stack_id!`, `buildpack_id!` and `process_type!` macros. +- `Process::new` no longer returns a `Result` and it's `type` argument now is of type `ProcessType`. - Made it easier to work with buildpack errors during all phases of a `LayerLifecycle`. - `LayerEnv` was integrated into the `LayerLifecycle`, allowing buildpack authors to both write environment variables in a declarative way and using them between different layers without explicit IO. diff --git a/Cargo.toml b/Cargo.toml index 7ea7e395..c7c4c041 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,6 +2,7 @@ members = [ "libcnb", "libcnb-data", + "libcnb-proc-macros", "libcnb/examples/example-01-basics", "libcnb/examples/example-02-ruby-sample" ] diff --git a/libcnb-data/Cargo.toml b/libcnb-data/Cargo.toml index 464390e7..712f1b93 100644 --- a/libcnb-data/Cargo.toml +++ b/libcnb-data/Cargo.toml @@ -13,8 +13,9 @@ include = ["src/**/*", "../LICENSE", "../README.md"] [dependencies] lazy_static = "^1.4.0" -regex = "^1.5.4" +fancy-regex = "^0.7.1" semver = { version = "^1.0.4", features = ["serde"] } serde = { version = "^1.0.126", features = ["derive"] } thiserror = "^1.0.26" toml = "^0.5.8" +libcnb-proc-macros = { path = "../libcnb-proc-macros", version = "0.1.0" } diff --git a/libcnb-data/src/buildpack.rs b/libcnb-data/src/buildpack.rs index 8d9dd4d9..fbe6ca12 100644 --- a/libcnb-data/src/buildpack.rs +++ b/libcnb-data/src/buildpack.rs @@ -1,6 +1,6 @@ use crate::newtypes::libcnb_newtype; +use fancy_regex::Regex; use lazy_static::lazy_static; -use regex::Regex; use semver::Version; use serde::Deserialize; use std::convert::TryFrom; @@ -147,7 +147,7 @@ impl FromStr for BuildpackApi { static ref RE: Regex = Regex::new(r"^(?P\d+)(\.(?P\d+))?$").unwrap(); } - if let Some(captures) = RE.captures(value) { + if let Some(captures) = RE.captures(value).unwrap_or_default() { if let Some(major) = captures.name("major") { // these should never panic since we check with the regex unless it's greater than // `std::u32::MAX` @@ -178,6 +178,19 @@ impl Display for BuildpackApi { } libcnb_newtype!( + buildpack, + /// Construct a [`BuildpackId`] value at compile time. + /// + /// Passing a string that is not a valid `BuildpackId` value will yield a compilation error. + /// + /// # Examples: + /// ``` + /// use libcnb_data::buildpack_id; + /// use libcnb_data::buildpack::BuildpackId; + /// + /// let buildpack_id: BuildpackId = buildpack_id!("heroku/java"); + /// ``` + buildpack_id, /// buildpack.toml Buildpack Id. This is a newtype wrapper around a String. /// It MUST only contain numbers, letters, and the characters ., /, and -. /// It also cannot be `config` or `app`. @@ -196,11 +209,23 @@ libcnb_newtype!( /// ``` BuildpackId, BuildpackIdError, - r"^[[:alnum:]./-]+$", - |id| { id != "app" && id != "config" } + r"^(?!app$|config$)[[:alnum:]./-]+$" ); libcnb_newtype!( + buildpack, + /// Construct a [`StackId`] value at compile time. + /// + /// Passing a string that is not a valid `StackId` value will yield a compilation error. + /// + /// # Examples: + /// ``` + /// use libcnb_data::stack_id; + /// use libcnb_data::buildpack::StackId; + /// + /// let stack_id: StackId = stack_id!("heroku-20"); + /// ``` + stack_id, /// buildpack.toml Stack Id. This is a newtype wrapper around a String. /// It MUST only contain numbers, letters, and the characters ., /, and -. /// or be `*`. diff --git a/libcnb-data/src/internals.rs b/libcnb-data/src/internals.rs new file mode 100644 index 00000000..78bfd635 --- /dev/null +++ b/libcnb-data/src/internals.rs @@ -0,0 +1,8 @@ +// This macro is used by all newtype literal macros to verify if the value matches the regex. It +// is not intended to be used outside of this crate. But since the code that macros expand to is +// just regular code, we need to expose this to users of this crate. +// +// We cannot use `::libcnb_proc_macros::verify_regex` in our macros directly as this would require +// every crate to explicitly import the `libcnb_proc_macros` crate as crates can't use code from +// transitive dependencies. +pub use libcnb_proc_macros::verify_regex; diff --git a/libcnb-data/src/launch.rs b/libcnb-data/src/launch.rs index e942c003..137f1a6f 100644 --- a/libcnb-data/src/launch.rs +++ b/libcnb-data/src/launch.rs @@ -1,7 +1,6 @@ use crate::bom; use crate::newtypes::libcnb_newtype; use serde::{Deserialize, Serialize}; -use std::str::FromStr; #[derive(Deserialize, Serialize, Debug)] pub struct Launch { @@ -20,9 +19,11 @@ pub struct Launch { /// # Examples /// ``` /// use libcnb_data::launch; +/// use libcnb_data::process_type; +/// /// let mut launch_toml = launch::Launch::new(); -/// let web = launch::Process::new("web", "bundle", vec!["exec", "ruby", "app.rb"], -/// false, false).unwrap(); +/// let web = launch::Process::new(process_type!("web"), "bundle", vec!["exec", "ruby", "app.rb"], +/// false, false); /// /// launch_toml.processes.push(web); /// assert!(toml::to_string(&launch_toml).is_ok()); @@ -67,19 +68,19 @@ pub struct Process { impl Process { pub fn new( - r#type: impl AsRef, + r#type: ProcessType, command: impl Into, args: impl IntoIterator>, direct: bool, default: bool, - ) -> Result { - Ok(Self { - r#type: ProcessType::from_str(r#type.as_ref())?, + ) -> Self { + Self { + r#type, command: command.into(), args: args.into_iter().map(std::convert::Into::into).collect(), direct, default, - }) + } } } @@ -89,6 +90,19 @@ pub struct Slice { } libcnb_newtype!( + launch, + /// Construct a [`ProcessType`] value at compile time. + /// + /// Passing a string that is not a valid `ProcessType` value will yield a compilation error. + /// + /// # Examples: + /// ``` + /// use libcnb_data::launch::ProcessType; + /// use libcnb_data::process_type; + /// + /// let process_type: ProcessType = process_type!("web"); + /// ``` + process_type, /// launch.toml Process Type. This is a newtype wrapper around a String. It MUST only contain numbers, letters, and the characters ., _, and -. Use [`std::str::FromStr`] to create a new instance of this struct. /// /// # Examples diff --git a/libcnb-data/src/lib.rs b/libcnb-data/src/lib.rs index 3086834e..25928e09 100644 --- a/libcnb-data/src/lib.rs +++ b/libcnb-data/src/lib.rs @@ -24,3 +24,7 @@ pub mod layer_content_metadata; pub mod store; mod newtypes; + +// Internals that need to be public for macros +#[doc(hidden)] +pub mod internals; diff --git a/libcnb-data/src/newtypes.rs b/libcnb-data/src/newtypes.rs index 79baa554..4d86a09d 100644 --- a/libcnb-data/src/newtypes.rs +++ b/libcnb-data/src/newtypes.rs @@ -1,4 +1,4 @@ -/// Macro to generate newtypes backed by `String` +/// Macro to generate a newtype backed by `String` that is validated by a regular expression. /// /// Automatically implements the following traits for the newtype: /// - [`Debug`] @@ -12,43 +12,45 @@ /// - [`Deref`] /// - [`AsRef`] /// +/// This macro also generates another macro that can be used to construct values of the newtype from +/// literal strings at compile time. Compilation will fail if such a macro is used with a string +/// that is not valid for the corresponding newtype. This removes the need for explicit `unwrap` +/// calls that might fail at runtime. +/// /// # Usage: /// ``` /// libcnb_newtype!( +/// // The module of this crate that exports the newtype publicly. Since it might differ from +/// // the actual module structure, the macro needs a way to determine how to import the type +/// // from a user's buildpack crate. +/// tests::doctest +/// /// RustDoc for the macro (optional) +/// buildpack_id, /// /// RustDoc for the newtype itself (optional) /// BuildpackId, /// /// RustDoc for the newtype error (optional) /// BuildpackIdError, -/// // The regular expression that must match for the String to be valid +/// // The regular expression that must match for the String to be valid. Uses the `fancy_regex` +/// // crate which supports negative lookarounds. /// r"^[[:alnum:]./-]+$", -/// // Additional predicate function to do further validation (optional) -/// |id| { id != "app" && id != "config" } /// ); +/// +/// // Using the type: +/// let bp_id = "bar".parse::().unwrap(); +/// +/// // Using the macro for newtype literals with compile-type checks: +/// let bp_id = buildpack_id!("foo"); /// ``` macro_rules! libcnb_newtype { ( + $path:path, + $(#[$macro_attributes:meta])* + $macro_name:ident, $(#[$type_attributes:meta])* $name:ident, $(#[$error_type_attributes:meta])* $error_name:ident, $regex:expr - ) => { - libcnb_newtype!( - $(#[$type_attributes])* - $name, - $(#[$error_type_attributes])* - $error_name, - $regex, - |_| true - ); - }; - ( - $(#[$type_attributes:meta])* - $name:ident, - $(#[$error_type_attributes:meta])* - $error_name:ident, - $regex:expr, - $extra_predicate:expr ) => { #[derive(Debug, Eq, PartialEq, ::serde::Deserialize, ::serde::Serialize)] $(#[$type_attributes])* @@ -74,10 +76,11 @@ macro_rules! libcnb_newtype { type Err = $error_name; fn from_str(value: &str) -> Result { - let regex_matches = ::regex::Regex::new($regex).unwrap().is_match(value); - let predicate_matches = $extra_predicate(value); + let regex_matches = ::fancy_regex::Regex::new($regex) + .and_then(|regex| regex.is_match(value)) + .unwrap_or(false); - if regex_matches && predicate_matches { + if regex_matches { Ok(Self(String::from(value))) } else { Err($error_name::InvalidValue(String::from(value))) @@ -110,6 +113,27 @@ macro_rules! libcnb_newtype { ::std::write!(f, "{}", self.0) } } + + #[macro_export] + $(#[$macro_attributes])* + macro_rules! $macro_name { + ($value:expr) => { + $crate::internals::verify_regex!( + $regex, + $value, + { + use $crate::$path as base; + $value.parse::().unwrap() + }, + compile_error!(concat!( + stringify!($value), + " is not a valid ", + stringify!($name), + " value!" + )) + ) + } + } }; } @@ -119,28 +143,17 @@ pub(crate) use libcnb_newtype; mod test { use super::libcnb_newtype; - #[test] - fn test() { - libcnb_newtype!(CapitalizedName, CapitalizedNameError, r"^[A-Z][a-z]*$"); - - assert!("Manuel".parse::().is_ok()); - - assert_eq!( - "manuel".parse::(), - Err(CapitalizedNameError::InvalidValue(String::from("manuel"))) - ); - } + libcnb_newtype!( + newtypes::test, + capitalized_name, + CapitalizedName, + CapitalizedNameError, + r"^(?!Manuel$)[A-Z][a-z]*$" + ); #[test] - fn test_extra_predicate() { - libcnb_newtype!( - CapitalizedName, - CapitalizedNameError, - r"^[A-Z][a-z]*$", - |value| value != "Manuel" - ); - - assert!("Jonas".parse::().is_ok()); + fn test() { + assert!("Katrin".parse::().is_ok()); assert_eq!( "manuel".parse::(), @@ -154,9 +167,12 @@ mod test { } #[test] - fn test_deref() { - libcnb_newtype!(CapitalizedName, CapitalizedNameError, r"^[A-Z][a-z]*$"); + fn test_literal_macro_success() { + assert_eq!("Jonas", capitalized_name!("Jonas").as_ref()); + } + #[test] + fn test_deref() { fn foo(name: &str) { assert_eq!(name, "Johanna"); } diff --git a/libcnb-proc-macros/Cargo.toml b/libcnb-proc-macros/Cargo.toml new file mode 100644 index 00000000..f63c0d4f --- /dev/null +++ b/libcnb-proc-macros/Cargo.toml @@ -0,0 +1,12 @@ +[package] +name = "libcnb-proc-macros" +version = "0.1.0" +edition = "2021" + +[lib] +proc-macro = true + +[dependencies] +syn = {version = "1.0.81", features = ["full"]} +quote = "1.0.10" +fancy-regex = "0.7.1" diff --git a/libcnb-proc-macros/src/lib.rs b/libcnb-proc-macros/src/lib.rs new file mode 100644 index 00000000..40a890d7 --- /dev/null +++ b/libcnb-proc-macros/src/lib.rs @@ -0,0 +1,73 @@ +// Enable rustc and Clippy lints that are disabled by default. +// https://doc.rust-lang.org/rustc/lints/listing/allowed-by-default.html#unused-crate-dependencies +#![warn(unused_crate_dependencies)] +// https://rust-lang.github.io/rust-clippy/stable/index.html +#![warn(clippy::pedantic)] + +use proc_macro::TokenStream; +use quote::quote; +use syn::parse::{Parse, ParseStream}; +use syn::parse_macro_input; +use syn::Token; + +/// Compiles the given regex using the `fancy_regex` crate and tries to match the given value. If +/// the value matches the regex, the macro will expand to the first expression. Otherwise it will +/// expand to the second expression. +/// +/// It is designed to be used within other macros to produce compile time errors when the regex +/// doesn't match but it might work for other use-cases as well. +/// +/// ```no_run +/// libcnb_proc_macros::verify_regex!("^A-Z+$", "foobar", println!("It did match!"), println!("It did not match!")); +/// ``` +#[proc_macro] +pub fn verify_regex(input: TokenStream) -> TokenStream { + let input = parse_macro_input!(input as VerifyRegexInput); + + let token_stream = match fancy_regex::Regex::new(&input.regex.value()) { + Ok(regex) => { + let regex_matches = regex.is_match(&input.value.value()).unwrap_or(false); + + let expression = if regex_matches { + input.expression_when_matched + } else { + input.expression_when_unmatched + }; + + quote! { #expression } + } + Err(_) => { + quote! { + compile_error!(concat!("Could not compile regular expression: ", #(verify_regex_input.regex))); + } + } + }; + + token_stream.into() +} + +struct VerifyRegexInput { + regex: syn::LitStr, + value: syn::LitStr, + expression_when_matched: syn::Expr, + expression_when_unmatched: syn::Expr, +} + +impl Parse for VerifyRegexInput { + fn parse(input: ParseStream) -> syn::Result { + let regex: syn::LitStr = input.parse()?; + input.parse::()?; + let value: syn::LitStr = input.parse()?; + input.parse::()?; + let expression_when_matched: syn::Expr = input.parse()?; + input.parse::()?; + let expression_when_unmatched: syn::Expr = input.parse()?; + + Ok(Self { + regex, + value, + expression_when_matched, + expression_when_unmatched, + }) + } +} diff --git a/libcnb/examples/example-02-ruby-sample/src/main.rs b/libcnb/examples/example-02-ruby-sample/src/main.rs index 04b16ed1..301c579a 100644 --- a/libcnb/examples/example-02-ruby-sample/src/main.rs +++ b/libcnb/examples/example-02-ruby-sample/src/main.rs @@ -1,6 +1,7 @@ use crate::layers::{BundlerLayer, RubyLayer}; use libcnb::build::{BuildContext, BuildResult, BuildResultBuilder}; use libcnb::data::launch::{Launch, Process}; +use libcnb::data::process_type; use libcnb::detect::{DetectContext, DetectResult, DetectResultBuilder}; use libcnb::generic::GenericPlatform; use libcnb::layer_env::TargetLifecycle; @@ -48,19 +49,19 @@ impl Buildpack for RubyBuildpack { .launch( Launch::new() .process(Process::new( - "web", + process_type!("web"), "bundle", vec!["exec", "ruby", "app.rb"], false, true, - )?) + )) .process(Process::new( - "worker", + process_type!("worker"), "bundle", vec!["exec", "ruby", "worker.rb"], false, false, - )?), + )), ) .build()) } diff --git a/libcnb/src/build.rs b/libcnb/src/build.rs index 3be97e91..7fcebfca 100644 --- a/libcnb/src/build.rs +++ b/libcnb/src/build.rs @@ -130,12 +130,13 @@ pub(crate) enum InnerBuildResult { /// # Examples: /// ``` /// use libcnb::build::BuildResultBuilder; -/// use libcnb_data::launch::{Launch, Process}; +/// use libcnb::data::launch::{Launch, Process}; +/// use libcnb::data::process_type; /// /// let simple = BuildResultBuilder::new().build(); /// /// let with_launch = BuildResultBuilder::new() -/// .launch(Launch::new().process(Process::new("type", "command", vec!["-v"], false, false).unwrap())) +/// .launch(Launch::new().process(Process::new(process_type!("type"), "command", vec!["-v"], false, false))) /// .build(); /// ``` pub struct BuildResultBuilder {