Skip to content

Commit 3c0ebf5

Browse files
apapirovskiapapirovski
committed
tools: enable additional eslint rules
Enable additional rules that node either already adheres to or it makes sense to do so going forward: for-direction, accessor-pairs, no-lonely-if and symbol-description. Fix all instances of no-lonely-if in lib & test and disable accessor-pairs in test-util-inspect. PR-URL: nodejs#16243 Refs: https://eslint.org/docs/rules/for-direction Refs: https://eslint.org/docs/rules/accessor-pairs Refs: https://eslint.org/docs/rules/no-lonely-if Refs: https://eslint.org/docs/rules/symbol-description Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent bf1bace commit 3c0ebf5

22 files changed

+118
-138
lines changed

‎.eslintrc.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ overrides:
1818
rules:
1919
# Possible Errors
2020
# http://eslint.org/docs/rules/#possible-errors
21+
for-direction: error
2122
no-control-regex: error
2223
no-debugger: error
2324
no-dupe-args: error
@@ -41,6 +42,7 @@ rules:
4142

4243
# Best Practices
4344
# http://eslint.org/docs/rules/#best-practices
45+
accessor-pairs: error
4446
dot-location: [error, property]
4547
eqeqeq: [error, smart]
4648
no-fallthrough: error
@@ -122,6 +124,7 @@ rules:
122124
ignoreUrls: true,
123125
tabWidth: 2}]
124126
new-parens: error
127+
no-lonely-if: error
125128
no-mixed-spaces-and-tabs: error
126129
no-multiple-empty-lines: [error, {max: 2, maxEOF: 0, maxBOF: 0}]
127130
no-restricted-syntax: [error, {
@@ -172,6 +175,7 @@ rules:
172175
no-this-before-super: error
173176
prefer-const: [error, {ignoreReadBeforeAssign: true}]
174177
rest-spread-spacing: error
178+
symbol-description: error
175179
template-curly-spacing: error
176180

177181
# Custom rules in tools/eslint-rules

‎doc/.eslintrc.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ rules:
66
no-undef: off
77
no-unused-vars: off
88
strict: off
9+
symbol-description: off
910

1011
# add new ECMAScript features gradually
1112
no-var: error

‎lib/_http_incoming.js‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -301,10 +301,9 @@ function _addHeaderLine(field, value, dest) {
301301
} else {
302302
dest['set-cookie'] = [value];
303303
}
304-
} else {
304+
} else if (dest[field] === undefined) {
305305
// Drop duplicates
306-
if (dest[field] === undefined)
307-
dest[field] = value;
306+
dest[field] = value;
308307
}
309308
}
310309

‎lib/_http_outgoing.js‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -404,20 +404,18 @@ function _storeHeader(firstLine, headers) {
404404
this.chunkedEncoding = false;
405405
} else if (!this.useChunkedEncodingByDefault) {
406406
this._last = true;
407+
} else if (!state.trailer &&
408+
!this._removedContLen &&
409+
typeof this._contentLength === 'number') {
410+
state.header += 'Content-Length: ' + this._contentLength + CRLF;
411+
} else if (!this._removedTE) {
412+
state.header += 'Transfer-Encoding: chunked\r\n';
413+
this.chunkedEncoding = true;
407414
} else {
408-
if (!state.trailer &&
409-
!this._removedContLen &&
410-
typeof this._contentLength === 'number') {
411-
state.header += 'Content-Length: ' + this._contentLength + CRLF;
412-
} else if (!this._removedTE) {
413-
state.header += 'Transfer-Encoding: chunked\r\n';
414-
this.chunkedEncoding = true;
415-
} else {
416-
// We should only be able to get here if both Content-Length and
417-
// Transfer-Encoding are removed by the user.
418-
// See: test/parallel/test-http-remove-header-stays-removed.js
419-
debug('Both Content-Length and Transfer-Encoding are removed');
420-
}
415+
// We should only be able to get here if both Content-Length and
416+
// Transfer-Encoding are removed by the user.
417+
// See: test/parallel/test-http-remove-header-stays-removed.js
418+
debug('Both Content-Length and Transfer-Encoding are removed');
421419
}
422420
}
423421

‎lib/_http_server.js‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -433,8 +433,8 @@ function socketOnEnd(server, socket, parser, state) {
433433
state.outgoing[state.outgoing.length - 1]._last = true;
434434
} else if (socket._httpMessage) {
435435
socket._httpMessage._last = true;
436-
} else {
437-
if (socket.writable) socket.end();
436+
} else if (socket.writable) {
437+
socket.end();
438438
}
439439
}
440440

