From a14270080149180c5aaae5aa2ef3be1303e2db29 Mon Sep 17 00:00:00 2001 From: Mark Dumay <61946753+markdumay@users.noreply.github.com> Date: Sun, 12 Jul 2026 13:44:16 +0200 Subject: [PATCH 1/2] test: characterize deprecated snake-twin default collision Add a 'twins' test group covering deprecated snake_case arguments that collide with their kebab-case canonical twin on the same camelKey: an inline-declared pair (flag-a/flag_a) and a pair mirroring the real-world shape where the canonical inherits a default from the module's global _arguments.yml (show-preview/show_preview). The pinned golden shows the defect: an explicit `show-preview=false` is silently overwritten to `true` because the unset deprecated twin inherits the canonical's global default and sorts after it (underscore > hyphen) in the schema loop, and `show_preview` incorrectly appears in `defaulted`. A second case shows the same unconditional last-write-wins ordering flips the result when both spellings are explicitly provided. Co-Authored-By: Claude Fable 5 --- exampleSite/content/tests/twins.md | 4 + exampleSite/data/structures/test-twin.yml | 23 ++++ exampleSite/data/tests/twins.yml | 18 +++ tests/golden/twins.json | 128 ++++++++++++++++++++++ 4 files changed, 173 insertions(+) create mode 100644 exampleSite/content/tests/twins.md create mode 100644 exampleSite/data/structures/test-twin.yml create mode 100644 exampleSite/data/tests/twins.yml create mode 100644 tests/golden/twins.json diff --git a/exampleSite/content/tests/twins.md b/exampleSite/content/tests/twins.md new file mode 100644 index 0000000..a77d17c --- /dev/null +++ b/exampleSite/content/tests/twins.md @@ -0,0 +1,4 @@ +--- +title: twins +outputs: ["json"] +--- diff --git a/exampleSite/data/structures/test-twin.yml b/exampleSite/data/structures/test-twin.yml new file mode 100644 index 0000000..f966058 --- /dev/null +++ b/exampleSite/data/structures/test-twin.yml @@ -0,0 +1,23 @@ +comment: >- + Test fixture for the deprecated snake_case twin vs. canonical kebab-case argument + camelKey collision: both names camelize to the same key. 'flag-a'/'flag_a' is a fully + inline-declared pair (no global argument definition involved). 'show-preview'/'show_preview' + mirrors the real-world shape: the canonical argument inherits its type and default from the + module's global _arguments.yml, and the deprecated twin inherits the SAME global definition + via ArgsSchema's snake->kebab normKey lookup. +arguments: + flag-a: + type: bool + optional: true + default: true + flag_a: + type: bool + optional: true + deprecated: 1.0.0 + alternative: flag-a + show-preview: + optional: true + show_preview: + optional: true + deprecated: 1.0.0 + alternative: show-preview diff --git a/exampleSite/data/tests/twins.yml b/exampleSite/data/tests/twins.yml new file mode 100644 index 0000000..0fee8d1 --- /dev/null +++ b/exampleSite/data/tests/twins.yml @@ -0,0 +1,18 @@ +cases: + # BUG: the canonical's explicit false is expected to survive, but the deprecated + # twin (unset) inherits a default and overwrites it once the twin sorts after the + # canonical (underscore > hyphen) in the schema loop. + - name: explicit-false-on-canonical + structure: test-twin + args: {flag-a: "false", show-preview: "false"} + - name: unset-uses-canonical-default + structure: test-twin + args: {} + - name: explicit-value-on-deprecated-twin + structure: test-twin + args: {flag_a: "false", show_preview: "false"} + # BUG: both spellings provided; the canonical's explicit false must win over the + # deprecated twin's explicit true, regardless of iteration order. + - name: both-spellings-provided + structure: test-twin + args: {flag-a: "false", flag_a: "true", show-preview: "false", show_preview: "true"} diff --git a/tests/golden/twins.json b/tests/golden/twins.json new file mode 100644 index 0000000..871a313 --- /dev/null +++ b/tests/golden/twins.json @@ -0,0 +1,128 @@ +{ + "both-spellings-provided": { + "args": { + "args": { + "flagA": true, + "showPreview": true + }, + "defaulted": [], + "err": false, + "errmsg": [], + "warnmsg": [ + "[test-twin] argument 'flag_a': deprecated in v1.0.0, use 'flag-a' instead", + "[test-twin] argument 'show_preview': deprecated in v1.0.0, use 'show-preview' instead" + ] + }, + "initargs": { + "default": [], + "err": false, + "errmsg": [], + "flag-a": true, + "flagA": true, + "flag_a": true, + "show-preview": true, + "showPreview": true, + "show_preview": true, + "warnmsg": [ + "[test-twin] argument 'flag_a': deprecated in v1.0.0, use 'flag-a' instead", + "[test-twin] argument 'show_preview': deprecated in v1.0.0, use 'show-preview' instead" + ] + } + }, + "explicit-false-on-canonical": { + "args": { + "args": { + "flagA": false, + "showPreview": true + }, + "defaulted": [ + "show_preview" + ], + "err": false, + "errmsg": [], + "warnmsg": [] + }, + "initargs": { + "default": [ + "show_preview" + ], + "err": false, + "errmsg": [], + "flag-a": false, + "flagA": false, + "flag_a": false, + "show-preview": true, + "showPreview": true, + "show_preview": true, + "warnmsg": [] + } + }, + "explicit-value-on-deprecated-twin": { + "args": { + "args": { + "flagA": false, + "showPreview": false + }, + "defaulted": [ + "flag-a", + "show-preview" + ], + "err": false, + "errmsg": [], + "warnmsg": [ + "[test-twin] argument 'flag_a': deprecated in v1.0.0, use 'flag-a' instead", + "[test-twin] argument 'show_preview': deprecated in v1.0.0, use 'show-preview' instead" + ] + }, + "initargs": { + "default": [ + "flag-a", + "show-preview" + ], + "err": false, + "errmsg": [], + "flag-a": false, + "flagA": false, + "flag_a": false, + "show-preview": false, + "showPreview": false, + "show_preview": false, + "warnmsg": [ + "[test-twin] argument 'flag_a': deprecated in v1.0.0, use 'flag-a' instead", + "[test-twin] argument 'show_preview': deprecated in v1.0.0, use 'show-preview' instead" + ] + } + }, + "unset-uses-canonical-default": { + "args": { + "args": { + "flagA": true, + "showPreview": true + }, + "defaulted": [ + "flag-a", + "show-preview", + "show_preview" + ], + "err": false, + "errmsg": [], + "warnmsg": [] + }, + "initargs": { + "default": [ + "flag-a", + "show-preview", + "show_preview" + ], + "err": false, + "errmsg": [], + "flag-a": true, + "flagA": true, + "flag_a": true, + "show-preview": true, + "showPreview": true, + "show_preview": true, + "warnmsg": [] + } + } +} \ No newline at end of file From be76dbd9091b9f3c63b9cb3b14da875eab7c1819 Mon Sep 17 00:00:00 2001 From: Mark Dumay <61946753+markdumay@users.noreply.github.com> Date: Sun, 12 Jul 2026 13:47:55 +0200 Subject: [PATCH 2/2] fix: never default deprecated arguments and guard camelKey collisions A deprecated snake_case twin (e.g. show_preview) and its kebab-case canonical (show-preview) camelize to the same output key. ArgsSchema.html's snake->kebab normKey lookup let the twin inherit the canonical's global default, and Args.html's schema loop iterates map keys in sorted order, so the twin (sorting after the canonical, since '_' > '-') silently overwrote an explicit show-preview=false with its inherited default of true. Two changes fix this: - ArgsSchema.html: a node carrying "deprecated" never stores "default" or "config", whether inherited from the global definition or declared inline. - Args.html: the camelKey merge is now precedence-safe and order-independent. Provided-canonical values beat provided-deprecated values, which beat defaulted values, and a defaulted value never overwrites an existing entry. Explicit values now always win over any default, on both the strict Args.html path and the InitArgs.html compatibility shim. Verified against a new "twins" golden fixture covering both an inline-declared twin pair and a pair that inherits its default from the module's global _arguments.yml. Co-Authored-By: Claude Fable 5 --- README.md | 18 ++++++++++ layouts/_partials/utilities/Args.html | 29 ++++++++++++++-- layouts/_partials/utilities/ArgsSchema.html | 16 +++++++-- tests/golden/twins.json | 38 +++++++++------------ 4 files changed, 74 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index 0c56fb0..420a7a9 100644 --- a/README.md +++ b/README.md @@ -132,6 +132,24 @@ This is distinct from a value the caller *explicitly* provided as `false`, `0`, (every blueprint key present, typically `null`, plus `_bookshop_name`/`_ordinal`) build clean while a genuinely wrong explicit value is still caught. +### Deprecated arguments and camelKey collisions + +A deprecated argument (e.g. a legacy snake_case twin of a kebab-case canonical, such as +`show_preview` next to `show-preview`) is compiled by `ArgsSchema.html` without a `default` or +`config` — even when the canonical argument it replaces carries one in the global +`_arguments.yml` definition. A deprecated alias exists only to accept an explicit legacy value +and emit a warning; defaults belong solely to the canonical replacement. This keeps an unset +deprecated twin out of the `defaulted` list entirely. + +Because hyphens and underscores both camelize to the same key (`show-preview` and `show_preview` +both become `showPreview`), `Args.html` can visit the same output entry more than once while +walking the schema. The merge follows a fixed precedence lattice rather than relying on +iteration order: a caller-provided value on the canonical argument always wins, followed by a +caller-provided value on the deprecated twin, followed by a defaulted value. A defaulted value +never overwrites an existing entry, and a value provided through a deprecated argument never +overwrites a value already provided through its canonical replacement — regardless of which one +the schema loop happens to visit first. + ### Warnings-first strictness rollout The following newly detectable problem classes surface as **warnings** (`warnmsg`, `err: false`) diff --git a/layouts/_partials/utilities/Args.html b/layouts/_partials/utilities/Args.html index 1dbfb73..f64e992 100644 --- a/layouts/_partials/utilities/Args.html +++ b/layouts/_partials/utilities/Args.html @@ -343,19 +343,42 @@ {{ end }} {{ end }} - {{/* validate every schema argument and enforce required ones */}} + {{/* validate every schema argument and enforce required ones. + + Two or more declared names (e.g. a kebab-case canonical and its deprecated snake_case + twin) can camelize to the same camelKey, so the schema loop below can visit the same + $out entry more than once. Map iteration order is sorted by key, which is NOT a + reliable precedence signal (it's an accident of "-" < "_" in ASCII) — so track, per + camelKey, whether the current entry came from a caller-provided value in $providedKeys + and merge under this lattice: + - provided-canonical > provided-deprecated > defaulted + - a defaulted value never overwrites an existing entry (provided or defaulted) + - a provided value overwrites a defaulted entry + - a provided value from a deprecated node never overwrites an existing provided entry + (so a provided canonical always beats a provided deprecated twin, whichever of the + two is visited first) */}} + {{ $providedKeys := dict }} {{ range $key, $node := $schema }} {{ $isProvided := isset $normalized $key }} {{ $val := index $normalized $key }} + {{ $wasProvided := and $isProvided (ne $val nil) }} {{ $res := partial "inline/validate-node.html" (dict "value" $val "node" $node "path" $key "name" $name - "depth" 0 "provided" (and $isProvided (ne $val nil))) }} + "depth" 0 "provided" $wasProvided) }} {{ range $res.errmsg }}{{ $errmsg = $errmsg | append . }}{{ end }} {{ range $res.newmsg }}{{ $newmsg = $newmsg | append . }}{{ end }} {{ range $res.warnmsg }}{{ $warnmsg = $warnmsg | append . }}{{ end }} {{ range $res.defaulted }}{{ $defaulted = $defaulted | append . }}{{ end }} {{ if ne $res.value nil }} - {{ $out = merge $out (dict $node.camelKey $res.value) }} + {{ $alreadyProvided := index $providedKeys $node.camelKey }} + {{ if $wasProvided }} + {{ if or (not $alreadyProvided) (not $node.deprecated) }} + {{ $out = merge $out (dict $node.camelKey $res.value) }} + {{ $providedKeys = merge $providedKeys (dict $node.camelKey true) }} + {{ end }} + {{ else if not $alreadyProvided }} + {{ $out = merge $out (dict $node.camelKey $res.value) }} + {{ end }} {{ end }} {{ $skip := false }} diff --git a/layouts/_partials/utilities/ArgsSchema.html b/layouts/_partials/utilities/ArgsSchema.html index c134062..80a287a 100644 --- a/layouts/_partials/utilities/ArgsSchema.html +++ b/layouts/_partials/utilities/ArgsSchema.html @@ -12,6 +12,13 @@ "reflects" field: a []string of raw package-qualified Go reflect-type names (e.g. "template.HTML") declared in the data files, matched verbatim against printf "%T" at validation time. + + A node carrying "deprecated" never stores "default" or "config", regardless of whether that + default/config would have come from the global _arguments.yml definition (via the snake->kebab + normKey lookup) or from an inline override in the structure file. A deprecated alias exists + only to accept an explicit legacy value and warn the caller; defaults belong solely to the + canonical replacement argument. This also keeps a deprecated twin out of Args.html's + "defaulted" list when the caller leaves it unset. */}} {{ define "_partials/inline/camel-key.html" }} @@ -162,8 +169,13 @@ }} {{ if $children }}{{ $node = merge $node (dict "children" $children "udtType" $udtType) }}{{ end }} {{ if $reflects }}{{ $node = merge $node (dict "reflects" $reflects) }}{{ end }} - {{ if ne $def.default nil }}{{ $node = merge $node (dict "default" $def.default) }}{{ end }} - {{ with $def.config }}{{ $node = merge $node (dict "config" .) }}{{ end }} + {{/* a deprecated argument never carries a default or a config lookup — see the header + doc. This applies whether "default"/"config" were inherited from the global + definition via the normKey lookup above or declared inline on this node. */}} + {{ if not $def.deprecated }} + {{ if ne $def.default nil }}{{ $node = merge $node (dict "default" $def.default) }}{{ end }} + {{ with $def.config }}{{ $node = merge $node (dict "config" .) }}{{ end }} + {{ end }} {{ with $def.options }}{{ $node = merge $node (dict "options" .) }}{{ end }} {{ if isset $def "position" }}{{ $node = merge $node (dict "position" $def.position) }}{{ end }} {{ with $def.group }}{{ $node = merge $node (dict "group" (slice | append .)) }}{{ end }} diff --git a/tests/golden/twins.json b/tests/golden/twins.json index 871a313..b731f99 100644 --- a/tests/golden/twins.json +++ b/tests/golden/twins.json @@ -2,8 +2,8 @@ "both-spellings-provided": { "args": { "args": { - "flagA": true, - "showPreview": true + "flagA": false, + "showPreview": false }, "defaulted": [], "err": false, @@ -17,12 +17,12 @@ "default": [], "err": false, "errmsg": [], - "flag-a": true, - "flagA": true, - "flag_a": true, - "show-preview": true, - "showPreview": true, - "show_preview": true, + "flag-a": false, + "flagA": false, + "flag_a": false, + "show-preview": false, + "showPreview": false, + "show_preview": false, "warnmsg": [ "[test-twin] argument 'flag_a': deprecated in v1.0.0, use 'flag-a' instead", "[test-twin] argument 'show_preview': deprecated in v1.0.0, use 'show-preview' instead" @@ -33,27 +33,23 @@ "args": { "args": { "flagA": false, - "showPreview": true + "showPreview": false }, - "defaulted": [ - "show_preview" - ], + "defaulted": [], "err": false, "errmsg": [], "warnmsg": [] }, "initargs": { - "default": [ - "show_preview" - ], + "default": [], "err": false, "errmsg": [], "flag-a": false, "flagA": false, "flag_a": false, - "show-preview": true, - "showPreview": true, - "show_preview": true, + "show-preview": false, + "showPreview": false, + "show_preview": false, "warnmsg": [] } }, @@ -101,8 +97,7 @@ }, "defaulted": [ "flag-a", - "show-preview", - "show_preview" + "show-preview" ], "err": false, "errmsg": [], @@ -111,8 +106,7 @@ "initargs": { "default": [ "flag-a", - "show-preview", - "show_preview" + "show-preview" ], "err": false, "errmsg": [],