Skip to content

Commit 509b3b8

Browse files
apapirovskiMylesBorins
apapirovski
authored and
MylesBorins
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: #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 0ddb00d commit 509b3b8

22 files changed

Lines changed: 118 additions & 138 deletions

‎.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
@@ -429,8 +429,8 @@ function socketOnEnd(server, socket, parser, state) {
429429
state.outgoing[state.outgoing.length - 1]._last = true;
430430
} else if (socket._httpMessage) {
431431
socket._httpMessage._last = true;
432-
} else {
433-
if (socket.writable) socket.end();
432+
} else if (socket.writable) {
433+
socket.end();
434434
}
435435
}
436436

@@ -590,13 +590,11 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
590590
res.writeContinue();
591591
server.emit('request', req, res);
592592
}
593+
} else if (server.listenerCount('checkExpectation') > 0) {
594+
server.emit('checkExpectation', req, res);
593595
} else {
594-
if (server.listenerCount('checkExpectation') > 0) {
595-
server.emit('checkExpectation', req, res);
596-
} else {
597-
res.writeHead(417);
598-
res.end();
599-
}
596+
res.writeHead(417);
597+
res.end();
600598
}
601599
} else {
602600
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
@@ -323,11 +323,10 @@ exports.execFile = function(file /*, args, options, callback*/) {
323323
if (stdoutLen > options.maxBuffer) {
324324
ex = new Error('stdout maxBuffer exceeded');
325325
kill();
326+
} else if (encoding) {
327+
_stdout += chunk;
326328
} else {
327-
if (encoding)
328-
_stdout += chunk;
329-
else
330-
_stdout.push(chunk);
329+
_stdout.push(chunk);
331330
}
332331
});
333332
}
@@ -342,11 +341,10 @@ exports.execFile = function(file /*, args, options, callback*/) {
342341
if (stderrLen > options.maxBuffer) {
343342
ex = new Error('stderr maxBuffer exceeded');
344343
kill();
344+
} else if (encoding) {
345+
_stderr += chunk;
345346
} else {
346-
if (encoding)
347-
_stderr += chunk;
348-
else
349-
_stderr.push(chunk);
347+
_stderr.push(chunk);
350348
}
351349
});
352350
}

‎lib/events.js‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -264,13 +264,11 @@ function _addListener(target, type, listener, prepend) {
264264
// Adding the second element, need to change to array.
265265
existing = events[type] =
266266
prepend ? [listener, existing] : [existing, listener];
267-
} else {
268267
// If we've already got an array, just append.
269-
if (prepend) {
270-
existing.unshift(listener);
271-
} else {
272-
existing.push(listener);
273-
}
268+
} else if (prepend) {
269+
existing.unshift(listener);
270+
} else {
271+
existing.push(listener);
274272
}
275273

276274
// Check for listener leak

‎lib/fs.js‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1241,21 +1241,19 @@ function writeAll(fd, isUserFd, buffer, offset, length, position, callback) {
12411241
callback(writeErr);
12421242
});
12431243
}
1244-
} else {
1245-
if (written === length) {
1246-
if (isUserFd) {
1247-
callback(null);
1248-
} else {
1249-
fs.close(fd, callback);
1250-
}
1244+
} else if (written === length) {
1245+
if (isUserFd) {
1246+
callback(null);
12511247
} else {
1252-
offset += written;
1253-
length -= written;
1254-
if (position !== null) {
1255-
position += written;
1256-
}
1257-
writeAll(fd, isUserFd, buffer, offset, length, position, callback);
1248+
fs.close(fd, callback);
1249+
}
1250+
} else {
1251+
offset += written;
1252+
length -= written;
1253+
if (position !== null) {
1254+
position += written;
12581255
}
1256+
writeAll(fd, isUserFd, buffer, offset, length, position, callback);
12591257
}
12601258
});
12611259
}

0 commit comments

Comments
 (0)