diff --git a/packages/@n8n/eslint-plugin-community-nodes/README.md b/packages/@n8n/eslint-plugin-community-nodes/README.md index db539c78ff6..96a5fd63178 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/README.md +++ b/packages/@n8n/eslint-plugin-community-nodes/README.md @@ -77,6 +77,7 @@ export default [ | [node-connection-type-literal](docs/rules/node-connection-type-literal.md) | Disallow string literals in node description `inputs`/`outputs` — use `NodeConnectionTypes` enum instead | ✅ ☑️ | | 🔧 | | | | [node-registration-complete](docs/rules/node-registration-complete.md) | Ensure every `.node.ts` file in the `nodes/` directory is registered in the "n8n.nodes" array of package.json | | ✅ ☑️ | | | | | [node-usable-as-tool](docs/rules/node-usable-as-tool.md) | Ensure node classes have usableAsTool property | ✅ ☑️ | | 🔧 | | | +| [open-ended-node-fields](docs/rules/open-ended-node-fields.md) | Discourage optional open-ended JSON fields in node properties | | ✅ ☑️ | | | | | [package-name-convention](docs/rules/package-name-convention.md) | Enforce correct package naming convention for n8n community nodes | ✅ ☑️ | | | 💡 | | | [require-files-array](docs/rules/require-files-array.md) | Require a non-empty "files" array in package.json | | ✅ ☑️ | | | | | [require-homepage](docs/rules/require-homepage.md) | Require a "homepage" field with a valid URL in package.json | | ✅ ☑️ | | | | diff --git a/packages/@n8n/eslint-plugin-community-nodes/docs/rules/open-ended-node-fields.md b/packages/@n8n/eslint-plugin-community-nodes/docs/rules/open-ended-node-fields.md new file mode 100644 index 00000000000..5907ae66bdc --- /dev/null +++ b/packages/@n8n/eslint-plugin-community-nodes/docs/rules/open-ended-node-fields.md @@ -0,0 +1,35 @@ +# Discourage optional open-ended JSON fields in node properties (`@n8n/community-nodes/open-ended-node-fields`) + +⚠️ This rule _warns_ in the following configs: ✅ `recommended`, ☑️ `recommendedWithoutN8nCloudSupport`. + + + +## Rule Details + +This rule warns about optional top-level JSON properties in node descriptions. A JSON configuration field can hide parameters from the editor. Typed parameters make configuration easier to find, validate, and change. + +Place optional parameters under Additional Options or Additional Fields. The rule does not report required JSON properties or typed collection options. + +## Examples + +### Incorrect + +```typescript +properties: [ + { displayName: 'Configuration', name: 'configuration', type: 'json', default: '{}' }, +]; +``` + +### Correct + +```typescript +properties: [ + { + displayName: 'Additional Fields', + name: 'additionalFields', + type: 'collection', + default: {}, + options: [{ displayName: 'Limit', name: 'limit', type: 'number', default: 10 }], + }, +]; +``` diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/plugin.ts b/packages/@n8n/eslint-plugin-community-nodes/src/plugin.ts index 622119334fb..049c9234493 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/src/plugin.ts +++ b/packages/@n8n/eslint-plugin-community-nodes/src/plugin.ts @@ -29,6 +29,7 @@ const configs = { '@n8n/community-nodes/no-deprecated-workflow-functions': 'error', '@n8n/community-nodes/no-emoji-in-options': 'error', '@n8n/community-nodes/node-usable-as-tool': 'error', + '@n8n/community-nodes/open-ended-node-fields': 'warn', '@n8n/community-nodes/package-name-convention': 'error', '@n8n/community-nodes/credential-test-required': 'error', '@n8n/community-nodes/no-credential-reuse': 'error', @@ -79,6 +80,7 @@ const configs = { '@n8n/community-nodes/no-deprecated-workflow-functions': 'error', '@n8n/community-nodes/no-emoji-in-options': 'error', '@n8n/community-nodes/node-usable-as-tool': 'error', + '@n8n/community-nodes/open-ended-node-fields': 'warn', '@n8n/community-nodes/package-name-convention': 'error', '@n8n/community-nodes/credential-test-required': 'error', '@n8n/community-nodes/no-credential-reuse': 'error', diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/rules/index.ts b/packages/@n8n/eslint-plugin-community-nodes/src/rules/index.ts index d178a210639..cb51527cb18 100644 --- a/packages/@n8n/eslint-plugin-community-nodes/src/rules/index.ts +++ b/packages/@n8n/eslint-plugin-community-nodes/src/rules/index.ts @@ -31,6 +31,7 @@ import { NodeClassDescriptionIconMissingRule } from './node-class-description-ic import { NodeConnectionTypeLiteralRule } from './node-connection-type-literal.js'; import { NodeRegistrationCompleteRule } from './node-registration-complete.js'; import { NodeUsableAsToolRule } from './node-usable-as-tool.js'; +import { OpenEndedNodeFieldsRule } from './open-ended-node-fields.js'; import { PackageNameConventionRule } from './package-name-convention.js'; import { RequireFilesArrayRule } from './require-files-array.js'; import { RequireHomepageRule } from './require-homepage.js'; @@ -54,6 +55,7 @@ export const rules = { 'no-deprecated-workflow-functions': NoDeprecatedWorkflowFunctionsRule, 'no-emoji-in-options': NoEmojiInOptionsRule, 'node-usable-as-tool': NodeUsableAsToolRule, + 'open-ended-node-fields': OpenEndedNodeFieldsRule, 'package-name-convention': PackageNameConventionRule, 'credential-test-required': CredentialTestRequiredRule, 'credential-unnecessary-password': CredentialUnnecessaryPasswordRule, diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/rules/open-ended-node-fields.test.ts b/packages/@n8n/eslint-plugin-community-nodes/src/rules/open-ended-node-fields.test.ts new file mode 100644 index 00000000000..5b50fe308cc --- /dev/null +++ b/packages/@n8n/eslint-plugin-community-nodes/src/rules/open-ended-node-fields.test.ts @@ -0,0 +1,122 @@ +import { Linter } from 'eslint'; +import { describe, expect, it } from 'vitest'; + +import { configs } from '../plugin.js'; + +const linter = new Linter(); + +function createNodeCode(property: string): string { + return ` +export class TestNode extends Node { + description = { + displayName: 'Test Node', + name: 'testNode', + group: ['transform'], + version: 1, + inputs: ['main'], + outputs: ['main'], + properties: [${property}], + }; +}`; +} + +function lintNode(code: string, config: keyof typeof configs) { + return linter.verify(code, [{ ...configs[config], files: ['**/*.node.ts'] }], { + filename: 'src/rules/TestNode.node.ts', + }); +} + +describe.each(['recommended', 'recommendedWithoutN8nCloudSupport'] as const)( + '%s node property review', + (config) => { + // CE-1879: Warn when an optional JSON field replaces typed node parameters. + it('warns about an optional JSON configuration catch-all', () => { + const code = createNodeCode(`{ + displayName: 'Configuration', + name: 'configuration', + type: 'json', + required: false, + default: '{}', + }`); + + const warnings = lintNode(code, config).filter( + ({ severity, message }) => severity === 1 && /Additional (Options|Fields)/i.test(message), + ); + + expect(warnings).toEqual([ + expect.objectContaining({ + severity: 1, + message: expect.stringMatching(/Additional (Options|Fields)/i), + }), + ]); + }); + + it('does not warn about typed parameters inside Additional Fields', () => { + const code = createNodeCode(`{ + displayName: 'Additional Fields', + name: 'additionalFields', + type: 'collection', + default: {}, + options: [{ displayName: 'Limit', name: 'limit', type: 'number', default: 10 }], + }`); + + const messages = lintNode(code, config); + expect(messages).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + ruleId: '@n8n/community-nodes/require-node-description-fields', + }), + ]), + ); + + const warnings = messages.filter( + ({ severity, message }) => severity === 1 && /Additional (Options|Fields)/i.test(message), + ); + + expect(warnings).toEqual([]); + }); + + it('warns when an open-ended JSON field does not declare required', () => { + const code = createNodeCode(`{ + displayName: 'Configuration', + name: 'configuration', + type: 'json', + default: '{}', + }`); + + expect( + lintNode(code, config).filter( + ({ ruleId }) => ruleId === '@n8n/community-nodes/open-ended-node-fields', + ), + ).toHaveLength(1); + }); + + it('does not warn about required JSON fields', () => { + const code = createNodeCode(`{ + displayName: 'Request Body', + name: 'body', + type: 'json', + required: true, + default: '{}', + }`); + + expect( + lintNode(code, config).filter( + ({ ruleId }) => ruleId === '@n8n/community-nodes/open-ended-node-fields', + ), + ).toEqual([]); + }); + + it('does not inspect classes that are not nodes', () => { + const code = `class Helper { + description = { properties: [{ type: 'json', required: false }] }; + }`; + + expect( + lintNode(code, config).filter( + ({ ruleId }) => ruleId === '@n8n/community-nodes/open-ended-node-fields', + ), + ).toEqual([]); + }); + }, +); diff --git a/packages/@n8n/eslint-plugin-community-nodes/src/rules/open-ended-node-fields.ts b/packages/@n8n/eslint-plugin-community-nodes/src/rules/open-ended-node-fields.ts new file mode 100644 index 00000000000..58e99caca0e --- /dev/null +++ b/packages/@n8n/eslint-plugin-community-nodes/src/rules/open-ended-node-fields.ts @@ -0,0 +1,55 @@ +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; + +import { + createRule, + findArrayLiteralProperty, + findNodeDescriptionObject, + findObjectProperty, + getBooleanLiteralValue, + getStringLiteralValue, + isFileType, + isNodeTypeClass, +} from '../utils/index.js'; + +export const OpenEndedNodeFieldsRule = createRule({ + name: 'open-ended-node-fields', + meta: { + type: 'suggestion', + docs: { + description: 'Discourage optional open-ended JSON fields in node properties', + }, + messages: { + optionalJson: + 'Optional JSON fields can hide configuration. Use typed parameters under Additional Options or Additional Fields instead.', + }, + schema: [], + }, + defaultOptions: [], + create(context) { + if (!isFileType(context.filename, '.node.ts')) return {}; + + return { + ClassDeclaration(node) { + if (!isNodeTypeClass(node)) return; + + const description = findNodeDescriptionObject(node); + if (!description) return; + + const properties = findArrayLiteralProperty(description, 'properties'); + if (!properties) return; + + for (const property of properties.elements) { + if (property?.type !== AST_NODE_TYPES.ObjectExpression) continue; + + const type = findObjectProperty(property, 'type'); + if (getStringLiteralValue(type?.value ?? null) !== 'json') continue; + + const required = findObjectProperty(property, 'required'); + if (getBooleanLiteralValue(required?.value ?? null) === true) continue; + + context.report({ node: property, messageId: 'optionalJson' }); + } + }, + }; + }, +});