diff --git a/lib/eslint.config_partial.mjs b/lib/eslint.config_partial.mjs index d4c3fd688314c2..cdc2cb0dcbf8db 100644 --- a/lib/eslint.config_partial.mjs +++ b/lib/eslint.config_partial.mjs @@ -421,6 +421,7 @@ export default [ 'node-core/alphabetize-errors': 'error', 'node-core/alphabetize-primordials': 'error', 'node-core/avoid-prototype-pollution': 'error', + 'node-core/iterator-result-done-first': 'error', 'node-core/lowercase-name-for-primitive': 'error', 'node-core/non-ascii-character': 'error', 'node-core/no-array-destructuring': 'error', diff --git a/lib/events.js b/lib/events.js index c6fc170d590aea..7044423692e1bf 100644 --- a/lib/events.js +++ b/lib/events.js @@ -1032,7 +1032,7 @@ async function once(emitter, name, options = kEmptyObject) { } function createIterResult(value, done) { - return { value, done }; + return { done, value }; } function eventTargetAgnosticRemoveListener(emitter, name, listener, flags) { diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 0aa01d9b39dc3e..e8694843987c2b 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -657,7 +657,7 @@ if (getOptionValue('--experimental-stream-iter')) { done = true; cleanup(); } - return { value: undefined, done: true }; + return { done: true, value: undefined }; } const toRead = remaining > 0 ? MathMin(readSize, remaining) : readSize; @@ -673,20 +673,20 @@ if (getOptionValue('--experimental-stream-iter')) { if (bytesRead === 0) { done = true; cleanup(); - return { value: undefined, done: true }; + return { done: true, value: undefined }; } if (pos >= 0) pos += bytesRead; if (remaining > 0) remaining -= bytesRead; const chunk = bytesRead < toRead ? buf.subarray(0, bytesRead) : buf; - return { value: [chunk], done: false }; + return { done: false, value: [chunk] }; }, return() { if (!done) { done = true; cleanup(); } - return { value: undefined, done: true }; + return { done: true, value: undefined }; }, }; }, diff --git a/lib/internal/streams/iter/push.js b/lib/internal/streams/iter/push.js index 1c367ff02bae71..34d3334f84a0b9 100644 --- a/lib/internal/streams/iter/push.js +++ b/lib/internal/streams/iter/push.js @@ -397,17 +397,17 @@ class PushQueue { if (this.#writerState === 'closing' && this.#slots.length === 0) { this.endDrained(); } - return { __proto__: null, value: result, done: false }; + return { __proto__: null, done: false, value: result }; } // Buffer empty and writer closing = drain complete if (this.#writerState === 'closing') { this.endDrained(); - return { __proto__: null, value: undefined, done: true }; + return { __proto__: null, done: true, value: undefined }; } if (this.#writerState === 'closed') { - return { __proto__: null, value: undefined, done: true }; + return { __proto__: null, done: true, value: undefined }; } if (this.#writerState === 'errored' && this.#error) { @@ -471,14 +471,14 @@ class PushQueue { const pending = this.#pendingReads.shift(); const result = this.#drain(); this.#resolvePendingWrites(); - pending.resolve({ __proto__: null, value: result, done: false }); + pending.resolve({ __proto__: null, done: false, value: result }); } else if (this.#writerState === 'closing' && this.#slots.length === 0) { this.endDrained(); const pending = this.#pendingReads.shift(); - pending.resolve({ __proto__: null, value: undefined, done: true }); + pending.resolve({ __proto__: null, done: true, value: undefined }); } else if (this.#writerState === 'closed') { const pending = this.#pendingReads.shift(); - pending.resolve({ __proto__: null, value: undefined, done: true }); + pending.resolve({ __proto__: null, done: true, value: undefined }); } else if (this.#writerState === 'errored' && this.#error) { const pending = this.#pendingReads.shift(); pending.reject(this.#error); @@ -687,11 +687,11 @@ function createReadable(queue) { }, async return() { queue.consumerReturn(); - return { __proto__: null, value: undefined, done: true }; + return { __proto__: null, done: true, value: undefined }; }, async throw(error) { queue.consumerThrow(error); - return { __proto__: null, value: undefined, done: true }; + return { __proto__: null, done: true, value: undefined }; }, }; }, diff --git a/lib/internal/url.js b/lib/internal/url.js index dfcd071073a8d3..446c22f5b66067 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -242,8 +242,8 @@ class URLSearchParamsIterator { const len = values.length; if (index >= len) { return { - value: undefined, done: true, + value: undefined, }; } @@ -261,8 +261,8 @@ class URLSearchParamsIterator { } return { - value: result, done: false, + value: result, }; } diff --git a/lib/internal/vfs/watcher.js b/lib/internal/vfs/watcher.js index f64ba91ba3c9a6..2b6b807ee85b44 100644 --- a/lib/internal/vfs/watcher.js +++ b/lib/internal/vfs/watcher.js @@ -599,7 +599,7 @@ class VFSWatchAsyncIterable { const event = { eventType, filename }; if (this.#pendingResolvers.length > 0) { const { resolve } = this.#pendingResolvers.shift(); - resolve({ value: event, done: false }); + resolve({ done: false, value: event }); } else if (this.#pendingEvents.length < kMaxPendingEvents) { ArrayPrototypePush(this.#pendingEvents, event); } @@ -611,7 +611,7 @@ class VFSWatchAsyncIterable { // Resolve any pending iterators while (this.#pendingResolvers.length > 0) { const { resolve } = this.#pendingResolvers.shift(); - resolve({ value: undefined, done: true }); + resolve({ done: true, value: undefined }); } }); @@ -648,12 +648,12 @@ class VFSWatchAsyncIterable { */ next() { if (this.#closed) { - return PromiseResolve({ value: undefined, done: true }); + return PromiseResolve({ done: true, value: undefined }); } if (this.#pendingEvents.length > 0) { const event = this.#pendingEvents.shift(); - return PromiseResolve({ value: event, done: false }); + return PromiseResolve({ done: false, value: event }); } return new Promise((resolve, reject) => { @@ -667,7 +667,7 @@ class VFSWatchAsyncIterable { */ return() { this.#watcher.close(); - return PromiseResolve({ value: undefined, done: true }); + return PromiseResolve({ done: true, value: undefined }); } /** @@ -677,7 +677,7 @@ class VFSWatchAsyncIterable { */ throw(error) { this.#watcher.close(); - return PromiseResolve({ value: undefined, done: true }); + return PromiseResolve({ done: true, value: undefined }); } } diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js index 876e3a5bf6e2f0..a46289723a7f43 100644 --- a/lib/internal/webstreams/readablestream.js +++ b/lib/internal/webstreams/readablestream.js @@ -761,7 +761,7 @@ class ReadableStreamAsyncIteratorReadRequest { [kChunk](chunk) { this.state.current = undefined; - this.promise.resolve({ value: chunk, done: false }); + this.promise.resolve({ done: false, value: chunk }); } [kClose]() { @@ -785,11 +785,11 @@ class DefaultReadRequest { } [kChunk](value) { - this[kState].resolve?.({ value, done: false }); + this[kState].resolve?.({ done: false, value }); } [kClose]() { - this[kState].resolve?.({ value: undefined, done: true }); + this[kState].resolve?.({ done: true, value: undefined }); } [kError](error) { @@ -805,11 +805,11 @@ class ReadIntoRequest { } [kChunk](value) { - this[kState].resolve?.({ value, done: false }); + this[kState].resolve?.({ done: false, value }); } [kClose](value) { - this[kState].resolve?.({ value, done: true }); + this[kState].resolve?.({ done: true, value }); } [kError](error) { @@ -875,7 +875,7 @@ class ReadableStreamDefaultReader { readableStreamDefaultControllerCallPullIfNeeded(controller); } - return PromiseResolve({ value: chunk, done: false }); + return PromiseResolve({ done: false, value: chunk }); } // Slow path: create request and go through normal flow diff --git a/test/parallel/test-eslint-iterator-result-done-first.js b/test/parallel/test-eslint-iterator-result-done-first.js new file mode 100644 index 00000000000000..f65f6bd72ebe7a --- /dev/null +++ b/test/parallel/test-eslint-iterator-result-done-first.js @@ -0,0 +1,60 @@ +'use strict'; + +const common = require('../common'); +if ((!common.hasCrypto) || (!common.hasIntl)) { + common.skip('ESLint tests require crypto and Intl'); +} + +common.skipIfEslintMissing(); + +const { RuleTester } = require('../../tools/eslint/node_modules/eslint'); +const rule = require('../../tools/eslint-rules/iterator-result-done-first'); + +const message = 'Iterator result objects should place `done` before `value`.'; + +new RuleTester().run('iterator-result-done-first', rule, { + valid: [ + 'function next() { return { done: true, value: undefined }; }', + 'function next() { return { __proto__: null, done: false, value: chunk }; }', + 'function next() { return { done, value }; }', + 'function next() { return { value }; }', + 'function next() { return { done }; }', + 'function next() { return { value: 1, other: 2 }; }', + 'function next() { return { [value]: 1, done: true }; }', + 'function next() { return { value: 1, [done]: true }; }', + 'function next() { return { "done": true, "value": undefined }; }', + 'function next() { return { ["done"]: true, ["value"]: undefined }; }', + ], + invalid: [ + { + code: 'function next() { return { value: undefined, done: true }; }', + errors: [{ message }], + output: 'function next() { return { done: true, value: undefined }; }', + }, + { + code: 'function next() { return { __proto__: null, value: chunk, done: false }; }', + errors: [{ message }], + output: 'function next() { return { __proto__: null, done: false, value: chunk }; }', + }, + { + code: 'function next() { return { value, done }; }', + errors: [{ message }], + output: 'function next() { return { done, value }; }', + }, + { + code: 'function next() { return { "value": undefined, "done": true }; }', + errors: [{ message }], + output: 'function next() { return { "done": true, "value": undefined }; }', + }, + { + code: 'function next() { return { ["value"]: undefined, ["done"]: true }; }', + errors: [{ message }], + output: 'function next() { return { ["done"]: true, ["value"]: undefined }; }', + }, + { + code: 'function next() { return { value: result, extra: true, done: false }; }', + errors: [{ message }], + output: 'function next() { return { done: false, extra: true, value: result }; }', + }, + ], +}); diff --git a/tools/eslint-rules/iterator-result-done-first.js b/tools/eslint-rules/iterator-result-done-first.js new file mode 100644 index 00000000000000..1c3ee9035e99f9 --- /dev/null +++ b/tools/eslint-rules/iterator-result-done-first.js @@ -0,0 +1,66 @@ +'use strict'; + +const MESSAGE = 'Iterator result objects should place `done` before `value`.'; + +function getStaticPropertyName(property) { + const { key } = property; + + if (!key) { + return; + } + + if (key.type === 'Identifier' && !property.computed) { + return key.name; + } + + if (key.type === 'Literal') { + return key.value; + } +} + +module.exports = { + meta: { + type: 'suggestion', + fixable: 'code', + schema: [], + }, + + create(context) { + const sourceCode = context.sourceCode; + + return { + ObjectExpression(node) { + let doneProperty; + let valueProperty; + + for (const property of node.properties) { + if (property.type !== 'Property') { + continue; + } + + switch (getStaticPropertyName(property)) { + case 'done': + doneProperty ??= property; + break; + case 'value': + valueProperty ??= property; + break; + } + } + + if (doneProperty && valueProperty && valueProperty.range[0] < doneProperty.range[0]) { + context.report({ + node: valueProperty, + message: MESSAGE, + fix(fixer) { + return [ + fixer.replaceText(valueProperty, sourceCode.getText(doneProperty)), + fixer.replaceText(doneProperty, sourceCode.getText(valueProperty)), + ]; + }, + }); + } + }, + }; + }, +};