From 706183b79e5c4cde71098125eccc920dcd73ae68 Mon Sep 17 00:00:00 2001 From: subaru <79771445+subaru-hello@users.noreply.github.com> Date: Wed, 16 Sep 2026 07:13:39 +0000 Subject: [PATCH] fix(core): Read property keys by the name they stand for (no-changelog) (#34972) Co-authored-by: Claude Opus 5 (1M context) --- .../rules/no-silent-error-swallowing.test.ts | 26 ++++++++++++ .../src/rules/no-silent-error-swallowing.ts | 11 +---- .../rules/webhook-lifecycle-complete.test.ts | 42 +++++++++++++++++++ .../src/rules/webhook-lifecycle-complete.ts | 15 ++----- .../src/utils/ast-utils.ts | 35 +++++++++++++--- 5 files changed, 102 insertions(+), 27 deletions(-) diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-silent-error-swallowing.test.ts b/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-silent-error-swallowing.test.ts index 95ee4062fc8..d543e483b67 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-silent-error-swallowing.test.ts +++ b/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-silent-error-swallowing.test.ts @@ -174,6 +174,32 @@ export class RegularClass { }`), errors: [{ messageId: 'silentReturn', data: { method: 'delete' } }], }, + { + name: 'lifecycle method written with a computed literal key', + code: createTriggerNode(`{ + default: { + ['delete']: async function (this: IHookFunctions): Promise { + try { + return await this.helpers.httpRequest({ url: 'https://example.com' }); + } catch (error) {} + }, + }, + }`), + errors: [{ messageId: 'emptyCatch', data: { method: 'delete' } }], + }, + { + name: 'lifecycle group written with a quoted key', + code: createTriggerNode(`{ + 'default': { + async delete(this: IHookFunctions): Promise { + try { + return await this.helpers.httpRequest({ url: 'https://example.com' }); + } catch (error) {} + }, + }, + }`), + errors: [{ messageId: 'emptyCatch', data: { method: 'delete' } }], + }, { name: 'catch block with a bare return', code: createTriggerNode(`{ diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-silent-error-swallowing.ts b/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-silent-error-swallowing.ts index 4978761538d..f44a1cc638a 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-silent-error-swallowing.ts +++ b/packages/@n8n/eslint-plugin-community-nodes/src/rules/no-silent-error-swallowing.ts @@ -3,20 +3,13 @@ import { AST_NODE_TYPES, type TSESTree } from '@typescript-eslint/utils'; import { createRule, findClassProperty, + getPropertyKeyName, isNodeTypeClass, WEBHOOK_LIFECYCLE_METHODS, } from '../utils/index.js'; const LIFECYCLE_METHODS: readonly string[] = WEBHOOK_LIFECYCLE_METHODS; -/** Returns the static name of a non-computed object property key, or null. */ -function getPropertyName(property: TSESTree.Property): string | null { - if (property.computed) return null; - if (property.key.type === AST_NODE_TYPES.Identifier) return property.key.name; - if (property.key.type === AST_NODE_TYPES.Literal) return String(property.key.value); - return null; -} - function isFunctionNode(node: TSESTree.Node): boolean { return ( node.type === AST_NODE_TYPES.FunctionDeclaration || @@ -81,7 +74,7 @@ export const NoSilentErrorSwallowingRule = createRule({ for (const methodProperty of groupProperty.value.properties) { if (methodProperty.type !== AST_NODE_TYPES.Property) continue; - const methodName = getPropertyName(methodProperty); + const methodName = getPropertyKeyName(methodProperty); if (methodName === null || !LIFECYCLE_METHODS.includes(methodName)) continue; const fn = methodProperty.value; diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/rules/webhook-lifecycle-complete.test.ts b/packages/@n8n/eslint-plugin-community-nodes/src/rules/webhook-lifecycle-complete.test.ts index 885d3bc3e17..eb6e8013a78 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/src/rules/webhook-lifecycle-complete.test.ts +++ b/packages/@n8n/eslint-plugin-community-nodes/src/rules/webhook-lifecycle-complete.test.ts @@ -88,6 +88,30 @@ export class RegularClass { }`, }), }, + { + name: 'lifecycle methods written with computed literal keys', + code: createTriggerNode({ + webhookMethods: `{ + default: { + ['checkExists']: async () => true, + ['create']: async () => true, + [\`delete\`]: async () => true, + }, + }`, + }), + }, + { + name: 'lifecycle group and methods written with quoted keys', + code: createTriggerNode({ + webhookMethods: `{ + 'default': { + 'checkExists': async () => true, + 'create': async () => true, + 'delete': async () => true, + }, + }`, + }), + }, { name: 'webhook lifecycle defined via arrow functions', code: createTriggerNode({ @@ -189,6 +213,24 @@ export class RegularClass { }), errors: [{ messageId: 'missingWebhookMethods' }], }, + { + name: 'lifecycle method behind a key only known at runtime', + code: createTriggerNode({ + webhookMethods: `{ + default: { + async checkExists() { return true; }, + async create() { return true; }, + [methodName]: async () => true, + }, + }`, + }), + errors: [ + { + messageId: 'missingLifecycleMethod', + data: { group: 'default', missing: '`delete`' }, + }, + ], + }, { name: 'multiple webhook groups each missing methods', code: createTriggerNode({ diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/rules/webhook-lifecycle-complete.ts b/packages/@n8n/eslint-plugin-community-nodes/src/rules/webhook-lifecycle-complete.ts index bddd5c2c301..c7dce0ef63e 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/src/rules/webhook-lifecycle-complete.ts +++ b/packages/@n8n/eslint-plugin-community-nodes/src/rules/webhook-lifecycle-complete.ts @@ -4,6 +4,7 @@ import { createRule, findClassProperty, findObjectProperty, + getPropertyKeyName, isNodeTypeClass, WEBHOOK_LIFECYCLE_METHODS, type WebhookLifecycleMethod, @@ -25,12 +26,7 @@ function hasWebhooksDeclared(descriptionValue: TSESTree.ObjectExpression): boole /** Returns true when the property defines a (possibly async) method named `name`. */ function isMethodProperty(property: TSESTree.ObjectLiteralElement, name: string): boolean { if (property.type !== AST_NODE_TYPES.Property) return false; - if (property.computed) return false; - - const keyMatches = - (property.key.type === AST_NODE_TYPES.Identifier && property.key.name === name) || - (property.key.type === AST_NODE_TYPES.Literal && property.key.value === name); - if (!keyMatches) return false; + if (getPropertyKeyName(property) !== name) return false; return ( property.value.type === AST_NODE_TYPES.FunctionExpression || @@ -104,12 +100,7 @@ export const WebhookLifecycleCompleteRule = createRule({ if (groupProperty.type !== AST_NODE_TYPES.Property) continue; if (groupProperty.value.type !== AST_NODE_TYPES.ObjectExpression) continue; - const groupName = - groupProperty.key.type === AST_NODE_TYPES.Identifier - ? groupProperty.key.name - : groupProperty.key.type === AST_NODE_TYPES.Literal - ? String(groupProperty.key.value) - : 'default'; + const groupName = getPropertyKeyName(groupProperty) ?? 'default'; const missing = findMissingMethods(groupProperty.value); if (missing.length === 0) continue; diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/utils/ast-utils.ts b/packages/@n8n/eslint-plugin-community-nodes/src/utils/ast-utils.ts index c575046fe47..69a5ff9d00c 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/src/utils/ast-utils.ts +++ b/packages/@n8n/eslint-plugin-community-nodes/src/utils/ast-utils.ts @@ -55,6 +55,33 @@ export function isTriggerNode( return hasTriggerGroup(descriptionValue) || isTriggerNodeClass(node); } +/** + * Returns the name a property key stands for, or null when it is only known at + * runtime. `delete`, `'delete'` and `['delete']` all name the same property, so + * how the key is written must not decide whether a rule sees it. + */ +export function getPropertyKeyName( + property: TSESTree.Property | TSESTree.PropertyDefinition | TSESTree.MethodDefinition, +): string | null { + const { key } = property; + + if (key.type === AST_NODE_TYPES.Literal) { + return key.value === null ? null : String(key.value); + } + + // A computed key still names one property when it is a template without + // substitutions; written plainly, an identifier is the name itself. + if (key.type === AST_NODE_TYPES.TemplateLiteral) { + return key.expressions.length === 0 ? (key.quasis[0]?.value.cooked ?? null) : null; + } + + if (key.type === AST_NODE_TYPES.Identifier) { + return property.computed ? null : key.name; + } + + return null; +} + export function findClassProperty( node: TSESTree.ClassDeclaration, propertyName: string, @@ -62,8 +89,7 @@ export function findClassProperty( const property = node.body.body.find( (member) => member.type === AST_NODE_TYPES.PropertyDefinition && - member.key?.type === AST_NODE_TYPES.Identifier && - member.key.name === propertyName, + getPropertyKeyName(member) === propertyName, ); return property?.type === AST_NODE_TYPES.PropertyDefinition ? property : null; } @@ -140,10 +166,7 @@ export function findObjectProperty( propertyName: string, ): TSESTree.Property | null { const property = obj.properties.find( - (prop) => - prop.type === AST_NODE_TYPES.Property && - prop.key.type === AST_NODE_TYPES.Identifier && - prop.key.name === propertyName, + (prop) => prop.type === AST_NODE_TYPES.Property && getPropertyKeyName(prop) === propertyName, ); return property?.type === AST_NODE_TYPES.Property ? property : null; }