tools: improve js linter

This commit switches from the eslint command-line tool to a custom
tool that uses eslint programmatically in order to perform linting
in parallel and to display linting results incrementally instead of
buffering them until the end.

Fixes: https://github.com/nodejs/node/issues/5596
PR-URL: https://github.com/nodejs/node/pull/5638
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>

Makefile

@@ -603,8 +603,12 @@ bench-idle:
 	$(NODE) benchmark/idle_clients.js &
 
 jslint:
-	$(NODE) tools/eslint/bin/eslint.js benchmark lib src test tools/doc \
-		tools/eslint-rules --rulesdir tools/eslint-rules
+	$(NODE) tools/jslint.js -J benchmark lib src test tools/doc \
+	  tools/eslint-rules tools/jslint.js
+
+jslint-ci:
+	$(NODE) tools/jslint.js -f tap -o test-eslint.tap benchmark lib src test \
+	  tools/doc tools/eslint-rules tools/jslint.js
 
 CPPLINT_EXCLUDE ?=
 CPPLINT_EXCLUDE += src/node_lttng.cc
@@ -633,14 +637,15 @@ cpplint:
 
 ifneq ("","$(wildcard tools/eslint/bin/eslint.js)")
 lint: jslint cpplint
+lint-ci: jslint-ci cpplint
 else
 lint:
 	@echo "Linting is not available through the source tarball."
 	@echo "Use the git repo instead:" \
 		"$ git clone https://github.com/nodejs/node.git"
-endif
 
 lint-ci: lint
+endif
 
 .PHONY: lint cpplint jslint bench clean docopen docclean doc dist distclean \
 	check uninstall install install-includes install-bin all staticlib \
@@ -648,4 +653,4 @@ lint-ci: lint
 	blog blogclean tar binary release-only bench-http-simple bench-idle \
 	bench-all bench bench-misc bench-array bench-buffer bench-net \
 	bench-http bench-fs bench-tls cctest run-ci test-v8 test-v8-intl \
-	test-v8-benchmarks test-v8-all v8 lint-ci bench-ci
+	test-v8-benchmarks test-v8-all v8 lint-ci bench-ci jslint-ci

tools/jslint.js

