Skip to content

Commit

Permalink
tools: improve js linter
Browse files Browse the repository at this point in the history
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: #5596
PR-URL: #5638
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
  • Loading branch information
mscdex authored and jasnell committed Apr 26, 2016
1 parent 5cce87d commit 396fbeb
Show file tree
Hide file tree
Showing 3 changed files with 279 additions and 5 deletions.
13 changes: 9 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -633,19 +637,20 @@ 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 \
dynamiclib test test-all test-addons build-addons website-upload pkg \
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
262 changes: 262 additions & 0 deletions tools/jslint.js
Original file line number Diff line number Diff line change
@@ -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;
}
});
}
9 changes: 8 additions & 1 deletion vcbuild.bat
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 396fbeb

Please sign in to comment.