Skip to content

Commit 713f6ff

Browse files
mscdexMylesBorins
authored andcommitted
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: #5596 PR-URL: #5638 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Johan Bergström <[email protected]>
1 parent 7ea5e43 commit 713f6ff

File tree

4 files changed

+278
-6
lines changed

4 files changed

+278
-6
lines changed

Makefile

+8-5
Original file line numberDiff line numberDiff line change
@@ -592,8 +592,12 @@ bench-idle:
592592
$(NODE) benchmark/idle_clients.js &
593593

594594
jslint:
595-
$(NODE) tools/eslint/bin/eslint.js benchmark lib src test tools/doc \
596-
tools/eslint-rules --rulesdir tools/eslint-rules
595+
$(NODE) tools/jslint.js -J benchmark lib src test tools/doc \
596+
tools/eslint-rules tools/jslint.js
597+
598+
jslint-ci:
599+
$(NODE) tools/jslint.js -f tap -o test-eslint.tap benchmark lib src test \
600+
tools/doc tools/eslint-rules tools/jslint.js
597601

598602
CPPLINT_EXCLUDE ?=
599603
CPPLINT_EXCLUDE += src/node_lttng.cc
@@ -621,13 +625,12 @@ cpplint:
621625
@$(PYTHON) tools/cpplint.py $(CPPLINT_FILES)
622626

623627
lint: jslint cpplint
624-
625-
lint-ci: lint
628+
lint-ci: jslint-ci cpplint
626629

627630
.PHONY: lint cpplint jslint bench clean docopen docclean doc dist distclean \
628631
check uninstall install install-includes install-bin all staticlib \
629632
dynamiclib test test-all test-addons build-addons website-upload pkg \
630633
blog blogclean tar binary release-only bench-http-simple bench-idle \
631634
bench-all bench bench-misc bench-array bench-buffer bench-net \
632635
bench-http bench-fs bench-tls cctest run-ci test-v8 test-v8-intl \
633-
test-v8-benchmarks test-v8-all v8 lint-ci bench-ci
636+
test-v8-benchmarks test-v8-all v8 lint-ci bench-ci jslint-ci

test-eslint.tap

Whitespace-only changes.

tools/jslint.js

