Refactor code-style

Author: wooorm

lib/index.js

@@ -1,44 +1,85 @@
-import process from 'node:process'
+/**
+ * @typedef {import('vfile').VFile} VFile
+ */
+
+/**
+ * @typedef {'patch' | 'stats' | 'raw'} DiffType
+ *
+ * @typedef PatchData
+ *   Data of a patch.
+ * @property {string} aPath
+ *   From.
+ * @property {string} bPath
+ *   To.
+ * @property {Array<string>} lines
+ *   Changes.
+ * @property {boolean} isBlacklisted
+ *   No idea.
+ *
+ * @typedef {[originalRev: string, rev: string]} Range
+ *   Range of two refs (such as commits).
+ *
+ * @typedef {[from: number, to: number]} Diff
+ *   Diff range, two line numbers between which there’s been a change.
+ */
+
 import path from 'node:path'
-// @ts-expect-error: hush
+import process from 'node:process'
+import {ok as assert} from 'devlop'
+// @ts-expect-error: not typed.
 import gitDiffTree from 'git-diff-tree'
 import {findUp} from 'vfile-find-up'
 
-const own = {}.hasOwnProperty
-
+// This is mostly to enable the tests to mimick different CIs.
+// Normally, a Node process exits between CI runs.
 /** @type {string} */
 let previousRange
 
