feat(conventional-changelog): handle BREAKING CHANGE in footer and bo…

…dy (#32)

lib/utils.js

@@ -1,22 +1,27 @@
 const visit = require('unist-util-visit')
+const visitWithAncestors = require('unist-util-visit-parents')
 const NUMBER_REGEX = /^[0-9]+$/
 
 // Converts conventional commit AST into conventional-changelog's
 // output format, see: https://www.npmjs.com/package/conventional-commits-parser
 function toConventionalChangelogFormat (ast) {
   const cc = {
-    body: null,
+    body: '',
+    subject: '',
+    type: '',
+    scope: null,
     notes: [],
     references: [],
     mentions: [],
     merge: null,
-    revert: null
+    revert: null,
+    header: '',
+    footer: null
   }
-  let breaking
-  let body
-  let summary
   // Separate the body and summary nodes, this simplifies the subsequent
   // tree walking logic:
+  let body
+  let summary
   visit(ast, ['body', 'summary'], (node) => {
     switch (node.type) {
       case 'body':
@@ -25,54 +30,77 @@ function toConventionalChangelogFormat (ast) {
       case 'summary':
         summary = node
         break
-      default:
-        break
     }
   })
 
-  visit(summary, () => true, (node) => {
+  // <type>, "(", <scope>, ")", ["!"], ":", <whitespace>*, <text>
+  visit(summary, (node) => {
     switch (node.type) {
       case 'type':
         cc.type = node.value
+        cc.header += node.value
         break
       case 'scope':
         cc.scope = node.value
+        cc.header += `(${node.value})`
+        break
+      case 'breaking-change':
+        cc.header += '!'
         break
       case 'text':
         cc.subject = node.value
-        break
-      case 'breaking-change':
-        breaking = {
-          title: 'BREAKING CHANGE'
-          // "text" node should be added with subject after walk.
-        }
+        cc.header += `: ${node.value}`
         break
       default:
         break
     }
   })
-  // The header contains the recombined components of the summary:
-  cc.header = `${cc.type}${cc.scope ? `(${cc.scope})` : ''}${breaking ? '!' : ''}: ${cc.subject}`
 
+  // [<any body-text except pre-footer>]
   if (body) {
-    let text = ''
     visit(body, 'text', (node, _i, parent) => {
-      if (parent.type !== 'body') return
-      if (text !== '') text += '\n'
-      text += node.value
+      // TODO(@bcoe): once we have \n tokens in tree we can drop this:
+      if (cc.body !== '') cc.body += '\n'
+      cc.body += node.value
     })
-    if (text !== '') cc.body = text
   }
 
-  // A breaking change note was found either in the body, the header, or
-  // in one of the footers:
-  // TODO(bcoe): if we refactor the grammar slightly, so that footer is a
-  // direct parent of `breaking-change`, the logic for extracting a breaking
-  // change would be easier.
-  if (breaking) {
-    if (!breaking.text) breaking.text = cc.subject
-    cc.notes.push(breaking)
+  // Extract BREAKING CHANGE notes, regardless of whether they fall in
+  // summary, body, or footer:
+  const breaking = {
+    title: 'BREAKING CHANGE',
+    text: '' // "text" will be populated if a BREAKING CHANGE token is parsed.
   }
+  visitWithAncestors(ast, ['breaking-change'], (node, ancestors) => {
+    let parent = ancestors.pop()
+    let startCollecting = false
+    switch (parent.type) {
+      case 'summary':
+        breaking.text = cc.subject
+        break
+      case 'body':
+        breaking.text = ''
+        // We treat text from the BREAKING CHANGE marker forward as
+        // the breaking change notes:
+        visit(parent, ['text', 'breaking-change'], (node) => {
+          // TODO(@bcoe): once we have \n tokens in tree we can drop this:
+          if (startCollecting && node.type === 'text') {
+            if (breaking.text !== '') breaking.text += '\n'
+            breaking.text += node.value
+          } else if (node.type === 'breaking-change') {
+            startCollecting = true
+          }
+        })
+        break
+      case 'token':
+        parent = ancestors.pop()
+        visit(parent, 'text', (node) => {
+          breaking.text = node.value
+        })
+        break
+    }
+  })
+  if (breaking.text !== '') cc.notes.push(breaking)
 
   // Populates references array from footers:
   // references: [{
@@ -109,8 +137,6 @@ function toConventionalChangelogFormat (ast) {
             reference.issue = node.value
           }
           break
-        default:
-          break
       }
     })
     // TODO(@bcoe): how should references like "Refs: v8:8940" work.

package.json

@@ -44,6 +44,7 @@
     "yargs": "^16.2.0"
   },
   "dependencies": {
-    "unist-util-visit": "^2.0.3"
+    "unist-util-visit": "^2.0.3",
+    "unist-util-visit-parents": "^3.1.1"
   }
 }

scripts/inspect.js

@@ -1,12 +1,17 @@
 #!/usr/bin/env node
 
-const { parser } = require('..')
+const { parser, toConventionalChangelogFormat } = require('..')
 const inspect = require('unist-util-inspect')
 const { hideBin } = require('yargs/helpers')
 const yargs = require('yargs/yargs')(hideBin(process.argv))
 
 yargs
-  .command('$0 <message>', 'Output the parsed syntax tree', () => {}, (argv) => {
+  .command('$0 <message>', 'output the parsed syntax tree', () => {}, (argv) => {
     console.log(inspect(parser(argv.message)))
   })
+  .command('cc <message>', 'output conventional changelog format commit', () => {}, (argv) => {
+    const cc = toConventionalChangelogFormat(parser(argv.message))
+    console.log('-----')
+    console.log(JSON.stringify(cc, null, 2))
+  })
   .parse()

test/utils.js

@@ -17,13 +17,6 @@ describe('utils', () => {
       const parsed = toConventionalChangelogFormat(parser('foo: bar\n\nthe body of commit\nsecond line'))
       assert.strictEqual(parsed.body, 'the body of commit\nsecond line')
     })
-    it('extracts BREAKING CHANGE from header', () => {
-      const parsed = toConventionalChangelogFormat(parser('foo!: hello world'))
-      assert.strictEqual(parsed.notes.length, 1)
-      const note = parsed.notes[0]
-      assert.strictEqual(note.title, 'BREAKING CHANGE')
-      assert.strictEqual(note.text, 'hello world')
-    })
     it('populates references entry from footer', () => {
       const parsed = toConventionalChangelogFormat(parser('foo: summary\n\nRefs #34'))
       assert.strictEqual(parsed.references.length, 1)
@@ -40,5 +33,34 @@ describe('utils', () => {
       const parsed = toConventionalChangelogFormat(parser('foo: summary\n\nRefs #batman'))
       assert.strictEqual(parsed.references.length, 0)
     })
+    it('extracts BREAKING CHANGE from header', () => {
+      const parsed = toConventionalChangelogFormat(parser('foo!: hello world'))
+      assert.strictEqual(parsed.notes.length, 1)
+      const note = parsed.notes[0]
+      assert.strictEqual(note.title, 'BREAKING CHANGE')
+      assert.strictEqual(note.text, 'hello world')
+    })
+    it('extracts BREAKING CHANGE from body', () => {
+      const parsed = toConventionalChangelogFormat(parser('foo!: hello world\n\nBREAKING CHANGE: this change is breaking\nsecond line'))
+      assert.strictEqual(parsed.notes.length, 1)
+      const note = parsed.notes[0]
+      assert.strictEqual(note.title, 'BREAKING CHANGE')
+      assert.strictEqual(note.text, 'this change is breaking\nsecond line')
+    })
+    it('only extracts text after BREAKING CHANGE token in body', () => {
+      const parsed = toConventionalChangelogFormat(parser('foo!: hello world\n\nstart of body\nBREAKING CHANGE: this change is breaking\nsecond line'))
+      assert.strictEqual(parsed.notes.length, 1)
+      const note = parsed.notes[0]
+      assert.strictEqual(note.title, 'BREAKING CHANGE')
+      assert.strictEqual(note.text, 'this change is breaking\nsecond line')
+    })
+    it('extracts BREAKING CHANGE from footer', () => {
+      const parsed = toConventionalChangelogFormat(parser('foo!: hello world\n\nthis is the body\n\nBREAKING CHANGE: this change is breaking'))
+      assert.strictEqual(parsed.notes.length, 1)
+      assert.strictEqual(parsed.body, 'this is the body')
+      const note = parsed.notes[0]
+      assert.strictEqual(note.title, 'BREAKING CHANGE')
+      assert.strictEqual(note.text, 'this change is breaking')
+    })
   })
 })