+262
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,262 @@
1+
'use strict';
2+
3+
const rulesDirs = ['tools/eslint-rules'];
4+
// This is the maximum number of files to be linted per worker at any given time
5+
const maxWorkload = 40;
6+
7+
const cluster = require('cluster');
8+
const path = require('path');
9+
const fs = require('fs');
10+
const totalCPUs = require('os').cpus().length;
11+
12+
const CLIEngine = require('./eslint').CLIEngine;
13+
const glob = require('./eslint/node_modules/glob');
14+
15+
const cwd = process.cwd();
16+
const cli = new CLIEngine({
17+
rulePaths: rulesDirs
18+
});
19+
20+
if (cluster.isMaster) {
21+
var numCPUs = 1;
22+
const paths = [];
23+
var files = null;
24+
var totalPaths = 0;
25+
var failures = 0;
26+
var successes = 0;
27+
var lastLineLen = 0;
28+
var curPath = 'Starting ...';
29+
var showProgress = true;
30+
const globOptions = {
31+
nodir: true
32+
};
33+
const workerConfig = {};
34+
var startTime;
35+
var formatter;
36+
var outFn;
37+
var fd;
38+
var i;
39+
40+
// Check if spreading work among all cores/cpus
41+
if (process.argv.indexOf('-J') !== -1)
42+
numCPUs = totalCPUs;
43+
44+
// Check if spreading work among an explicit number of cores/cpus
45+
i = process.argv.indexOf('-j');
46+
if (i !== -1) {
47+
if (!process.argv[i + 1])
48+
throw new Error('Missing parallel job count');
49+
numCPUs = parseInt(process.argv[i + 1], 10);
50+
if (!isFinite(numCPUs) || numCPUs <= 0)
51+
throw new Error('Bad parallel job count');
52+
}
53+
54+
// Check for custom JSLint report formatter
55+
i = process.argv.indexOf('-f');
56+
if (i !== -1) {
57+
if (!process.argv[i + 1])
58+
throw new Error('Missing format name');
59+
const format = process.argv[i + 1];
60+
formatter = cli.getFormatter(format);
61+
if (!formatter)
62+
throw new Error('Invalid format name');
63+
// Automatically disable progress display
64+
showProgress = false;
65+
// Tell worker to send all results, not just linter errors
66+
workerConfig.sendAll = true;
67+
} else {
68+
// Use default formatter
69+
formatter = cli.getFormatter();
70+
}
71+
72+
// Check if outputting JSLint report to a file instead of stdout
73+
i = process.argv.indexOf('-o');
74+
if (i !== -1) {
75+
if (!process.argv[i + 1])
76+
throw new Error('Missing output filename');
77+
var outPath = process.argv[i + 1];
78+
if (!path.isAbsolute(outPath))
79+
outPath = path.join(cwd, outPath);
80+
fd = fs.openSync(outPath, 'w');
81+
outFn = function(str) {
82+
fs.writeSync(fd, str, 'utf8');
83+
};
84+
process.on('exit', function() {
85+
fs.closeSync(fd);
86+
});
87+
} else {
88+
outFn = function(str) {
89+
process.stdout.write(str);
90+
};
91+
}
92+
93+
// Process the rest of the arguments as paths to lint, ignoring any unknown
94+
// flags
95+
for (i = 2; i < process.argv.length; ++i) {
96+
if (process.argv[i][0] === '-') {
97+
switch (process.argv[i]) {
98+
case '-f': // Skip format name
99+
case '-o': // Skip filename
100+
case '-j': // Skip parallel job count number
101+
++i;
102+
break;
103+
}
104+
continue;
105+
}
106+
paths.push(process.argv[i]);
107+
}
108+
109+
if (paths.length === 0)
110+
return;
111+
totalPaths = paths.length;
112+
113+
if (showProgress) {
114+
// Start the progress display update timer when the first worker is ready
115+
cluster.once('online', function(worker) {
116+
startTime = process.hrtime();
117+
setInterval(printProgress, 1000).unref();
118+
printProgress();
119+
});
120+
}
121+
122+
cluster.on('online', function(worker) {
123+
// Configure worker and give it some initial work to do
124+
worker.send(workerConfig);
125+
sendWork(worker);
126+
});
127+
128+
cluster.on('message', function(worker, results) {
129+
if (typeof results !== 'number') {
130+
// The worker sent us results that are not all successes
131+
if (!workerConfig.sendAll)
132+
failures += results.length;
133+
outFn(formatter(results) + '\r\n');
134+
printProgress();
135+
} else {
136+
successes += results;
137+
}
138+
// Try to give the worker more work to do
139+
sendWork(worker);
140+
});
141+
142+
process.on('exit', function() {
143+
if (showProgress) {
144+
curPath = 'Done';
145+
printProgress();
146+
outFn('\r\n');
147+
}
148+
process.exit(failures ? 1 : 0);
149+
});
150+
151+
for (i = 0; i < numCPUs; ++i)
152+
cluster.fork();
153+
154+
function sendWork(worker) {
155+
if (!files || !files.length) {
156+
// We either just started or we have no more files to lint for the current
157+
// path. Find the next path that has some files to be linted.
158+
while (paths.length) {
159+
var dir = paths.shift();
160+
curPath = dir;
161+
if (dir.indexOf('/') > 0)
162+
dir = path.join(cwd, dir);
163+
const patterns = cli.resolveFileGlobPatterns([dir]);
164+
dir = path.resolve(patterns[0]);
165+
files = glob.sync(dir, globOptions);
166+
if (files.length)
167+
break;
168+
}
169+
if ((!files || !files.length) && !paths.length) {
170+
// We exhausted all input paths and thus have nothing left to do, so end
171+
// the worker
172+
return worker.disconnect();
173+
}
174+
}
175+
// Give the worker an equal portion of the work left for the current path,
176+
// but not exceeding a maximum file count in order to help keep *all*
177+
// workers busy most of the time instead of only a minority doing most of
178+
// the work.
179+
const sliceLen = Math.min(maxWorkload, Math.ceil(files.length / numCPUs));
180+
var slice;
181+
if (sliceLen === files.length) {
182+
// Micro-ptimization to avoid splicing to an empty array
183+
slice = files;
184+
files = null;
185+
} else {
186+
slice = files.splice(0, sliceLen);
187+
}
188+
worker.send(slice);
189+
}
190+
191+
function printProgress() {
192+
if (!showProgress)
193+
return;
194+
195+
// Clear line
196+
outFn('\r' + ' '.repeat(lastLineLen) + '\r');
197+
198+
// Calculate and format the data for displaying
199+
const elapsed = process.hrtime(startTime)[0];
200+
const mins = padString(Math.floor(elapsed / 60), 2, '0');
201+
const secs = padString(elapsed % 60, 2, '0');
202+
const passed = padString(successes, 6, ' ');
203+
const failed = padString(failures, 6, ' ');
204+
var pct = Math.ceil(((totalPaths - paths.length) / totalPaths) * 100);
205+
pct = padString(pct, 3, ' ');
206+
207+
var line = `[${mins}:${secs}|%${pct}|+${passed}|-${failed}]: ${curPath}`;
208+
209+
// Truncate line like cpplint does in case it gets too long
210+
if (line.length > 75)
211+
line = line.slice(0, 75) + '...';
212+
213+
// Store the line length so we know how much to erase the next time around
214+
lastLineLen = line.length;
215+
216+
outFn(line);
217+
}
218+
219+
function padString(str, len, chr) {
220+
str = '' + str;
221+
if (str.length >= len)
222+
return str;
223+
return chr.repeat(len - str.length) + str;
224+
}
225+
} else {
226+
// Worker
227+
228+
var config = {};
229+
process.on('message', function(files) {
230+
if (files instanceof Array) {
231+
// Lint some files
232+
const report = cli.executeOnFiles(files);
233+
if (config.sendAll) {
234+
// Return both success and error results
235+
236+
const results = report.results;
237+
// Silence warnings for files with no errors while keeping the "ok"
238+
// status
239+
if (report.warningCount > 0) {
240+
for (var i = 0; i < results.length; ++i) {
241+
const result = results[i];
242+
if (result.errorCount === 0 && result.warningCount > 0) {
243+
result.warningCount = 0;
244+
result.messages = [];
245+
}
246+
}
247+
}
248+
process.send(results);
249+
} else if (report.errorCount === 0) {
250+
// No errors, return number of successful lint operations
251+
process.send(files.length);
252+
} else {
253+
// One or more errors, return the error results only
254+
process.send(CLIEngine.getErrorResults(report.results));
255+
}
256+
} else if (typeof files === 'object') {
257+
// The master process is actually sending us our configuration and not a
258+
// list of files to lint
259+
config = files;
260+
}
261+
});
262+
}