@@ -0,0 +1,262 @@
+'use strict';
+
+const rulesDirs = ['tools/eslint-rules'];
+// This is the maximum number of files to be linted per worker at any given time
+const maxWorkload = 40;
+
+const cluster = require('cluster');
+const path = require('path');
+const fs = require('fs');
+const totalCPUs = require('os').cpus().length;
+
+const CLIEngine = require('./eslint').CLIEngine;
+const glob = require('./eslint/node_modules/glob');
+
+const cwd = process.cwd();
+const cli = new CLIEngine({
+  rulePaths: rulesDirs
+});
+
+if (cluster.isMaster) {
+  var numCPUs = 1;
+  const paths = [];
+  var files = null;
+  var totalPaths = 0;
+  var failures = 0;
+  var successes = 0;
+  var lastLineLen = 0;
+  var curPath = 'Starting ...';
+  var showProgress = true;
+  const globOptions = {
+    nodir: true
+  };
+  const workerConfig = {};
+  var startTime;
+  var formatter;
+  var outFn;
+  var fd;
+  var i;
+
+  // Check if spreading work among all cores/cpus
+  if (process.argv.indexOf('-J') !== -1)
+    numCPUs = totalCPUs;
+
+  // Check if spreading work among an explicit number of cores/cpus
+  i = process.argv.indexOf('-j');
+  if (i !== -1) {
+    if (!process.argv[i + 1])
+      throw new Error('Missing parallel job count');
+    numCPUs = parseInt(process.argv[i + 1], 10);
+    if (!isFinite(numCPUs) || numCPUs <= 0)
+      throw new Error('Bad parallel job count');
+  }
+
+  // Check for custom JSLint report formatter
+  i = process.argv.indexOf('-f');
+  if (i !== -1) {
+    if (!process.argv[i + 1])
+      throw new Error('Missing format name');
+    const format = process.argv[i + 1];
+    formatter = cli.getFormatter(format);
+    if (!formatter)
+      throw new Error('Invalid format name');
+    // Automatically disable progress display
+    showProgress = false;
+    // Tell worker to send all results, not just linter errors
+    workerConfig.sendAll = true;
+  } else {
+    // Use default formatter
+    formatter = cli.getFormatter();
+  }
+
+  // Check if outputting JSLint report to a file instead of stdout
+  i = process.argv.indexOf('-o');
+  if (i !== -1) {
+    if (!process.argv[i + 1])
+      throw new Error('Missing output filename');
+    var outPath = process.argv[i + 1];
+    if (!path.isAbsolute(outPath))
+      outPath = path.join(cwd, outPath);
+    fd = fs.openSync(outPath, 'w');
+    outFn = function(str) {
+      fs.writeSync(fd, str, 'utf8');
+    };
+    process.on('exit', function() {
+      fs.closeSync(fd);
+    });
+  } else {
+    outFn = function(str) {
+      process.stdout.write(str);
+    };
+  }
+
+  // Process the rest of the arguments as paths to lint, ignoring any unknown
+  // flags
+  for (i = 2; i < process.argv.length; ++i) {
+    if (process.argv[i][0] === '-') {
+      switch (process.argv[i]) {
+        case '-f': // Skip format name
+        case '-o': // Skip filename
+        case '-j': // Skip parallel job count number
+          ++i;
+          break;
+      }
+      continue;
+    }
+    paths.push(process.argv[i]);
+  }
+
+  if (paths.length === 0)
+    return;
+  totalPaths = paths.length;
+
+  if (showProgress) {
+    // Start the progress display update timer when the first worker is ready
+    cluster.once('online', function(worker) {
+      startTime = process.hrtime();
+      setInterval(printProgress, 1000).unref();
+      printProgress();
+    });
+  }
+
+  cluster.on('online', function(worker) {
+    // Configure worker and give it some initial work to do
+    worker.send(workerConfig);
+    sendWork(worker);
+  });
+
+  cluster.on('message', function(worker, results) {
+    if (typeof results !== 'number') {
+      // The worker sent us results that are not all successes
+      if (!workerConfig.sendAll)
+        failures += results.length;
+      outFn(formatter(results) + '\r\n');
+      printProgress();
+    } else {
+      successes += results;
+    }
+    // Try to give the worker more work to do
+    sendWork(worker);
+  });
+
+  process.on('exit', function() {
+    if (showProgress) {
+      curPath = 'Done';
+      printProgress();
+      outFn('\r\n');
+    }
+    process.exit(failures ? 1 : 0);
+  });
+
+  for (i = 0; i < numCPUs; ++i)
+    cluster.fork();
+
+  function sendWork(worker) {
+    if (!files || !files.length) {
+      // We either just started or we have no more files to lint for the current
+      // path. Find the next path that has some files to be linted.
+      while (paths.length) {
+        var dir = paths.shift();
+        curPath = dir;
+        if (dir.indexOf('/') > 0)
+          dir = path.join(cwd, dir);
+        const patterns = cli.resolveFileGlobPatterns([dir]);
+        dir = path.resolve(patterns[0]);
+        files = glob.sync(dir, globOptions);
+        if (files.length)
+          break;
+      }
+      if ((!files || !files.length) && !paths.length) {
+        // We exhausted all input paths and thus have nothing left to do, so end
+        // the worker
+        return worker.disconnect();
+      }
+    }
+    // Give the worker an equal portion of the work left for the current path,
+    // but not exceeding a maximum file count in order to help keep *all*
+    // workers busy most of the time instead of only a minority doing most of
+    // the work.
+    const sliceLen = Math.min(maxWorkload, Math.ceil(files.length / numCPUs));
+    var slice;
+    if (sliceLen === files.length) {
+      // Micro-ptimization to avoid splicing to an empty array
+      slice = files;
+      files = null;
+    } else {
+      slice = files.splice(0, sliceLen);
+    }
+    worker.send(slice);
+  }
+
+  function printProgress() {
+    if (!showProgress)
+      return;
+
+    // Clear line
+    outFn('\r' + ' '.repeat(lastLineLen) + '\r');
+
+    // Calculate and format the data for displaying
+    const elapsed = process.hrtime(startTime)[0];
+    const mins = padString(Math.floor(elapsed / 60), 2, '0');
+    const secs = padString(elapsed % 60, 2, '0');
+    const passed = padString(successes, 6, ' ');
+    const failed = padString(failures, 6, ' ');
+    var pct = Math.ceil(((totalPaths - paths.length) / totalPaths) * 100);
+    pct = padString(pct, 3, ' ');
+
+    var line = `[${mins}:${secs}|%${pct}|+${passed}|-${failed}]: ${curPath}`;
+
+    // Truncate line like cpplint does in case it gets too long
+    if (line.length > 75)
+      line = line.slice(0, 75) + '...';
+
+    // Store the line length so we know how much to erase the next time around
+    lastLineLen = line.length;
+
+    outFn(line);
+  }
+
+  function padString(str, len, chr) {
+    str = '' + str;
+    if (str.length >= len)
+      return str;
+    return chr.repeat(len - str.length) + str;
+  }
+} else {
+  // Worker
+
+  var config = {};
+  process.on('message', function(files) {
+    if (files instanceof Array) {
+      // Lint some files
+      const report = cli.executeOnFiles(files);
+      if (config.sendAll) {
+        // Return both success and error results
+
+        const results = report.results;
+        // Silence warnings for files with no errors while keeping the "ok"
+        // status
+        if (report.warningCount > 0) {
+          for (var i = 0; i < results.length; ++i) {
+            const result = results[i];
+            if (result.errorCount === 0 && result.warningCount > 0) {
+              result.warningCount = 0;
+              result.messages = [];
+            }
+          }
+        }
+        process.send(results);
+      } else if (report.errorCount === 0) {
+        // No errors, return number of successful lint operations
+        process.send(files.length);
+      } else {
+        // One or more errors, return the error results only
+        process.send(CLIEngine.getErrorResults(report.results));
+      }
+    } else if (typeof files === 'object') {
+      // The master process is actually sending us our configuration and not a
+      // list of files to lint
+      config = files;
+    }
+  });
+}

