Skip to content
This repository has been archived by the owner on Aug 31, 2018. It is now read-only.

Commit

Permalink
tools: enable additional eslint rules
Browse files Browse the repository at this point in the history
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/node#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>
  • Loading branch information
apapirovski authored and addaleax committed Dec 7, 2017
1 parent 1173d25 commit 2549fa6
Show file tree
Hide file tree
Showing 22 changed files with 118 additions and 138 deletions.
4 changes: 4 additions & 0 deletions .eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ overrides:
rules:
# Possible Errors
# http://eslint.org/docs/rules/#possible-errors
for-direction: error
no-control-regex: error
no-debugger: error
no-dupe-args: error
Expand All @@ -41,6 +42,7 @@ rules:

# Best Practices
# http://eslint.org/docs/rules/#best-practices
accessor-pairs: error
dot-location: [error, property]
eqeqeq: [error, smart]
no-fallthrough: error
Expand Down Expand Up @@ -122,6 +124,7 @@ rules:
ignoreUrls: true,
tabWidth: 2}]
new-parens: error
no-lonely-if: error
no-mixed-spaces-and-tabs: error
no-multiple-empty-lines: [error, {max: 2, maxEOF: 0, maxBOF: 0}]
no-restricted-syntax: [error, {
Expand Down Expand Up @@ -172,6 +175,7 @@ rules:
no-this-before-super: error
prefer-const: [error, {ignoreReadBeforeAssign: true}]
rest-spread-spacing: error
symbol-description: error
template-curly-spacing: error

# Custom rules in tools/eslint-rules
Expand Down
1 change: 1 addition & 0 deletions doc/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ rules:
no-undef: off
no-unused-vars: off
strict: off
symbol-description: off

# add new ECMAScript features gradually
no-var: error
Expand Down
5 changes: 2 additions & 3 deletions lib/_http_incoming.js
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,9 @@ function _addHeaderLine(field, value, dest) {
} else {
dest['set-cookie'] = [value];
}
} else {
} else if (dest[field] === undefined) {
// Drop duplicates
if (dest[field] === undefined)
dest[field] = value;
dest[field] = value;
}
}

Expand Down
24 changes: 11 additions & 13 deletions lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -404,20 +404,18 @@ function _storeHeader(firstLine, headers) {
this.chunkedEncoding = false;
} else if (!this.useChunkedEncodingByDefault) {
this._last = true;
} else if (!state.trailer &&
!this._removedContLen &&
typeof this._contentLength === 'number') {
state.header += 'Content-Length: ' + this._contentLength + CRLF;
} else if (!this._removedTE) {
state.header += 'Transfer-Encoding: chunked\r\n';
this.chunkedEncoding = true;
} else {
if (!state.trailer &&
!this._removedContLen &&
typeof this._contentLength === 'number') {
state.header += 'Content-Length: ' + this._contentLength + CRLF;
} else if (!this._removedTE) {
state.header += 'Transfer-Encoding: chunked\r\n';
this.chunkedEncoding = true;
} else {
// We should only be able to get here if both Content-Length and
// Transfer-Encoding are removed by the user.
// See: test/parallel/test-http-remove-header-stays-removed.js
debug('Both Content-Length and Transfer-Encoding are removed');
}
// We should only be able to get here if both Content-Length and
// Transfer-Encoding are removed by the user.
// See: test/parallel/test-http-remove-header-stays-removed.js
debug('Both Content-Length and Transfer-Encoding are removed');
}
}

Expand Down
14 changes: 6 additions & 8 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -433,8 +433,8 @@ function socketOnEnd(server, socket, parser, state) {
state.outgoing[state.outgoing.length - 1]._last = true;
} else if (socket._httpMessage) {
socket._httpMessage._last = true;
} else {
if (socket.writable) socket.end();
} else if (socket.writable) {
socket.end();
}
}

