From b1cc0e99bdf2fc5a25db06e663b2d7b2ed69b54f Mon Sep 17 00:00:00 2001 From: Alex Kocharin Date: Thu, 10 Dec 2020 20:39:03 +0300 Subject: [PATCH] Remember cursor position for 'duplicate mapping key' exception ref https://github.com/nodeca/js-yaml/issues/452 ref https://github.com/nodeca/js-yaml/issues/243 --- CHANGELOG.md | 1 + lib/loader.js | 48 ++++++++++++++++++++++++++------------ test/issues/0243.js | 56 ++++++++++++++++++++++++++++++--------------- 3 files changed, 73 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8becad8e..c9a4cf43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Astral characters are no longer encoded by dump/safeDump, #587. +- "duplicate mapping key" exception now points at the correct column, #452. - Removed `bower.json`. diff --git a/lib/loader.js b/lib/loader.js index 42eddd02..0ebb3929 100644 --- a/lib/loader.js +++ b/lib/loader.js @@ -286,7 +286,9 @@ function mergeMappings(state, destination, source, overridableKeys) { } } -function storeMappingPair(state, _result, overridableKeys, keyTag, keyNode, valueNode, startLine, startPos) { +function storeMappingPair(state, _result, overridableKeys, keyTag, keyNode, valueNode, + startLine, startLineStart, startPos) { + var index, quantity; // The output is a plain object here, so keys can only be strings. @@ -333,6 +335,7 @@ function storeMappingPair(state, _result, overridableKeys, keyTag, keyNode, valu !_hasOwnProperty.call(overridableKeys, keyNode) && _hasOwnProperty.call(_result, keyNode)) { state.line = startLine || state.line; + state.lineStart = startLineStart || state.lineStart; state.position = startPos || state.position; throwError(state, 'duplicated mapping key'); } @@ -670,6 +673,8 @@ function readDoubleQuotedScalar(state, nodeIndent) { function readFlowCollection(state, nodeIndent) { var readNext = true, _line, + _lineStart, + _pos, _tag = state.tag, _result, _anchor = state.anchor, @@ -733,7 +738,9 @@ function readFlowCollection(state, nodeIndent) { } } - _line = state.line; + _line = state.line; // Save the current line. + _lineStart = state.lineStart; + _pos = state.position; composeNode(state, nodeIndent, CONTEXT_FLOW_IN, false, true); keyTag = state.tag; keyNode = state.result; @@ -750,9 +757,9 @@ function readFlowCollection(state, nodeIndent) { } if (isMapping) { - storeMappingPair(state, _result, overridableKeys, keyTag, keyNode, valueNode); + storeMappingPair(state, _result, overridableKeys, keyTag, keyNode, valueNode, _line, _lineStart, _pos); } else if (isPair) { - _result.push(storeMappingPair(state, null, overridableKeys, keyTag, keyNode, valueNode)); + _result.push(storeMappingPair(state, null, overridableKeys, keyTag, keyNode, valueNode, _line, _lineStart, _pos)); } else { _result.push(keyNode); } @@ -981,7 +988,9 @@ function readBlockMapping(state, nodeIndent, flowIndent) { var following, allowCompact, _line, - _pos, + _keyLine, + _keyLineStart, + _keyPos, _tag = state.tag, _anchor = state.anchor, _result = {}, @@ -1002,7 +1011,6 @@ function readBlockMapping(state, nodeIndent, flowIndent) { while (ch !== 0) { following = state.input.charCodeAt(state.position + 1); _line = state.line; // Save the current line. - _pos = state.position; // // Explicit notation case. There are two separate blocks: @@ -1012,7 +1020,7 @@ function readBlockMapping(state, nodeIndent, flowIndent) { if (ch === 0x3F/* ? */) { if (atExplicitKey) { - storeMappingPair(state, _result, overridableKeys, keyTag, keyNode, null); + storeMappingPair(state, _result, overridableKeys, keyTag, keyNode, null, _keyLine, _keyLineStart, _keyPos); keyTag = keyNode = valueNode = null; } @@ -1035,7 +1043,16 @@ function readBlockMapping(state, nodeIndent, flowIndent) { // // Implicit notation case. Flow-style node as the key first, then ":", and the value. // - } else if (composeNode(state, flowIndent, CONTEXT_FLOW_OUT, false, true)) { + } else { + _keyLine = state.line; + _keyLineStart = state.lineStart; + _keyPos = state.position; + + if (!composeNode(state, flowIndent, CONTEXT_FLOW_OUT, false, true)) { + // Neither implicit nor explicit notation. + // Reading is done. Go to the epilogue. + break; + } if (state.line === _line) { ch = state.input.charCodeAt(state.position); @@ -1052,7 +1069,7 @@ function readBlockMapping(state, nodeIndent, flowIndent) { } if (atExplicitKey) { - storeMappingPair(state, _result, overridableKeys, keyTag, keyNode, null); + storeMappingPair(state, _result, overridableKeys, keyTag, keyNode, null, _keyLine, _keyLineStart, _keyPos); keyTag = keyNode = valueNode = null; } @@ -1079,15 +1096,18 @@ function readBlockMapping(state, nodeIndent, flowIndent) { state.anchor = _anchor; return true; // Keep the result of `composeNode`. } - - } else { - break; // Reading is done. Go to the epilogue. } // // Common reading code for both explicit and implicit notations. // if (state.line === _line || state.lineIndent > nodeIndent) { + if (atExplicitKey) { + _keyLine = state.line; + _keyLineStart = state.lineStart; + _keyPos = state.position; + } + if (composeNode(state, nodeIndent, CONTEXT_BLOCK_OUT, true, allowCompact)) { if (atExplicitKey) { keyNode = state.result; @@ -1097,7 +1117,7 @@ function readBlockMapping(state, nodeIndent, flowIndent) { } if (!atExplicitKey) { - storeMappingPair(state, _result, overridableKeys, keyTag, keyNode, valueNode, _line, _pos); + storeMappingPair(state, _result, overridableKeys, keyTag, keyNode, valueNode, _keyLine, _keyLineStart, _keyPos); keyTag = keyNode = valueNode = null; } @@ -1118,7 +1138,7 @@ function readBlockMapping(state, nodeIndent, flowIndent) { // Special case: last mapping's node contains only the key in explicit notation. if (atExplicitKey) { - storeMappingPair(state, _result, overridableKeys, keyTag, keyNode, null); + storeMappingPair(state, _result, overridableKeys, keyTag, keyNode, null, _keyLine, _keyLineStart, _keyPos); } // Expose the resulting mapping. diff --git a/test/issues/0243.js b/test/issues/0243.js index 52306e6b..2ff6fa49 100644 --- a/test/issues/0243.js +++ b/test/issues/0243.js @@ -6,24 +6,44 @@ var yaml = require('../../'); var readFileSync = require('fs').readFileSync; -it('Duplicated mapping key errors on top level throw at beginning of key', function () { - var src = readFileSync(require('path').join(__dirname, '/0243-basic.yml'), 'utf8'); - var lines = src.split('\n'); - - try { - yaml.load(src); - } catch (e) { - assert.strictEqual(lines[e.mark.line], 'duplicate: # 2'); - } -}); +describe('Duplicated mapping key errors throw at beginning of key', function () { + it('on top level', function () { + var src = readFileSync(require('path').join(__dirname, '/0243-basic.yml'), 'utf8'); + var lines = src.split('\n'); + + try { + yaml.load(src); + } catch (e) { + assert.strictEqual(lines[e.mark.line], 'duplicate: # 2'); + assert(e.message.match(/line 10, column 1/), e.message); + } + }); + + it('inside of mapping values', function () { + var src = readFileSync(require('path').join(__dirname, '/0243-nested.yml'), 'utf8'); + var lines = src.split('\n'); + + try { + yaml.load(src); + } catch (e) { + assert.strictEqual(lines[e.mark.line], ' duplicate: # 2'); + assert(e.message.match(/line 10, column 3/), e.message); + } + }); -it('Duplicated mapping key errors inside of mapping values throw at beginning of key', function () { - var src = readFileSync(require('path').join(__dirname, '/0243-nested.yml'), 'utf8'); - var lines = src.split('\n'); + it('inside flow collection', function () { + try { + yaml.load('{ foo: 123, foo: 456 }'); + } catch (e) { + assert(e.message.match(/line 1, column 13/), e.message); + } + }); - try { - yaml.load(src); - } catch (e) { - assert.strictEqual(lines[e.mark.line], ' duplicate: # 2'); - } + it('inside a set', function () { + try { + yaml.load(' ? foo\n ? foo\n ? bar'); + } catch (e) { + assert(e.message.match(/line 2, column 5/), e.message); + } + }); });