feat: Check key uniqueness; add uniqueKeys option (#271)

Author: eemeli

docs/03_options.md

@@ -22,12 +22,13 @@ Parse options affect the parsing and composition of a YAML Document from it sour
 
 Used by: `parse()`, `parseDocument()`, `parseAllDocuments()`, `new Composer()`, and `new Document()`
 
-| Name         | Type          | Default | Description                                                                                                                             |
-| ------------ | ------------- | ------- | --------------------------------------------------------------------------------------------------------------------------------------- |
-| intAsBigInt  | `boolean`     | `false` | Whether integers should be parsed into [BigInt] rather than `number` values.                                                            |
-| lineCounter  | `LineCounter` |         | If set, newlines will be tracked, to allow for `lineCounter.linePos(offset)` to provide the `{ line, col }` positions within the input. |
-| prettyErrors | `boolean`     | `true`  | Include line/col position in errors, along with an extract of the source string.                                                        |
-| strict       | `boolean`     | `true`  | When parsing, do not ignore errors required by the YAML 1.2 spec, but caused by unambiguous content.                                    |
+| Name         | Type                          | Default | Description                                                                                                                                                                |
+| ------------ | ----------------------------- | ------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| intAsBigInt  | `boolean`                     | `false` | Whether integers should be parsed into [BigInt] rather than `number` values.                                                                                               |
+| lineCounter  | `LineCounter`                 |         | If set, newlines will be tracked, to allow for `lineCounter.linePos(offset)` to provide the `{ line, col }` positions within the input.                                    |
+| prettyErrors | `boolean`                     | `true`  | Include line/col position in errors, along with an extract of the source string.                                                                                           |
+| strict       | `boolean`                     | `true`  | When parsing, do not ignore errors required by the YAML 1.2 spec, but caused by unambiguous content.                                                                       |
+| uniqueKeys   | `boolean ⎮ (a, b) => boolean` | `true`  | Whether key uniqueness is checked, or customised. If set to be a function, it will be passed two parsed nodes and should return a boolean value indicating their equality. |
 
 [bigint]: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/BigInt
 

docs/08_errors.md

@@ -33,6 +33,7 @@ To identify errors for special handling, you should primarily use `code` to diff
 | `BLOCK_AS_IMPLICIT_KEY`  | There's probably something wrong with the indentation, or you're trying to parse something like `a: b: c`, where it's not clear what's the key and what's the value.         |
 | `BLOCK_IN_FLOW`          | YAML scalars and collections both have block and flow styles. Flow is allowed within block, but not the other way around.                                                    |
 | `COMMENT_SPACE`          | Comments need to be separated from their preceding content by a space.                                                                                                       |
+| `DUPLICATE_KEY`          | Map keys must be unique. Use the `uniqueKeys` option to disable or customise this check when parsing.                                                                        |
 | `IMPOSSIBLE`             | This really should not happen. If you encounter this error code, please file a bug.                                                                                          |
 | `KEY_OVER_1024_CHARS`    | Due to legacy reasons, implicit keys must have their following `:` indicator after at most 1k characters.                                                                    |
 | `MISSING_ANCHOR`         | Aliases can only dereference anchors that are before them in the document.                                                                                                   |

src/compose/resolve-block-map.ts

@@ -1,10 +1,12 @@
+import type { ParsedNode } from '../nodes/Node.js'
 import { Pair } from '../nodes/Pair.js'
 import { YAMLMap } from '../nodes/YAMLMap.js'
 import type { BlockMap, Token } from '../parse/cst.js'
 import type { ComposeContext, ComposeNode } from './compose-node.js'
 import type { ComposeErrorHandler } from './composer.js'
 import { resolveProps } from './resolve-props.js'
 import { containsNewline } from './util-contains-newline.js'
+import { mapIncludes } from './util-map-includes.js'
 
 const startColMsg = 'All mapping items must start at the same column'
 
@@ -14,7 +16,7 @@ export function resolveBlockMap(
   bm: BlockMap,
   onError: ComposeErrorHandler
 ) {
-  const map = new YAMLMap(ctx.schema)
+  const map = new YAMLMap<ParsedNode, ParsedNode>(ctx.schema)
 
   let offset = bm.offset
   for (const { start, key, sep, value } of bm.items) {
@@ -61,6 +63,9 @@ export function resolveBlockMap(
       ? composeNode(ctx, key, keyProps, onError)
       : composeEmptyNode(ctx, keyStart, start, null, keyProps, onError)
 
+    if (mapIncludes(ctx, map.items, keyNode))
+      onError(keyStart, 'DUPLICATE_KEY', 'Map keys must be unique')
+
     // value properties
     const valueProps = resolveProps(sep || [], {
       ctx,

src/compose/resolve-flow-collection.ts

@@ -8,6 +8,7 @@ import type { ComposeErrorHandler } from './composer.js'
 import { resolveEnd } from './resolve-end.js'
 import { resolveProps } from './resolve-props.js'
 import { containsNewline } from './util-contains-newline.js'
+import { mapIncludes } from './util-map-includes.js'
 
 const blockMsg = 'Block collections are not allowed within flow collections'
 const isBlock = (token: Token | null | undefined) =>
@@ -169,8 +170,12 @@ export function resolveFlowCollection(
       }
 
       const pair = new Pair(keyNode, valueNode)
-      if (isMap) (coll as YAMLMap.Parsed).items.push(pair)
-      else {
+      if (isMap) {
+        const map = coll as YAMLMap.Parsed
+        if (mapIncludes(ctx, map.items, keyNode))
+          onError(keyStart, 'DUPLICATE_KEY', 'Map keys must be unique')
+        map.items.push(pair)
+      } else {
         const map = new YAMLMap(ctx.schema)
         map.flow = true
         map.items.push(pair)

src/compose/util-map-includes.ts

@@ -0,0 +1,22 @@
+import { isScalar, ParsedNode } from '../nodes/Node'
+import { Pair } from '../nodes/Pair'
+import { ComposeContext } from './compose-node'
+
+export function mapIncludes(
+  ctx: ComposeContext,
+  items: Pair<ParsedNode>[],
+  search: ParsedNode
+) {
+  const { uniqueKeys } = ctx.options
+  if (uniqueKeys === false) return false
+  const isEqual =
+    typeof uniqueKeys === 'function'
+      ? uniqueKeys
+      : (a: ParsedNode, b: ParsedNode) =>
+          a === b ||
+          (isScalar(a) &&
+            isScalar(b) &&
+            a.value === b.value &&
+            !(a.value === '<<' && ctx.schema.merge))
+  return items.some(pair => isEqual(pair.key, search))
+}

src/errors.ts

@@ -10,6 +10,7 @@ export type ErrorCode =
   | 'BLOCK_AS_IMPLICIT_KEY'
   | 'BLOCK_IN_FLOW'
   | 'COMMENT_SPACE'
+  | 'DUPLICATE_KEY'
   | 'IMPOSSIBLE'
   | 'KEY_OVER_1024_CHARS'
   | 'MISSING_ANCHOR'

src/options.ts

@@ -1,6 +1,7 @@
 import type { Reviver } from './doc/applyReviver.js'
 import type { Directives } from './doc/directives.js'
 import type { LogLevelId } from './log.js'
+import type { ParsedNode } from './nodes/Node.js'
 import type { Pair } from './nodes/Pair.js'
 import type { Scalar } from './nodes/Scalar.js'
 import type { LineCounter } from './parse/line-counter.js'
@@ -38,6 +39,20 @@ export type ParseOptions = {
    * Default: `true`
    */
   strict?: boolean
+
+  /**
+   * YAML requires map keys to be unique. By default, this is checked by
+   * comparing scalar values with `===`; deep equality is not checked for
+   * aliases or collections. If merge keys are enabled by the schema,
+   * multiple `<<` keys are allowed.
+   *
+   * Set `false` to disable, or provide your own comparator function to
+   * customise. The comparator will be passed two `ParsedNode` values, and
+   * is expected to return a `boolean` indicating their equality.
+   *
+   * Default: `true`
+   */
+  uniqueKeys?: boolean | ((a: ParsedNode, b: ParsedNode) => boolean)
 }
 
 export type DocumentOptions = {
@@ -305,5 +320,6 @@ export const defaultOptions: Required<
   logLevel: 'warn',
   prettyErrors: true,
   strict: true,
+  uniqueKeys: true,
   version: '1.2'
 }

tests/doc/parse.js

@@ -612,6 +612,40 @@ test('Anchor for empty node (6KGN)', () => {
   expect(YAML.parse(src)).toMatchObject({ a: null, b: null })
 })
 
+describe('duplicate keys', () => {
+  test('block collection scalars', () => {
+    const doc = YAML.parseDocument('foo: 1\nbar: 2\nfoo: 3\n')
+    expect(doc.errors).toMatchObject([{ code: 'DUPLICATE_KEY' }])
+  })
+
+  test('flow collection scalars', () => {
+    const doc = YAML.parseDocument('{ foo: 1, bar: 2, foo: 3 }\n')
+    expect(doc.errors).toMatchObject([{ code: 'DUPLICATE_KEY' }])
+  })
+
+  test('merge key (disabled)', () => {
+    const doc = YAML.parseDocument('<<: 1\nbar: 2\n<<: 3\n', { merge: false })
+    expect(doc.errors).toMatchObject([{ code: 'DUPLICATE_KEY' }])
+  })
+
+  test('disable with option', () => {
+    const doc = YAML.parseDocument('foo: 1\nbar: 2\nfoo: 3\n', {
+      uniqueKeys: false
+    })
+    expect(doc.errors).toMatchObject([])
+  })
+
+  test('customise with option', () => {
+    const doc = YAML.parseDocument('foo: 1\nbar: 2\nfoo: 3\n', {
+      uniqueKeys: () => true
+    })
+    expect(doc.errors).toMatchObject([
+      { code: 'DUPLICATE_KEY' },
+      { code: 'DUPLICATE_KEY' }
+    ])
+  })
+})
+
 describe('handling complex keys', () => {
   let origEmitWarning
   beforeAll(() => {