Better errors for bad indents (#169)

* Add better errors for broken collections

* Improve error handling for trailing content

src/cst/Collection.js

@@ -1,4 +1,5 @@
 import { Type } from '../constants.js'
+import { YAMLSyntaxError } from '../errors.js'
 import { BlankLine } from './BlankLine.js'
 import { CollectionItem } from './CollectionItem.js'
 import { Comment } from './Comment.js'
@@ -138,24 +139,30 @@ export class Collection extends Node {
         break
       }
       if (offset !== lineStart + indent && (atLineStart || ch !== ':')) {
-        trace: 'end:unindent',
-          { offset, lineStart, indent, ch: JSON.stringify(ch) }
-        if (lineStart > start) offset = lineStart
-        break
-      }
-      if ((firstItem.type === Type.SEQ_ITEM) !== (ch === '-')) {
-        let typeswitch = true
-        if (ch === '-') {
-          // map key may start with -, as long as it's followed by a non-whitespace char
-          const next = src[offset + 1]
-          typeswitch = !next || next === '\n' || next === '\t' || next === ' '
+        if (offset < lineStart + indent) {
+          trace: 'end:unindent',
+            { offset, lineStart, indent, ch: JSON.stringify(ch) }
+          if (lineStart > start) offset = lineStart
+          break
+        } else if (!this.error) {
+          const msg = 'All collection items must start at the same column'
+          this.error = new YAMLSyntaxError(this, msg)
         }
-        if (typeswitch) {
+      }
+      if (firstItem.type === Type.SEQ_ITEM) {
+        if (ch !== '-') {
           trace: 'end:typeswitch',
             { offset, lineStart, indent, ch: JSON.stringify(ch) }
           if (lineStart > start) offset = lineStart
           break
         }
+      } else if (ch === '-' && !this.error) {
+        // map key may start with -, as long as it's followed by a non-whitespace char
+        const next = src[offset + 1]
+        if (!next || next === '\n' || next === '\t' || next === ' ') {
+          const msg = 'A collection cannot be both a mapping and a sequence'
+          this.error = new YAMLSyntaxError(this, msg)
+        }
       }
       trace: 'item-start', this.items.length, { ch: JSON.stringify(ch) }
       const node = parseNode(

src/doc/parseContents.js

@@ -5,27 +5,29 @@ import { resolveNode } from '../resolve/resolveNode.js'
 
 export function parseContents(doc, contents) {
   const comments = { before: [], after: [] }
-  const contentNodes = []
+  let body = undefined
   let spaceBefore = false
   for (const node of contents) {
     if (node.valueRange) {
-      if (contentNodes.length === 1) {
-        const msg = 'Document is not valid YAML (bad indentation?)'
+      if (body !== undefined) {
+        const msg =
+          'Document contains trailing content not separated by a ... or --- line'
         doc.errors.push(new YAMLSyntaxError(node, msg))
+        break
       }
       const res = resolveNode(doc, node)
       if (spaceBefore) {
         res.spaceBefore = true
         spaceBefore = false
       }
-      contentNodes.push(res)
+      body = res
     } else if (node.comment !== null) {
-      const cc = contentNodes.length === 0 ? comments.before : comments.after
+      const cc = body === undefined ? comments.before : comments.after
       cc.push(node.comment)
     } else if (node.type === Type.BLANK_LINE) {
       spaceBefore = true
       if (
-        contentNodes.length === 0 &&
+        body === undefined &&
         comments.before.length > 0 &&
         !doc.commentBefore
       ) {
@@ -36,39 +38,18 @@ export function parseContents(doc, contents) {
     }
   }
 
-  switch (contentNodes.length) {
-    // empty document
-    case 0:
-      doc.contents = null
-      comments.after = comments.before
-      break
-
-    case 1:
-      doc.contents = contentNodes[0]
-      if (doc.contents) {
-        const cb = comments.before.join('\n') || null
-        if (cb) {
-          const cbNode =
-            doc.contents instanceof Collection && doc.contents.items[0]
-              ? doc.contents.items[0]
-              : doc.contents
-          cbNode.commentBefore = cbNode.commentBefore
-            ? `${cb}\n${cbNode.commentBefore}`
-            : cb
-        }
-      } else {
-        comments.after = comments.before.concat(comments.after)
-      }
-      break
-
-    // invalid source
-    default:
-      doc.contents = contentNodes
-      if (doc.contents[0]) {
-        doc.contents[0].commentBefore = comments.before.join('\n') || null
-      } else {
-        comments.after = comments.before.concat(comments.after)
-      }
+  doc.contents = body || null
+  if (!body) {
+    doc.comment = comments.before.concat(comments.after).join('\n') || null
+  } else {
+    const cb = comments.before.join('\n')
+    if (cb) {
+      const cbNode =
+        body instanceof Collection && body.items[0] ? body.items[0] : body
+      cbNode.commentBefore = cbNode.commentBefore
+        ? `${cb}\n${cbNode.commentBefore}`
+        : cb
+    }
+    doc.comment = comments.after.join('\n') || null
   }
-  doc.comment = comments.after.join('\n') || null
 }

tests/doc/errors.js

@@ -77,6 +77,52 @@ describe('eemeli/yaml#7', () => {
   })
 })
 
+describe('block collections', () => {
+  test('mapping with bad indentation', () => {
+    const src = 'foo: "1"\n bar: 2\n'
+    const doc = YAML.parseDocument(src)
+    expect(doc.errors).toMatchObject([
+      { message: 'All collection items must start at the same column' }
+    ])
+    expect(doc.contents).toMatchObject({
+      type: 'MAP',
+      items: [
+        { key: { value: 'foo' }, value: { value: '1' } },
+        { key: { value: 'bar' }, value: { value: 2 } }
+      ]
+    })
+  })
+
+  test('sequence with bad indentation', () => {
+    const src = '- "foo"\n - bar\n'
+    const doc = YAML.parseDocument(src)
+    expect(doc.errors).toMatchObject([
+      { message: 'All collection items must start at the same column' }
+    ])
+    expect(doc.contents).toMatchObject({
+      type: 'SEQ',
+      items: [{ value: 'foo' }, { value: 'bar' }]
+    })
+  })
+
+  test('seq item in mapping', () => {
+    const src = 'foo: "1"\n- bar\n'
+    const doc = YAML.parseDocument(src)
+    expect(doc.errors).toMatchObject([
+      { message: 'A collection cannot be both a mapping and a sequence' },
+      { message: 'Failed to resolve SEQ_ITEM node here' },
+      { message: 'Implicit map keys need to be followed by map values' }
+    ])
+    expect(doc.contents).toMatchObject({
+      type: 'MAP',
+      items: [
+        { key: { value: 'foo' }, value: { value: '1' } },
+        { key: null, value: null }
+      ]
+    })
+  })
+})
+
 describe('missing flow collection terminator', () => {
   test('start only of flow map (eemeli/yaml#8)', () => {
     const doc = YAML.parseDocument('{', { prettyErrors: true })
@@ -197,10 +243,14 @@ describe('invalid options', () => {
 
 test('broken document with comment before first node', () => {
   const doc = YAML.parseDocument('#c\n*x\nfoo\n')
-  expect(doc.contents).toMatchObject([null, { type: 'PLAIN' }])
+  expect(doc.contents).toBeNull()
   expect(doc.errors).toMatchObject([
-    { name: 'YAMLReferenceError' },
-    { name: 'YAMLSyntaxError' }
+    { name: 'YAMLReferenceError', message: 'Aliased anchor not found: x' },
+    {
+      name: 'YAMLSyntaxError',
+      message:
+        'Document contains trailing content not separated by a ... or --- line'
+    }
   ])
 })