tools: add lint rule for aborted AbortController

test/eslint.config_partial.mjs

@@ -162,6 +162,7 @@ export default [
       'node-core/require-common-first': 'error',
       'node-core/no-duplicate-requires': 'off',
       'node-core/must-call-assert': 'error',
+      'node-core/prefer-abort-signal-abort': 'error',
     },
   },
   {

test/parallel/test-abortcontroller.js

@@ -161,6 +161,7 @@ test('AbortController inspection depth 1 or null works', () => {
 
 test('AbortSignal reason is set correctly', () => {
   // Test AbortSignal.reason
+  // eslint-disable-next-line node-core/prefer-abort-signal-abort
   const ac = new AbortController();
   ac.abort('reason');
   assert.strictEqual(ac.signal.reason, 'reason');
@@ -235,6 +236,7 @@ test('AbortSignal.reason should default', () => {
   assert.ok(signal.reason instanceof DOMException);
   assert.strictEqual(signal.reason.code, 20);
 
+  // eslint-disable-next-line node-core/prefer-abort-signal-abort
   const ac = new AbortController();
   ac.abort();
   assert.ok(ac.signal.reason instanceof DOMException);

test/parallel/test-eslint-prefer-abort-signal-abort.js

@@ -0,0 +1,118 @@
+'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').RuleTester;
+const rule = require('../../tools/eslint-rules/prefer-abort-signal-abort');
+
+const message = 'Use AbortSignal.abort() instead of creating and aborting an AbortController.';
+
+new RuleTester().run('prefer-abort-signal-abort', rule, {
+  valid: [
+    'const signal = AbortSignal.abort();',
+    `
+      const controller = new AbortController();
+      controller.abort();
+      controller.abort();
+      fn(controller.signal);
+    `,
+    `
+      const controller = new AbortController();
+      controller.abort();
+      console.log(controller);
+      fn(controller.signal);
+    `,
+    `
+      const controller = new AbortController();
+      // This comment should not be removed.
+      controller.abort();
+      fn(controller.signal);
+    `,
+    `
+      const controller = new AbortController();
+      setImmediate(() => controller.abort());
+      fn(controller.signal);
+    `,
+    `
+      const controller = new AbortController();
+      controller.abort('reason', 'extra');
+      fn(controller.signal);
+    `,
+  ],
+  invalid: [
+    {
+      code: `
+        const controller = new AbortController();
+        controller.abort();
+        fn(controller.signal);
+      `,
+      errors: [{ message }],
+      output: `
+        fn(AbortSignal.abort());
+      `,
+    },
+    {
+      code: `
+        const abortController = new AbortController();
+        abortController.abort(new Error('aborted'));
+        fn({ signal: abortController.signal });
+      `,
+      errors: [{ message }],
+      output: `
+        fn({ signal: AbortSignal.abort(new Error('aborted')) });
+      `,
+    },
+    {
+      code: `
+        {
+          const ac = new AbortController();
+          ac.abort();
+          await wait({ signal: ac.signal });
+        }
+      `,
+      errors: [{ message }],
+      output: `
+        {
+          await wait({ signal: AbortSignal.abort() });
+        }
+      `,
+    },
+    {
+      code: `
+        {
+          const controller = new AbortController();
+          controller.abort();
+          fn(controller.signal, controller.signal);
+        }
+      `,
+      errors: [{ message }],
+      output: `
+        {
+          const controller = AbortSignal.abort();
+          fn(controller, controller);
+        }
+      `,
+    },
+    {
+      code: `
+        {
+          const controller = new AbortController();
+          controller.abort("reason");
+          fn(controller.signal, controller.signal);
+        }
+      `,
+      errors: [{ message }],
+      output: `
+        {
+          const controller = AbortSignal.abort("reason");
+          fn(controller, controller);
+        }
+      `,
+    },
+  ]
+});

test/parallel/test-quic-writer-abort-signal.mjs

@@ -33,12 +33,11 @@ const stream = await clientSession.createBidirectionalStream();
 const w = stream.writer;
 
 // Create an already-aborted signal.
-const ac = new AbortController();
-ac.abort(new Error('already aborted'));
+const signal = AbortSignal.abort(new Error('already aborted'));
 
 // write() with an already-aborted signal should reject immediately.
 await rejects(
-  w.write(encoder.encode('data'), { signal: ac.signal }),
+  w.write(encoder.encode('data'), { signal }),
   { message: 'already aborted' },
 );
 

tools/eslint-rules/prefer-abort-signal-abort.js

@@ -0,0 +1,154 @@
+/**
+ * @file Prefer AbortSignal.abort() for already-aborted signals.
+ */
+'use strict';
+
+const message = 'Use AbortSignal.abort() instead of creating and aborting an AbortController.';
+
+function isAbortControllerConstruction(node) {
+  return node?.type === 'NewExpression' &&
+         node.callee.type === 'Identifier' &&
+         node.callee.name === 'AbortController' &&
+         node.arguments.length === 0;
+}
+
+function isIdentifier(node, name) {
+  return node?.type === 'Identifier' && node.name === name;
+}
+
+function isProperty(node, name) {
+  return !node.computed && isIdentifier(node.property, name);
+}
+
+function isAbortCallStatement(node, name) {
+  const expression = node?.expression;
+  const callee = expression?.callee;
+  return node?.type === 'ExpressionStatement' &&
+         expression.type === 'CallExpression' &&
+         callee.type === 'MemberExpression' &&
+         isIdentifier(callee.object, name) &&
+         isProperty(callee, 'abort') &&
+         expression.arguments.length <= 1;
+}
+
+function isSignalReference(reference, name) {
+  const { identifier } = reference;
+  const parent = identifier.parent;
+  return isIdentifier(identifier, name) &&
+         parent?.type === 'MemberExpression' &&
+         parent.object === identifier &&
+         isProperty(parent, 'signal');
+}
+
+function isAbortReference(reference, abortStatement, name) {
+  const { identifier } = reference;
+  const parent = identifier.parent;
+  return isIdentifier(identifier, name) &&
+         parent?.type === 'MemberExpression' &&
+         parent.object === identifier &&
+         isProperty(parent, 'abort') &&
+         parent.parent === abortStatement.expression;
+}
+
+module.exports = {
+  meta: {
+    fixable: 'code',
+  },
+
+  create(context) {
+    const sourceCode = context.sourceCode;
+    const candidates = [];
+
+    function hasCommentsBetween(left, right) {
+      return sourceCode.getCommentsBefore(right)
+        .some((comment) => comment.range[0] > left.range[1]);
+    }
+
+    function rangeIncludingTrailingLine(statement) {
+      const tokenAfter = sourceCode.getTokenAfter(statement, { includeComments: true });
+      if (tokenAfter && tokenAfter.loc.start.line > statement.loc.end.line) {
+        return [statement.range[0], tokenAfter.range[0]];
+      }
+      return statement.range;
+    }
+
+    return {
+      VariableDeclarator(node) {
+        if (node.id.type !== 'Identifier' ||
+            !isAbortControllerConstruction(node.init) ||
+            node.parent.declarations.length !== 1) {
+          return;
+        }
+
+        const variableDeclaration = node.parent;
+        const parent = variableDeclaration.parent;
+        if (parent.type !== 'BlockStatement' && parent.type !== 'Program') {
+          return;
+        }
+
+        const index = parent.body.indexOf(variableDeclaration);
+        const abortStatement = parent.body[index + 1];
+        if (!isAbortCallStatement(abortStatement, node.id.name) ||
+            hasCommentsBetween(variableDeclaration, abortStatement)) {
+          return;
+        }
+
+        candidates.push({
+          abortStatement,
+          declarator: node,
+          variableDeclaration,
+        });
+      },
+
+      'Program:exit'() {
+        for (const { abortStatement, declarator, variableDeclaration } of candidates) {
+          const [variable] = sourceCode.scopeManager.getDeclaredVariables(declarator);
+          if (!variable) {
+            continue;
+          }
+
+          const name = declarator.id.name;
+          const references = variable.references.filter((reference) => {
+            return reference.identifier !== declarator.id;
+          });
+          const signalReferences = references.filter((reference) => {
+            return isSignalReference(reference, name);
+          });
+          const abortReferences = references.filter((reference) => {
+            return isAbortReference(reference, abortStatement, name);
+          });
+
+          if (references.length !== (1 + signalReferences.length) ||
+              abortReferences.length !== 1) {
+            continue;
+          }
+
+          const signalNode = signalReferences[0].identifier.parent;
+          const abortArguments = abortStatement.expression.arguments;
+          const abortReason = abortArguments.length === 0 ?
+            '' : sourceCode.getText(abortArguments[0]);
+
+          context.report({
+            node: declarator,
+            message,
+            fix(fixer) {
+              const abortSignalCreationCall = `AbortSignal.abort(${abortReason})`;
+              if (signalReferences.length > 1) {
+                return [
+                  fixer.replaceText(declarator.init, abortSignalCreationCall),
+                  fixer.removeRange(rangeIncludingTrailingLine(abortStatement)),
+                  ...signalReferences.map(({ identifier: { parent } }) => fixer.replaceText(parent, name)),
+                ];
+              }
+              return [
+                fixer.removeRange(rangeIncludingTrailingLine(variableDeclaration)),
+                fixer.removeRange(rangeIncludingTrailingLine(abortStatement)),
+                fixer.replaceText(signalNode, abortSignalCreationCall),
+              ];
+            },
+          });
+        }
+      },
+    };
+  },
+};