From a09162b05813035923b75038f99f8b575460334e Mon Sep 17 00:00:00 2001
From: default <216188+jdx@users.noreply.github.com>
Date: Sat, 15 Aug 2026 01:27:20 +0000
Subject: [PATCH] fix(config): give a deprecation notice from anywhere along a
 rename chain
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Found by writing the corpus vector for a rule the page already stated. `explain`
walked the chain of renames looking for a notice — `a` renamed to `b`, and `b` the
one that says to use `c` — and the merge read the notice off the declaration the
user wrote. So `config explain threads` told a user to stop using a key that
running the CLI said nothing about, and the quieter of the two is the one a CLI
actually surfaces.

One walker now, on `Registry`, called by both. The message still names the key
that was written, since that is the line in the file they would go and edit.
---
 config/src/explain.rs        | 24 ++------------
 config/src/registry.rs       | 22 +++++++++++++
 config/src/resolve.rs        | 61 +++++++++++++++++++++++++++++++++---
 corpus/config/04-renames.kdl | 14 +++++++++
 4 files changed, 94 insertions(+), 27 deletions(-)

diff --git a/config/src/explain.rs b/config/src/explain.rs
index 35e9f170f..6b4ba2ab0 100644
--- a/config/src/explain.rs
+++ b/config/src/explain.rs
@@ -14,7 +14,7 @@
 
 use std::fmt::Write as _;
 
-use crate::registry::{PropId, Registry};
+use crate::registry::PropId;
 use crate::resolve::Resolved;
 use crate::source::SourceKind;
 use crate::value::{one_line, shown};
@@ -121,7 +121,7 @@ pub fn explain(resolved: &Resolved, key: &str) -> Option<String> {
     // reading it off the setting that replaced it printed nothing at all for the one case where it
     // matters — and following the renames from there, because a notice can sit anywhere along a
     // chain: `a` renamed to `b`, and `b` the one carrying the notice that says to use `c`.
-    let deprecated = deprecation_along(registry, found.renamed_from.unwrap_or(meta.key));
+    let deprecated = registry.deprecation(found.renamed_from.unwrap_or(meta.key));
     if let Some(why) = deprecated {
         let _ = writeln!(out, "\n  deprecated: {}", one_line(why));
     }
@@ -129,26 +129,6 @@ pub fn explain(resolved: &Resolved, key: &str) -> Option<String> {
     Some(out)
 }
 
-/// The first deprecation notice along the rename chain that starts at `key`.
-///
-/// Bounded by the number of settings there are, so a registry whose renames form a cycle stops
-/// rather than following them forever — the same guard [`Registry::lookup`] uses, and for the same
-/// reason: this is an authoring mistake, and hanging is a worse way to report one than nothing at
-/// all. `usage-config-build` refuses such a registry outright.
-fn deprecation_along(registry: Registry, key: &str) -> Option<&'static str> {
-    let mut current = registry.lookup_exact(key)?;
-    for _ in 0..registry.props.len() {
-        let meta = registry.get(current);
-        if let Some(why) = meta.deprecated {
-            return Some(why);
-        }
-        current = meta
-            .renamed_to
-            .and_then(|next| registry.lookup_exact(next))?;
-    }
-    None
-}
-
 /// Every warning the resolution produced, as lines.
 ///
 /// Separate from [`explain`] because they answer different questions and belong in different
diff --git a/config/src/registry.rs b/config/src/registry.rs
index b6f306cfa..cfade0cbf 100644
--- a/config/src/registry.rs
+++ b/config/src/registry.rs
@@ -254,6 +254,28 @@ impl Registry {
             .map(|index| PropId(index as u16))
     }
 
+    /// The first deprecation notice along the rename chain that starts at `key`.
+    ///
+    /// The chain, not the declaration named: `a` renamed to `b`, and `b` the one carrying the notice
+    /// that says to use `c`. A user who wrote `a` is being told the same thing either way, and which
+    /// release the notice was attached in is not something they can see.
+    ///
+    /// Bounded by the number of settings there are, so a registry whose renames form a cycle stops
+    /// rather than following them forever — the same guard [`Registry::lookup`] uses, and for the
+    /// same reason: this is an authoring mistake, and hanging is a worse way to report one than
+    /// nothing at all. `usage-config-build` refuses such a registry outright.
+    pub fn deprecation(&self, key: &str) -> Option<&'static str> {
+        let mut current = self.lookup_exact(key)?;
+        for _ in 0..self.props.len() {
+            let meta = self.get(current);
+            if let Some(why) = meta.deprecated {
+                return Some(why);
+            }
+            current = meta.renamed_to.and_then(|next| self.lookup_exact(next))?;
+        }
+        None
+    }
+
     /// The settings an environment variable sets, and the variable that set them.
     ///
     /// Several names per setting are aliases in descending precedence, which the env layer
