Skip to content

Commit 325087b

Browse files
trivikraduh95
trivikr
authored and
aduh95
committed
tools: enforce iterator result property order
Add a custom ESLint rule requiring iterator result objects to place `done` before `value`, and update existing lib iterator result objects to follow the rule. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.5 PR-URL: #63526 Reviewed-By: Mattias Buelens <mattias@buelens.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 7bc93a6 commit 325087b

9 files changed

Lines changed: 154 additions & 27 deletions

File tree

‎lib/eslint.config_partial.mjs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,7 @@ export default [
417417
'node-core/alphabetize-errors': 'error',
418418
'node-core/alphabetize-primordials': 'error',
419419
'node-core/avoid-prototype-pollution': 'error',
420+
'node-core/iterator-result-done-first': 'error',
420421
'node-core/lowercase-name-for-primitive': 'error',
421422
'node-core/non-ascii-character': 'error',
422423
'node-core/no-array-destructuring': 'error',

‎lib/events.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1032,7 +1032,7 @@ async function once(emitter, name, options = kEmptyObject) {
10321032
}
10331033

10341034
function createIterResult(value, done) {
1035-
return { value, done };
1035+
return { done, value };
10361036
}
10371037

10381038
function eventTargetAgnosticRemoveListener(emitter, name, listener, flags) {

‎lib/internal/fs/promises.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -660,7 +660,7 @@ if (getOptionValue('--experimental-stream-iter')) {
660660
done = true;
661661
cleanup();
662662
}
663-
return { value: undefined, done: true };
663+
return { done: true, value: undefined };
664664
}
665665
const toRead = remaining > 0 ?
666666
MathMin(readSize, remaining) : readSize;
@@ -676,20 +676,20 @@ if (getOptionValue('--experimental-stream-iter')) {
676676
if (bytesRead === 0) {
677677
done = true;
678678
cleanup();
679-
return { value: undefined, done: true };
679+
return { done: true, value: undefined };
680680
}
681681
if (pos >= 0) pos += bytesRead;
682682
if (remaining > 0) remaining -= bytesRead;
683683
const chunk = bytesRead < toRead ?
684684
buf.subarray(0, bytesRead) : buf;
685-
return { value: [chunk], done: false };
685+
return { done: false, value: [chunk] };
686686
},
687687
return() {
688688
if (!done) {
689689
done = true;
690690
cleanup();
691691
}
692-
return { value: undefined, done: true };
692+
return { done: true, value: undefined };
693693
},
694694
};
695695
},

‎lib/internal/streams/iter/push.js‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -385,17 +385,17 @@ class PushQueue {
385385
if (this.#writerState === 'closing' && this.#slots.length === 0) {
386386
this.endDrained();
387387
}
388-
return { __proto__: null, value: result, done: false };
388+
return { __proto__: null, done: false, value: result };
389389
}
390390

391391
// Buffer empty and writer closing = drain complete
392392
if (this.#writerState === 'closing') {
393393
this.endDrained();
394-
return { __proto__: null, value: undefined, done: true };
394+
return { __proto__: null, done: true, value: undefined };
395395
}
396396

397397
if (this.#writerState === 'closed') {
398-
return { __proto__: null, value: undefined, done: true };
398+
return { __proto__: null, done: true, value: undefined };
399399
}
400400

