fix: values have wrong end positions (#4)

* fix: wrong nodes endPosition

* fix: keyNode is undefined for invalid mappings

src/loader.ts

@@ -408,8 +408,11 @@ function storeMappingPair(
   mapping.parent = _result;
   keyNode.parent = mapping;
 
-  if (valueNode != null) {
-      valueNode.parent = mapping;
+  if (valueNode == null) {
+    // due to shifting in `ast.newMapping` function
+    mapping.endPosition = keyNode.endPosition;
+  } else {
+    valueNode.parent = mapping;
   }
 
   !state.ignoreDuplicateKeys && _result.mappings.forEach(sibling => {
@@ -495,10 +498,6 @@ function skipSeparationSpace(state:State, allowComments, nodeIndent, toEndBlock
     }
 
     if (is_EOL(ch)) {
-      if (toEndBlock && -1 !== nodeIndent && state.lineIndent < nodeIndent) {
-        state.position -= state.lineIndent;
-        break;
-      }
       readLineBreak(state);
 
       ch = state.input.charCodeAt(state.position);
@@ -664,6 +663,7 @@ function readPlainScalar(state:State, nodeIndent, withinFlowCollection) {
 
   if (state.result.startPosition!=-1) {
     state_result.rawValue = state.input.substring(state_result.startPosition, state_result.endPosition);
+    state.result.endPosition = skipSeparationSpace(state, false, -1) ? state.position - 1 : state.position;
     return true;
   }
 
@@ -1110,6 +1110,7 @@ function readBlockSequence(state:State, nodeIndent) {
     state.position++;
 
     if (skipSeparationSpace(state, true, -1)) {
+      // have line breaks
       if (state.lineIndent <= nodeIndent) {
         _result.items.push(null);
         ch = state.input.charCodeAt(state.position);
@@ -1124,8 +1125,7 @@ function readBlockSequence(state:State, nodeIndent) {
       _result.items.push(state.result);
     }
 
-    skipSeparationSpace(state, true, nodeIndent, true);
-    _result.endPosition = state.position - 1;
+    _result.endPosition = skipSeparationSpace(state, true, nodeIndent, true) ? state.position - 1 : state.position;
 
     ch = state.input.charCodeAt(state.position);
 
@@ -1135,14 +1135,15 @@ function readBlockSequence(state:State, nodeIndent) {
       break;
     }
   }
+
   if (detected) {
     state.tag = _tag;
     state.anchor = _anchor;
     state.kind = 'sequence';
     state.result = _result;
     return true;
   }
-  _result.endPosition=state.position
+
   return false;
 }
 
@@ -1273,21 +1274,29 @@ function readBlockMapping(state:State, nodeIndent, flowIndent) {
     // Common reading code for both explicit and implicit notations.
     //
     if (state.line === _line || state.lineIndent > nodeIndent) {
+      // mapping value on the same line or inside the node
+      const lineBeforeValue = state.line;
+
       if (composeNode(state, nodeIndent, CONTEXT_BLOCK_OUT, true, allowCompact)) {
-        if (state.lineIndent && nodeIndent && state.lineIndent >= nodeIndent) {
-          skipSeparationSpace(state, true, nodeIndent, true);
-          state.result.endPosition = state.position - 1;
-        }
+        skipSeparationSpace(state, true, nodeIndent);
+
         if (atExplicitKey) {
           keyNode = state.result;
         } else {
           valueNode = state.result;
+          // `skipSeparationSpace` and `composeNode` could change `state.line` so check again if we are still on the same line
+          valueNode.endPosition = state.line === lineBeforeValue ? state.position : state.position - 1;
+        }
+      } else {
+        // we dont have a value yet
+        if (keyNode) {
+          // we dont have a keyNode when yaml is invalid (e.g. when colon is missed after field)
+          keyNode.endPosition = state.line === lineBeforeValue ? state.position : state.position - 1;
         }
       }
 
+
       if (!atExplicitKey) {
-        if (keyNode?.endPosition) keyNode.endPosition = state.position - 1;
-        if (valueNode?.endPosition) valueNode.endPosition = state.position - 1;
         storeMappingPair(state, _result, keyTag, keyNode, valueNode, nodeIndent);
         keyTag = keyNode = valueNode = null;
       }

test/loader.test.ts

@@ -87,7 +87,7 @@ foo: bar`;
   test('should not contain spaces', function () {
     const input = `tags:
   - email
- 
+  
  
  
  
@@ -103,9 +103,13 @@ foo: bar`;
                     YAML.newScalar('tags'),
                     seq)]);
 
+    const docSeq = <YAML.YAMLSequence>doc.mappings[0].value;
+
     assert.equal(doc.endPosition, input.length);
     assert.equal(doc.mappings[0].startPosition, 0);
-    assert.equal(doc.mappings[0].endPosition, 15);
+    assert.equal(docSeq.items[0].endPosition, input.length);
+    assert.equal(docSeq.endPosition, input.length);
+    assert.equal(doc.mappings[0].endPosition, input.length);
 
     assert.deepEqual(structure(doc), expected_structure)
     assert.lengthOf(doc.errors, 0)
@@ -142,11 +146,152 @@ newTags:
     assert.equal(doc.mappings[0].endPosition, seq.length);
     assert.equal(doc.mappings[1].endPosition, input.length);
     assert.isTrue(doc.mappings[0].endPosition < doc.mappings[1].startPosition);
+
+    assert.lengthOf(doc.errors, 0);
+  });
+
+  test('mapping values must end 1 character before next mapping starts', () => {
+    const input = `
+openapi: 3.1.0
+servers: 
+  - url:  anything
+    description: some description
+  - url: //petstore.swagger.io/sandbox
+    description: Sandbox server
+    variables: 
+      varName: default
+`;
+    const doc = YAML.safeLoad(input) as YamlMap;
+    const [openApiMapping, serversMapping] = doc.mappings;
+    const docSeq = <YAML.YAMLSequence>doc.mappings[1].value;
+    const [firstMap, secondMap] = <YAML.YamlMap[]>docSeq.items;
+
+    assert.equal(doc.endPosition, input.length);
+
+    assert.equal(openApiMapping.startPosition, 1);
+    assert.equal(openApiMapping.endPosition, 15);
+
+    assert.equal(serversMapping.startPosition, 16);
+    assert.equal(serversMapping.endPosition, input.length - 1);
+
+    assert.equal(firstMap.mappings[0].endPosition, 48);
+    assert.equal(firstMap.mappings[1].endPosition, 80);
+
+    assert.equal(secondMap.mappings[0].endPosition, 121);
+    assert.equal(secondMap.mappings[1].endPosition, 153);
+
+    assert.equal(secondMap.mappings[2].startPosition, 154);
+    assert.equal(secondMap.mappings[2].endPosition, input.length - 1);
+
+    const variablesMap = <YAML.YamlMap>secondMap.mappings[2].value;
+
+    assert.equal(variablesMap.startPosition, 172);
+    assert.equal(variablesMap.mappings[0].startPosition, 172);
+    assert.equal(variablesMap.mappings[0].endPosition, input.length - 1);
+  });
+
+  test('mapping with multiline value must end at the end of document', () => {
+    const input = `
+mapping: |
+  some 
+  multiline value
+  ends
+  here`;
+
+    const doc = YAML.safeLoad(input) as YamlMap;
+    const [mapping] = doc.mappings;
+
+    assert.equal(mapping.endPosition, input.length);
+    assert.equal(mapping.value.endPosition, input.length);
+  });
+
+  test('every scalar value in sequence must end 1 character before next sequence item starts', () => {
+    const input = `
+enum:
+  - clueless
+  - lazy
+  - adventurous
+  - aggressive
+  - |
+    multiline
+    string
+    should work
+`;
+        const doc = YAML.safeLoad(input) as YamlMap;
+        const [enumMapping] = doc.mappings;
+        const enumSeq = <YAML.YAMLSequence>enumMapping.value;
+        const [clueless, lazy, adventurous, aggressive, multiline] = <YAML.YamlMap[]>enumSeq.items;
+
+        assert.equal(doc.endPosition, input.length);
+
+        assert.equal(clueless.startPosition, 11);
+        assert.equal(clueless.endPosition, 21);
+
+        assert.equal(lazy.startPosition, 24);
+        assert.equal(lazy.endPosition, 30);
+
+        assert.equal(adventurous.startPosition, 33);
+        assert.equal(adventurous.endPosition, 46);
+
+        assert.equal(aggressive.startPosition, 49);
+        assert.equal(aggressive.endPosition, 61);
+
+        assert.equal(multiline.startPosition, 64);
+        assert.equal(multiline.endPosition, 106);
+  });
+
+  test('empty mapping value should end where next mapping starts', () => {
+    const input = `
+openapi: 3.1.0
+servers: 
+  - url:     
+    description: some description
+`;
+
+    const doc = YAML.safeLoad(input) as YamlMap;
+    const [, serversMapping] = doc.mappings;
+    const docSeq = <YAML.YAMLSequence>doc.mappings[1].value;
+    const [firstMap] = <YAML.YamlMap[]>docSeq.items;
+
+    assert.equal(doc.endPosition, input.length);
+
+    assert.equal(serversMapping.endPosition, input.length - 1);
+
+    assert.equal(firstMap.mappings[0].startPosition, 30);
+    assert.equal(firstMap.mappings[0].endPosition, 43);
+
+    assert.equal(firstMap.mappings[1].startPosition, 44);
+    assert.equal(firstMap.mappings[1].endPosition, 73);
 
     assert.lengthOf(doc.errors, 0)
   });
 });
 
+test.only('should not throw when multiline invalid mapping key', function () {
+  const input = `
+test: 
+  - firstlevel: val
+    sdsds: v
+    %%%
+  - second: val
+responses:
+  '405':
+    description: Invalid input
+security:
+  `;
+
+  let err;
+
+  try {
+    const doc = YAML.safeLoad(input) as YamlMap
+    console.log(doc);
+  } catch (e) {
+    err = e;
+  }
+
+  assert.equal(err, undefined);  
+});
+
 suite('Loading multiple documents', () => {
     test('should work with document-end delimiters', function () {
         const docs = []