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/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/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 new file mode 100644 index 0000000..b731f99 --- /dev/null +++ b/tests/golden/twins.json @@ -0,0 +1,122 @@ +{ + "both-spellings-provided": { + "args": { + "args": { + "flagA": false, + "showPreview": false + }, + "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": 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" + ] + } + }, + "explicit-false-on-canonical": { + "args": { + "args": { + "flagA": false, + "showPreview": false + }, + "defaulted": [], + "err": false, + "errmsg": [], + "warnmsg": [] + }, + "initargs": { + "default": [], + "err": false, + "errmsg": [], + "flag-a": false, + "flagA": false, + "flag_a": false, + "show-preview": false, + "showPreview": false, + "show_preview": false, + "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" + ], + "err": false, + "errmsg": [], + "warnmsg": [] + }, + "initargs": { + "default": [ + "flag-a", + "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