vcbuild.bat

@@ -66,6 +66,7 @@ if /i "%1"=="test-pummel"   set test_args=%test_args% pummel&goto arg-ok
 if /i "%1"=="test-all"      set test_args=%test_args% sequential parallel message gc internet pummel&set buildnodeweak=1&set jslint=1&goto arg-ok
 if /i "%1"=="test-known-issues" set test_args=%test_args% known_issues --expect-fail&goto arg-ok
 if /i "%1"=="jslint"        set jslint=1&goto arg-ok
+if /i "%1"=="jslint-ci"     set jslint_ci=1&goto arg-ok
 if /i "%1"=="msi"           set msi=1&set licensertf=1&set download_arg="--download=all"&set i18n_arg=small-icu&goto arg-ok
 if /i "%1"=="build-release" set build_release=1&goto arg-ok
 if /i "%1"=="upload"        set upload=1&goto arg-ok
@@ -286,10 +287,16 @@ python tools\test.py %test_args%
 goto jslint
 
 :jslint
+if defined jslint_ci goto jslint-ci
 if not defined jslint goto exit
 if not exist tools\eslint\bin\eslint.js goto no-lint
 echo running jslint
-%config%\node tools\eslint\bin\eslint.js benchmark lib src test tools\doc tools\eslint-rules --rulesdir tools\eslint-rules
+%config%\node tools\jslint.js -J benchmark lib src test tools\doc tools\eslint-rules tools\jslint.js
+goto exit
+
+:jslint-ci
+echo running jslint-ci
+%config%\node tools\jslint.js -J -f tap -o test-eslint.tap benchmark lib src test tools\doc tools\eslint-rules tools\jslint.js
 goto exit
 
 :no-lint