tools: enforce iterator result property order

trivikr · web-flow · commit 92f48f43fec1 · 2026-05-26T17:19:42.000Z
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>
diff --git 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
@@ -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
@@ -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
@@ -400,17 +400,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) {
@@ -474,14 +474,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);
@@ -694,11 +694,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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)),
+              ];
+            },
+          });
+        }
+      },
+    };
+  },
+};

Original file line number	Diff line number	Diff line change
`@@ -1032,7 +1032,7 @@ async function once(emitter, name, options = kEmptyObject) {`
`1032`	`1032`	`}`
`1033`	`1033`
`1034`	`1034`	`function createIterResult(value, done) {`
`1035`		`- return { value, done };`
	`1035`	`+ return { done, value };`
`1036`	`1036`	`}`
`1037`	`1037`
`1038`	`1038`	`function eventTargetAgnosticRemoveListener(emitter, name, listener, flags) {`
Original file line number	Diff line number	Diff line change
`@@ -242,8 +242,8 @@ class URLSearchParamsIterator {`
`242`	`242`	`const len = values.length;`
`243`	`243`	`if (index >= len) {`
`244`	`244`	`return {`
`245`		`- value: undefined,`
`246`	`245`	`done: true,`
	`246`	`+ value: undefined,`
`247`	`247`	`};`
`248`	`248`	`}`
`249`	`249`
`@@ -261,8 +261,8 @@ class URLSearchParamsIterator {`
`261`	`261`	`}`
`262`	`262`
`263`	`263`	`return {`
`264`		`- value: result,`
`265`	`264`	`done: false,`
	`265`	`+ value: result,`
`266`	`266`	`};`
`267`	`267`	`}`
`268`	`268`
Original file line number	Diff line number	Diff line change
`@@ -761,7 +761,7 @@ class ReadableStreamAsyncIteratorReadRequest {`
`761`	`761`
`762`	`762`	`[kChunk](chunk) {`
`763`	`763`	`this.state.current = undefined;`
`764`		`- this.promise.resolve({ value: chunk, done: false });`
	`764`	`+ this.promise.resolve({ done: false, value: chunk });`
`765`	`765`	`}`
`766`	`766`
`767`	`767`	`[kClose]() {`
`@@ -785,11 +785,11 @@ class DefaultReadRequest {`
`785`	`785`	`}`
`786`	`786`
`787`	`787`	`[kChunk](value) {`
`788`		`- this[kState].resolve?.({ value, done: false });`
	`788`	`+ this[kState].resolve?.({ done: false, value });`
`789`	`789`	`}`
`790`	`790`
`791`	`791`	`[kClose]() {`
`792`		`- this[kState].resolve?.({ value: undefined, done: true });`
	`792`	`+ this[kState].resolve?.({ done: true, value: undefined });`
`793`	`793`	`}`
`794`	`794`
`795`	`795`	`[kError](error) {`
`@@ -805,11 +805,11 @@ class ReadIntoRequest {`
`805`	`805`	`}`
`806`	`806`
`807`	`807`	`[kChunk](value) {`
`808`		`- this[kState].resolve?.({ value, done: false });`
	`808`	`+ this[kState].resolve?.({ done: false, value });`
`809`	`809`	`}`
`810`	`810`
`811`	`811`	`[kClose](value) {`
`812`		`- this[kState].resolve?.({ value, done: true });`
	`812`	`+ this[kState].resolve?.({ done: true, value });`
`813`	`813`	`}`
`814`	`814`
`815`	`815`	`[kError](error) {`
`@@ -875,7 +875,7 @@ class ReadableStreamDefaultReader {`
`875`	`875`	`readableStreamDefaultControllerCallPullIfNeeded(controller);`
`876`	`876`	`}`
`877`	`877`
`878`		`- return PromiseResolve({ value: chunk, done: false });`
	`878`	`+ return PromiseResolve({ done: false, value: chunk });`
`879`	`879`	`}`
`880`	`880`
`881`	`881`	`// Slow path: create request and go through normal flow`