From 2cd59c6aafbd30eeb544276dd64a12ee26288d8a Mon Sep 17 00:00:00 2001 From: "Kamat, Trivikram" <16024985+trivikr@users.noreply.github.com> Date: Sun, 24 May 2026 09:08:48 -0700 Subject: [PATCH 1/4] tools: add lint rule for aborted AbortController Refs: https://github.com/nodejs/node/pull/63489 Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: openai:gpt-5.5 --- test/eslint.config_partial.mjs | 1 + test/parallel/test-abortcontroller.js | 5 +- .../test-eslint-prefer-abort-signal-abort.js | 91 +++++++++++ .../test-quic-writer-abort-signal.mjs | 5 +- .../eslint-rules/prefer-abort-signal-abort.js | 147 ++++++++++++++++++ 5 files changed, 243 insertions(+), 6 deletions(-) create mode 100644 test/parallel/test-eslint-prefer-abort-signal-abort.js create mode 100644 tools/eslint-rules/prefer-abort-signal-abort.js diff --git a/test/eslint.config_partial.mjs b/test/eslint.config_partial.mjs index 6fbdf277044679..70c00ef5d4dcda 100644 --- a/test/eslint.config_partial.mjs +++ b/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', }, }, { diff --git a/test/parallel/test-abortcontroller.js b/test/parallel/test-abortcontroller.js index 948bd208cd2b90..b3de0e07bdca42 100644 --- a/test/parallel/test-abortcontroller.js +++ b/test/parallel/test-abortcontroller.js @@ -161,9 +161,8 @@ test('AbortController inspection depth 1 or null works', () => { test('AbortSignal reason is set correctly', () => { // Test AbortSignal.reason - const ac = new AbortController(); - ac.abort('reason'); - assert.strictEqual(ac.signal.reason, 'reason'); + const signal = AbortSignal.abort('reason'); + assert.strictEqual(signal.reason, 'reason'); }); test('AbortSignal reasonable is set correctly with AbortSignal.abort()', () => { diff --git a/test/parallel/test-eslint-prefer-abort-signal-abort.js b/test/parallel/test-eslint-prefer-abort-signal-abort.js new file mode 100644 index 00000000000000..509b62436b9d2c --- /dev/null +++ b/test/parallel/test-eslint-prefer-abort-signal-abort.js @@ -0,0 +1,91 @@ +'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(); +fn(controller.signal, 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() }); +} +`, + }, + ] +}); diff --git a/test/parallel/test-quic-writer-abort-signal.mjs b/test/parallel/test-quic-writer-abort-signal.mjs index 8d0dbb0351d931..a00e5904f670d1 100644 --- a/test/parallel/test-quic-writer-abort-signal.mjs +++ b/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' }, ); diff --git a/tools/eslint-rules/prefer-abort-signal-abort.js b/tools/eslint-rules/prefer-abort-signal-abort.js new file mode 100644 index 00000000000000..a4977ea33b3aa8 --- /dev/null +++ b/tools/eslint-rules/prefer-abort-signal-abort.js @@ -0,0 +1,147 @@ +/** + * @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 !== 2 || + signalReferences.length !== 1 || + 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: signalNode, + message, + fix(fixer) { + return [ + fixer.removeRange(rangeIncludingTrailingLine(variableDeclaration)), + fixer.removeRange(rangeIncludingTrailingLine(abortStatement)), + fixer.replaceText(signalNode, `AbortSignal.abort(${abortReason})`), + ]; + }, + }); + } + }, + }; + }, +}; From e395f746e65126d7d84c9368d950d521aad07391 Mon Sep 17 00:00:00 2001 From: "Kamat, Trivikram" <16024985+trivikr@users.noreply.github.com> Date: Mon, 25 May 2026 08:28:22 -0700 Subject: [PATCH 2/4] test: disable prefer-abort-signal-abort in abort-controller test --- test/parallel/test-abortcontroller.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-abortcontroller.js b/test/parallel/test-abortcontroller.js index b3de0e07bdca42..bdaf222d60ee4a 100644 --- a/test/parallel/test-abortcontroller.js +++ b/test/parallel/test-abortcontroller.js @@ -161,8 +161,10 @@ test('AbortController inspection depth 1 or null works', () => { test('AbortSignal reason is set correctly', () => { // Test AbortSignal.reason - const signal = AbortSignal.abort('reason'); - assert.strictEqual(signal.reason, 'reason'); + const ac = new AbortController(); + ac.abort('reason'); + // eslint-disable-next-line node-core/prefer-abort-signal-abort + assert.strictEqual(ac.signal.reason, 'reason'); }); test('AbortSignal reasonable is set correctly with AbortSignal.abort()', () => { From 69dedf764e9acc7c8b2b6caaed818d59b4b5abab Mon Sep 17 00:00:00 2001 From: "Kamat, Trivikram" <16024985+trivikr@users.noreply.github.com> Date: Mon, 25 May 2026 08:31:59 -0700 Subject: [PATCH 3/4] test: improve prefer-abort-signal-abort case formatting --- .../test-eslint-prefer-abort-signal-abort.js | 98 +++++++++---------- 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/test/parallel/test-eslint-prefer-abort-signal-abort.js b/test/parallel/test-eslint-prefer-abort-signal-abort.js index 509b62436b9d2c..4a7917cb341064 100644 --- a/test/parallel/test-eslint-prefer-abort-signal-abort.js +++ b/test/parallel/test-eslint-prefer-abort-signal-abort.js @@ -16,76 +16,76 @@ 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(); + controller.abort(); + fn(controller.signal); + `, ` -const controller = new AbortController(); -controller.abort(); -fn(controller.signal, controller.signal); -`, + const controller = new AbortController(); + controller.abort(); + fn(controller.signal, controller.signal); + `, ` -const controller = new AbortController(); -controller.abort(); -console.log(controller); -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(); + // 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(); + setImmediate(() => controller.abort()); + fn(controller.signal); + `, ` -const controller = new AbortController(); -controller.abort('reason', 'extra'); -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); -`, + const controller = new AbortController(); + controller.abort(); + fn(controller.signal); + `, errors: [{ message }], output: ` -fn(AbortSignal.abort()); -`, + fn(AbortSignal.abort()); + `, }, { code: ` -const abortController = new AbortController(); -abortController.abort(new Error('aborted')); -fn({ signal: abortController.signal }); -`, + const abortController = new AbortController(); + abortController.abort(new Error('aborted')); + fn({ signal: abortController.signal }); + `, errors: [{ message }], output: ` -fn({ signal: AbortSignal.abort(new Error('aborted')) }); -`, + fn({ signal: AbortSignal.abort(new Error('aborted')) }); + `, }, { code: ` -{ - const ac = new AbortController(); - ac.abort(); - await wait({ signal: ac.signal }); -} -`, + { + const ac = new AbortController(); + ac.abort(); + await wait({ signal: ac.signal }); + } + `, errors: [{ message }], output: ` -{ - await wait({ signal: AbortSignal.abort() }); -} -`, + { + await wait({ signal: AbortSignal.abort() }); + } + `, }, ] }); From 9df000b3ddf0453fa97a524d946157986ad3db5b Mon Sep 17 00:00:00 2001 From: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Date: Mon, 25 May 2026 14:06:35 -0700 Subject: [PATCH 4/4] tools: check for refs as one plus signal refs Co-authored-by: Antoine du Hamel --- tools/eslint-rules/prefer-abort-signal-abort.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/eslint-rules/prefer-abort-signal-abort.js b/tools/eslint-rules/prefer-abort-signal-abort.js index a4977ea33b3aa8..1b44b55f65e541 100644 --- a/tools/eslint-rules/prefer-abort-signal-abort.js +++ b/tools/eslint-rules/prefer-abort-signal-abort.js @@ -118,8 +118,7 @@ module.exports = { return isAbortReference(reference, abortStatement, name); }); - if (references.length !== 2 || - signalReferences.length !== 1 || + if (references.length !== (1 + signalReferences.length) || abortReferences.length !== 1) { continue; }