vcbuild.bat

+8-1
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ if /i "%1"=="test-pummel" set test_args=%test_args% pummel&goto arg-ok
6666
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
6767
if /i "%1"=="test-known-issues" set test_args=%test_args% known_issues --expect-fail&goto arg-ok
6868
if /i "%1"=="jslint" set jslint=1&goto arg-ok
69+
if /i "%1"=="jslint-ci" set jslint_ci=1&goto arg-ok
6970
if /i "%1"=="msi" set msi=1&set licensertf=1&set download_arg="--download=all"&set i18n_arg=small-icu&goto arg-ok
7071
if /i "%1"=="build-release" set build_release=1&goto arg-ok
7172
if /i "%1"=="upload" set upload=1&goto arg-ok
@@ -286,9 +287,15 @@ python tools\test.py %test_args%
286287
goto jslint
287288

288289
:jslint
290+
if defined jslint_ci goto jslint-ci
289291
if not defined jslint goto exit
290292
echo running jslint
291-
%config%\node tools\eslint\bin\eslint.js benchmark lib src test tools\doc tools\eslint-rules --rulesdir tools\eslint-rules
293+
%config%\node tools\jslint.js -J benchmark lib src test tools\doc tools\eslint-rules tools\jslint.js
294+
goto exit
295+
296+
:jslint-ci
297+
echo running jslint-ci
298+
%config%\node tools\jslint.js -J -f tap -o test-eslint.tap benchmark lib src test tools\doc tools\eslint-rules tools\jslint.js
292299
goto exit
293300

294301
:create-msvs-files-failed

0 commit comments

Comments
 (0)