Merge pull request #2864 from spotify/orkohunter/remove-friction-by-vale

Run vale linter only if it is installed, skip otherwise

package.json

@@ -41,6 +41,7 @@
     "@changesets/cli": "^2.11.0",
     "@spotify/eslint-config-oss": "^1.0.1",
     "@spotify/prettier-config": "^8.0.0",
+    "command-exists": "^1.2.9",
     "concurrently": "^5.2.0",
     "fs-extra": "^9.0.0",
     "husky": "^4.2.3",
@@ -65,7 +66,7 @@
       "prettier --write"
     ],
     "*.md": [
-      "vale"
+      "node ./scripts/check-docs-quality"
     ]
   },
   "jest": {

scripts/check-docs-quality.js

@@ -1,4 +1,3 @@
-#!/usr/bin/env node
 /*
  * Copyright 2020 Spotify AB
  *
@@ -15,54 +14,76 @@
  * limitations under the License.
  */
 const { execSync, spawnSync } = require('child_process');
-const path = require('path');
-
-const listFilesTrackedByGit = 'git ls-files';
+// eslint-disable-next-line import/no-extraneous-dependencies
+const commandExists = require('command-exists');
 
 const inheritStdIo = {
   stdio: 'inherit',
 };
 
-const ERROR_MESSAGE =
-  'Please install vale linter(https://docs.errata.ai/vale/install). Ignore this message if already installed.\n';
+const LINT_SKIPPED_MESSAGE =
+  'Skipping documentation quality check (vale not found). Install vale linter (https://docs.errata.ai/vale/install) to enable.\n';
+const LINT_ERROR_MESSAGE = `Language linter (vale) generated errors. Please check the errors and review any markdown files that you changed.
+  Possibly update .github/styles/vocab.txt to add new valid words.\n`;
+const VALE_NOT_FOUND_MESSAGE = `Language linter (vale) was not found. Please install vale linter (https://docs.errata.ai/vale/install).\n`;
 
-// xargs is not supported by shx.
-if (process.platform === 'win32') {
-  const validMDFilesCommand = `${listFilesTrackedByGit} | .\\node_modules\\.bin\\shx grep ".md"`;
-  try {
-    // get list of all md files except in directories of gitignore.
-    let filesToLint = execSync(validMDFilesCommand, {
-      stdio: ['ignore', 'pipe', 'inherit'],
-    });
+// Note: Make sure the script is run as `node check-docs-quality.js [FILES]` instead of `./check-docs-quality.js [FILES]`
+// If the script receives arguments (file paths), the script is run exclusively on them. (e.g. when run via pre-commit hook)
+const getFilesToLint = () => {
+  // Files have been provided as arguments
+  if (process.argv.length > 2) {
+    return process.argv.slice(2);
+  }
 
-    // set all file(s) path as absolute path
-    filesToLint = filesToLint
-      .toString()
-      .split('\n')
-      .map(filepath => (filepath ? path.join(process.cwd(), filepath) : null))
-      .filter(Boolean);
+  let command = `git ls-files | ./node_modules/.bin/shx grep ".md"`;
+  if (process.platform === 'win32') {
+    command = `git ls-files | .\\node_modules\\.bin\\shx grep ".md"`;
+  }
 
-    const output = spawnSync('vale', filesToLint, inheritStdIo);
+  return execSync(command, {
+    stdio: ['ignore', 'pipe', 'inherit'],
+  })
+    .toString()
+    .split('\n');
+};
 
-    // if the command does not succeed
-    if (output.status !== 0) {
-      // if it contains system level error. [in this case vale does not exist]
-      if (output.error) {
-        console.error(ERROR_MESSAGE);
-      }
+// Proceed with the script only if Vale linter is installed. Limit the friction and surprises caused by the script.
+// On CI, we want to ensure vale linter is run.
+commandExists('vale')
+  .catch(() => {
+    if (process.env.CI) {
+      console.log(VALE_NOT_FOUND_MESSAGE);
       process.exit(1);
     }
-  } catch (e) {
-    console.error(e.message);
-    process.exit(1);
-  }
-} else {
-  const validMDFilesCommand = `${listFilesTrackedByGit} | ./node_modules/.bin/shx grep ".md"`;
-  // use xargs
-  try {
-    execSync(`${validMDFilesCommand} | xargs vale`, inheritStdIo);
-  } catch (e) {
-    console.error(ERROR_MESSAGE);
-    process.exit(1);
-  }
-}
+    console.log(LINT_SKIPPED_MESSAGE);
+    process.exit(0);
+  })
+  .then(() => {
+    const filesToLint = getFilesToLint();
+
+    if (process.platform === 'win32') {
+      // Windows
+      try {
+        const output = spawnSync('vale', filesToLint, inheritStdIo);
+
+        // If the command does not succeed
+        if (output.status !== 0) {
+          // If it contains system level error. In this case vale does not exist.
+          if (output.error) {
+            console.log(LINT_ERROR_MESSAGE);
+          }
+          process.exit(1);
+        }
+      } catch (e) {
+        console.log(e.message);
+        process.exit(1);
+      }
+    } else {
+      // Unix
+      const output = spawnSync('vale', filesToLint, inheritStdIo);
+      if (output.status !== 0) {
+        console.log(LINT_ERROR_MESSAGE);
+        process.exit(1);
+      }
+    }
+  });

yarn.lock

@@ -8554,6 +8554,11 @@ command-exists-promise@^2.0.2:
   resolved "https://registry.npmjs.org/command-exists-promise/-/command-exists-promise-2.0.2.tgz#7beecc4b218299f3c61fa69a4047aa0b36a64a99"
   integrity sha512-T6PB6vdFrwnHXg/I0kivM3DqaCGZLjjYSOe0a5WgFKcz1sOnmOeIjnhQPXVXX3QjVbLyTJ85lJkX6lUpukTzaA==
 
+command-exists@^1.2.9:
+  version "1.2.9"
+  resolved "https://registry.npmjs.org/command-exists/-/command-exists-1.2.9.tgz#c50725af3808c8ab0260fd60b01fbfa25b954f69"
+  integrity sha512-LTQ/SGc+s0Xc0Fu5WaKnR0YiygZkm9eKFvyS+fRsU7/ZWFF8ykFM6Pc9aCVf1+xasOOZpO3BAVgVrKvsqKHV7w==
+
 commander@^2.11.0, commander@^2.19.0, commander@^2.20.0, commander@^2.20.3, commander@~2.20.3:
   version "2.20.3"
   resolved "https://registry.npmjs.org/commander/-/commander-2.20.3.tgz#fd485e84c03eb4881c20722ba48035e8531aeb33"