Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tools: enable no-unused-expressions lint rule #36248

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.js
Expand Up @@ -227,6 +227,7 @@ module.exports = {
'no-unreachable': 'error',
'no-unsafe-finally': 'error',
'no-unsafe-negation': 'error',
'no-unused-expressions': ['error', { allowShortCircuit: true }],
'no-unused-labels': 'error',
'no-unused-vars': ['error', { args: 'none', caughtErrors: 'all' }],
'no-use-before-define': ['error', {
Expand Down
1 change: 0 additions & 1 deletion benchmark/_benchmark_progress.js
Expand Up @@ -39,7 +39,6 @@ class BenchmarkProgress {
this.completedConfig = 0;
// Total number of configurations for the current file
this.scheduledConfig = 0;
this.interval; // Updates the elapsed time.
}

startQueue(index) {
Expand Down
6 changes: 4 additions & 2 deletions benchmark/es/destructuring-bench.js
Expand Up @@ -12,7 +12,8 @@ function runSwapManual(n) {
let x, y, r;
bench.start();
for (let i = 0; i < n; i++) {
x = 1, y = 2;
x = 1;
y = 2;
r = x;
x = y;
y = r;
Expand All @@ -26,7 +27,8 @@ function runSwapDestructured(n) {
let x, y;
bench.start();
for (let i = 0; i < n; i++) {
x = 1, y = 2;
x = 1;
y = 2;
[x, y] = [y, x];
assert.strictEqual(x, 2);
assert.strictEqual(y, 1);
Expand Down
2 changes: 1 addition & 1 deletion benchmark/perf_hooks/bench-eventlooputil.js
Expand Up @@ -33,7 +33,7 @@ function main({ method, n }) {
function benchIdleTime(n) {
bench.start();
for (let i = 0; i < n; i++)
nodeTiming.idleTime;
nodeTiming.idleTime; // eslint-disable-line no-unused-expressions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be wrapped inside a noop or would it significantly affect the benchmark?

+function noop() {
+    return;
+}
...
-    nodeTiming.idleTime; // eslint-disable-line no-unused-expressions
+    noop(nodeTiming.idleTime);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to explicitly disable a rule when necessary instead of working around it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok i agree - gives more eyeballs to scrutinize these weird pieces of code.
maybe adding a comment justifying naked-property-access could help catch more bugs in the pr (like whether they're actually needed or not)?

+    // benchmark getter() idleTime
     nodeTiming.idleTime; // eslint-disable-line no-unused-expressions

bench.end(n);
}

Expand Down
4 changes: 2 additions & 2 deletions benchmark/process/bench-env.js
Expand Up @@ -13,7 +13,7 @@ function main({ n, operation }) {
case 'get':
bench.start();
for (let i = 0; i < n; i++) {
process.env.PATH;
process.env.PATH; // eslint-disable-line no-unused-expressions
}
bench.end(n);
break;
Expand Down Expand Up @@ -42,7 +42,7 @@ function main({ n, operation }) {
case 'query':
bench.start();
for (let i = 0; i < n; i++) {
'PATH' in process.env;
'PATH' in process.env; // eslint-disable-line no-unused-expressions
}
bench.end(n);
break;
Expand Down
3 changes: 1 addition & 2 deletions benchmark/string_decoder/string-decoder-create.js
Expand Up @@ -12,8 +12,7 @@ const bench = common.createBenchmark(main, {
function main({ encoding, n }) {
bench.start();
for (let i = 0; i < n; ++i) {
const sd = new StringDecoder(encoding);
!!sd.encoding;
new StringDecoder(encoding);
}
bench.end(n);
}
1 change: 1 addition & 0 deletions doc/.eslintrc.yaml
Expand Up @@ -4,6 +4,7 @@ rules:
# ease some restrictions in doc examples
no-restricted-properties: off
no-undef: off
no-unused-expressions: off
no-unused-vars: off
symbol-description: off

Expand Down
2 changes: 1 addition & 1 deletion lib/internal/assert/assertion_error.js
Expand Up @@ -455,7 +455,7 @@ class AssertionError extends Error {
}
ErrorCaptureStackTrace(this, stackStartFn || stackStartFunction);
// Create error message including the error code in the name.
this.stack;
this.stack; // eslint-disable-line no-unused-expressions
// Reset the name.
this.name = 'AssertionError';
}
Expand Down
24 changes: 12 additions & 12 deletions lib/internal/buffer.js
Expand Up @@ -952,18 +952,18 @@ function writeFloatBackwards(val, offset = 0) {
class FastBuffer extends Uint8Array {}

function addBufferPrototypeMethods(proto) {
proto.readBigUInt64LE = readBigUInt64LE,
proto.readBigUInt64BE = readBigUInt64BE,
proto.readBigUint64LE = readBigUInt64LE,
proto.readBigUint64BE = readBigUInt64BE,
proto.readBigInt64LE = readBigInt64LE,
proto.readBigInt64BE = readBigInt64BE,
proto.writeBigUInt64LE = writeBigUInt64LE,
proto.writeBigUInt64BE = writeBigUInt64BE,
proto.writeBigUint64LE = writeBigUInt64LE,
proto.writeBigUint64BE = writeBigUInt64BE,
proto.writeBigInt64LE = writeBigInt64LE,
proto.writeBigInt64BE = writeBigInt64BE,
proto.readBigUInt64LE = readBigUInt64LE;
proto.readBigUInt64BE = readBigUInt64BE;
proto.readBigUint64LE = readBigUInt64LE;
proto.readBigUint64BE = readBigUInt64BE;
proto.readBigInt64LE = readBigInt64LE;
proto.readBigInt64BE = readBigInt64BE;
proto.writeBigUInt64LE = writeBigUInt64LE;
proto.writeBigUInt64BE = writeBigUInt64BE;
proto.writeBigUint64LE = writeBigUInt64LE;
proto.writeBigUint64BE = writeBigUInt64BE;
proto.writeBigInt64LE = writeBigInt64LE;
proto.writeBigInt64BE = writeBigInt64BE;

proto.readUIntLE = readUIntLE;
proto.readUInt32LE = readUInt32LE;
Expand Down
8 changes: 4 additions & 4 deletions lib/internal/errors.js
Expand Up @@ -332,7 +332,7 @@ function addCodeToName(err, name, code) {
err.name = `${name} [${code}]`;
// Access the stack to generate the error message including the error code
// from the name.
err.stack;
err.stack; // eslint-disable-line no-unused-expressions
// Reset the name to the actual name.
if (name === 'SystemError') {
ObjectDefineProperty(err, 'name', {
Expand Down Expand Up @@ -1411,11 +1411,11 @@ E('ERR_TLS_CERT_ALTNAME_INVALID', function(reason, host, cert) {
}, Error);
E('ERR_TLS_DH_PARAM_SIZE', 'DH parameter size %s is less than 2048', Error);
E('ERR_TLS_HANDSHAKE_TIMEOUT', 'TLS handshake timeout', Error);
E('ERR_TLS_INVALID_CONTEXT', '%s must be a SecureContext', TypeError),
E('ERR_TLS_INVALID_STATE', 'TLS socket connection must be securely established',
Error),
E('ERR_TLS_INVALID_CONTEXT', '%s must be a SecureContext', TypeError);
E('ERR_TLS_INVALID_PROTOCOL_VERSION',
'%j is not a valid %s TLS protocol version', TypeError);
E('ERR_TLS_INVALID_STATE', 'TLS socket connection must be securely established',
Error);
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
E('ERR_TLS_PROTOCOL_VERSION_CONFLICT',
'TLS protocol version %j conflicts with secureProtocol %j', TypeError);
E('ERR_TLS_RENEGOTIATION_DISABLED',
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/streams/destroy.js
Expand Up @@ -11,7 +11,7 @@ const kConstruct = Symbol('kConstruct');
function checkError(err, w, r) {
if (err) {
// Avoid V8 leak, https://github.com/nodejs/node/pull/34103#issuecomment-652002364
err.stack;
err.stack; // eslint-disable-line no-unused-expressions

if (w && !w.errored) {
w.errored = err;
Expand Down Expand Up @@ -118,7 +118,7 @@ function _destroy(self, err, cb) {
function(err) {
const r = self._readableState;
const w = self._writableState;
err.stack;
err.stack; // eslint-disable-line no-unused-expressions

called = true;

Expand Down Expand Up @@ -237,7 +237,7 @@ function errorOrDestroy(stream, err, sync) {
stream.destroy(err);
else if (err) {
// Avoid V8 leak, https://github.com/nodejs/node/pull/34103#issuecomment-652002364
err.stack;
err.stack; // eslint-disable-line no-unused-expressions

if (w && !w.errored) {
w.errored = err;
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/streams/writable.js
Expand Up @@ -438,7 +438,7 @@ function onwrite(stream, er) {

if (er) {
// Avoid V8 leak, https://github.com/nodejs/node/pull/34103#issuecomment-652002364
er.stack;
er.stack; // eslint-disable-line no-unused-expressions

if (!state.errored) {
state.errored = er;
Expand Down
2 changes: 1 addition & 1 deletion test/common/index.js
Expand Up @@ -424,7 +424,7 @@ function getCallSite(top) {
const err = new Error();
Error.captureStackTrace(err, top);
// With the V8 Error API, the stack is not formatted until it is accessed
err.stack;
err.stack; // eslint-disable-line no-unused-expressions
Error.prepareStackTrace = originalStackFormatter;
return err.stack;
}
Expand Down
2 changes: 1 addition & 1 deletion test/js-native-api/test_general/testEnvCleanup.js
Expand Up @@ -30,7 +30,7 @@ if (process.argv[2] === 'child') {
// Make sure that only the latest attached version of a re-wrapped item's
// finalizer gets called at env cleanup.
module.exports['first wrap'] =
test_general.envCleanupWrap({}, finalizerMessages['first wrap']),
test_general.envCleanupWrap({}, finalizerMessages['first wrap']);
test_general.removeWrap(module.exports['first wrap']);
test_general.envCleanupWrap(module.exports['first wrap'],
finalizerMessages['second wrap']);
Expand Down
2 changes: 1 addition & 1 deletion test/message/nexttick_throw.js
Expand Up @@ -26,7 +26,7 @@ process.nextTick(function() {
process.nextTick(function() {
process.nextTick(function() {
process.nextTick(function() {
// eslint-disable-next-line no-undef
// eslint-disable-next-line no-undef,no-unused-expressions
undefined_reference_error_maker;
});
});
Expand Down
2 changes: 1 addition & 1 deletion test/message/timeout_throw.js
Expand Up @@ -23,6 +23,6 @@
require('../common');

setTimeout(function() {
// eslint-disable-next-line no-undef
// eslint-disable-next-line no-undef,no-unused-expressions
undefined_reference_error_maker;
}, 1);
5 changes: 3 additions & 2 deletions test/parallel/test-accessor-properties.js
Expand Up @@ -17,7 +17,7 @@ const UDP = internalBinding('udp_wrap').UDP;
{
// Should throw instead of raise assertions
assert.throws(() => {
UDP.prototype.fd;
UDP.prototype.fd; // eslint-disable-line no-unused-expressions
}, TypeError);

const StreamWrapProto = Object.getPrototypeOf(TTY.prototype);
Expand All @@ -26,7 +26,7 @@ const UDP = internalBinding('udp_wrap').UDP;
properties.forEach((property) => {
// Should throw instead of raise assertions
assert.throws(() => {
TTY.prototype[property];
TTY.prototype[property]; // eslint-disable-line no-unused-expressions
}, TypeError, `Missing expected TypeError for TTY.prototype.${property}`);

// Should not throw for Object.getOwnPropertyDescriptor
Expand All @@ -42,6 +42,7 @@ const UDP = internalBinding('udp_wrap').UDP;
const crypto = internalBinding('crypto');

assert.throws(() => {
// eslint-disable-next-line no-unused-expressions
crypto.SecureContext.prototype._external;
}, TypeError);

Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-buffer-backing-arraybuffer.js
Expand Up @@ -31,7 +31,7 @@ for (const { length, expectOnHeap } of tests) {
`for ${array.constructor.name}, length = ${length}`);

// Consistency check: Accessing .buffer should create it.
array.buffer;
array.buffer; // eslint-disable-line no-unused-expressions
assert(arrayBufferViewHasBuffer(array));
}
}
2 changes: 1 addition & 1 deletion test/parallel/test-buffer-constructor-deprecation-error.js
Expand Up @@ -14,4 +14,4 @@ process.on('warning', common.mustCall());

Error.prepareStackTrace = (err, trace) => new Buffer(10);

new Error().stack;
new Error().stack; // eslint-disable-line no-unused-expressions
2 changes: 1 addition & 1 deletion test/parallel/test-buffer-fakes.js
Expand Up @@ -14,7 +14,7 @@ assert.throws(function() {
}, TypeError);

assert.throws(function() {
+Buffer.prototype;
+Buffer.prototype; // eslint-disable-line no-unused-expressions
}, TypeError);

assert.throws(function() {
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-child-process-stdin-ipc.js
Expand Up @@ -27,7 +27,7 @@ const spawn = require('child_process').spawn;

if (process.argv[2] === 'child') {
// Just reference stdin, it should start it
process.stdin;
process.stdin; // eslint-disable-line no-unused-expressions
return;
}

Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-dgram-deprecation-error.js
Expand Up @@ -31,7 +31,7 @@ const propertyCases = propertiesToTest.map((propName) => {
`Socket.prototype.${propName} is deprecated`,
'DEP0112'
);
sock[propName];
sock[propName]; // eslint-disable-line no-unused-expressions
},
() => {
// Test property setter
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-disable-proto-throw.js
Expand Up @@ -10,7 +10,7 @@ const { Worker, isMainThread } = require('worker_threads');
assert(Object.prototype.hasOwnProperty('__proto__'));

assert.throws(() => {
// eslint-disable-next-line no-proto
// eslint-disable-next-line no-proto,no-unused-expressions
({}).__proto__;
}, {
code: 'ERR_PROTO_ACCESS'
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-error-prepare-stack-trace.js
Expand Up @@ -13,7 +13,7 @@ const assert = require('assert');
try {
throw new Error('foo');
} catch (err) {
err.stack;
err.stack; // eslint-disable-line no-unused-expressions
}
assert(prepareCalled);
}
2 changes: 1 addition & 1 deletion test/parallel/test-fs-readv-promises.js
Expand Up @@ -18,7 +18,7 @@ function getFileName() {
const allocateEmptyBuffers = (combinedLength) => {
const bufferArr = [];
// Allocate two buffers, each half the size of exptectedBuff
bufferArr[0] = Buffer.alloc(Math.floor(combinedLength / 2)),
bufferArr[0] = Buffer.alloc(Math.floor(combinedLength / 2));
bufferArr[1] = Buffer.alloc(combinedLength - bufferArr[0].length);

return bufferArr;
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-fs-readv-sync.js
Expand Up @@ -19,7 +19,7 @@ fs.writeFileSync(filename, exptectedBuff);
const allocateEmptyBuffers = (combinedLength) => {
const bufferArr = [];
// Allocate two buffers, each half the size of exptectedBuff
bufferArr[0] = Buffer.alloc(Math.floor(combinedLength / 2)),
bufferArr[0] = Buffer.alloc(Math.floor(combinedLength / 2));
bufferArr[1] = Buffer.alloc(combinedLength - bufferArr[0].length);

return bufferArr;
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-fs-readv.js
Expand Up @@ -17,7 +17,7 @@ const exptectedBuff = Buffer.from(expected);
const allocateEmptyBuffers = (combinedLength) => {
const bufferArr = [];
// Allocate two buffers, each half the size of exptectedBuff
bufferArr[0] = Buffer.alloc(Math.floor(combinedLength / 2)),
bufferArr[0] = Buffer.alloc(Math.floor(combinedLength / 2));
bufferArr[1] = Buffer.alloc(combinedLength - bufferArr[0].length);

return bufferArr;
Expand Down
Expand Up @@ -9,5 +9,5 @@ common.expectWarning('DeprecationWarning', warn, 'DEP0066');
{
// Tests for _headerNames get method
const outgoingMessage = new OutgoingMessage();
outgoingMessage._headerNames;
outgoingMessage._headerNames; // eslint-disable-line no-unused-expressions
}
2 changes: 1 addition & 1 deletion test/parallel/test-http-outgoing-internal-headers.js
Expand Up @@ -13,7 +13,7 @@ common.expectWarning('DeprecationWarning', warn, 'DEP0066');
// Tests for _headers get method
const outgoingMessage = new OutgoingMessage();
outgoingMessage.getHeaders = common.mustCall();
outgoingMessage._headers;
outgoingMessage._headers; // eslint-disable-line no-unused-expressions
}

{
Expand Down
7 changes: 4 additions & 3 deletions test/parallel/test-http-same-map.js
Expand Up @@ -39,9 +39,10 @@ onresponse.responses = [];

function allSame(list) {
assert(list.length >= 2);
// Use |elt| in no-op position to pacify eslint.
for (const elt of list) elt, eval('%DebugPrint(elt)');
for (const elt of list) elt, assert(eval('%HaveSameMap(list[0], elt)'));
// eslint-disable-next-line no-unused-vars
for (const elt of list) eval('%DebugPrint(elt)');
// eslint-disable-next-line no-unused-vars
for (const elt of list) assert(eval('%HaveSameMap(list[0], elt)'));
}

process.on('exit', () => {
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-http2-unbound-socket-proxy.js
Expand Up @@ -26,7 +26,7 @@ server.listen(0, common.mustCall(() => {
// informative error.
setImmediate(common.mustCall(() => {
assert.throws(() => {
socket.example;
socket.example; // eslint-disable-line no-unused-expressions
}, {
code: 'ERR_HTTP2_SOCKET_UNBOUND'
});
Expand All @@ -36,6 +36,7 @@ server.listen(0, common.mustCall(() => {
code: 'ERR_HTTP2_SOCKET_UNBOUND'
});
assert.throws(() => {
// eslint-disable-next-line no-unused-expressions
socket instanceof net.Socket;
}, {
code: 'ERR_HTTP2_SOCKET_UNBOUND'
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-inspector-tracing-domain.js
Expand Up @@ -30,7 +30,7 @@ function post(message, data) {
function generateTrace() {
return new Promise((resolve) => setTimeout(() => {
for (let i = 0; i < 1000000; i++) {
'test' + i;
'test' + i; // eslint-disable-line no-unused-expressions
}
resolve();
}, 1));
Expand Down
Expand Up @@ -19,8 +19,7 @@ session.post('Runtime.evaluate', {
expression: 'a',
throwOnSideEffect: true,
contextId: 2 // context's id
}, common.mustCall((error, res) => {
assert.ifError(error),
}, common.mustSucceed((res) => {
assert.deepStrictEqual(res, {
result: {
type: 'number',
Expand Down