Skip to content

Commit

Permalink
fix: do not report duplicate keys errors for keys added by merge keys (
Browse files Browse the repository at this point in the history
…#25)

* fix: do not report duplicate keys errors for keys added by merge keys

* test: extra override scenario
  • Loading branch information
P0lip authored Sep 17, 2019
1 parent f90b45e commit f5d83fe
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 9 deletions.
36 changes: 36 additions & 0 deletions src/__tests__/fixtures/merge-keys-with-duplicate-props.yaml
Original file line number Diff line number Diff line change
@@ -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: {}
72 changes: 72 additions & 0 deletions src/__tests__/parseWithPointers.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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 });

Expand All @@ -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([]);
});
});
});
27 changes: 18 additions & 9 deletions src/parseWithPointers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit f5d83fe

Please sign in to comment.