Skip to content

Commit

Permalink
Remember cursor position for 'duplicate mapping key' exception
Browse files Browse the repository at this point in the history 
ref #452
ref #243
  • Loading branch information
@rlidwka
rlidwka committed Dec 11, 2020
1 parent 421ed22 commit b1cc0e9
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 32 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`.


Expand Down
48 changes: 34 additions & 14 deletions lib/loader.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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');
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down Expand Up @@ -981,7 +988,9 @@ function readBlockMapping(state, nodeIndent, flowIndent) {
var following,
allowCompact,
_line,
_pos,
_keyLine,
_keyLineStart,
_keyPos,
_tag = state.tag,
_anchor = state.anchor,
_result = {},
Expand All @@ -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:
Expand All @@ -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;
}

Expand All @@ -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);
Expand All @@ -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;
}

Expand All @@ -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;
Expand All @@ -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;
}

Expand All @@ -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.
Expand Down
56 changes: 38 additions & 18 deletions test/issues/0243.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
});

0 comments on commit b1cc0e9

Please # to comment.