401401
if (this.#writerState === 'errored' && this.#error) {
@@ -465,14 +465,14 @@ class PushQueue {
465465
const pending = this.#pendingReads.shift();
466466
const result = this.#drain();
467467
this.#resolvePendingWrites();
468-
pending.resolve({ __proto__: null, value: result, done: false });
468+
pending.resolve({ __proto__: null, done: false, value: result });
469469
} else if (this.#writerState === 'closing' && this.#slots.length === 0) {
470470
this.endDrained();
471471
const pending = this.#pendingReads.shift();
472-
pending.resolve({ __proto__: null, value: undefined, done: true });
472+
pending.resolve({ __proto__: null, done: true, value: undefined });
473473
} else if (this.#writerState === 'closed') {
474474
const pending = this.#pendingReads.shift();
475-
pending.resolve({ __proto__: null, value: undefined, done: true });
475+
pending.resolve({ __proto__: null, done: true, value: undefined });
476476
} else if (this.#writerState === 'errored' && this.#error) {
477477
const pending = this.#pendingReads.shift();
478478
pending.reject(this.#error);
@@ -659,11 +659,11 @@ function createReadable(queue) {
659659
},
660660
async return() {
661661
queue.consumerReturn();
662-
return { __proto__: null, value: undefined, done: true };
662+
return { __proto__: null, done: true, value: undefined };
663663
},
664664
async throw(error) {
665665
queue.consumerThrow(error);
666-
return { __proto__: null, value: undefined, done: true };
666+
return { __proto__: null, done: true, value: undefined };
667667
},
668668
};
669669
},

‎lib/internal/url.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,8 @@ class URLSearchParamsIterator {
242242
const len = values.length;
243243
if (index >= len) {
244244
return {
245-
value: undefined,
246245
done: true,
246+
value: undefined,
247247
};
248248
}
249249

@@ -261,8 +261,8 @@ class URLSearchParamsIterator {
261261
}
262262

263263
return {
264-
value: result,
265264
done: false,
265+
value: result,
266266
};
267267
}
268268

‎lib/internal/vfs/watcher.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,7 @@ class VFSWatchAsyncIterable {
599599
const event = { eventType, filename };
600600
if (this.#pendingResolvers.length > 0) {
601601
const { resolve } = this.#pendingResolvers.shift();
602-
resolve({ value: event, done: false });
602+
resolve({ done: false, value: event });
603603
} else if (this.#pendingEvents.length < kMaxPendingEvents) {
604604
ArrayPrototypePush(this.#pendingEvents, event);
605605
}
@@ -611,7 +611,7 @@ class VFSWatchAsyncIterable {
611611
// Resolve any pending iterators
612612
while (this.#pendingResolvers.length > 0) {
613613
const { resolve } = this.#pendingResolvers.shift();
614-
resolve({ value: undefined, done: true });
614+
resolve({ done: true, value: undefined });
615615
}
616616
});
617617

@@ -648,12 +648,12 @@ class VFSWatchAsyncIterable {
648648
*/
649649
next() {
650650
if (this.#closed) {
651-
return PromiseResolve({ value: undefined, done: true });
651+
return PromiseResolve({ done: true, value: undefined });
652652
}
653653

654654
if (this.#pendingEvents.length > 0) {
655655
const event = this.#pendingEvents.shift();
656-
return PromiseResolve({ value: event, done: false });
656+
return PromiseResolve({ done: false, value: event });
657657
}
658658

659659
return new Promise((resolve, reject) => {
@@ -667,7 +667,7 @@ class VFSWatchAsyncIterable {
667667
*/
668668
return() {
669669
this.#watcher.close();
670-
return PromiseResolve({ value: undefined, done: true });
670+
return PromiseResolve({ done: true, value: undefined });
671671
}
672672

673673
/**
@@ -677,7 +677,7 @@ class VFSWatchAsyncIterable {
677677
*/
678678
throw(error) {
679679
this.#watcher.close();
680-
return PromiseResolve({ value: undefined, done: true });
680+
return PromiseResolve({ done: true, value: undefined });
681681
}
682682
}
683683

‎lib/internal/webstreams/readablestream.js‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -824,7 +824,7 @@ class ReadableStreamAsyncIteratorReadRequest {
824824

825825
[kChunk](chunk) {
826826
this.state.current = undefined;
827-
this.promise.resolve({ value: chunk, done: false });
827+
this.promise.resolve({ done: false, value: chunk });
828828
}
829829

830830
[kClose]() {
@@ -848,11 +848,11 @@ class DefaultReadRequest {
848848
}
849849

850850
[kChunk](value) {
851-
this[kState].resolve?.({ value, done: false });
851+
this[kState].resolve?.({ done: false, value });
852852
}
853853

854854
[kClose]() {
855-
this[kState].resolve?.({ value: undefined, done: true });
855+
this[kState].resolve?.({ done: true, value: undefined });
856856
}
857857

858858
[kError](error) {
@@ -868,11 +868,11 @@ class ReadIntoRequest {
868868
}
869869

870870
[kChunk](value) {
871-
this[kState].resolve?.({ value, done: false });
871+
this[kState].resolve?.({ done: false, value });
872872
}
873873

874874
[kClose](value) {
875-
this[kState].resolve?.({ value, done: true });
875+
this[kState].resolve?.({ done: true, value });
876876
}
877877

878878
[kError](error) {
@@ -935,7 +935,7 @@ class ReadableStreamDefaultReader {
935935
readableStreamDefaultControllerCallPullIfNeeded(controller);
936936
}
937937

938-
return PromiseResolve({ value: chunk, done: false });
938+
return PromiseResolve({ done: false, value: chunk });
939939
}
940940

941941
// Slow path: create request and go through normal flow

‎test/parallel/test-eslint-iterator-result-done-first.js‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if ((!common.hasCrypto) || (!common.hasIntl)) {
5+
common.skip('ESLint tests require crypto and Intl');
6+
}
7+
8+
common.skipIfEslintMissing();
9+
10+
const { RuleTester } = require('../../tools/eslint/node_modules/eslint');
11+
const rule = require('../../tools/eslint-rules/iterator-result-done-first');
12+
13+
const message = 'Iterator result objects should place `done` before `value`.';
14+
15+
new RuleTester().run('iterator-result-done-first', rule, {
16+
valid: [
17+
'function next() { return { done: true, value: undefined }; }',
18+
'function next() { return { __proto__: null, done: false, value: chunk }; }',
19+
'function next() { return { done, value }; }',
20+
'function next() { return { value }; }',
21+
'function next() { return { done }; }',
22+
'function next() { return { value: 1, other: 2 }; }',
23+
'function next() { return { [value]: 1, done: true }; }',
24+
'function next() { return { value: 1, [done]: true }; }',
25+
'function next() { return { "done": true, "value": undefined }; }',
26+
'function next() { return { ["done"]: true, ["value"]: undefined }; }',
27+
],
28+
invalid: [
29+
{
30+
code: 'function next() { return { value: undefined, done: true }; }',
31+
errors: [{ message }],
32+
output: 'function next() { return { done: true, value: undefined }; }',
33+
},
34+
{
35+
code: 'function next() { return { __proto__: null, value: chunk, done: false }; }',
36+
errors: [{ message }],
37+
output: 'function next() { return { __proto__: null, done: false, value: chunk }; }',
38+
},
39+
{
40+
code: 'function next() { return { value, done }; }',
41+
errors: [{ message }],
42+
output: 'function next() { return { done, value }; }',
43+
},
44+
{
45+
code: 'function next() { return { "value": undefined, "done": true }; }',
46+
errors: [{ message }],
47+
output: 'function next() { return { "done": true, "value": undefined }; }',
48+
},
49+
{
50+
code: 'function next() { return { ["value"]: undefined, ["done"]: true }; }',
51+
errors: [{ message }],
52+
output: 'function next() { return { ["done"]: true, ["value"]: undefined }; }',
53+
},
54+
{
55+
code: 'function next() { return { value: result, extra: true, done: false }; }',
56+
errors: [{ message }],
57+
output: 'function next() { return { done: false, extra: true, value: result }; }',
58+
},
59+
],
60+
});

‎tools/eslint-rules/iterator-result-done-first.js‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
'use strict';
2+
3+
const MESSAGE = 'Iterator result objects should place `done` before `value`.';
4+
5+
function getStaticPropertyName(property) {
6+
const { key } = property;
7+
8+
if (!key) {
9+
return;
10+
}
11+
12+
if (key.type === 'Identifier' && !property.computed) {
13+
return key.name;
14+
}
15+
16+
if (key.type === 'Literal') {
17+
return key.value;
18+
}
19+
}
20+
21+
module.exports = {
22+
meta: {
23+
type: 'suggestion',
24+
fixable: 'code',
25+
schema: [],
26+
},
27+
28+
create(context) {
29+
const sourceCode = context.sourceCode;
30+
31+
return {
32+
ObjectExpression(node) {
33+
let doneProperty;
34+
let valueProperty;
35+
36+
for (const property of node.properties) {
37+
if (property.type !== 'Property') {
38+
continue;
39+
}
40+
41+
switch (getStaticPropertyName(property)) {
42+
case 'done':
43+
doneProperty ??= property;
44+
break;
45+
case 'value':
46+
valueProperty ??= property;
47+
break;
48+
}
49+
}
50+
51+
if (doneProperty && valueProperty && valueProperty.range[0] < doneProperty.range[0]) {
52+
context.report({
53+
node: valueProperty,
54+
message: MESSAGE,
55+
fix(fixer) {
56+
return [
57+
fixer.replaceText(valueProperty, sourceCode.getText(doneProperty)),
58+
fixer.replaceText(doneProperty, sourceCode.getText(valueProperty)),
59+
];
60+
},
61+
});
62+
}
63+
},
64+
};
65+
},
66+
};

0 commit comments

Comments
 (0)