Remove sync fs operations from algorithm

remarkjs · Aug 10, 2018 · e9f1ee0 · e9f1ee0
1 parent fad3b67
commit e9f1ee0
Show file tree

Hide file tree

Showing 6 changed files with 208 additions and 147 deletions.
diff --git a/index.js b/index.js
@@ -12,9 +12,9 @@ try {
   path = require('path')
 } catch (err) {}
 
-var EXPRESSION = /^([^<(]+?)?[ \t]*(?:<([^>(]+?)>)?[ \t]*(?:\(([^)]+?)\)|$)/
-var LICENSE = /^licen[cs]e(?=$|\.)/i
-
+var authorRegexp = /^([^<(]+?)?[ \t]*(?:<([^>(]+?)>)?[ \t]*(?:\(([^)]+?)\)|$)/
+var licenseRegexp = /^licen[cs]e(?=$|\.)/i
+var licenseHeadingRegexp = /^licen[cs]e$/i
 var http = 'http://'
 var https = 'https://'
 
@@ -26,112 +26,156 @@ function license(options) {
 
   return transformer
 
-  function transformer(tree, file) {
-    var settings = {}
-    var pack = {}
-    var entries = []
+  function transformer(tree, file, next) {
     var cwd = file.cwd
-    var url
-    var length
-    var index
-
-    try {
-      pack = require(path.resolve(cwd, 'package.json'))
-    } catch (err) {}
-
-    if (typeof pack.author === 'string') {
-      url = EXPRESSION.exec(pack.author)
-      settings.name = url[1]
-      settings.url = url[3]
-    } else if (pack.author && pack.author.name) {
-      settings.name = pack.author.name
-      settings.url = pack.author.url
+    var left = 2 // Two async operations.
+    var defaultName
+    var defaultUrl
+    var defaultLicense
+    var defaultLicenseFile
+
+    // Skip package loading if we have all info in `options`.
+    if (options.url && options.name && options.license) {
+      one()
+    } else {
+      fs.readFile(path.resolve(cwd, 'package.json'), onpackage)
     }
 
     if (options.file) {
-      settings.file = options.file
+      one()
     } else {
-      try {
-        entries = fs.readdirSync(cwd)
-      } catch (err) {
-        /* Empty */
-      }
+      fs.readdir(cwd, onfiles)
+    }
 
-      length = entries.length
-      index = -1
+    function onpackage(err, buf) {
+      var pack = {}
+      var match
 
-      while (++index < length) {
-        if (LICENSE.test(entries[index])) {
-          settings.file = entries[index]
-          break
+      if (buf) {
+        try {
+          pack = JSON.parse(buf)
+        } catch (err) {
+          return one(err)
         }
       }
-    }
 
-    if (options.url) {
-      settings.url = options.url
-    }
+      /* istanbul ignore if - hard to test. */
+      if (err && err.code !== 'ENOENT') {
+        one(err)
+      } else {
+        defaultLicense = pack.license
+
+        if (typeof pack.author === 'string') {
+          match = authorRegexp.exec(pack.author)
+          defaultName = match[1]
+          defaultUrl = match[3]
+        } else if (pack.author && pack.author.name) {
+          defaultName = pack.author.name
+          defaultUrl = pack.author.url
+        }
 
-    if (options.name) {
-      settings.name = options.name
+        one()
+      }
     }
 
-    settings.license = options.license || pack.license
+    function onfiles(err, files) {
+      var length
+      var index
 
-    if (!settings.license) {
-      throw new Error(
-        'Missing required `license` in settings.\n' +
-          'Either add a `license` to a `package.json` file\n' +
-          'or pass it into `remark-license`'
-      )
-    }
+      /* istanbul ignore if - hard to test. */
+      if (err) {
+        one(err)
+      } else {
+        length = files.length
+        index = -1
+
+        while (++index < length) {
+          if (licenseRegexp.test(files[index])) {
+            defaultLicenseFile = files[index]
+            break
+          }
+        }
 
-    if (!settings.name) {
-      throw new Error(
-        'Missing required `name` in settings.\n' +
-          'Either add an `author` to a `package.json` file\n' +
-          'or pass it into `remark-license`'
-      )
+        one()
+      }
     }
 
-    heading(tree, /^licen[cs]e$/i, onheading)
+    function one(err) {
+      if (err) {
+        next(err)
+        left = Infinity
+      } else if (--left === 0) {
+        done()
+      }
+    }
 
-    function onheading(start, nodes, end) {
-      var children = []
-      var node = {type: 'paragraph', children: children}
-      var url
-      var parent
+    function done() {
+      var url = options.url || defaultUrl
+      var name = options.name || defaultName
+      var license = options.license || defaultLicense
+      var licenseFile = options.file || defaultLicenseFile
+
+      if (!license) {
+        return next(
+          new Error(
+            'Missing required `license` in settings.\n' +
+              'Either add a `license` to a `package.json` file\n' +
+              'or pass it into `remark-license`'
+          )
+        )
+      }
 
-      if (settings.file) {
-        parent = {type: 'link', title: null, url: settings.file, children: []}
-        children.push(parent)
-      } else {
-        parent = node
+      if (!name) {
+        return next(
+          new Error(
+            'Missing required `name` in settings.\n' +
+              'Either add an `author` to a `package.json` file\n' +
+              'or pass it into `remark-license`'
+          )
+        )
       }
 
-      parent.children.push({type: 'text', value: settings.license})
+      heading(tree, licenseHeadingRegexp, onheading)
 
-      children.push({type: 'text', value: ' © '})
+      next()
 
-      if (settings.url) {
-        url = settings.url
+      function onheading(start, nodes, end) {
+        var children = []
+        var node = {type: 'paragraph', children: children}
+        var link
+        var parent
 
-        if (
-          url.slice(0, http.length) !== http &&
-          url.slice(0, https.length) !== https
-        ) {
-          url = http + url
+        if (licenseFile) {
+          parent = {type: 'link', title: null, url: licenseFile, children: []}
+          children.push(parent)
+        } else {
+          parent = node
         }
 
-        parent = {type: 'link', title: null, url: url, children: []}
-        children.push(parent)
-      } else {
-        parent = node
-      }
+        parent.children.push({type: 'text', value: license})
+
+        children.push({type: 'text', value: ' © '})
+
+        if (url) {
+          if (
+            url.slice(0, http.length) !== http &&
+            url.slice(0, https.length) !== https
+          ) {
+            link = http + url
+          } else {
+            link = url
+          }
 
-      parent.children.push({type: 'text', value: settings.name})
+          parent = {type: 'link', title: null, url: link, children: []}
+          children.push(parent)
+        } else {
+          parent = node
+        }
+
+        parent.children.push({type: 'text', value: name})
 
-      return [start, node, end]
+        return [start, node, end]
+      }
     }
   }
 }
diff --git a/readme.md b/readme.md
@@ -1,6 +1,6 @@
 # remark-license [![Build Status][build-badge]][build-status] [![Coverage Status][coverage-badge]][coverage-status] [![Chat][chat-badge]][chat]
 
-Add a [license][sec] section to a README with [**remark**][remark].
+Add a [license][section] section to a README with [**remark**][remark].
 
 ## Installation
 
@@ -23,24 +23,24 @@ Something nondescript.
 And our script, `example.js`, looks as follows:
 
 ```javascript
-var fs = require('fs');
-var remark = require('remark');
-var license = require('remark-license');
+var fs = require('fs')
+var remark = require('remark')
+var license = require('remark-license')
 
 remark()
   .use(license)
-  .process(fs.readFileSync('example.md'), function (err, file) {
-    if (err) throw err;
-    console.log(String(file));
-  });
+  .process(fs.readFileSync('example.md'), function(err, file) {
+    if (err) throw err
+    console.log(String(file))
+  })
 ```
 
 Now, running `node example` yields:
 
 ```markdown
 ## License
 
-[MIT](LICENSE) © [Titus Wormer](http://remarkjs.com)
+[MIT](LICENSE) © [Titus Wormer](http://wooorm.com)
 ```
 
 ## API
@@ -88,20 +88,23 @@ If you want to set the cwd yourself (the default is `process.cwd()`), you can
 pass in a `vfile` or [`vfile` options][vfile-options] to `.process` like so:
 
 ```js
-var fs = require('fs');
-var path = require('path');
-var remark = require('remark');
-var license = require('remark-license');
+var fs = require('fs')
+var path = require('path')
+var remark = require('remark')
+var license = require('remark-license')
 
 remark()
   .use(license)
-  .process({
-    cwd: path.join('.', 'some', 'path', 'to', 'a', 'directory'),
-    contents: fs.readFileSync('example.md')
-  }, function (err, file) {
-    if (err) throw err;
-    console.log(String(file));
-  });
+  .process(
+    {
+      cwd: path.join('.', 'some', 'path', 'to', 'a', 'directory'),
+      contents: fs.readFileSync('example.md')
+    },
+    function(err, file) {
+      if (err) throw err
+      console.log(String(file))
+    }
+  )
 ```
 
 ## Related
@@ -157,7 +160,7 @@ repository, organisation, or community you agree to abide by its terms.
 
 [vfile-options]: https://github.com/vfile/vfile#vfileoptions
 
-[sec]: #license
+[section]: #license
 
 [detected]: #detection
 

diff --git a/test/fixtures/fail-missing-required-license-in-settings/readme.md b/test/fixtures/fail-missing-required-license-in-settings/readme.md
@@ -0,0 +1,3 @@
+# Pi
+
+## License
diff --git a/test/fixtures/fail-unexpected-end-of-json/package.json b/test/fixtures/fail-unexpected-end-of-json/package.json
@@ -0,0 +1 @@
+{
diff --git a/test/fixtures/fail-unexpected-end-of-json/readme.md b/test/fixtures/fail-unexpected-end-of-json/readme.md
@@ -0,0 +1,3 @@
+# Pi
+
+## License