From d38696ea60672d838555d89a2a8122990069cc37 Mon Sep 17 00:00:00 2001
From: Marc Schoolderman <marc@tweedegolf.com>
Date: Mon, 16 Dec 2024 17:07:46 +0100
Subject: [PATCH] change DSL to emit warnings about unused variables by using
 getters

---
 src/defaults/mod.rs          | 72 ++++++++++++++++++------------------
 src/defaults/settings_dsl.rs | 60 ++++++++++++++++++++++++------
 src/sudoers/mod.rs           |  2 +-
 src/sudoers/policy.rs        | 18 ++++-----
 src/sudoers/test/mod.rs      | 33 +++++++++--------
 5 files changed, 114 insertions(+), 71 deletions(-)

diff --git a/src/defaults/mod.rs b/src/defaults/mod.rs
index d150cf831..e4241fba2 100644
--- a/src/defaults/mod.rs
+++ b/src/defaults/mod.rs
@@ -16,7 +16,9 @@ pub enum SettingKind {
 }
 
 mod settings_dsl;
-use settings_dsl::{defaults, ifdef, modifier_of, negate_of, type_of, value_of};
+use settings_dsl::{
+    defaults, ifdef, initializer_of, modifier_of, negator_of, referent_of, result_of, storage_of,
+};
 
 defaults! {
     always_query_group_plugin = false
@@ -68,35 +70,35 @@ mod test {
     #[test]
     fn check() {
         let mut def = Settings::default();
-        assert_eq! { def.always_query_group_plugin, false };
-        assert_eq! { def.always_set_home, false };
-        assert_eq! { def.env_reset, true };
-        assert_eq! { def.mail_badpass, true };
-        assert_eq! { def.match_group_by_gid, false };
-        assert_eq! { def.use_pty, true };
-        assert_eq! { def.visiblepw, false };
-        assert_eq! { def.env_editor, true };
-        assert_eq! { def.passwd_tries, 3 };
-        assert_eq! { def.secure_path, None };
-        assert_eq! { def.env_check, ["COLORTERM", "LANG", "LANGUAGE", "LC_*", "LINGUAS", "TERM", "TZ"].iter().map(|s| s.to_string()).collect() };
-        assert_eq! { def.verifypw, enums::verifypw::all };
+        assert_eq! { def.always_query_group_plugin(), false };
+        assert_eq! { def.always_set_home(), false };
+        assert_eq! { def.env_reset(), true };
+        assert_eq! { def.mail_badpass(), true };
+        assert_eq! { def.match_group_by_gid(), false };
+        assert_eq! { def.use_pty(), true };
+        assert_eq! { def.visiblepw(), false };
+        assert_eq! { def.env_editor(), true };
+        assert_eq! { def.passwd_tries(), 3 };
+        assert_eq! { def.secure_path(), None };
+        assert_eq! { def.env_check(), &["COLORTERM", "LANG", "LANGUAGE", "LC_*", "LINGUAS", "TERM", "TZ"].iter().map(|s| s.to_string()).collect() };
+        assert_eq! { def.verifypw(), enums::verifypw::all };
 
         negate("env_check").unwrap()(&mut def);
         negate("env_reset").unwrap()(&mut def);
         negate("secure_path").unwrap()(&mut def);
         negate("verifypw").unwrap()(&mut def);
-        assert_eq! { def.always_query_group_plugin, false };
-        assert_eq! { def.always_set_home, false };
-        assert_eq! { def.env_reset, false };
-        assert_eq! { def.mail_badpass, true };
-        assert_eq! { def.match_group_by_gid, false };
-        assert_eq! { def.use_pty, true };
-        assert_eq! { def.visiblepw, false };
-        assert_eq! { def.env_editor, true };
-        assert_eq! { def.passwd_tries, 3 };
-        assert_eq! { def.secure_path, None };
+        assert_eq! { def.always_query_group_plugin(), false };
+        assert_eq! { def.always_set_home(), false };
+        assert_eq! { def.env_reset(), false };
+        assert_eq! { def.mail_badpass(), true };
+        assert_eq! { def.match_group_by_gid(), false };
+        assert_eq! { def.use_pty(), true };
+        assert_eq! { def.visiblepw(), false };
+        assert_eq! { def.env_editor(), true };
+        assert_eq! { def.passwd_tries(), 3 };
+        assert_eq! { def.secure_path(), None };
         assert! { def.env_check.is_empty() };
-        assert_eq! { def.verifypw, enums::verifypw::never };
+        assert_eq! { def.verifypw(), enums::verifypw::never };
 
         let SettingKind::Flag(f) = set("env_reset").unwrap() else {
             panic!()
@@ -114,18 +116,18 @@ mod test {
             panic!()
         };
         f("any").unwrap()(&mut def);
-        assert_eq! { def.always_query_group_plugin, false };
-        assert_eq! { def.always_set_home, false };
-        assert_eq! { def.env_reset, true };
-        assert_eq! { def.mail_badpass, true };
-        assert_eq! { def.match_group_by_gid, false };
-        assert_eq! { def.use_pty, true };
-        assert_eq! { def.visiblepw, false };
-        assert_eq! { def.env_editor, true };
-        assert_eq! { def.passwd_tries, 5 };
-        assert_eq! { def.secure_path, Some("/bin".into()) };
+        assert_eq! { def.always_query_group_plugin(), false };
+        assert_eq! { def.always_set_home(), false };
+        assert_eq! { def.env_reset(), true };
+        assert_eq! { def.mail_badpass(), true };
+        assert_eq! { def.match_group_by_gid(), false };
+        assert_eq! { def.use_pty(), true };
+        assert_eq! { def.visiblepw(), false };
+        assert_eq! { def.env_editor(), true };
+        assert_eq! { def.passwd_tries(), 5 };
+        assert_eq! { def.secure_path(), Some("/bin") };
         assert! { def.env_check.is_empty() };
-        assert_eq! { def.verifypw, enums::verifypw::any };
+        assert_eq! { def.verifypw(), enums::verifypw::any };
 
         assert!(set("notanoption").is_none());
         assert!(f("notanoption").is_none());
diff --git a/src/defaults/settings_dsl.rs b/src/defaults/settings_dsl.rs
index f29e0d756..a2b0b7cd2 100644
--- a/src/defaults/settings_dsl.rs
+++ b/src/defaults/settings_dsl.rs
@@ -1,4 +1,4 @@
-macro_rules! type_of {
+macro_rules! storage_of {
     ($id:ident, true) => { bool };
     ($id:ident, false) => { bool };
     ($id:ident, [ $($value: expr),* ]) => { std::collections::HashSet<String> };
@@ -7,7 +7,16 @@ macro_rules! type_of {
     ($id:ident, $_: expr) => { Option<Box<str>> };
 }
 
-macro_rules! value_of {
+macro_rules! referent_of {
+    ($id:ident, true) => { bool };
+    ($id:ident, false) => { bool };
+    ($id:ident, [ $($value: expr),* ]) => { &std::collections::HashSet<String> };
+    ($id:ident, $(=int $check: expr;)+ $_: expr) => { i64 };
+    ($id:ident, $(=enum $k: ident;)+ $_: ident) => { $crate::defaults::enums::$id };
+    ($id:ident, $_: expr) => { Option<&str> };
+}
+
+macro_rules! initializer_of {
     ($id:ident, true) => { true };
     ($id:ident, false) => { false };
     ($id:ident, [ $($value: expr),* ]) => { [$($value),*].into_iter().map(|s: &str| s.to_string()).collect::<std::collections::HashSet<_>>() };
@@ -18,7 +27,7 @@ macro_rules! value_of {
     ($id:ident, $($_: tt)*) => { ifdef![] };
 }
 
-macro_rules! negate_of {
+macro_rules! negator_of {
     ($id:ident, true) => {
         false
     };
@@ -45,6 +54,24 @@ macro_rules! negate_of {
     };
 }
 
+macro_rules! result_of {
+    ($id:expr, true) => {
+        $id
+    };
+    ($id:expr, false) => {
+        $id
+    };
+    ($id:expr, [ $($value: expr),* ]) => {
+        &$id
+    };
+    ($id:expr, $(=value $k: expr;)+ $_: expr) => {
+        $id
+    };
+    ($id:expr, $_: expr) => {
+        $id.as_deref()
+    };
+}
+
 macro_rules! modifier_of {
     ($id:ident, true) => {
         $crate::defaults::SettingKind::Flag(Box::new(move |obj: &mut Settings| obj.$id = true))
@@ -130,7 +157,7 @@ macro_rules! defaults {
         #[allow(non_camel_case_types)]
         mod enums {
             $($(
-                #[derive(Clone,Debug)]
+                #[derive(Clone,Copy,Debug)]
                 #[cfg_attr(test, derive(PartialEq, Eq))]
                 pub enum $name { $($key),* }
             )?)*
@@ -138,14 +165,23 @@ macro_rules! defaults {
 
         #[derive(Clone)]
         pub struct Settings {
-            $(pub $name: type_of!($name, $(=int $fn;)?$(=int $first;)?$($(=enum $key;)*)? $value)),*
+            $($name: storage_of!($name, $(=int $fn;)?$(=int $first;)?$($(=enum $key;)*)? $value)),*
+        }
+
+        // we add setters to make sure the settings-object is read only, and to generate 'unused variable' warnings
+        impl Settings {
+            $(
+            pub fn $name(&self) -> referent_of!($name, $(=int $fn;)?$(=int $first;)?$($(=enum $key;)*)? $value) {
+                result_of!(self.$name, $(=value $fn;)?$(=value $first;)?$($(=value $key;)*)? $value)
+            }
+            )*
         }
 
         impl Default for Settings {
             #[allow(unused_parens)]
             fn default() -> Self {
                 Self {
-                    $($name: value_of!($name, $(=int $fn;)?$(=int $first;)?$($(=enum $key;)*)? $value)),*
+                    $($name: initializer_of!($name, $(=int $fn;)?$(=int $first;)?$($(=enum $key;)*)? $value)),*
                 }
             }
         }
@@ -157,8 +193,8 @@ macro_rules! defaults {
                 $(
                 stringify!($name) if ifdef!($($negate)?; true; false) || matches!(stringify!($value), "true" | "false") || stringify!($value).starts_with('[') => {
                     let _value = ifdef!($($negate)?;
-                        value_of!($name, $(=int $fn;)?$(=int $first;)?$($(=enum $key;)*)? $($negate)?);
-                        negate_of!($name, $(=int $fn;)?$(=int $first;)?$($(=enum $key;)*)? $value)
+                        initializer_of!($name, $(=int $fn;)?$(=int $first;)?$($(=enum $key;)*)? $($negate)?);
+                        negator_of!($name, $(=int $fn;)?$(=int $first;)?$($(=enum $key;)*)? $value)
                     );
                     Some(Box::new(move |obj: &mut Settings| obj.$name = _value))
                 },
@@ -180,7 +216,9 @@ macro_rules! defaults {
 
 pub(super) use defaults;
 pub(super) use ifdef;
+pub(super) use initializer_of;
 pub(super) use modifier_of;
-pub(super) use negate_of;
-pub(super) use type_of;
-pub(super) use value_of;
+pub(super) use negator_of;
+pub(super) use referent_of;
+pub(super) use result_of;
+pub(super) use storage_of;
diff --git a/src/sudoers/mod.rs b/src/sudoers/mod.rs
index debc1351f..063bd6403 100644
--- a/src/sudoers/mod.rs
+++ b/src/sudoers/mod.rs
@@ -188,7 +188,7 @@ impl Sudoers {
     }
 
     pub(crate) fn solve_editor_path(&self) -> Option<PathBuf> {
-        if self.settings.env_editor {
+        if self.settings.env_editor() {
             for key in ["SUDO_EDITOR", "VISUAL", "EDITOR"] {
                 if let Some(var) = std::env::var_os(key) {
                     let path = Path::new(&var);
diff --git a/src/sudoers/policy.rs b/src/sudoers/policy.rs
index 984e9b670..2dd7a4942 100644
--- a/src/sudoers/policy.rs
+++ b/src/sudoers/policy.rs
@@ -53,8 +53,8 @@ pub enum DirChange<'a> {
 impl Policy for Judgement {
     fn authorization(&self) -> Authorization {
         if let Some(tag) = &self.flags {
-            let allowed_attempts = self.settings.passwd_tries.try_into().unwrap();
-            let valid_seconds = self.settings.timestamp_timeout;
+            let allowed_attempts = self.settings.passwd_tries().try_into().unwrap();
+            let valid_seconds = self.settings.timestamp_timeout();
             Authorization::Allowed(AuthorizationAllowed {
                 must_authenticate: tag.needs_passwd(),
                 allowed_attempts,
@@ -66,11 +66,11 @@ impl Policy for Judgement {
     }
 
     fn env_keep(&self) -> &HashSet<String> {
-        &self.settings.env_keep
+        self.settings.env_keep()
     }
 
     fn env_check(&self) -> &HashSet<String> {
-        &self.settings.env_check
+        self.settings.env_check()
     }
 
     fn chdir(&self) -> DirChange {
@@ -82,11 +82,11 @@ impl Policy for Judgement {
     }
 
     fn secure_path(&self) -> Option<String> {
-        self.settings.secure_path.as_ref().map(|s| s.to_string())
+        self.settings.secure_path().as_ref().map(|s| s.to_string())
     }
 
     fn use_pty(&self) -> bool {
-        self.settings.use_pty
+        self.settings.use_pty()
     }
 }
 
@@ -97,14 +97,14 @@ pub trait PreJudgementPolicy {
 
 impl PreJudgementPolicy for Sudoers {
     fn secure_path(&self) -> Option<String> {
-        self.settings.secure_path.as_ref().map(|s| s.to_string())
+        self.settings.secure_path().as_ref().map(|s| s.to_string())
     }
 
     fn validate_authorization(&self) -> Authorization {
         Authorization::Allowed(AuthorizationAllowed {
             must_authenticate: true,
-            allowed_attempts: self.settings.passwd_tries.try_into().unwrap(),
-            prior_validity: Duration::seconds(self.settings.timestamp_timeout),
+            allowed_attempts: self.settings.passwd_tries().try_into().unwrap(),
+            prior_validity: Duration::seconds(self.settings.timestamp_timeout()),
         })
     }
 }
diff --git a/src/sudoers/test/mod.rs b/src/sudoers/test/mod.rs
index ea27e8715..774315749 100644
--- a/src/sudoers/test/mod.rs
+++ b/src/sudoers/test/mod.rs
@@ -257,11 +257,11 @@ fn default_bool_test() {
             "Defaults !env_editor"
         ],
     );
-    assert!(settings.env_reset);
-    assert!(!settings.use_pty);
-    assert!(settings.env_keep.is_empty());
-    assert_eq!(settings.secure_path, None);
-    assert!(!settings.env_editor);
+    assert!(settings.env_reset());
+    assert!(!settings.use_pty());
+    assert!(settings.env_keep().is_empty());
+    assert_eq!(settings.secure_path(), None);
+    assert!(!settings.env_editor());
 }
 
 #[test]
@@ -279,18 +279,18 @@ fn default_set_test() {
         ],
     );
     assert_eq!(
-        settings.env_keep,
-        ["FOO", "BAR"].into_iter().map(|x| x.to_string()).collect()
+        settings.env_keep(),
+        &["FOO", "BAR"].into_iter().map(|x| x.to_string()).collect()
     );
     assert_eq!(
-        settings.env_check,
-        ["FOO", "XYZZY"]
+        settings.env_check(),
+        &["FOO", "XYZZY"]
             .into_iter()
             .map(|x| x.to_string())
             .collect()
     );
-    assert_eq!(settings.secure_path.as_deref(), Some("/etc"));
-    assert_eq!(settings.passwd_tries, 5);
+    assert_eq!(settings.secure_path(), Some("/etc"));
+    assert_eq!(settings.passwd_tries(), 5);
 
     assert!(parse_string::<Sudo>("Defaults verifypw = \"sometimes\"").is_err());
     assert!(parse_string::<Sudo>("Defaults verifypw = sometimes").is_err());
@@ -305,10 +305,13 @@ fn default_multi_test() {
         "Defaults env_reset, !use_pty, secure_path=/etc, env_keep = \"FOO BAR\", env_keep -= BAR"
     ],
     );
-    assert!(settings.env_reset);
-    assert!(!settings.use_pty);
-    assert_eq!(settings.secure_path.as_deref(), Some("/etc"));
-    assert_eq!(settings.env_keep, ["FOO".to_string()].into_iter().collect());
+    assert!(settings.env_reset());
+    assert!(!settings.use_pty());
+    assert_eq!(settings.secure_path(), Some("/etc"));
+    assert_eq!(
+        settings.env_keep(),
+        &["FOO".to_string()].into_iter().collect()
+    );
 }
 
 #[test]