github-mirror/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2026-10-18 14:56:50 +02:00
Code Issues Actions 13 Packages Projects Releases Wiki Activity Graph

fix: Limit declarative style guidance to input item loops

Browse Source
This commit is contained in:
n8n-cat-bot[bot] 2026-09-24 22:01:47 +00:00
parent ea4681b09b
commit 8d2e9ee429
3 changed files with 277 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
packages/@n8n/eslint-plugin-community-nodes/docs/rules/prefer-declarative-style.md
View File

@ -6,6 +6,6 @@
## Rule Details
This rule reports a programmatic node when its `execute()` method only sends one HTTP request for each input item. Use `routing`, `requestDefaults`, and credential `authenticate` settings for this node instead.
This rule reports a programmatic node when its `execute()` method sends one HTTP request for each input item. It recognizes loops over `getInputData()` and indexed loops that use the input item index. Use `routing`, `requestDefaults`, and credential `authenticate` settings for this node instead.
The rule does not report trigger nodes, pagination logic, binary data handling, non-HTTP transports, multiple requests, multiple outputs, or workflow static data access.
The rule does not report trigger nodes, pagination logic, binary data handling, non-item or nested request loops, non-HTTP transports, multiple requests, multiple outputs, or workflow static data access.

110
packages/@n8n/eslint-plugin-community-nodes/src/rules/prefer-declarative-style.test.ts
View File

@ -5,12 +5,39 @@ import communityNodesPlugin, { configs } from '../plugin.js';
const ruleName = '@n8n/community-nodes/prefer-declarative-style';
function lint(code: string) {
return new Linter().verify(
code,
[
{
files: ['**/*.ts'],
plugins: { '@n8n/community-nodes': communityNodesPlugin },
rules: { [ruleName]: 'warn' },
},
],
{ filename: 'Test.node.ts' },
);
}
function nodeWithExecute(body: string, group = 'transform', outputs = "['main']") {
return `
class TestNode extends Node {
description = { group: ['${group}'], outputs: ${outputs} };
async execute() {
const items = this.getInputData();
${body}
}
}`;
}
const request = "await this.helpers.httpRequest({ url: 'https://api.example.com/items' });";
describe('prefer-declarative-style', () => {
it('warns for a non-trigger node that sends one HTTP request per item', () => {
const severity = Reflect.get(configs.recommended.rules ?? {}, ruleName);
expect(severity).toBe('warn');
const messages = new Linter().verify(
const messages = lint(
`
class TestNode extends Node {
description = {
@ -41,14 +68,6 @@ class TestNode extends Node {
}
}
`,
[
{
files: ['**/*.ts'],
plugins: { '@n8n/community-nodes': communityNodesPlugin },
rules: { [ruleName]: 'warn' },
},
],
{ filename: 'Test.node.ts' },
);
expect(messages).toEqual([
@ -58,4 +77,77 @@ class TestNode extends Node {
}),
]);
});
it.each([
['for-of', `for (const item of items) { ${request} }`],
[
'while',
`let itemIndex = 0; while (itemIndex < items.length) { this.getNodeParameter('id', itemIndex); ${request} itemIndex++; }`,
],
[
'do-while',
`let itemIndex = 0; do { this.getNodeParameter('id', itemIndex); ${request} itemIndex++; } while (itemIndex < items.length);`,
],
])('warns for a per-item %s loop', (_name, body) => {
expect(lint(nodeWithExecute(body))).toEqual([
expect.objectContaining({ ruleId: ruleName, severity: 1 }),
]);
});
it.each([
['retry loop', `for (let retry = 0; retry < 3; retry++) { ${request} }`],
[
'retry bounded by item count',
`for (let retry = 0; retry < items.length; retry++) { ${request} }`,
],
[
'nested retry loop',
`for (let i = 0; i < items.length; i++) { for (let retry = 0; retry < 3; retry++) { ${request} } }`,
],
[
'retry before item loop',
`for (let retry = 0; retry < 3; retry++) { ${request} } for (let i = 0; i < items.length; i++) {}`,
],
[
'pagination parameter',
`for (const item of items) { this.getNodeParameter('returnAll', 0); ${request} }`,
],
[
'binary parameter',
`for (const item of items) { this.getNodeParameter('binaryPropertyName', 0); ${request} }`,
],
[
'template parameter',
`for (const item of items) { this.getNodeParameter(\`page\`, 0); ${request} }`,
],
['multiple requests', `for (const item of items) { ${request} ${request} }`],
['non-HTTP transport', `for (const item of items) { this.sendMessage(item); ${request} }`],
['static data', `for (const item of items) { this.getWorkflowStaticData('node'); ${request} }`],
])('does not warn for %s', (_name, body) => {
expect(lint(nodeWithExecute(body))).toEqual([]);
});
it('does not warn for a trigger node', () => {
expect(lint(nodeWithExecute(`for (const item of items) { ${request} }`, 'trigger'))).toEqual(
[],
);
});
it('does not warn for a node with multiple outputs', () => {
expect(
lint(
nodeWithExecute(
`for (const item of items) { ${request} }`,
'transform',
"['main', 'main']",
),
),
).toEqual([]);
});
it('does not warn for a class that is not a node', () => {
expect(
lint('class Test { async execute() { for (const item of items) { ' + request + ' } } }'),
).toEqual([]);
});
});

