Skip to content

Commit 00d2960

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 59badfd commit 00d2960

9 files changed

Lines changed: 152 additions & 25 deletions

File tree

‎lib/eslint.config_partial.mjs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,7 @@ export default [
395395
'node-core/alphabetize-errors': 'error',
396396
'node-core/alphabetize-primordials': 'error',
397397
'node-core/avoid-prototype-pollution': 'error',
398+
'node-core/iterator-result-done-first': 'error',
398399
'node-core/lowercase-name-for-primitive': 'error',
399400
'node-core/non-ascii-character': 'error',
400401
'node-core/no-array-destructuring': 'error',

‎lib/events.js‎

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

10451045
function createIterResult(value, done) {
1046-
return { value, done };
1046+
return { done, value };
10471047
}
10481048

10491049
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
@@ -661,7 +661,7 @@ if (getOptionValue('--experimental-stream-iter')) {
661661
done = true;
662662
cleanup();
663663
}
664-
return { value: undefined, done: true };
664+
return { done: true, value: undefined };
665665
}
666666
const toRead = remaining > 0 ?
667667
MathMin(readSize, remaining) : readSize;
@@ -677,20 +677,20 @@ if (getOptionValue('--experimental-stream-iter')) {
677677
if (bytesRead === 0) {
678678
done = true;
679679
cleanup();
680-
return { value: undefined, done: true };
680+
return { done: true, value: undefined };
681681
}
682682
if (pos >= 0) pos += bytesRead;
683683
if (remaining > 0) remaining -= bytesRead;
684684
const chunk = bytesRead < toRead ?
685685
buf.subarray(0, bytesRead) : buf;
686-
return { value: [chunk], done: false };
686+
return { done: false, value: [chunk] };
687687
},
688688
return() {
689689
if (!done) {
690690
done = true;
691691
cleanup();
692692
}
693-
return { value: undefined, done: true };
693+
return { done: true, value: undefined };
694694
},
695695
};
696696
},

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -447,17 +447,17 @@ class PushQueue {
447447
if (this.#writerState === 'closing' && this.#slots.length === 0) {
448448
this.endDrained();
449449
}
450-
return { __proto__: null, value: result, done: false };
450+
return { __proto__: null, done: false, value: result };
451451
}
452452

453453
// Buffer empty and writer closing = drain complete
454454
if (this.#writerState === 'closing') {
455455
this.endDrained();
456-
return { __proto__: null, value: undefined, done: true };
456+
return { __proto__: null, done: true, value: undefined };
457457
}
458458

459459
if (this.#writerState === 'closed') {
460-
return { __proto__: null, value: undefined, done: true };
460+
return { __proto__: null, done: true, value: undefined };
461461
}
462462

463463
if (this.#writerState === 'errored') {
@@ -546,14 +546,14 @@ class PushQueue {
546546
const pending = this.#pendingReads.shift();
547547
const result = this.#drain();
548548
this.#resolvePendingWrites();
549-
pending.resolve({ __proto__: null, value: result, done: false });
549+
pending.resolve({ __proto__: null, done: false, value: result });
550550
} else if (this.#writerState === 'closing' && this.#slots.length === 0) {
551551
this.endDrained();
552552
const pending = this.#pendingReads.shift();
553-
pending.resolve({ __proto__: null, value: undefined, done: true });
553+
pending.resolve({ __proto__: null, done: true, value: undefined });
554554
} else if (this.#writerState === 'closed') {
555555
const pending = this.#pendingReads.shift();
556-
pending.resolve({ __proto__: null, value: undefined, done: true });
556+
pending.resolve({ __proto__: null, done: true, value: undefined });
557557
} else if (this.#writerState === 'errored') {
558558
const pending = this.#pendingReads.shift();
559559
pending.reject(this.#error);
@@ -729,7 +729,7 @@ function createReadable(queue) {
729729
},
730730
async return() {
731731
queue.consumerReturn();
732-
return { __proto__: null, value: undefined, done: true };
732+
return { __proto__: null, done: true, value: undefined };
733733
},
734734
async throw(error) {
735735
queue.consumerThrow(error);

‎lib/internal/url.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,8 +274,8 @@ class URLSearchParamsIterator {
274274
const len = values.length;
275275
if (index >= len) {
276276
return {
277-
value: undefined,
278277
done: true,
278+
value: undefined,
279279
};
280280
}
281281

@@ -293,8 +293,8 @@ class URLSearchParamsIterator {
293293
}
294294

295295
return {
296-
value: result,
297296
done: false,
297+
value: result,
298298
};
299299
}
300300

‎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: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -845,7 +845,7 @@ class ReadableStreamAsyncIteratorReadRequest {
845845

846846
[kChunk](chunk) {
847847
this.state.current = undefined;
848-
this.promise.resolve({ value: chunk, done: false });
848+
this.promise.resolve({ done: false, value: chunk });
849849
}
850850

851851
[kClose]() {
@@ -869,11 +869,11 @@ class DefaultReadRequest {
869869
}
870870

871871
[kChunk](value) {
872-
this[kState].resolve({ value, done: false });
872+
this[kState].resolve({ done: false, value });
873873
}
874874

875875
[kClose]() {
876-
this[kState].resolve({ value: undefined, done: true });
876+
this[kState].resolve({ done: true, value: undefined });
877877
}
878878

879879
[kError](error) {
@@ -889,11 +889,11 @@ class ReadIntoRequest {
889889
}
890890

891891
[kChunk](value) {
892-
this[kState].resolve({ value, done: false });
892+
this[kState].resolve({ done: false, value });
893893
}
894894

895895
[kClose](value) {
896-
this[kState].resolve({ value, done: true });
896+
this[kState].resolve({ done: true, value });
897897
}
898898

899899
[kError](error) {

‎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)