tools: enforce iterator result property order

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',

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

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 };
           },
         };
       },

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 };
         },
       };
     },

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,
     };
   }
 

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 });
   }
 }
 

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

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 }; }',
+    },
+  ],
+});

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)),
+              ];
+            },
+          });
+        }
+      },
+    };
+  },
+};