191
packages/@n8n/eslint-plugin-community-nodes/src/rules/prefer-declarative-style.ts
View File

@ -9,6 +9,7 @@ import {
isFileType,
isNodeTypeClass,
isThisHelpersAccess,
isThisMethodCall,
isTriggerNode,
} from '../utils/index.js';
@ -21,25 +22,98 @@ type ExecuteState = {
httpRequestCount: number;
hasEscapeHatch: boolean;
requestInLoop: boolean;
loopDepth: number;
inputItems: Set<string>;
zeroIndexes: Set<string>;
loopStack: Array<{ index: string | null; referencesItem: boolean; hasRequest: boolean }>;
};
function isInputItems(node: TSESTree.Expression, inputItems: Set<string>): boolean {
return (
(node.type === AST_NODE_TYPES.Identifier && inputItems.has(node.name)) ||
(node.type === AST_NODE_TYPES.CallExpression && isThisMethodCall(node, 'getInputData'))
);
}
function isInputItemsLength(node: TSESTree.Expression, inputItems: Set<string>): boolean {
return (
node.type === AST_NODE_TYPES.MemberExpression &&
!node.computed &&
node.property.type === AST_NODE_TYPES.Identifier &&
node.property.name === 'length' &&
isInputItems(node.object, inputItems)
);
}
function getBoundedIndex(test: TSESTree.Expression | null, inputItems: Set<string>): string | null {
return test?.type === AST_NODE_TYPES.BinaryExpression &&
test.operator === '<' &&
test.left.type === AST_NODE_TYPES.Identifier &&
isInputItemsLength(test.right, inputItems)
? test.left.name
: null;
}
function incrementsIndex(node: TSESTree.Expression, index: string): boolean {
return (
node.type === AST_NODE_TYPES.UpdateExpression &&
node.operator === '++' &&
node.argument.type === AST_NODE_TYPES.Identifier &&
node.argument.name === index
);
}
function getItemLoopIndex(
node:
| TSESTree.ForStatement
| TSESTree.ForInStatement
| TSESTree.ForOfStatement
| TSESTree.WhileStatement
| TSESTree.DoWhileStatement,
state: ExecuteState,
): string | null {
if (node.type === AST_NODE_TYPES.ForOfStatement || node.type === AST_NODE_TYPES.ForInStatement) {
return isInputItems(node.right, state.inputItems) ? '' : null;
}
const index = getBoundedIndex(node.test, state.inputItems);
if (!index) return null;
if (node.type === AST_NODE_TYPES.ForStatement) {
const isItemLoop =
node.init?.type === AST_NODE_TYPES.VariableDeclaration &&
node.init.declarations.some(
(declaration) =>
declaration.id.type === AST_NODE_TYPES.Identifier &&
declaration.id.name === index &&
declaration.init?.type === AST_NODE_TYPES.Literal &&
declaration.init.value === 0,
) &&
node.update !== null &&
incrementsIndex(node.update, index);
return isItemLoop ? index : null;
}
const isItemLoop =
state.zeroIndexes.has(index) &&
node.body.type === AST_NODE_TYPES.BlockStatement &&
node.body.body.some(
(statement) =>
statement.type === AST_NODE_TYPES.ExpressionStatement &&
incrementsIndex(statement.expression, index),
);
return isItemLoop ? index : null;
}
function getMemberName(node: TSESTree.CallExpression): string | null {
if (node.callee.type !== AST_NODE_TYPES.MemberExpression) return null;
return node.callee.property.type === AST_NODE_TYPES.Identifier
? node.callee.property.name
: null;
return node.callee.property.type === AST_NODE_TYPES.Identifier ? node.callee.property.name : null;
}
function getHttpRequestMethod(node: TSESTree.CallExpression): string | null {
if (node.callee.type !== AST_NODE_TYPES.MemberExpression) return null;
const directMethod = getMemberName(node);
if (
directMethod &&
HTTP_REQUEST_METHODS.has(directMethod) &&
isThisHelpersAccess(node.callee)
) {
if (directMethod && HTTP_REQUEST_METHODS.has(directMethod) && isThisHelpersAccess(node.callee)) {
return directMethod;
}
@ -76,11 +150,28 @@ export const PreferDeclarativeStyleRule = createRule({
const eligibleClassStack: boolean[] = [];
let executeState: ExecuteState | null = null;
const enterLoop = () => {
if (executeState) executeState.loopDepth++;
const enterLoop = (
node:
| TSESTree.ForStatement
| TSESTree.ForInStatement
| TSESTree.ForOfStatement
| TSESTree.WhileStatement
| TSESTree.DoWhileStatement,
) => {
if (executeState) {
const index = getItemLoopIndex(node, executeState);
executeState.loopStack.push({ index, referencesItem: index === '', hasRequest: false });
}
};
const exitLoop = () => {
if (executeState) executeState.loopDepth--;
if (!executeState) return;
const loop = executeState.loopStack.pop();
executeState.requestInLoop ||=
executeState.loopStack.length === 0 &&
loop !== undefined &&
loop.index !== null &&
loop.referencesItem &&
loop.hasRequest;
};
return {
@ -88,7 +179,8 @@ export const PreferDeclarativeStyleRule = createRule({
const description = findNodeDescriptionObject(node);
const outputs = description ? findObjectProperty(description, 'outputs') : null;
const hasOneOutput =
outputs?.value.type === AST_NODE_TYPES.ArrayExpression && outputs.value.elements.length === 1;
outputs?.value.type === AST_NODE_TYPES.ArrayExpression &&
outputs.value.elements.length === 1;
eligibleClassStack.push(
isNodeTypeClass(node) &&
@ -108,7 +200,9 @@ export const PreferDeclarativeStyleRule = createRule({
httpRequestCount: 0,
hasEscapeHatch: false,
requestInLoop: false,
loopDepth: 0,
inputItems: new Set(),
zeroIndexes: new Set(),
loopStack: [],
};
}
},
@ -125,15 +219,71 @@ export const PreferDeclarativeStyleRule = createRule({
executeState = null;
},
VariableDeclarator(node) {
if (!executeState || node.id.type !== AST_NODE_TYPES.Identifier) return;
if (
node.init?.type === AST_NODE_TYPES.CallExpression &&
isThisMethodCall(node.init, 'getInputData')
) {
executeState.inputItems.add(node.id.name);
}
if (node.init?.type === AST_NODE_TYPES.Literal && node.init.value === 0) {
executeState.zeroIndexes.add(node.id.name);
}
},
ForStatement: enterLoop,
ForInStatement: enterLoop,
ForOfStatement: enterLoop,
WhileStatement: enterLoop,
DoWhileStatement: enterLoop,
'ForStatement:exit': exitLoop,
'ForInStatement:exit': exitLoop,
'ForOfStatement:exit': exitLoop,
'WhileStatement:exit': exitLoop,
'DoWhileStatement:exit': exitLoop,
Identifier(node) {
if (executeState && ESCAPE_HATCH_NAME.test(node.name)) {
if (!executeState) return;
if (ESCAPE_HATCH_NAME.test(node.name)) {
executeState.hasEscapeHatch = true;
}
const loop = executeState.loopStack.at(-1);
if (loop?.index !== node.name) return;
if (
(node.parent.type === AST_NODE_TYPES.MemberExpression &&
node.parent.computed &&
node.parent.property === node &&
isInputItems(node.parent.object, executeState.inputItems)) ||
(node.parent.type === AST_NODE_TYPES.CallExpression &&
isThisMethodCall(node.parent, 'getNodeParameter') &&
node.parent.arguments[1] === node) ||
(node.parent.type === AST_NODE_TYPES.Property &&
node.parent.value === node &&
getPropertyKeyName(node.parent) === 'item')
) {
loop.referencesItem = true;
}
},
Literal(node) {
if (!executeState || typeof node.value !== 'string' || !ESCAPE_HATCH_NAME.test(node.value))
return;
if (
(node.parent.type === AST_NODE_TYPES.CallExpression &&
getMemberName(node.parent) === 'getNodeParameter' &&
node.parent.arguments[0] === node) ||
(node.parent.type === AST_NODE_TYPES.Property && node.parent.key === node)
) {
executeState.hasEscapeHatch = true;
}
},
TemplateLiteral(node) {
if (
executeState &&
node.parent.type === AST_NODE_TYPES.CallExpression &&
getMemberName(node.parent) === 'getNodeParameter' &&
node.parent.arguments[0] === node &&
node.quasis.some((part) => ESCAPE_HATCH_NAME.test(part.value.cooked ?? ''))
) {
executeState.hasEscapeHatch = true;
}
},
@ -143,12 +293,19 @@ export const PreferDeclarativeStyleRule = createRule({
const httpRequestMethod = getHttpRequestMethod(node);
if (httpRequestMethod) {
executeState.httpRequestCount++;
executeState.requestInLoop ||= executeState.loopDepth > 0;
const loop = executeState.loopStack.length === 1 ? executeState.loopStack[0] : null;
if (loop) {
loop.hasRequest = true;
}
return;
}
const methodName = getMemberName(node);
if (methodName === 'getWorkflowStaticData' || !methodName || !ALLOWED_CALLS.has(methodName)) {
if (
methodName === 'getWorkflowStaticData' ||
!methodName ||
!ALLOWED_CALLS.has(methodName)
) {
executeState.hasEscapeHatch = true;
}
},