feat(config): hold a value to the choices its setting declares

config/src/explain.rs

@@ -66,6 +66,11 @@ pub fn explain(resolved: &Resolved, key: &str) -> Option<String> {
     // The spec's own spelling, not the prose an error message uses: a reader searching the docs
     // for "a non-negative integer" finds nothing, and `uint` is what the author wrote.
     let _ = writeln!(out, "  {:<8}{}", "type", meta.ty.name());
+    // What it will take, when it says. A user reading an explanation because their value was refused
+    // needs the list here rather than in a warning they have already scrolled past.
+    if !meta.choices.is_empty() {
+        let _ = writeln!(out, "  {:<8}{}", "one of", one_line(&meta.allowed()));
+    }
     if let Some(help) = meta.help {
         // Through the same helper as everything else: an adopter's help is a doc comment, and a
         // doc comment with a second paragraph in it split one fact into several records.

config/src/layer.rs

@@ -166,10 +166,16 @@ impl LayerCtx {
         // binding carries the same information as one built from a key.
         let (id, renamed_from) = self.folded(id);
         match self.parse(id, raw) {
-            Ok(value) => Ok(Entry {
-                renamed_from,
-                ..Entry::new(id, value, origin)
-            }),
+            Ok(value) => {
+                let key = renamed_from.unwrap_or(self.registry.get(id).key);
+                if let Some(refused) = self.refused(id, &value, key, &origin) {
+                    return Err(refused);
+                }
+                Ok(Entry {
+                    renamed_from,
+                    ..Entry::new(id, value, origin)
+                })
+            }
             Err(err) => {
                 // The name that was written, not the one it folded to: a message about a key
                 // the user cannot find in their own file is no help.
@@ -187,6 +193,26 @@ impl LayerCtx {
 }
 
 impl LayerCtx {
+    /// The warning for a value the setting's `choice` nodes do not allow, if it is one.
+    ///
+    /// Beside the type check and for the same reason: a declared type and a declared set of values
+    /// are both the spec saying what may be here, and a value that is neither costs its own key and
+    /// nothing else. Until this, choices reached the docs, the JSON schema and completions, and
+    /// nothing that *resolved* a value — so a CLI documenting three allowed values took a fourth
+    /// without a word, and only failed later, somewhere that could not say why.
+    fn refused(&self, id: PropId, value: &Value, key: &str, origin: &Origin) -> Option<Warning> {
+        let meta = self.registry.get(id);
+        let refused = meta.refuses(value)?;
+        Some(Warning::at(
+            format!(
+                "{key} expected one of {} but has `{}`",
+                meta.allowed(),
+                crate::value::shown(refused)
+            ),
+            origin.clone(),
+        ))
+    }
+
     /// An entry for a dotted key, which is what a layer reading a file has in hand.
     ///
     /// The path worth taking: it looks the key up, follows a rename while remembering the name
@@ -222,10 +248,16 @@ impl LayerCtx {
         };
         let meta = self.registry.get(found.id);
         match meta.ty.coerce(value) {
-            Ok(value) => Ok(Entry {
-                renamed_from: found.renamed_from,
-                ..Entry::new(found.id, value, origin)
-            }),
+            Ok(value) => {
+                let key = found.renamed_from.unwrap_or(meta.key);
+                if let Some(refused) = self.refused(found.id, &value, key, &origin) {
+                    return Err(refused);
+                }
+                Ok(Entry {
+                    renamed_from: found.renamed_from,
+                    ..Entry::new(found.id, value, origin)
+                })
+            }
             Err(err) => Err(Warning::at(
                 format!(
                     "{} expected {} but has `{}`",
@@ -253,16 +285,101 @@ mod tests {
     use super::*;
     use crate::registry::PropMeta;
     use crate::ty::{Parser, Ty};
+    use crate::value::Const;
 
     static PROPS: &[PropMeta] = &[
         PropMeta::new("jobs", Ty::Uint),
         PropMeta {
             parse: Some(Parser::ListByComma),
             ..PropMeta::new("exclude", Ty::List(&Ty::String))
         },
+        // A setting the spec limits to three values, which is hk's `stash` and mise's nine
+        // enum-valued settings.
+        PropMeta {
+            choices: &[
+                Const::Str("git"),
+                Const::Str("patch-file"),
+                Const::Str("none"),
+            ],
+            ..PropMeta::new("stash", Ty::String)
+        },
+        // Choices on a list: each *item* is one of them, the way the JSON schema reads it.
+        PropMeta {
+            parse: Some(Parser::ListByComma),
+            choices: &[Const::Str("lint"), Const::Str("test")],
+            ..PropMeta::new("skip", Ty::List(&Ty::String))
+        },
     ];
     const REGISTRY: Registry = Registry::new(PROPS);
 
+    #[test]
+    fn a_value_the_spec_does_not_allow_is_refused_with_the_list_of_what_is() {
+        // Choices reached the docs, the JSON schema and completions, and nothing that *resolved* a
+        // value: a CLI documenting three allowed values took a fourth in silence, and failed later
+        // somewhere that could not say why.
+        let ctx = LayerCtx::new(REGISTRY);
+        let origin = Origin::new(SourceKind::ENV, "HK_STASH");
+        let warning = ctx
+            .entry_for_key("stash", "svn", origin.clone())
+            .expect_err("not one of the three");
+        assert_eq!(
+            warning.message,
+            "stash expected one of git, patch-file, none but has `svn`"
+        );
+        // And a value that is one of them is just a value.
+        assert_eq!(
+            ctx.entry_for_key("stash", "git", origin.clone())
+                .map(|entry| entry.value),
+            Ok(Value::from("git"))
+        );
+
+        // A list is checked item by item, and the *item* is what the message quotes — naming the
+        // whole list would leave the user to work out which of five items was the problem.
+        let warning = ctx
+            .entry_for_key("skip", "lint,fmt", origin.clone())
+            .expect_err("`fmt` is not one of them");
+        assert_eq!(
+            warning.message,
+            "skip expected one of lint, test but has `fmt`"
+        );
+        assert!(ctx.entry_for_key("skip", "lint,test", origin).is_ok());
+    }
+
+    #[test]
+    fn a_setting_with_no_choices_takes_what_its_type_takes() {
+        // Most settings say nothing about their values, and the check has to cost them nothing and
+        // refuse them nothing.
+        let ctx = LayerCtx::new(REGISTRY);
+        let origin = Origin::new(SourceKind::ENV, "HK_JOBS");
+        assert_eq!(
+            ctx.entry_for_key("jobs", "8", origin).map(|e| e.value),
+            Ok(Value::Int(8))
+        );
+    }
+
+    #[test]
+    fn a_structured_value_is_held_to_the_same_choices() {
+        // The other way a value arrives — a table or a list out of a file, which never passes
+        // through a parser. Checking one path and not the other is how a rule ends up applying to
+        // the environment and not to the file beside it.
+        let ctx = LayerCtx::new(REGISTRY);
+        let origin = Origin::new(SourceKind::FILE, "hk.toml");
+        let warning = ctx
+            .entry_from_value(
+                "skip",
+                Value::List(vec![Value::from("test"), Value::from("deploy")]),
+                origin.clone(),
+            )
+            .expect_err("`deploy` is not one of them");
+        assert_eq!(
+            warning.message,
+            "skip expected one of lint, test but has `deploy`"
+        );
+        assert!(ctx
+            .entry_from_value("skip", Value::List(vec![Value::from("test")]), origin)
+            .is_ok());
+    }
+
     #[test]
     fn a_raw_string_is_read_the_way_the_spec_says() {
         let ctx = LayerCtx::new(REGISTRY);

config/src/registry.rs

@@ -10,7 +10,7 @@
 
 use crate::source::SourceKind;
 use crate::ty::{Parser, Ty};
-use crate::value::Const;
+use crate::value::{Const, Value};
 
 /// A setting's index in its registry.
 ///
@@ -76,6 +76,12 @@ pub struct PropMeta {
     /// asks the registry for its own kind and iterates what it finds, which is the whole
     /// mechanism behind hk's git and pkl layers and aube's `.npmrc`.
     pub bindings: &'static [(&'static str, &'static str)],
+    /// The only values this setting accepts, when it says.
+    ///
+    /// Empty means anything the type allows. Declared in the spec as `choice` nodes, where they
+    /// already reach the docs, the JSON schema and completions — and, until this, nothing that
+    /// *resolved* a value, so a CLI documenting three allowed values accepted a fourth in silence.
+    pub choices: &'static [Const],
     /// Kept out of documentation and completions. Still settable.
     pub hide: bool,
     /// Why not to use this any more.
@@ -98,6 +104,7 @@ impl PropMeta {
             parse: None,
             envs: &[],
             bindings: &[],
+            choices: &[],
             hide: false,
             deprecated: None,
             renamed_to: None,
@@ -121,6 +128,37 @@ pub struct Lookup {
     pub renamed_from: Option<&'static str>,
 }
 
+impl PropMeta {
+    /// The first value here that this setting does not allow, if there is one.
+    ///
+    /// A collection is checked item by item, because choices on a `list<string>` mean each item is
+    /// one of them — the same rule `usage g json-schema` follows, which puts the enum on every value
+    /// position rather than on the container. Returning the offender rather than a bool is what lets
+    /// the warning quote the item that is wrong instead of the whole list it was in.
+    pub fn refuses<'v>(&self, value: &'v Value) -> Option<&'v Value> {
+        if self.choices.is_empty() {
+            return None;
+        }
+        match value {
+            Value::List(items) => items.iter().find_map(|item| self.refuses(item)),
+            Value::Map(entries) => entries.values().find_map(|item| self.refuses(item)),
+            scalar => match self.choices.iter().any(|choice| choice.matches(scalar)) {
+                true => None,
+                false => Some(scalar),
+            },
+        }
+    }
+
+    /// What it allows, written the way the spec declared them, for a message.
+    pub fn allowed(&self) -> String {
+        self.choices
+            .iter()
+            .map(|choice| choice.to_value().display())
+            .collect::<Vec<_>>()
+            .join(", ")
+    }
+}
+
 impl Registry {
     pub const fn new(props: &'static [PropMeta]) -> Self {
         Self { props }

config/src/value.rs

@@ -157,6 +157,23 @@ pub enum Const {
 }
 
 impl Const {
+    /// Whether `value` is this constant.
+    ///
+    /// Without building the `Value` it stands for: this runs once per declared choice for every
+    /// value a layer supplies, and a setting with choices is usually a string, where the comparison
+    /// would otherwise allocate a copy of the choice to throw away.
+    pub fn matches(self, value: &Value) -> bool {
+        match (self, value) {
+            (Self::Bool(a), Value::Bool(b)) => a == *b,
+            (Self::Int(a), Value::Int(b)) => a == *b,
+            (Self::Float(a), Value::Float(b)) => a == *b,
+            (Self::Str(a), Value::String(b)) => a == b,
+            // A list or a table is not something a `choice` node can hold, and comparing one to a
+            // scalar is not a near miss to be generous about.
+            _ => false,
+        }
+    }
+
     /// The owned value this stands for.
     pub fn to_value(self) -> Value {
         match self {