@@ -602,13 +602,11 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
602602
res.writeContinue();
603603
server.emit('request', req, res);
604604
}
605+
} else if (server.listenerCount('checkExpectation') > 0) {
606+
server.emit('checkExpectation', req, res);
605607
} else {
606-
if (server.listenerCount('checkExpectation') > 0) {
607-
server.emit('checkExpectation', req, res);
608-
} else {
609-
res.writeHead(417);
610-
res.end();
611-
}
608+
res.writeHead(417);
609+
res.end();
612610
}
613611
} else {
614612
server.emit('request', req, res);

‎lib/_stream_readable.js‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,20 +40,19 @@ const kProxyEvents = ['error', 'close', 'destroy', 'pause', 'resume'];
4040
function prependListener(emitter, event, fn) {
4141
// Sadly this is not cacheable as some libraries bundle their own
4242
// event emitter implementation with them.
43-
if (typeof emitter.prependListener === 'function') {
43+
if (typeof emitter.prependListener === 'function')
4444
return emitter.prependListener(event, fn);
45-
} else {
46-
// This is a hack to make sure that our error handler is attached before any
47-
// userland ones. NEVER DO THIS. This is here only because this code needs
48-
// to continue to work with older versions of Node.js that do not include
49-
// the prependListener() method. The goal is to eventually remove this hack.
50-
if (!emitter._events || !emitter._events[event])
51-
emitter.on(event, fn);
52-
else if (Array.isArray(emitter._events[event]))
53-
emitter._events[event].unshift(fn);
54-
else
55-
emitter._events[event] = [fn, emitter._events[event]];
56-
}
45+
46+
// This is a hack to make sure that our error handler is attached before any
47+
// userland ones. NEVER DO THIS. This is here only because this code needs
48+
// to continue to work with older versions of Node.js that do not include
49+
// the prependListener() method. The goal is to eventually remove this hack.
50+
if (!emitter._events || !emitter._events[event])
51+
emitter.on(event, fn);
52+
else if (Array.isArray(emitter._events[event]))
53+
emitter._events[event].unshift(fn);
54+
else
55+
emitter._events[event] = [fn, emitter._events[event]];
5756
}
5857

5958
function ReadableState(options, stream) {

‎lib/_tls_legacy.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -878,8 +878,8 @@ SecurePair.prototype.error = function(returnOnly) {
878878
/peer did not return a certificate/.test(err.message)) {
879879
// Not really an error.
880880
this.destroy();
881-
} else {
882-
if (!returnOnly) this.cleartext.emit('error', err);
881+
} else if (!returnOnly) {
882+
this.cleartext.emit('error', err);
883883
}
884884
return err;
885885
};

‎lib/child_process.js‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -324,11 +324,10 @@ exports.execFile = function(file /*, args, options, callback*/) {
324324
if (stdoutLen > options.maxBuffer) {
325325
ex = new Error('stdout maxBuffer exceeded');
326326
kill();
327+
} else if (encoding) {
328+
_stdout += chunk;
327329
} else {
328-
if (encoding)
329-
_stdout += chunk;
330-
else
331-
_stdout.push(chunk);
330+
_stdout.push(chunk);
332331
}
333332
});
334333
}
@@ -343,11 +342,10 @@ exports.execFile = function(file /*, args, options, callback*/) {
343342
if (stderrLen > options.maxBuffer) {
344343
ex = new Error('stderr maxBuffer exceeded');
345344
kill();
345+
} else if (encoding) {
346+
_stderr += chunk;
346347
} else {
347-
if (encoding)
348-
_stderr += chunk;
349-
else
350-
_stderr.push(chunk);
348+
_stderr.push(chunk);
351349
}
352350
});
353351
}

‎lib/events.js‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -280,13 +280,11 @@ function _addListener(target, type, listener, prepend) {
280280
// Adding the second element, need to change to array.
281281
existing = events[type] =
282282
prepend ? [listener, existing] : [existing, listener];
283-
} else {
284283
// If we've already got an array, just append.
285-
if (prepend) {
286-
existing.unshift(listener);
287-
} else {
288-
existing.push(listener);
289-
}
284+
} else if (prepend) {
285+
existing.unshift(listener);
286+
} else {
287+
existing.push(listener);
290288
}
291289

292290
// Check for listener leak

‎lib/fs.js‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1262,21 +1262,19 @@ function writeAll(fd, isUserFd, buffer, offset, length, position, callback) {
12621262
callback(writeErr);
12631263
});
12641264
}
1265-
} else {
1266-
if (written === length) {
1267-
if (isUserFd) {
1268-
callback(null);
1269-
} else {
1270-
fs.close(fd, callback);
1271-
}
1265+
} else if (written === length) {
1266+
if (isUserFd) {
1267+
callback(null);
12721268
} else {
1273-
offset += written;
1274-
length -= written;
1275-
if (position !== null) {
1276-
position += written;
1277-
}
1278-
writeAll(fd, isUserFd, buffer, offset, length, position, callback);
1269+
fs.close(fd, callback);
1270+
}
1271+
} else {
1272+
offset += written;
1273+
length -= written;
1274+
if (position !== null) {
1275+
position += written;
12791276
}
1277+
writeAll(fd, isUserFd, buffer, offset, length, position, callback);
12801278
}
12811279
});
12821280
}

0 commit comments

Comments
 (0)