Expand Down Expand Up @@ -602,13 +602,11 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
res.writeContinue();
server.emit('request', req, res);
}
} else if (server.listenerCount('checkExpectation') > 0) {
server.emit('checkExpectation', req, res);
} else {
if (server.listenerCount('checkExpectation') > 0) {
server.emit('checkExpectation', req, res);
} else {
res.writeHead(417);
res.end();
}
res.writeHead(417);
res.end();
}
} else {
server.emit('request', req, res);
Expand Down
25 changes: 12 additions & 13 deletions lib/_stream_readable.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,20 +40,19 @@ const kProxyEvents = ['error', 'close', 'destroy', 'pause', 'resume'];
function prependListener(emitter, event, fn) {
// Sadly this is not cacheable as some libraries bundle their own
// event emitter implementation with them.
if (typeof emitter.prependListener === 'function') {
if (typeof emitter.prependListener === 'function')
return emitter.prependListener(event, fn);
} else {
// This is a hack to make sure that our error handler is attached before any
// userland ones. NEVER DO THIS. This is here only because this code needs
// to continue to work with older versions of Node.js that do not include
// the prependListener() method. The goal is to eventually remove this hack.
if (!emitter._events || !emitter._events[event])
emitter.on(event, fn);
else if (Array.isArray(emitter._events[event]))
emitter._events[event].unshift(fn);
else
emitter._events[event] = [fn, emitter._events[event]];
}

// This is a hack to make sure that our error handler is attached before any
// userland ones. NEVER DO THIS. This is here only because this code needs
// to continue to work with older versions of Node.js that do not include
// the prependListener() method. The goal is to eventually remove this hack.
if (!emitter._events || !emitter._events[event])
emitter.on(event, fn);
else if (Array.isArray(emitter._events[event]))
emitter._events[event].unshift(fn);
else
emitter._events[event] = [fn, emitter._events[event]];
}

function ReadableState(options, stream) {
Expand Down
4 changes: 2 additions & 2 deletions lib/_tls_legacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -878,8 +878,8 @@ SecurePair.prototype.error = function(returnOnly) {
/peer did not return a certificate/.test(err.message)) {
// Not really an error.
this.destroy();
} else {
if (!returnOnly) this.cleartext.emit('error', err);
} else if (!returnOnly) {
this.cleartext.emit('error', err);
}
return err;
};
Expand Down
14 changes: 6 additions & 8 deletions lib/child_process.js
Original file line number Diff line number Diff line change
Expand Up @@ -324,11 +324,10 @@ exports.execFile = function(file /*, args, options, callback*/) {
if (stdoutLen > options.maxBuffer) {
ex = new Error('stdout maxBuffer exceeded');
kill();
} else if (encoding) {
_stdout += chunk;
} else {
if (encoding)
_stdout += chunk;
else
_stdout.push(chunk);
_stdout.push(chunk);
}
});
}
Expand All @@ -343,11 +342,10 @@ exports.execFile = function(file /*, args, options, callback*/) {
if (stderrLen > options.maxBuffer) {
ex = new Error('stderr maxBuffer exceeded');
kill();
} else if (encoding) {
_stderr += chunk;
} else {
if (encoding)
_stderr += chunk;
else
_stderr.push(chunk);
_stderr.push(chunk);
}
});
}
Expand Down
10 changes: 4 additions & 6 deletions lib/extras/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -280,13 +280,11 @@ function _addListener(target, type, listener, prepend) {
// Adding the second element, need to change to array.
existing = events[type] =
prepend ? [listener, existing] : [existing, listener];
} else {
} else if (prepend) {
// If we've already got an array, just append.
if (prepend) {
existing.unshift(listener);
} else {
existing.push(listener);
}
existing.unshift(listener);
} else {
existing.push(listener);
}

// Check for listener leak
Expand Down
24 changes: 11 additions & 13 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -1262,21 +1262,19 @@ function writeAll(fd, isUserFd, buffer, offset, length, position, callback) {
callback(writeErr);
});
}
} else {
if (written === length) {
if (isUserFd) {
callback(null);
} else {
fs.close(fd, callback);
}
} else if (written === length) {
if (isUserFd) {
callback(null);
} else {
offset += written;
length -= written;
if (position !== null) {
position += written;
}
writeAll(fd, isUserFd, buffer, offset, length, position, callback);
fs.close(fd, callback);
}
} else {
offset += written;
length -= written;
if (position !== null) {
position += written;
}
writeAll(fd, isUserFd, buffer, offset, length, position, callback);
}
});
}
Expand Down
11 changes: 5 additions & 6 deletions lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,11 @@ function onSessionHeaders(id, cat, flags, headers) {
}
} else if (cat === NGHTTP2_HCAT_PUSH_RESPONSE) {
event = 'push';
} else { // cat === NGHTTP2_HCAT_HEADERS:
if (!endOfStream && status !== undefined && status >= 200) {
event = 'response';
} else {
event = endOfStream ? 'trailers' : 'headers';
}
// cat === NGHTTP2_HCAT_HEADERS:
} else if (!endOfStream && status !== undefined && status >= 200) {
event = 'response';
} else {
event = endOfStream ? 'trailers' : 'headers';
}
debug(`[${sessionName(owner[kType])}] emitting stream '${event}' event`);
process.nextTick(emit, stream, event, obj, flags, headers);
Expand Down
18 changes: 8 additions & 10 deletions lib/path.js
Original file line number Diff line number Diff line change
Expand Up @@ -455,17 +455,15 @@ const win32 = {
} else {
return '';
}
} else if (isAbsolute) {
if (tail.length > 0)
return device + '\\' + tail;
else
return device + '\\';
} else if (tail.length > 0) {
return device + tail;
} else {
if (isAbsolute) {
if (tail.length > 0)
return device + '\\' + tail;
else
return device + '\\';
} else if (tail.length > 0) {
return device + tail;
} else {
return device;
}
return device;
}
},

