From f5d83fe0111530e88da13df25b3ff4bfe531d3a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ro=C5=BCek?= Date: Tue, 17 Sep 2019 09:50:54 +0200 Subject: [PATCH] fix: do not report duplicate keys errors for keys added by merge keys (#25) * fix: do not report duplicate keys errors for keys added by merge keys * test: extra override scenario --- .../merge-keys-with-duplicate-props.yaml | 36 ++++++++++ src/__tests__/parseWithPointers.spec.ts | 72 +++++++++++++++++++ src/parseWithPointers.ts | 27 ++++--- 3 files changed, 126 insertions(+), 9 deletions(-) create mode 100644 src/__tests__/fixtures/merge-keys-with-duplicate-props.yaml diff --git a/src/__tests__/fixtures/merge-keys-with-duplicate-props.yaml b/src/__tests__/fixtures/merge-keys-with-duplicate-props.yaml new file mode 100644 index 0000000..0a27e11 --- /dev/null +++ b/src/__tests__/fixtures/merge-keys-with-duplicate-props.yaml @@ -0,0 +1,36 @@ +openapi: 3.0.0 + +x-format-version: "1.0" + +info: + title: Merge key issue + description: https://yaml.org/type/merge.html + version: 1.0.0 + +x-center: &CENTER + x: 1 + y: 2 + +x-left: &LEFT + x: 0 + y: 2 + +x-big: &BIG + r: 10 + +x-small: &SMALL + r: 1 + +x-one: + <<: *CENTER + r: 10 + label: center/big +x-two: + <<: [*CENTER, *BIG] + label: center/big +x-three: + <<: [*BIG, *LEFT, *SMALL] + x: 1 + label: center/big + +paths: {} diff --git a/src/__tests__/parseWithPointers.spec.ts b/src/__tests__/parseWithPointers.spec.ts index 3fe80cd..c1f0a67 100644 --- a/src/__tests__/parseWithPointers.spec.ts +++ b/src/__tests__/parseWithPointers.spec.ts @@ -7,6 +7,10 @@ import { HugeYAML } from './fixtures/huge-yaml'; const diverse = fs.readFileSync(path.join(__dirname, './fixtures/diverse.yaml'), 'utf-8'); const duplicateMergeKeys = fs.readFileSync(path.join(__dirname, './fixtures/duplicate-merge-keys.yaml'), 'utf-8'); +const mergeKeysWithDuplicateProperties = fs.readFileSync( + path.join(__dirname, './fixtures/merge-keys-with-duplicate-props.yaml'), + 'utf-8' +); const spectral481 = fs.readFileSync(path.join(__dirname, './fixtures/spectral-481.yaml'), 'utf-8'); describe('yaml parser', () => { @@ -428,6 +432,56 @@ european-cities: &cities }); }); + test('handles overrides #2', () => { + const result = parseWithPointers(mergeKeysWithDuplicateProperties, { + mergeKeys: true, + ignoreDuplicateKeys: false, + }); + + expect(result.data).toEqual({ + openapi: '3.0.0', + 'x-format-version': '1.0', + info: { + description: 'https://yaml.org/type/merge.html', + title: 'Merge key issue', + version: '1.0.0', + }, + 'x-center': { + x: 1, + y: 2, + }, + 'x-left': { + x: 0, + y: 2, + }, + 'x-big': { + r: 10, + }, + 'x-small': { + r: 1, + }, + 'x-one': { + x: 1, + y: 2, + r: 10, + label: 'center/big', + }, + 'x-two': { + x: 1, + y: 2, + r: 10, + label: 'center/big', + }, + 'x-three': { + x: 1, + y: 2, + r: 10, + label: 'center/big', + }, + paths: {}, + }); + }); + test('handles duplicate merge keys', () => { const result = parseWithPointers(duplicateMergeKeys, { mergeKeys: true }); @@ -440,5 +494,23 @@ european-cities: &cities t: 4, }); }); + + test('does not report duplicate merge keys', () => { + const result = parseWithPointers(duplicateMergeKeys, { + mergeKeys: true, + ignoreDuplicateKeys: false, + }); + + expect(result.diagnostics).toEqual([]); + }); + + test('does not report duplicate errors for merged keys', () => { + const result = parseWithPointers(mergeKeysWithDuplicateProperties, { + mergeKeys: true, + ignoreDuplicateKeys: false, + }); + + expect(result.diagnostics).toEqual([]); + }); }); }); diff --git a/src/parseWithPointers.ts b/src/parseWithPointers.ts index 657fb7d..17c81c7 100644 --- a/src/parseWithPointers.ts +++ b/src/parseWithPointers.ts @@ -73,20 +73,29 @@ export const walkAST = ( case Kind.MAP: { const container = {}; // note, we don't handle null aka '~' keys on purpose - for (const mapping of (node as YamlMap).mappings) { - // typing is broken, value might be null - if (mapping.key.value in container) { - if (options !== void 0 && options.json === false) { - throw new Error('Duplicate YAML mapping key encountered'); - } + const seenKeys: string[] = []; + const handleMergeKeys = options !== void 0 && options.mergeKeys === true; + const handleDuplicates = (options !== void 0 && options.json === false) || duplicatedMappingKeys !== void 0; - if (duplicatedMappingKeys !== void 0) { - duplicatedMappingKeys.push(mapping.key); + for (const mapping of (node as YamlMap).mappings) { + const key = mapping.key.value; + + if (handleDuplicates && (!handleMergeKeys || key !== SpecialMappingKeys.MergeKey)) { + if (seenKeys.includes(mapping.key.value)) { + if (options !== void 0 && options.json === false) { + throw new Error('Duplicate YAML mapping key encountered'); + } + + if (duplicatedMappingKeys !== void 0) { + duplicatedMappingKeys.push(mapping.key); + } + } else { + seenKeys.push(key); } } // https://yaml.org/type/merge.html merge keys, not a part of YAML spec - if (options !== void 0 && options.mergeKeys === true && mapping.key.value === SpecialMappingKeys.MergeKey) { + if (handleMergeKeys && key === SpecialMappingKeys.MergeKey) { Object.assign(container, reduceMergeKeys(walkAST(mapping.value, options, duplicatedMappingKeys))); } else { container[mapping.key.value] = walkAST(mapping.value, options, duplicatedMappingKeys);