Skip to content

Commit f201b01

Browse files
mscdexMyles Borins
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 <info@bnoordhuis.nl> Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
1 parent be070d7 commit f201b01

4 files changed

Lines changed: 278 additions & 6 deletions

File tree

‎Makefile‎

Lines changed: 8 additions & 5 deletions
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‎

Lines changed: 262 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,262 @@
1+
'use strict';
2+
3+
construlesDirs=['tools/eslint-rules'];
4+
// This is the maximum number of files to be linted per worker at any given time
5+
constmaxWorkload=40;
6+
7+
constcluster=require('cluster');
8+
constpath=require('path');
9+
constfs=require('fs');
10+
consttotalCPUs=require('os').cpus().length;
11+
12+
constCLIEngine=require('./eslint').CLIEngine;
13+
constglob=require('./eslint/node_modules/glob');
14+
15+
constcwd=process.cwd();
16+
constcli=newCLIEngine({
17+
rulePaths: rulesDirs
18+
});
19+
20+
if(cluster.isMaster){
21+
varnumCPUs=1;
22+
constpaths=[];
23+
varfiles=null;
24+
vartotalPaths=0;
25+
varfailures=0;
26+
varsuccesses=0;
27+
varlastLineLen=0;
28+
varcurPath='Starting ...';
29+
varshowProgress=true;
30+
constglobOptions={
31+
nodir: true
32+
};
33+
constworkerConfig={};
34+
varstartTime;
35+
varformatter;
36+
varoutFn;
37+
varfd;
38+
vari;
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+
thrownewError('Missing parallel job count');
49+
numCPUs=parseInt(process.argv[i+1],10);
50+
if(!isFinite(numCPUs)||numCPUs<=0)
51+
thrownewError('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+
thrownewError('Missing format name');
59+
constformat=process.argv[i+1];
60+
formatter=cli.getFormatter(format);
61+
if(!formatter)
62+
thrownewError('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+
thrownewError('Missing output filename');
77+
varoutPath=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(typeofresults!=='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+
functionsendWork(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+
vardir=paths.shift();
160+
curPath=dir;
161+
if(dir.indexOf('/')>0)
162+
dir=path.join(cwd,dir);
163+
constpatterns=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+
returnworker.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+
constsliceLen=Math.min(maxWorkload,Math.ceil(files.length/numCPUs));
180+
varslice;
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+
functionprintProgress(){
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+
constelapsed=process.hrtime(startTime)[0];
200+
constmins=padString(Math.floor(elapsed/60),2,'0');
201+
constsecs=padString(elapsed%60,2,'0');
202+
constpassed=padString(successes,6,' ');
203+
constfailed=padString(failures,6,' ');
204+
varpct=Math.ceil(((totalPaths-paths.length)/totalPaths)*100);
205+
pct=padString(pct,3,' ');
206+
207+
varline=`[${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+
functionpadString(str,len,chr){
220+
str=''+str;
221+
if(str.length>=len)
222+
returnstr;
223+
returnchr.repeat(len-str.length)+str;
224+
}
225+
}else{
226+
// Worker
227+
228+
varconfig={};
229+
process.on('message',function(files){
230+
if(filesinstanceofArray){
231+
// Lint some files
232+
constreport=cli.executeOnFiles(files);
233+
if(config.sendAll){
234+
// Return both success and error results
235+
236+
constresults=report.results;
237+
// Silence warnings for files with no errors while keeping the "ok"
238+
// status
239+
if(report.warningCount>0){
240+
for(vari=0;i<results.length;++i){
241+
constresult=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+
}elseif(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+
}elseif(typeoffiles==='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‎

Lines changed: 8 additions & 1 deletion
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"settest_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"settest_args=%test_args% known_issues --expect-fail&goto arg-ok
6868
if /i "%1"=="jslint"setjslint=1&goto arg-ok
69+
if /i "%1"=="jslint-ci"setjslint_ci=1&goto arg-ok
6970
if /i "%1"=="msi"setmsi=1&set licensertf=1&set download_arg="--download=all"&set i18n_arg=small-icu&goto arg-ok
7071
if /i "%1"=="build-release"setbuild_release=1&goto arg-ok
7172
if /i "%1"=="upload"setupload=1&goto arg-ok
@@ -286,9 +287,15 @@ python tools\test.py %test_args%
286287
goto jslint
287288

288289
:jslint
290+
ifdefined jslint_ci goto jslint-ci
289291
ifnotdefined jslint gotoexit
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+
gotoexit
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
gotoexit
293300

294301
:create-msvs-files-failed

0 commit comments

Comments
 (0)