diff --git a/config/src/resolve.rs b/config/src/resolve.rs
index f5077f806..a9cf88302 100644
--- a/config/src/resolve.rs
+++ b/config/src/resolve.rs
@@ -180,10 +180,6 @@ pub fn resolve(registry: Registry, layers: Layers<'_>) -> Result<Resolved, Layer
             // key up and `LayerCtx` folded it, or as a raw id this loop folded just now.
             // Keyed on the fold alone, a file layer's deprecated key was folded in silence.
             let written_key = entry.renamed_from.unwrap_or(written.key);
-            let as_written = registry
-                .lookup_exact(written_key)
-                .map(|id| registry.get(id))
-                .unwrap_or(written);
             if let Some(refusal) = refuse(meta.scope, &entry.origin) {
                 // `written_key`, like the two warnings below it: after `LayerCtx` folds a
                 // rename, `written.key` is the *replacement's* name, so a refused value was
@@ -195,7 +191,11 @@ pub fn resolve(registry: Registry, layers: Layers<'_>) -> Result<Resolved, Layer
                 );
                 continue;
             }
-            if let Some(why) = as_written.deprecated {
+            // Along the chain rather than off the declaration written, which is what `explain` has
+            // always done: a notice can sit on a name further along, and reading only the one the
+            // user wrote meant `config explain` told them to stop using a key that running the CLI
+            // said nothing about.
+            if let Some(why) = registry.deprecation(written_key) {
                 resolved.warnings.push(
                     Warning::at(
                         format!("{written_key} is deprecated: {why}"),
@@ -1057,6 +1057,57 @@ mod tests {
         );
     }
 
+    #[test]
+    fn a_notice_further_along_a_chain_of_renames_is_still_given() {
+        // Two releases of renaming: `threads` became `concurrency`, which became `jobs` and carries
+        // the notice. `explain` walked the chain for it and the merge read only the declaration
+        // written, so `config explain threads` told a user to stop using a key that running the CLI
+        // said nothing about — one rule, two implementations, and the quieter one was the one a CLI
+        // actually surfaces.
+        static PROPS: &[PropMeta] = &[
+            PropMeta {
+                default: Some(Const::Int(1)),
+                ..PropMeta::new("jobs", Ty::Uint)
+            },
+            PropMeta {
+                renamed_to: Some("jobs"),
+                deprecated: Some("Use jobs instead."),
+                ..PropMeta::new("concurrency", Ty::Uint)
+            },
+            PropMeta {
+                renamed_to: Some("concurrency"),
+                ..PropMeta::new("threads", Ty::Uint)
+            },
+        ];
+        const CHAINED: Registry = Registry::new(PROPS);
+
+        struct Wrote;
+        impl Layer for Wrote {
+            fn source(&self) -> SourceKind {
+                SourceKind::FILE
+            }
+            fn load(&self, ctx: &LayerCtx) -> Result<LayerOutput, LayerError> {
+                let mut out = LayerOutput::new();
+                let origin = Origin::file("hk.toml", FileScope::Project);
+                match ctx.entry_for_key("threads", "8", origin) {
+                    Ok(entry) => out.push(entry),
+                    Err(warning) => out.warn(warning),
+                }
+                Ok(out)
+            }
+        }
+        let resolved = resolve(CHAINED, Layers::new().then(&Wrote)).expect("should resolve");
+        assert_eq!(resolved.get_key("jobs"), Some(&Value::Int(8)));
+        let kinds: Vec<_> = resolved.warnings.iter().map(|w| w.kind).collect();
+        assert_eq!(kinds, vec![WarningKind::Deprecated, WarningKind::Renamed]);
+        // Named by what the user wrote, since that is the line in the file they would go and edit.
+        assert!(
+            resolved.warnings[0].message == "threads is deprecated: Use jobs instead.",
+            "{:?}",
+            resolved.warnings[0].message
+        );
+    }
+
     #[test]
     fn an_unknown_key_is_a_warning_rather_than_a_failure() {
         // Newer config read by an older binary: the key it does not know is reported and the
diff --git a/corpus/config/04-renames.kdl b/corpus/config/04-renames.kdl
index 7c4599429..5ffa024a9 100644
--- a/corpus/config/04-renames.kdl
+++ b/corpus/config/04-renames.kdl
@@ -91,3 +91,17 @@ vector "a-chain-of-renames-resolves-to-its-end" doc="Two releases of renaming le
     warning "renamed"
   }
 }
+
+vector "a-notice-anywhere-along-a-chain-is-reported" doc="A deprecation notice may sit on a name further along than the one that was written, and it is still what the user is told." {
+  setting "jobs" type="uint" default=1
+  setting "concurrency" type="uint" renamed-to="jobs" deprecated="Use jobs instead."
+  setting "threads" type="uint" renamed-to="concurrency"
+  layer "file" id="hk.toml" {
+    value "threads" "8"
+  }
+  expect {
+    value "jobs" 8
+    warning "deprecated"
+    warning "renamed"
+  }
+}