From b3e62f57c7e2a6674160a962782b6461e6bd1bfd Mon Sep 17 00:00:00 2001 From: Brian White Date: Wed, 9 Mar 2016 22:40:54 -0500 Subject: [PATCH] tools: improve js linter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-By: Johan Bergström --- Makefile | 13 ++- test-eslint.tap | 0 tools/jslint.js | 262 ++++++++++++++++++++++++++++++++++++++++++++++++ vcbuild.bat | 9 +- 4 files changed, 278 insertions(+), 6 deletions(-) create mode 100644 test-eslint.tap create mode 100644 tools/jslint.js diff --git a/Makefile b/Makefile index b4edaee1bf29ba..46b4bbf338f29e 100644 --- a/Makefile +++ b/Makefile @@ -592,8 +592,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 @@ -621,8 +625,7 @@ cpplint: @$(PYTHON) tools/cpplint.py $(CPPLINT_FILES) lint: jslint cpplint - -lint-ci: lint +lint-ci: jslint-ci cpplint .PHONY: lint cpplint jslint bench clean docopen docclean doc dist distclean \ check uninstall install install-includes install-bin all staticlib \ @@ -630,4 +633,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 diff --git a/test-eslint.tap b/test-eslint.tap new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/tools/jslint.js b/tools/jslint.js new file mode 100644 index 00000000000000..7cd2fd7bcb60f2 --- /dev/null +++ b/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; + } + }); +} diff --git a/vcbuild.bat b/vcbuild.bat index 144e5420634fdb..4bc73c496d4b41 100644 --- a/vcbuild.bat +++ b/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,9 +287,15 @@ python tools\test.py %test_args% goto jslint :jslint +if defined jslint_ci goto jslint-ci if not defined jslint goto exit 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 :create-msvs-files-failed