Expand Down
5 changes: 2 additions & 3 deletions lib/querystring.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,9 +315,8 @@ function parse(qs, sep, eq, options) {
sepIdx = eqIdx = 0;
continue;
}
} else {
if (lastPos < end)
value += qs.slice(lastPos, end);
} else if (lastPos < end) {
value += qs.slice(lastPos, end);
}

if (key.length > 0 && keyEncoded)
Expand Down
16 changes: 7 additions & 9 deletions lib/url.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,16 +128,14 @@ Url.prototype.parse = function parse(url, parseQueryString, slashesDenoteHost) {
if (isWs)
continue;
lastPos = start = i;
} else {
if (inWs) {
if (!isWs) {
end = -1;
inWs = false;
}
} else if (isWs) {
end = i;
inWs = true;
} else if (inWs) {
if (!isWs) {
end = -1;
inWs = false;
}
} else if (isWs) {
end = i;
inWs = true;
}

// Only convert backslashes while we haven't seen a split character
Expand Down
1 change: 1 addition & 0 deletions test/.eslintrc.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ rules:
# http://eslint.org/docs/rules/#ecmascript-6
no-var: error
prefer-const: error
symbol-description: off

# Custom rules in tools/eslint-rules
prefer-assert-iferror: error
Expand Down
7 changes: 3 additions & 4 deletions test/abort/test-abort-uncaught-exception.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,10 @@ function run(flags, signals) {
assert.strictEqual(code, 0xC0000005);
else
assert.strictEqual(code, 1);
} else if (signals) {
assert(signals.includes(sig), `Unexpected signal ${sig}`);
} else {
if (signals)
assert(signals.includes(sig), `Unexpected signal ${sig}`);
else
assert.strictEqual(sig, null);
assert.strictEqual(sig, null);
}
}));
}
8 changes: 3 additions & 5 deletions test/parallel/test-cluster-disconnect-unshared-tcp.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,8 @@ if (cluster.isMaster) {
unbound.disconnect();
unbound.on('disconnect', cluster.disconnect);
}
} else {
if (process.env.BOUND === 'y') {
const source = net.createServer();
} else if (process.env.BOUND === 'y') {
const source = net.createServer();

source.listen(0);
}
source.listen(0);
}
8 changes: 3 additions & 5 deletions test/parallel/test-cluster-disconnect-unshared-udp.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,8 @@ if (cluster.isMaster) {
unbound.disconnect();
unbound.on('disconnect', cluster.disconnect);
}
} else {
if (process.env.BOUND === 'y') {
const source = dgram.createSocket('udp4');
} else if (process.env.BOUND === 'y') {
const source = dgram.createSocket('udp4');

source.bind(0);
}
source.bind(0);
}
22 changes: 10 additions & 12 deletions test/parallel/test-cluster-worker-destroy.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,15 @@ if (cluster.isMaster) {
worker.on('disconnect', common.mustCall());
worker.on('exit', common.mustCall());
});
} else {
if (cluster.worker.id === 1) {
// Call destroy when worker is disconnected
cluster.worker.process.on('disconnect', function() {
cluster.worker.destroy();
});

const w = cluster.worker.disconnect();
assert.strictEqual(w, cluster.worker);
} else {
// Call destroy when worker is not disconnected yet
} else if (cluster.worker.id === 1) {
// Call destroy when worker is disconnected
cluster.worker.process.on('disconnect', function() {
cluster.worker.destroy();
}
});

const w = cluster.worker.disconnect();
assert.strictEqual(w, cluster.worker);
} else {
// Call destroy when worker is not disconnected yet
cluster.worker.destroy();
}
Loading

0 comments on commit 2549fa6

Please sign in to comment.