-/** @type {import('unified').Plugin<[]>} */
-export default function diff() {
-  /** @type {Record<string, string>} */
-  let cache = {}
-
-  return function (_, file, next) {
+/**
+ * @returns
+ *   Transform.
+ */
+export default function unifiedDiff() {
+  /** @type {Map<string, string>} */
+  let cache = new Map()
+
+  /**
+   * @param {unknown} _
+   *   Tree.
+   * @param {VFile} file
+   *   File.
+   * @returns {Promise<undefined>}
+   *   Promise to nothing.
+   */
+  return async function (_, file) {
     const base = file.dirname
-    /** @type {string|undefined} */
+    /** @type {string | undefined} */
     let commitRange
-    /** @type {Array<string>|undefined} */
+    /** @type {Range | undefined} */
     let range
 
     // Looks like Travis.
     if (process.env.TRAVIS_COMMIT_RANGE) {
       commitRange = process.env.TRAVIS_COMMIT_RANGE
-      range = commitRange.split(/\.{3}/)
+      // Cast because we check `length` later.
+      range = /** @type {Range} */ (commitRange.split(/\.{3}/))
     }
     // Looks like GH Actions.
     else if (process.env.GITHUB_SHA) {
-      // @ts-expect-error: fine.
-      range =
-        // This is a PR: check the whole PR.
-        // Refs take the form `refs/heads/main`.
-        process.env.GITHUB_BASE_REF && process.env.GITHUB_HEAD_REF
-          ? [
-              process.env.GITHUB_BASE_REF.split('/').pop(),
-              process.env.GITHUB_HEAD_REF.split('/').pop()
-            ]
-          : [process.env.GITHUB_SHA + '^1', process.env.GITHUB_SHA]
-      // @ts-expect-error: We definitely just defined this
+      const sha = process.env.GITHUB_SHA
+      const base = process.env.GITHUB_BASE_REF
+      const head = process.env.GITHUB_HEAD_REF
+
+      if (base && head) {
+        const baseTail = base.split('/').pop()
+        const headTail = head.split('/').pop()
+        assert(baseTail)
+        assert(headTail)
+        range = [baseTail, headTail]
+      } else {
+        range = [sha + '^1', sha]
+      }
+
       commitRange = range.join('...')
     }
 
@@ -49,119 +90,124 @@ export default function diff() {
       !file.dirname ||
       range.length !== 2
     ) {
-      return next()
+      return
     }
 
-    if (commitRange !== previousRange) {
-      cache = {}
+    // Reset cache.
+    if (previousRange !== commitRange) {
+      cache = new Map()
       previousRange = commitRange
     }
 
-    /* c8 ignore next 3 */
-    if (own.call(cache, base)) {
-      tick(cache[base])
-    } else {
-      findUp('.git', file.dirname, (error, git) => {
-        // Never happens.
-        /* c8 ignore next */
-        if (error) return next(error)
-
-        // Not testable in a Git repo…
-        /* c8 ignore next 3 */
-        if (!git || !git.dirname) {
-          return next(new Error('Not in a git repository'))
-        }
+    let gitFolder = cache.get(base)
 
-        cache[base] = git.dirname
-        tick(git.dirname)
-      })
+    if (!gitFolder) {
+      const gitFolderFile = await findUp('.git', file.dirname)
+
+      /* c8 ignore next 3 -- not testable in a Git repo… */
+      if (!gitFolderFile || !gitFolderFile.dirname) {
+        throw new Error('Not in a git repository')
+      }
+
+      cache.set(base, gitFolderFile.dirname)
+      gitFolder = gitFolderFile.dirname
+    }
+
+    const diffs = await checkGit(gitFolder, range)
+    const ranges = diffs.get(path.resolve(file.cwd, file.path))
+
+    // Unchanged file: drop all messages.
+    if (!ranges || ranges.length === 0) {
+      file.messages.length = 0
+      return
     }
 
-    /**
-     * @param {string} root
-     */
-    function tick(root) {
-      /** @type {Record<string, Array<[number, number]>>} */
-      const diffs = {}
-
-      gitDiffTree(path.join(root, '.git'), {
-        // @ts-expect-error: fine.
-        originalRev: range[0],
-        // @ts-expect-error: fine.
-        rev: range[1]
+    file.messages = file.messages.filter(function (message) {
+      return ranges.some(function (range) {
+        return (
+          message.line && message.line >= range[0] && message.line <= range[1]
+        )
       })
-        .on('error', next)
-        .on(
-          'data',
-          /**
-           * @param {string} type
-           * @param {{lines: string, aPath: string, bPath: string}} data
-           */
-          (type, data) => {
-            if (type !== 'patch') return
-
-            const lines = data.lines
-            const re = /^@@ -(\d+),?(\d+)? \+(\d+),?(\d+)? @@/
-            const match = lines[0].match(re)
-
-            // Should not happen, maybe if Git returns weird diffs?
-            /* c8 ignore next */
-            if (!match) return
-
-            /** @type {Array<[number, number]>} */
-            const ranges = []
-            const start = Number.parseInt(match[3], 10) - 1
-            let index = 0
-            /** @type {number|undefined} */
-            let position
-
-            while (++index < lines.length) {
-              const line = lines[index]
-
-              if (line.charAt(0) === '+') {
-                const no = start + index
-
-                if (position === undefined) {
-                  position = ranges.length
-                  ranges.push([no, no])
-                } else {
-                  ranges[position][1] = no
-                }
+    })
+  }
+}
+
+/**
+ * Check a folder.
+ *
+ * @param {string} root
+ *   Folder.
+ * @param {Range} range
+ *   Range.
+ * @returns {Promise<Map<string, Array<Diff>>>}
+ *   Nothing.
+ */
+function checkGit(root, range) {
+  return new Promise(function (resolve, reject) {
+    /** @type {Map<string, Array<Diff>>} */
+    const diffs = new Map()
+    const [originalRev, rev] = range
+
+    gitDiffTree(path.join(root, '.git'), {originalRev, rev})
+      .on('error', reject)
+      .on(
+        'data',
+        /**
+         * @param {DiffType} type
+         *   Data type.
+         * @param {PatchData} data
+         *   Data.
+         * @returns {undefined}
+         *   Nothing.
+         */
+        function (type, data) {
+          if (type !== 'patch') return
+
+          const lines = data.lines
+          const re = /^@@ -(\d+),?(\d+)? \+(\d+),?(\d+)? @@/
+          const match = lines[0].match(re)
+
+          /* c8 ignore next -- should not happen, maybe if Git returns weird diffs? */
+          if (!match) return
+
+          /** @type {Array<Diff>} */
+          const ranges = []
+          const start = Number.parseInt(match[3], 10) - 1
+          let index = 0
+          /** @type {number | undefined} */
+          let position
+
+          while (++index < lines.length) {
+            const line = lines[index]
+
+            if (line.charAt(0) === '+') {
+              const no = start + index
+
+              if (position === undefined) {
+                position = ranges.length
+                ranges.push([no, no])
               } else {
-                position = undefined
+                ranges[position][1] = no
               }
+            } else {
+              position = undefined
             }
+          }
 
-            const fp = path.resolve(root, data.bPath)
+          const fp = path.resolve(root, data.bPath)
 
-            // Long diffs.
-            /* c8 ignore next */
-            if (!(fp in diffs)) diffs[fp] = []
+          let list = diffs.get(fp)
 
-            diffs[fp].push(...ranges)
-          }
-        )
-        .on('end', () => {
-          const fp = path.resolve(file.cwd, file.path)
-          const ranges = diffs[fp]
-
-          // Unchanged file.
-          if (!ranges || ranges.length === 0) {
-            file.messages = []
-            return next()
+          if (!list) {
+            list = []
+            diffs.set(fp, list)
           }
 
-          file.messages = file.messages.filter((message) =>
-            ranges.some(
-              (range) =>
-                message.line &&
-                message.line >= range[0] &&
-                message.line <= range[1]
-            )
-          )
-
-          next()
-        })
-    }
-  }
+          list.push(...ranges)
+        }
+      )
+      .on('end', function () {
+        resolve(diffs)
+      })
+  })
 }

package.json

@@ -35,19 +35,20 @@
     "index.js"
   ],
   "dependencies": {
+    "devlop": "^1.0.0",
     "git-diff-tree": "^1.0.0",
+    "vfile": "^6.0.0",
     "vfile-find-up": "^7.0.0"
   },
   "devDependencies": {
     "@types/nlcst": "^2.0.0",
-    "@types/tape": "^5.0.0",
+    "@types/node": "^20.0.0",
     "c8": "^8.0.0",
     "nlcst-to-string": "^4.0.0",
     "parse-english": "^7.0.0",
     "prettier": "^3.0.0",
     "remark-cli": "^11.0.0",
     "remark-preset-wooorm": "^9.0.0",
-    "tape": "^5.0.0",
     "to-vfile": "^8.0.0",
     "type-coverage": "^2.0.0",
     "typescript": "^5.0.0",

readme.md

@@ -104,7 +104,7 @@ const file = unified()
   .process(await read('example.md'))
 
 console.error(reporter(file))
-process.exit(file.messages.length > 0 ? 1 : 0)
+process.exitCode = file.messages.length > 0 ? 1 : 0
 ```
 
 …and our Travis configuration `.travis.yml` contains: