Allow a local key to override a merged non-string key

[spokodev]

Problem

A merge key (<<) folds another mapping's keys into the target and records them
as overridable, so a later explicit pair may override a merged key without a
duplicated mapping key error (the canonical YAML merge "Override" behavior).

For the default object-based map, that tracking breaks on non-string keys:

const { load, CORE_SCHEMA, mergeTag } = require('js-yaml');
const schema = CORE_SCHEMA.withTags(mergeTag);

load('base: &a {10: x}\nm:\n  <<: *a\n  10: y', { schema });
// throws YAMLException: duplicated mapping key

A string key in the same position works fine (ten: → {ten: 'y'}). Non-string
map keys are ordinary in YAML (on/off/yes/no, port numbers, numeric enum keys).

Root cause

The overridable set is populated from keys(), which for the object map is
Object.keys() — always strings ("10"). The duplicate check then queries
that set with the resolved key (the number 10):

if (!state.json && frame.tag.has(frame.value, key) && !frame.overridable?.has(key)) {
  throwError(state, 'duplicated mapping key')
}

frame.tag.has coerces (String(key)) so it correctly sees the duplicate, but
Set.has uses SameValueZero, so "10" !== 10 — the override exemption misses
and the pair is rejected. realMapTag/setTag store keys losslessly, so they
were never affected, which pinpoints the object-map string coercion as the cause.

Fix

Object-based maps already normalize keys to their string form in
addPair/has/get. Give a mapping tag a normalizeKey (defaulting to
identity, so Map/Set tags are unchanged) and key the overridable set through
it, so both sides use the same key identity.

• Genuine duplicate keys without a merge still raise ({10: x, 10: y} throws).
• String-key overrides are unchanged.

Test

Added a tags/merge case where a merged 10:/true: is overridden by a local
pair. Fails on main (duplicated mapping key), passes with the fix. Full
test/core suite green (284 pass / 0 fail), type-check and lint clean.

[puzrin]

I understand the problem, but not sure about implementation.

The enforced key stringification for JS objects is language-specific behavior, not a "normal" one. So:

1. Introducing a "normalize" method in the collection tag is very confusing.
2. This is a specific tag-level defect. It would be nice to avoid spreading it into the upper level, if possible.

IMHO, worth discussing alternatives in the issue first.

Also, note about the topic name - it's about scalar keys only. Non-scalar keys (complex objects) are usually not supported at all (PyYAML, Go) or are potentially vulnerable to DoS attacks.

[spokodev]

Yeah, that's fair. And you're right it's only scalar keys — the map's addPair already rejects complex ones.

The reason it ends up in the constructor is that the overridable set lives there, not in the tag. For the plain-object map a merged key comes back from keys() as "10", but the explicit override key is the resolved number 10, and they only line up once you apply the String() coercion the map already does internally. So the comparison has to go through the tag's idea of key identity somewhere — hard-coding String() up in the constructor would be the real leak you want to avoid.

I don't think there's a version that needs zero tag support: either a small opt-in method on the map tags (identity elsewhere), or tracking overridable through the tag's own has/get instead of a raw Set — though that second one gets awkward for !!set. Given it's more than a one-liner either way, I'm happy to open an issue and settle on the shape there first, like you suggested.

[puzrin]

Yes, create an issue with your vision on "how {}-based map should work and why". Prior to make any decision, I'd like to understand the whole situation.

1. Use cases when the user needs non-string scalar keys. Is this really required for all or a rare/specific need? How frequent? And so on...
2. {}-based maps are defective by nature (mutate any key kinds to strings). Is this a "bug or feature" :)? Should we continue adding kludges or advise an alternative?
   • I don't know if scalar non-string keys can be considered as really working with {} mappings. The user defines a number in the key but gets a string. That's not nice. Is it really needed to enhance features or just say "only string keys are normally supported?
   • We can just advise the realMap tag for full-featured key support (it should be ok with any key merge now).
   • Existing parser uses "fast path"; it translates events to JS in a single step. We can suggest a more formal alternative: translate events to AST, then construct JS. Current architecture allows building such chains

IMHO, globally, the question is not "how to make this work", but "how much the user agrees to pay for extra features" (performance, convenience, and so on). I don't think package features should be code-driven instead of user needs. But IMHO, continuing to hack objects may be not the best/optimal approach of available.

Personally, I'd mark Object-based maps as "restricted". But that depends on use cases; I don't know the user needs for YAML.

[spokodev]

Opened #778 with the full writeup — the concrete case, the two questions (use cases / bug-or-feature), what realMapTag already does with merge, the prior art (#329, #538), and where I'd land. Let's settle the direction there.

[puzrin]

Let's close this; changes to merge "as is" are low. This is referred to from an open issue, so any info will not be lost.