Empty collection items should only grab inline comments (#125, #126)

eemeli · Oct 6, 2019 · cc3e27d · cc3e27d
1 parent 2345db6
commit cc3e27d
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 25 deletions.
diff --git a/src/cst/CollectionItem.js b/src/cst/CollectionItem.js
@@ -32,6 +32,7 @@ export default class CollectionItem extends Node {
     const indent = atLineStart ? start - lineStart : context.indent
     let offset = Node.endOfWhiteSpace(src, start + 1)
     let ch = src[offset]
+    const inlineComment = ch === '#'
     const comments = []
     let blankLine = null
     while (ch === '\n' || ch === '#') {
@@ -67,22 +68,27 @@ export default class CollectionItem extends Node {
         { atLineStart, inCollection: false, indent, lineStart, parent: this },
         offset
       )
-      if (this.node) offset = this.node.range.end
     } else if (ch && lineStart > start + 1) {
       offset = lineStart - 1
     }
-    if (blankLine && !this.node) {
-      // Only blank lines preceding non-empty nodes are captured. Note that
-      // this means that collection item range start indices do not always
-      // increase monotonically. -- eemeli/yaml#126
-      offset = blankLine.range.end
-      blankLine = null
-    } else {
+    if (this.node) {
       if (blankLine) {
+        // Only blank lines preceding non-empty nodes are captured. Note that
+        // this means that collection item range start indices do not always
+        // increase monotonically. -- eemeli/yaml#126
         const items = context.parent.items || context.parent.contents
         if (items) items.push(blankLine)
       }
       if (comments.length) Array.prototype.push.apply(this.props, comments)
+      offset = this.node.range.end
+    } else {
+      if (inlineComment) {
+        const c = comments[0]
+        this.props.push(c)
+        offset = c.end
+      } else {
+        offset = Node.endOfLine(src, start + 1)
+      }
     }
     const end = this.node ? this.node.valueRange.end : offset
     trace: 'item-end', { start, end, offset }

diff --git a/tests/cst/corner-cases.js b/tests/cst/corner-cases.js
@@ -119,9 +119,16 @@ test('eemeli/yaml#10', () => {
 test('eemeli/yaml#19', () => {
   const src = 'a:\n  # 123'
   const doc = parse(src)[0]
-  const { items } = doc.contents[0]
-  expect(items).toHaveLength(2)
-  expect(items[1].comment).toBe(' 123')
+  expect(doc.contents).toMatchObject([
+    {
+      type: 'MAP',
+      items: [
+        { type: 'PLAIN', range: { start: 0, end: 1 } },
+        { type: 'MAP_VALUE', range: { start: 1, end: 2 } }
+      ]
+    },
+    { type: 'COMMENT', range: { start: 5, end: 10 } }
+  ])
 })
 
 test('eemeli/yaml#20', () => {
@@ -347,7 +354,7 @@ describe('blank lines before empty collection item value', () => {
     ])
   })
 
-  test('empty value with blank line after comment at document end', () => {
+  test('empty value with blank line after inline comment at document end', () => {
     const src = 'a: #c\n\n'
     const doc = parse(src)[0]
     expect(doc.contents).toMatchObject([
@@ -361,6 +368,21 @@ describe('blank lines before empty collection item value', () => {
     ])
   })
 
+  test('empty value with blank line after separate-line comment at document end', () => {
+    const src = 'a:\n#c\n\n'
+    const doc = parse(src)[0]
+    expect(doc.contents).toMatchObject([
+      {
+        type: 'MAP',
+        items: [
+          { type: 'PLAIN', props: [] },
+          { type: 'MAP_VALUE', node: null, props: [] }
+        ]
+      },
+      { type: 'COMMENT', range: { start: 3, end: 5 } }
+    ])
+  })
+
   test('empty value with blank line before & after comment at document end', () => {
     const src = 'a:\n\n#c\n\n'
     const doc = parse(src)[0]

diff --git a/tests/doc/parse.js b/tests/doc/parse.js
@@ -285,7 +285,7 @@ describe('eemeli/yaml#l19', () => {
   test('map', () => {
     const src = 'a:\n  # 123'
     const doc = YAML.parseDocument(src)
-    expect(String(doc)).toBe('a: null # 123\n')
+    expect(String(doc)).toBe('? a\n\n# 123\n')
   })
 
   test('seq', () => {
@@ -346,19 +346,13 @@ test('eemeli/yaml#120', () => {
   })
 })
 
-test('fake node should respect setOrigRanges()', () => {
-  const cst = YAML.parseCST('a:\r\n  # 123')
+test('empty node should respect setOrigRanges()', () => {
+  const cst = YAML.parseCST('\r\na: # 123\r\n')
+  expect(cst).toHaveLength(1)
   expect(cst.setOrigRanges()).toBe(true)
-  const ast = cst.map(doc =>
-    new YAML.Document({ keepCstNodes: true }).parse(doc)
-  )
-  const fakePlain = ast[0].contents.items[0].value.cstNode
-  expect(fakePlain.range).toEqual({
-    start: 2,
-    end: 2,
-    origStart: 2,
-    origEnd: 2
-  })
+  const doc = new YAML.Document({ keepCstNodes: true }).parse(cst[0])
+  const empty = doc.contents.items[0].value.cstNode
+  expect(empty.range).toEqual({ start: 3, end: 3, origStart: 4, origEnd: 4 })
 })
 
 test('parse an empty string as null', () => {