From fd7ea9487dea395b2dd14e5b705696100c5edcc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CStelios?= Date: Wed, 3 Dec 2025 23:33:06 +0200 Subject: [PATCH] [kbn-eslint-plugin-eslint] add scout_api_test_discourage_kbn_client rule --- .eslintrc.js | 1 + packages/kbn-eslint-plugin-eslint/index.js | 1 + .../scout_api_test_discourage_kbn_client.js | 177 ++++++++++++++++++ ...out_api_test_discourage_kbn_client.test.js | 102 ++++++++++ 4 files changed, 281 insertions(+) create mode 100644 packages/kbn-eslint-plugin-eslint/rules/scout_api_test_discourage_kbn_client.js create mode 100644 packages/kbn-eslint-plugin-eslint/rules/scout_api_test_discourage_kbn_client.test.js diff --git a/.eslintrc.js b/.eslintrc.js index bd96d42821c4b..7a45ed8234e34 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -2702,6 +2702,7 @@ module.exports = { rules: { '@kbn/eslint/scout_no_describe_configure': 'error', '@kbn/eslint/require_include_in_check_a11y': 'warn', + '@kbn/eslint/scout_api_test_discourage_kbn_client': 'warn', }, }, { diff --git a/packages/kbn-eslint-plugin-eslint/index.js b/packages/kbn-eslint-plugin-eslint/index.js index cd5a2124c3604..1d5ecb87c5f05 100644 --- a/packages/kbn-eslint-plugin-eslint/index.js +++ b/packages/kbn-eslint-plugin-eslint/index.js @@ -24,6 +24,7 @@ module.exports = { no_deprecated_imports: require('./rules/no_deprecated_imports'), deployment_agnostic_test_context: require('./rules/deployment_agnostic_test_context'), scout_no_describe_configure: require('./rules/scout_no_describe_configure'), + scout_api_test_discourage_kbn_client: require('./rules/scout_api_test_discourage_kbn_client'), require_kbn_fs: require('./rules/require_kbn_fs'), require_include_in_check_a11y: require('./rules/require_include_in_check_a11y'), }, diff --git a/packages/kbn-eslint-plugin-eslint/rules/scout_api_test_discourage_kbn_client.js b/packages/kbn-eslint-plugin-eslint/rules/scout_api_test_discourage_kbn_client.js new file mode 100644 index 0000000000000..4acc0bb1ac5d8 --- /dev/null +++ b/packages/kbn-eslint-plugin-eslint/rules/scout_api_test_discourage_kbn_client.js @@ -0,0 +1,177 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +/** @typedef {import("eslint").Rule.RuleModule} Rule */ +/** @typedef {import("@typescript-eslint/typescript-estree").TSESTree.CallExpression} CallExpression */ +/** @typedef {import("@typescript-eslint/typescript-estree").TSESTree.Identifier} Identifier */ + +const ERROR_MSG = 'API tests must not use the kbnClient fixture.'; + +const traverse = require('eslint-traverse'); + +const isApiTestCall = (node) => node.callee.type === 'Identifier' && node.callee.name === 'apiTest'; + +/** Get local param name for `kbnClient` (supports `{ kbnClient: alias }`). */ +const getKbnClientLocalName = (fnNode) => { + const firstParam = fnNode.params[0]; + if (!firstParam || firstParam.type !== 'ObjectPattern') { + return 'kbnClient'; + } + + const prop = firstParam.properties.find((p) => { + if (p.type === 'RestElement') return false; + return p.key && p.key.name === 'kbnClient'; + }); + + return prop?.value?.name || 'kbnClient'; +}; + +/** Helper: Check if an identifier is used anywhere in a function body (simple recursion). */ +const paramUsesIdentifierInBody = (node, paramName) => { + if (!node || typeof node !== 'object') return false; + + if (node.type === 'Identifier' && node.name === paramName) return true; + + for (const key in node) { + if ( + key === 'parent' || + key === 'loc' || + key === 'range' || + key === 'leadingComments' || + key === 'trailingComments' + ) { + continue; + } + const value = node[key]; + if (Array.isArray(value)) { + for (const item of value) { + if (paramUsesIdentifierInBody(item, paramName)) return true; + } + } else if (value && typeof value === 'object') { + if (paramUsesIdentifierInBody(value, paramName)) return true; + } + } + return false; +}; + +/** + * Determine whether `fnNode` uses `kbnClient`. + * + * Strategy: Single-pass traversal that: + * 1. Collects variable aliases pointing to kbnClient (e.g., `const client = kbnClient`) + * 2. Collects local function definitions and their parameter usage + * 3. Detects member access on kbnClient or calls passing kbnClient to functions that use it + */ +const functionUsesKbnClient = (fnNode, context) => { + const kbnClientName = getKbnClientLocalName(fnNode); + const variableAliases = new Set([kbnClientName]); + const localFnUsesParam = new Map(); + let found = false; + + traverse(context, fnNode.body, (path) => { + if (found) return traverse.SKIP; + + const node = path.node; + + if (node.type === 'VariableDeclarator' && node.init && node.id?.type === 'Identifier') { + if (node.init.type === 'Identifier' && variableAliases.has(node.init.name)) { + variableAliases.add(node.id.name); + } + + if (node.init.type === 'FunctionExpression' || node.init.type === 'ArrowFunctionExpression') { + const fnName = node.id.name; + const paramName = getKbnClientLocalName(node.init); + localFnUsesParam.set(fnName, paramUsesIdentifierInBody(node.init.body, paramName)); + } + } + + if (node.type === 'FunctionDeclaration' && node.id?.type === 'Identifier') { + const fnName = node.id.name; + const paramName = getKbnClientLocalName(node); + localFnUsesParam.set(fnName, paramUsesIdentifierInBody(node.body, paramName)); + } + + if ( + node.type === 'MemberExpression' && + node.object?.type === 'Identifier' && + variableAliases.has(node.object.name) + ) { + found = true; + return traverse.SKIP; + } + + if (node.type === 'CallExpression' && node.arguments) { + for (const arg of node.arguments) { + if (arg.type === 'Identifier' && variableAliases.has(arg.name)) { + const callee = node.callee; + if (callee.type === 'Identifier' && localFnUsesParam.has(callee.name)) { + if (localFnUsesParam.get(callee.name)) { + found = true; + return traverse.SKIP; + } + } else { + found = true; + return traverse.SKIP; + } + } + } + } + }); + + return found; +}; + +/** @type {Rule} */ +module.exports = { + meta: { + type: 'suggestion', + docs: { + description: 'Discourage `kbnClient` fixture usage in API tests.', + category: 'Best Practices', + }, + fixable: null, + schema: [], + }, + + create(context) { + return { + CallExpression(node) { + if (!isApiTestCall(node)) return; + + const callbackArg = node.arguments[node.arguments.length - 1]; + if ( + !callbackArg || + (callbackArg.type !== 'ArrowFunctionExpression' && + callbackArg.type !== 'FunctionExpression') + ) { + return; + } + + const sourceCode = context.getSourceCode(); + const comments = sourceCode.getCommentsBefore(node); + if ( + comments.some((c) => + /eslint-disable-next-line\s+@kbn\/eslint\/scout_api_test_discourage_kbn_client/.test( + c.value + ) + ) + ) { + return; + } + + if (functionUsesKbnClient(callbackArg, context)) { + context.report({ + node, + message: ERROR_MSG, + }); + } + }, + }; + }, +}; diff --git a/packages/kbn-eslint-plugin-eslint/rules/scout_api_test_discourage_kbn_client.test.js b/packages/kbn-eslint-plugin-eslint/rules/scout_api_test_discourage_kbn_client.test.js new file mode 100644 index 0000000000000..5da0185f78762 --- /dev/null +++ b/packages/kbn-eslint-plugin-eslint/rules/scout_api_test_discourage_kbn_client.test.js @@ -0,0 +1,102 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +const { RuleTester } = require('eslint'); +const rule = require('./scout_api_test_discourage_kbn_client'); +const dedent = require('dedent'); + +const ruleTester = new RuleTester({ + parser: require.resolve('@typescript-eslint/parser'), + parserOptions: { + sourceType: 'module', + ecmaVersion: 2020, + }, +}); + +ruleTester.run('@kbn/eslint/scout_api_test_discourage_kbn_client', rule, { + valid: [ + // No kbnClient present + { + code: dedent` + apiTest('no kbnClient', async ({ apiClient }) => { + await apiClient.get('/foo'); + }); + `, + }, + // beforeAll usage without kbnClient + { + code: dedent` + apiTest.beforeAll(async () => {}); + `, + }, + // beforeAll usage with kbnClient + { + code: dedent` + apiTest.beforeAll(async ({ kbnClient }) => {}); + `, + }, + // kbnClient referenced in outer scope but not in apiTest + { + code: dedent` + function helper(kbnClient) { kbnClient.doSomething(); } + apiTest('not using kbnClient', async ({ apiClient }) => { await apiClient.get('/'); }); + `, + }, + // eslint-disable for this rule + { + code: dedent` + // eslint-disable-next-line @kbn/eslint/scout_api_test_discourage_kbn_client + apiTest('override', ({ kbnClient }) => { kbnClient.doSomething(); }); + `, + }, + ], + + invalid: [ + // direct destructured kbnClient usage + { + code: dedent` + apiTest('uses kbnClient', async ({ kbnClient }) => { + await kbnClient.call('/x'); + }); + `, + errors: [{ message: 'API tests must not use the kbnClient fixture.' }], + }, + // aliasing kbnClient to variable + { + code: dedent` + apiTest('alias kbnClient', async ({ kbnClient }) => { + const client = kbnClient; + client.doSomething(); + }); + `, + errors: [{ message: 'API tests must not use the kbnClient fixture.' }], + }, + // passing kbnClient to helper that uses it + { + code: dedent` + function external(client) { client.doSomething(); } + apiTest('pass to external', async ({ kbnClient }) => { + external(kbnClient); + }); + `, + errors: [{ message: 'API tests must not use the kbnClient fixture.' }], + }, + // member expression inside nested block + { + code: dedent` + apiTest('nested member', ({ kbnClient }) => { + if (true) { + kbnClient.doSomething(); + } + }); + `, + errors: [{ message: 'API tests must not use the kbnClient fixture.' }], + }, + ], +});