Merge branch 'upgrade-derive-builder'

CHANGELOG.md

@@ -19,6 +19,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 * Upgrade crate to Rust 2021 edition.
 * MSRV bumped to 1.69 due to use of `CStr::from_bytes_until_nul`.
 * Replace `error-chain` generated errors with manually implemented error types.
+* Remove `build_internal` methods on `FilterRuleBuilder` and `RedirectRuleBuilder`.
+  This was never supposed to be public, but a side effect of using `derive-builder`.
 
 ### Removed
 * Remove `PoolAddrList::to_palist` from the public API. It should never have been exposed.

Cargo.toml

@@ -18,7 +18,7 @@ travis-ci = { repository = "mullvad/pfctl-rs" }
 [dependencies]
 ioctl-sys = "0.8.0"
 libc = "0.2.29"
-derive_builder = "0.9"
+derive_builder = "0.20"
 ipnetwork = "0.20.0"
 
 [dev-dependencies]

src/lib.rs

@@ -100,6 +100,8 @@ pub enum ErrorKind {
     DeviceOpen,
     /// The firewall rule is invalidly configured
     InvalidRuleCombination,
+    /// A required field on a builder was not set
+    UninitializedFieldError,
     /// The supplied network interface name is not compatible with PF
     InvalidInterfaceName,
     /// The supplied anchor name in not compatible with PF
@@ -123,6 +125,7 @@ pub struct Error(ErrorInternal);
 enum ErrorInternal {
     DeviceOpen(&'static str, std::io::Error),
     InvalidRuleCombination(String),
+    UninitializedFieldError(derive_builder::UninitializedFieldError),
     InvalidInterfaceName(&'static str),
     InvalidAnchorName(&'static str),
     InvalidPortRange,
@@ -139,6 +142,7 @@ impl Error {
         match self.0 {
             DeviceOpen(..) => ErrorKind::DeviceOpen,
             InvalidRuleCombination(_) => ErrorKind::InvalidRuleCombination,
+            UninitializedFieldError(_) => ErrorKind::UninitializedFieldError,
             InvalidInterfaceName(..) => ErrorKind::InvalidInterfaceName,
             InvalidAnchorName(..) => ErrorKind::InvalidAnchorName,
             InvalidPortRange => ErrorKind::InvalidPortRange,
@@ -164,6 +168,7 @@ impl fmt::Display for Error {
                 write!(f, "Unable to open PF device file ({device_path})")
             }
             InvalidRuleCombination(msg) => write!(f, "Invalid rule combination: {msg}"),
+            UninitializedFieldError(inner) => inner.fmt(f),
             InvalidInterfaceName(reason) => write!(f, "Invalid interface name ({reason})"),
             InvalidAnchorName(reason) => write!(f, "Invalid anchor name ({reason})"),
             InvalidPortRange => write!(f, "Lower port is greater than upper port"),
@@ -186,6 +191,14 @@ impl std::error::Error for Error {
     }
 }
 
+// Required in order to have derive_builder builders generate `build` methods that return
+// our error type directly.
+impl From<derive_builder::UninitializedFieldError> for Error {
+    fn from(value: derive_builder::UninitializedFieldError) -> Self {
+        Error::from(ErrorInternal::UninitializedFieldError(value))
+    }
+}
+
 /// Returns the given input result, except if it is an `Err` matching the given `ErrorKind`,
 /// then it returns `Ok(())` instead, so the error is ignored.
 macro_rules! ignore_error_kind {

src/rule/mod.rs

@@ -10,7 +10,6 @@ use crate::{
     conversion::{CopyTo, TryCopyTo},
     ffi, Error, ErrorInternal, Result,
 };
-use derive_builder::Builder;
 use ipnetwork::IpNetwork;
 use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
 
@@ -59,9 +58,9 @@ pub use self::rule_log::*;
 mod uid;
 pub use self::uid::*;
 
-#[derive(Debug, Clone, PartialEq, Eq, Hash, Builder)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash, derive_builder::Builder)]
 #[builder(setter(into))]
-#[builder(build_fn(name = "build_internal"))]
+#[builder(build_fn(error = "Error"))]
 pub struct FilterRule {
     action: FilterRuleAction,
     #[builder(default)]
@@ -96,13 +95,6 @@ pub struct FilterRule {
     icmp_type: Option<IcmpType>,
 }
 
-impl FilterRuleBuilder {
-    pub fn build(&self) -> Result<FilterRule> {
-        self.build_internal()
-            .map_err(|e| ErrorInternal::InvalidRuleCombination(e).into())
-    }
-}
-
 impl FilterRule {
     /// Returns the `AddrFamily` this rule matches against. Returns an `InvalidRuleCombination`
     /// error if this rule has an invalid combination of address families.
@@ -167,9 +159,9 @@ impl TryCopyTo<ffi::pfvar::pf_rule> for FilterRule {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Hash, Builder)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash, derive_builder::Builder)]
 #[builder(setter(into))]
-#[builder(build_fn(name = "build_internal"))]
+#[builder(build_fn(error = "Error"))]
 pub struct RedirectRule {
     action: RedirectRuleAction,
     #[builder(default)]
@@ -197,13 +189,6 @@ pub struct RedirectRule {
     redirect_to: Endpoint,
 }
 
-impl RedirectRuleBuilder {
-    pub fn build(&self) -> Result<RedirectRule> {
-        self.build_internal()
-            .map_err(|e| ErrorInternal::InvalidRuleCombination(e).into())
-    }
-}
-
 impl RedirectRule {
     /// Returns the `AddrFamily` this rule matches against. Returns an `InvalidRuleCombination`
     /// error if this rule has an invalid combination of address families.