From 1f281b4707b44fffe82143e0bdbe725ed6529dd9 Mon Sep 17 00:00:00 2001 From: DavertMik Date: Tue, 6 Oct 2026 12:50:18 +0300 Subject: [PATCH 1/2] feat: add codeceptjs lint command with Claude Code hook mode AST-based checker for CodeceptJS anti-patterns in tests, page objects and helpers: no-fixed-wait, no-sleep, no-only, no-pause, secret-credentials, await-grab, no-actor-in-helper, raw-browser-in-test. `--hook claude` reads a PreToolUse payload, lints the file before and after the edit and blocks only on new error findings. Co-Authored-By: Claude Opus 5.5 --- bin/codecept.js | 8 + docs/agents.md | 2 + docs/lint.md | 90 +++++ lib/command/lint.js | 196 ++++++++++ lib/lint.js | 460 ++++++++++++++++++++++++ package.json | 1 + test/data/lint/await-grab.js | 16 + test/data/lint/clean.js | 9 + test/data/lint/no-actor-in-helper.js | 22 ++ test/data/lint/no-fixed-wait.js | 8 + test/data/lint/no-only.js | 17 + test/data/lint/no-pause.js | 7 + test/data/lint/no-sleep-app.js | 5 + test/data/lint/no-sleep.js | 19 + test/data/lint/project/checkout_test.js | 6 + test/data/lint/project/codecept.conf.js | 17 + test/data/lint/project/custom_helper.js | 9 + test/data/lint/project/existing_test.js | 7 + test/data/lint/project/legacy_test.js | 5 + test/data/lint/project/pages/login.js | 7 + test/data/lint/project/steps_file.js | 3 + test/data/lint/raw-browser-in-test.js | 17 + test/data/lint/secret-credentials.js | 11 + test/data/lint/suppressed.js | 9 + test/data/lint/typescript-enum.ts | 11 + test/data/lint/typescript.ts | 15 + test/unit/command/lint_test.js | 300 ++++++++++++++++ 27 files changed, 1277 insertions(+) create mode 100644 docs/lint.md create mode 100644 lib/command/lint.js create mode 100644 lib/lint.js create mode 100644 test/data/lint/await-grab.js create mode 100644 test/data/lint/clean.js create mode 100644 test/data/lint/no-actor-in-helper.js create mode 100644 test/data/lint/no-fixed-wait.js create mode 100644 test/data/lint/no-only.js create mode 100644 test/data/lint/no-pause.js create mode 100644 test/data/lint/no-sleep-app.js create mode 100644 test/data/lint/no-sleep.js create mode 100644 test/data/lint/project/checkout_test.js create mode 100644 test/data/lint/project/codecept.conf.js create mode 100644 test/data/lint/project/custom_helper.js create mode 100644 test/data/lint/project/existing_test.js create mode 100644 test/data/lint/project/legacy_test.js create mode 100644 test/data/lint/project/pages/login.js create mode 100644 test/data/lint/project/steps_file.js create mode 100644 test/data/lint/raw-browser-in-test.js create mode 100644 test/data/lint/secret-credentials.js create mode 100644 test/data/lint/suppressed.js create mode 100644 test/data/lint/typescript-enum.ts create mode 100644 test/data/lint/typescript.ts create mode 100644 test/unit/command/lint_test.js diff --git a/bin/codecept.js b/bin/codecept.js index e270e2920..cb8ec7e65 100755 --- a/bin/codecept.js +++ b/bin/codecept.js @@ -108,6 +108,14 @@ program .option('--action ', 'show docs for a single action (e.g. amOnPage or I.amOnPage)') .action(commandHandler('../lib/command/list.js')) +program + .command('lint [paths...]') + .description('Checks tests, page objects and helpers for CodeceptJS anti-patterns') + .option(commandFlags.config.flag, commandFlags.config.description) + .option('--json', 'print findings as JSON') + .option('--hook ', 'run as a coding agent pre-write hook reading the payload from stdin (supported: claude)') + .action(commandHandler('../lib/command/lint.js')) + program .command('def [path]') .description('Generates TypeScript definitions for all I actions.') diff --git a/docs/agents.md b/docs/agents.md index ea7153c88..2c4acc1e6 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -55,6 +55,8 @@ codex mcp add codeceptjs -- npx codeceptjs-mcp See [/mcp](/mcp) for full client setup. Now the agent is ready to run the loop. +Optionally, add a lint hook. Skills tell the agent what not to do; `npx codeceptjs lint --hook claude` enforces it. As a Claude Code `PreToolUse` hook, it blocks an edit that adds a fixed `I.wait(5)`, an un-awaited grabber or a plain-text password, and returns the reason so the agent rewrites the edit. Setup and rules are in [/lint](/lint). + ## The loop Whether the agent is writing a new test or fixing an old one, it follows the same cycle. diff --git a/docs/lint.md b/docs/lint.md new file mode 100644 index 000000000..35c2d285a --- /dev/null +++ b/docs/lint.md @@ -0,0 +1,90 @@ +--- +permalink: /lint +title: Lint +--- + +# Lint + +`codeceptjs lint` checks tests, page objects and helpers for CodeceptJS anti-patterns: fixed sleeps, missing `await` on grabbers, plain-text credentials, leftover `pause()` and `.only`. It parses files into an AST, so it never runs a browser and finishes in a second. + +```bash +npx codeceptjs lint # tests, include and helpers from codecept.conf.js +npx codeceptjs lint tests/checkout_test.js pages/ +npx codeceptjs lint --json # machine-readable output +npx codeceptjs lint -c path/to/codecept.conf.js +``` + +Each finding is printed on one line: + +``` +tests/checkout_test.js:14:3 error no-fixed-wait I.wait(5) sleeps unconditionally. Wait for a condition: I.waitForElement / I.waitForText / I.see +``` + +Without paths, lint checks files matched by `tests`, local files from `include` (page objects, steps file) and custom helpers loaded with `require`. JavaScript and TypeScript files are supported. + +Exit codes: `0` no errors (warnings allowed), `1` errors found, `2` bad input or a file that can't be parsed. + +## Rules + +| Rule | Default | Detects | +| --- | --- | --- | +| `no-fixed-wait` | error | `I.wait(5)` with a number. Use `I.waitForElement`, `I.waitForText`, `I.see` | +| `no-sleep` | error | `setTimeout` (including `new Promise(r => setTimeout(r, ms))`) in a Scenario, hook or page object method | +| `no-only` | error on CI, warning locally | `Scenario.only`, `Feature.only`, `Data(...).only.Scenario` | +| `no-pause` | error on CI, warning locally | `pause()` | +| `secret-credentials` | error | `I.fillField` on a password, token, secret or API key field without `secret()`; `process.env.*` with such a name passed to an `I.*` call without `secret()` | +| `await-grab` | error | `I.grab*()` result assigned, returned or passed on without `await` | +| `no-actor-in-helper` | error | `I` (including `const { I } = inject()`) inside a class extending `Helper`. Use `this.helpers[...]` | +| `raw-browser-in-test` | warning | `I.usePlaywrightTo`, `I.usePuppeteerTo`, `I.useWebDriverTo` and other `use*To`, `I.executeScript` in a Scenario body. Move it into a helper or page object | + +"On CI" means the `CI` environment variable is set, which every CI provider does. Locally `pause()` and `.only` stay warnings, so a debugging stub doesn't fail the lint. + +## Configuration + +Add an optional `lint` section to `codecept.conf.js`: + +```js +lint: { + rules: { 'raw-browser-in-test': 'off', 'no-fixed-wait': 'warn' }, + ignore: ['tests/legacy/**'], +} +``` + +Rule levels are `error`, `warn` or `off`. `ignore` takes glob patterns relative to the config file. + +To allow a single case, suppress it inline. The rule id is required: + +```js +I.wait(1) // codeceptjs-lint-disable-line no-fixed-wait + +// codeceptjs-lint-disable-next-line no-fixed-wait +I.wait(1) +``` + +## CI + +Run lint before the tests: + +```yaml +- run: npx codeceptjs lint +- run: npx codeceptjs run +``` + +## Claude Code Hook + +Lint can block a bad edit before an agent writes it. Add a `PreToolUse` hook to `.claude/settings.json`: + +```json +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Write|Edit|MultiEdit", + "hooks": [{ "type": "command", "command": "npx codeceptjs lint --hook claude" }] + } + ] + } +} +``` + +The hook builds the file as it would look after the edit and lints it. The edit is blocked only when it adds a new error. The agent receives the findings and rewrites the edit. Errors already in the file and warnings never block, so an agent can still add a `pause()` stub or touch a legacy test. If the hook fails or the resulting file can't be parsed, the edit is allowed. diff --git a/lib/command/lint.js b/lib/command/lint.js new file mode 100644 index 000000000..2cc56e62e --- /dev/null +++ b/lib/command/lint.js @@ -0,0 +1,196 @@ +import fs from 'fs' +import path from 'path' +import output from '../output.js' +import Config from '../config.js' +import { captureStream } from './utils.js' +import { LINT_EXTENSIONS, collectFiles, isIgnored, lintFile, lintOptions, lintSource, newErrors } from '../lint.js' + +const HOOK_AGENTS = ['claude'] +const CONFIG_NAMES = ['codecept.config.js', 'codecept.conf.js', 'codecept.js', 'codecept.config.cjs', 'codecept.conf.cjs', 'codecept.config.ts', 'codecept.conf.ts'] + +function findConfig(dir) { + return CONFIG_NAMES.map(name => path.join(dir, name)).find(f => fs.existsSync(f)) || null +} + +async function loadConfig(configPath, dir) { + const file = configPath ? path.resolve(configPath) : findConfig(dir) + if (!file) return { config: null, root: dir } + const root = fs.existsSync(file) && fs.statSync(file).isDirectory() ? file : path.dirname(file) + return { config: await Config.load(file), root } +} + +function relative(file) { + const rel = path.relative(process.cwd(), file) + return rel.startsWith('..') ? file : rel +} + +function formatFinding(f, colors = true) { + const level = f.level === 'error' ? 'error' : 'warn ' + const levelText = colors ? (f.level === 'error' ? output.colors.red(level) : output.colors.yellow(level)) : level + const location = `${relative(f.file)}:${f.line}:${f.column}` + return `${colors ? output.colors.bold(location) : location} ${levelText} ${colors ? output.colors.grey(f.rule) : f.rule} ${f.message}` +} + +export default async function lint(paths = [], options = {}) { + if (options.hook) return runHookCommand(options) + + let loaded + try { + loaded = await loadConfig(options.config, process.cwd()) + } catch (err) { + output.error(`Can't load config: ${err.message}`) + process.exitCode = 2 + return + } + const { config, root } = loaded + if (!config && !paths.length) { + output.error('No codecept config found. Pass files to lint or use -c to point to a config') + process.exitCode = 2 + return + } + + const files = collectFiles(config || {}, root, paths) + const opts = lintOptions(config || {}) + const findings = [] + const failures = [] + const skipped = [] + + for (const file of files) { + try { + const result = await lintFile(file, opts) + if (result.skipped) skipped.push({ file, reason: result.skipped }) + findings.push(...result.findings) + } catch (err) { + failures.push({ file, message: err.message }) + } + } + + const errors = findings.filter(f => f.level === 'error').length + const warnings = findings.length - errors + + if (options.json) { + process.stdout.write(`${JSON.stringify({ files: files.length, errors, warnings, findings, failures, skipped }, null, 2)}\n`) + } else { + for (const f of findings) output.print(formatFinding(f)) + for (const s of skipped) output.print(`${output.colors.bold(relative(s.file))} ${output.colors.yellow('skip ')} ${s.reason}`) + for (const e of failures) output.print(`${output.colors.bold(relative(e.file))} ${output.colors.red('parse')} ${e.message}`) + const summary = `${files.length} file(s) checked, ${errors} error(s), ${warnings} warning(s)` + output.print(errors || failures.length ? output.colors.red(summary) : output.colors.green(summary)) + } + + if (failures.length || (!files.length && paths.length)) process.exitCode = 2 + else if (errors) process.exitCode = 1 +} + +function readStdin() { + return new Promise((resolve, reject) => { + let data = '' + process.stdin.setEncoding('utf8') + process.stdin.on('data', chunk => (data += chunk)) + process.stdin.on('end', () => resolve(data)) + process.stdin.on('error', reject) + }) +} + +function applyEdit(content, oldString, newString, replaceAll) { + if (typeof oldString !== 'string' || typeof newString !== 'string') return null + if (oldString === '') return content === '' ? newString : null + if (!content.includes(oldString)) return null + return replaceAll ? content.split(oldString).join(newString) : content.replace(oldString, () => newString) +} + +function resultingContent(toolName, input, current) { + if (toolName === 'Write') return typeof input.content === 'string' ? input.content : null + if (toolName === 'Edit') return applyEdit(current, input.old_string, input.new_string, input.replace_all) + if (toolName === 'MultiEdit') { + let content = current + for (const edit of input.edits || []) { + content = applyEdit(content, edit.old_string, edit.new_string, edit.replace_all) + if (content === null) return null + } + return content + } + return null +} + +export async function runHook(payload, { agent = 'claude', config: configPath } = {}) { + const allow = { code: 0, stderr: '' } + if (!HOOK_AGENTS.includes(agent)) return { code: 0, stderr: `codeceptjs lint: unsupported hook agent "${agent}", supported: ${HOOK_AGENTS.join(', ')}\n` } + if (!payload || typeof payload !== 'object') return allow + + const toolName = payload.tool_name + const input = payload.tool_input || {} + if (!['Write', 'Edit', 'MultiEdit'].includes(toolName) || typeof input.file_path !== 'string') return allow + + const projectDir = path.resolve(process.env.CLAUDE_PROJECT_DIR || payload.cwd || process.cwd()) + const file = path.resolve(payload.cwd || projectDir, input.file_path) + const rel = path.relative(projectDir, file) + if (rel.startsWith('..') || path.isAbsolute(rel) || rel.split(path.sep).includes('node_modules')) return allow + if (!LINT_EXTENSIONS.includes(path.extname(file))) return allow + + let config = {} + let root = projectDir + const stdout = captureStream(process.stdout) + stdout.startCapture() + try { + const loaded = await loadConfig(configPath, projectDir) + if (loaded.config) { + config = loaded.config + root = loaded.root + } + } catch { + config = {} + } finally { + stdout.stopCapture() + } + if (isIgnored(file, config, root)) return allow + + const current = fs.existsSync(file) ? fs.readFileSync(file, 'utf8') : '' + const after = resultingContent(toolName, input, current) + if (after === null) return allow + + const opts = lintOptions(config) + let afterResult + try { + afterResult = await lintSource(after, file, opts) + } catch (err) { + return { code: 0, stderr: `codeceptjs lint: ${relative(file)} could not be parsed after this edit (${err.message})\n` } + } + if (afterResult.skipped) return allow + + let beforeFindings = [] + if (current) { + try { + beforeFindings = (await lintSource(current, file, opts)).findings + } catch { + beforeFindings = [] + } + } + + const added = newErrors(beforeFindings, afterResult.findings) + if (!added.length) return allow + + const lines = added.map(f => formatFinding(f, false)) + return { + code: 2, + stderr: `codeceptjs lint blocked this edit, ${added.length} new error(s):\n${lines.join('\n')}\nFix the code and retry.\n`, + } +} + +async function runHookCommand(options) { + let result = { code: 0, stderr: '' } + try { + const raw = await readStdin() + let payload = null + try { + payload = JSON.parse(raw) + } catch { + payload = null + } + result = await runHook(payload, { agent: options.hook, config: options.config }) + } catch (err) { + result = { code: 0, stderr: `codeceptjs lint: hook failed (${err.message})\n` } + } + process.exitCode = result.code + process.stderr.write(result.stderr, () => process.exit(result.code)) +} diff --git a/lib/lint.js b/lib/lint.js new file mode 100644 index 000000000..ed079c8ff --- /dev/null +++ b/lib/lint.js @@ -0,0 +1,460 @@ +import fs from 'fs' +import path from 'path' +import module from 'module' +import * as acorn from 'acorn' +import * as walk from 'acorn-walk' +import { globSync } from 'glob' + +export const LINT_EXTENSIONS = ['.js', '.ts', '.mjs', '.cjs'] + +const TEST_BLOCKS = new Set(['Scenario', 'Before', 'After', 'BeforeSuite', 'AfterSuite', 'Background']) +const CREDENTIAL_WORDS = /pass(word|wd)|token|secret|api[\s_-]?key/i +const CREDENTIAL_ENV = /pass(word|wd)|(^|_)pass($|_)|token|secret|api_?key/i +const RAW_BROWSER_METHOD = /^use[A-Z]\w*To$/ +const PROMISE_COMBINATORS = new Set(['all', 'allSettled', 'race', 'any']) + +const isCI = () => !!process.env.CI + +function isIdentifier(node, name) { + return node?.type === 'Identifier' && (name === undefined || node.name === name) +} + +function propertyName(member) { + if (member?.type !== 'MemberExpression') return null + if (!member.computed && member.property.type === 'Identifier') return member.property.name + if (member.computed && member.property.type === 'Literal' && typeof member.property.value === 'string') return member.property.value + return null +} + +function actorMethod(node) { + if (node?.type !== 'CallExpression') return null + const callee = node.callee + if (callee.type !== 'MemberExpression' || !isIdentifier(callee.object, 'I')) return null + return propertyName(callee) +} + +function isFunction(node) { + return node?.type === 'FunctionExpression' || node?.type === 'ArrowFunctionExpression' || node?.type === 'FunctionDeclaration' +} + +function isDataCall(node) { + return node?.type === 'CallExpression' && isIdentifier(node.callee, 'Data') +} + +function testBlockName(callee) { + if (callee.type === 'Identifier' && TEST_BLOCKS.has(callee.name)) return callee.name + if (callee.type !== 'MemberExpression') return null + const prop = propertyName(callee) + if (isIdentifier(callee.object, 'Scenario') && ['only', 'skip', 'todo'].includes(prop)) return 'Scenario' + if (prop === 'Scenario') { + const obj = callee.object + if (isDataCall(obj)) return 'Scenario' + if (obj.type === 'MemberExpression' && isDataCall(obj.object)) return 'Scenario' + } + return null +} + +function enclosingTestBlock(ancestors) { + for (let i = ancestors.length - 2; i >= 0; i--) { + const node = ancestors[i] + if (node.type !== 'CallExpression' || !isFunction(ancestors[i + 1])) continue + if (!node.arguments.includes(ancestors[i + 1])) continue + const name = testBlockName(node.callee) + if (name) return name + } + return null +} + +function isHelperClass(node) { + if (node.type !== 'ClassDeclaration' && node.type !== 'ClassExpression') return false + const sup = node.superClass + if (!sup) return false + return isIdentifier(sup, 'Helper') || propertyName(sup) === 'Helper' +} + +function insideHelperClass(ancestors) { + return ancestors.some(isHelperClass) +} + +function insideMethod(ancestors) { + for (let i = ancestors.length - 2; i > 0; i--) { + const node = ancestors[i] + if (!isFunction(node)) continue + const parent = ancestors[i - 1] + if (parent.type === 'MethodDefinition') return true + if (parent.type === 'Property' && parent.value === node) return true + if (parent.type === 'PropertyDefinition' && parent.value === node) return true + } + return false +} + +function isSecretCall(node) { + return node?.type === 'CallExpression' && isIdentifier(node.callee, 'secret') +} + +function stringValue(node) { + if (node?.type === 'Literal' && typeof node.value === 'string') return node.value + if (node?.type === 'TemplateLiteral' && node.expressions.length === 0) return node.quasis[0].value.cooked + return null +} + +function envName(node) { + if (node?.type !== 'MemberExpression') return null + const obj = node.object + if (obj.type !== 'MemberExpression' || !isIdentifier(obj.object, 'process') || propertyName(obj) !== 'env') return null + return propertyName(node) +} + +function isPromiseCombinator(node) { + return node?.type === 'CallExpression' && node.callee.type === 'MemberExpression' && isIdentifier(node.callee.object, 'Promise') && PROMISE_COMBINATORS.has(propertyName(node.callee)) +} + +function grabResultUsed(ancestors) { + let i = ancestors.length - 1 + let child = ancestors[i] + let parent = ancestors[i - 1] + while (parent && parent.type === 'ChainExpression') { + child = parent + parent = ancestors[--i - 1] + } + if (!parent) return false + switch (parent.type) { + case 'AwaitExpression': + case 'ExpressionStatement': + case 'YieldExpression': + case 'SequenceExpression': + return false + case 'MemberExpression': + return parent.object !== child + case 'ArrayExpression': + return !isPromiseCombinator(ancestors[i - 2]) + case 'ArrowFunctionExpression': + return parent.body === child + case 'UnaryExpression': + return parent.operator !== 'void' + default: + return true + } +} + +function usesCodeceptGlobals(ast) { + let found = false + walk.full(ast, node => { + if (found) return + if (node.type === 'CallExpression' && (isIdentifier(node.callee, 'inject') || isIdentifier(node.callee, 'actor') || isIdentifier(node.callee, 'Feature') || testBlockName(node.callee))) found = true + }) + return found +} + +export const rules = [ + { + id: 'no-fixed-wait', + level: 'error', + check(node, ancestors, ctx) { + if (actorMethod(node) !== 'wait') return + const arg = node.arguments[0] + if (arg?.type !== 'Literal' || typeof arg.value !== 'number') return + ctx.report(node, `${ctx.source(node)} sleeps unconditionally. Wait for a condition: I.waitForElement / I.waitForText / I.see`) + }, + }, + { + id: 'no-sleep', + level: 'error', + check(node, ancestors, ctx) { + if (node.type !== 'CallExpression' || !isIdentifier(node.callee, 'setTimeout')) return + if (insideHelperClass(ancestors)) return + const inTest = enclosingTestBlock(ancestors) + if (!inTest && !(ctx.codeceptFile && insideMethod(ancestors))) return + ctx.report(node, 'setTimeout pauses for a fixed time. Wait for a condition: I.waitForElement / I.waitForText / I.waitForFunction') + }, + }, + { + id: 'no-only', + level: () => (isCI() ? 'error' : 'warn'), + check(node, ancestors, ctx) { + if (node.type !== 'MemberExpression' || propertyName(node) !== 'only') return + const obj = node.object + if (!isIdentifier(obj, 'Scenario') && !isIdentifier(obj, 'Feature') && !isDataCall(obj)) return + const name = isDataCall(obj) ? 'Data(...).only' : `${obj.name}.only` + ctx.report(node, `${name} limits the run to the focused tests. Remove .only before commit`) + }, + }, + { + id: 'no-pause', + level: () => (isCI() ? 'error' : 'warn'), + check(node, ancestors, ctx) { + if (node.type !== 'CallExpression' || !isIdentifier(node.callee, 'pause')) return + ctx.report(node, 'pause() stops the test for interactive debugging. Remove it before commit') + }, + }, + { + id: 'secret-credentials', + level: 'error', + check(node, ancestors, ctx) { + const method = actorMethod(node) + if (!method) return + let envReported = false + for (const arg of node.arguments) { + const name = envName(arg) + if (name && CREDENTIAL_ENV.test(name)) { + envReported = true + ctx.report(arg, `process.env.${name} is passed to I.${method} in plain text and will be printed in logs. Wrap it: secret(process.env.${name})`) + } + } + if (method !== 'fillField' || envReported) return + const [locator, value] = node.arguments + const text = stringValue(locator) + if (!text || !CREDENTIAL_WORDS.test(text) || !value || isSecretCall(value)) return + ctx.report(node, `I.fillField('${text}', ...) types a credential in plain text and it will be printed in logs. Wrap the value: secret(...)`) + }, + }, + { + id: 'await-grab', + level: 'error', + check(node, ancestors, ctx) { + const method = actorMethod(node) + if (!method || !method.startsWith('grab')) return + if (!grabResultUsed(ancestors)) return + ctx.report(node, `I.${method}() returns a promise. Use: await I.${method}(...)`) + }, + }, + { + id: 'no-actor-in-helper', + level: 'error', + check(node, ancestors, ctx) { + const isActorRef = + (node.type === 'Identifier' && node.name === 'I') || (node.type === 'MemberExpression' && propertyName(node) === 'I' && node.object.type === 'CallExpression' && isIdentifier(node.object.callee, 'inject')) + if (!isActorRef || !insideHelperClass(ancestors)) return + ctx.report(node, 'The I actor is not available inside a helper. Call other helpers via this.helpers[...]') + }, + }, + { + id: 'raw-browser-in-test', + level: 'warn', + check(node, ancestors, ctx) { + const method = actorMethod(node) + if (!method || !(RAW_BROWSER_METHOD.test(method) || method === 'executeScript')) return + if (enclosingTestBlock(ancestors) !== 'Scenario') return + ctx.report(node, `I.${method} runs raw browser code inside a test. Move it into a helper or page object`) + }, + }, +] + +export const ruleIds = rules.map(r => r.id) + +function withoutNodeWarnings(fn) { + const original = process.emitWarning + process.emitWarning = (warning, ...args) => { + const type = (typeof args[0] === 'string' ? args[0] : args[0]?.type) || warning?.name + if (type === 'ExperimentalWarning' || type === 'DeprecationWarning') return + return original.call(process, warning, ...args) + } + try { + return fn() + } finally { + process.emitWarning = original + } +} + +let typescriptModule +async function loadTypeScript() { + if (typescriptModule !== undefined) return typescriptModule + try { + const mod = await import('typescript') + typescriptModule = mod.default || mod + } catch { + typescriptModule = null + } + return typescriptModule +} + +export async function toJavaScript(code, file) { + if (path.extname(file) !== '.ts') return { code } + if (typeof module.stripTypeScriptTypes === 'function') { + try { + return { code: withoutNodeWarnings(() => module.stripTypeScriptTypes(code, { mode: 'strip' })) } + } catch {} + } + const ts = await loadTypeScript() + if (!ts) return { skipped: 'TypeScript file skipped: type stripping is not supported by this Node.js version and the "typescript" package is not installed' } + const result = ts.transpileModule(code, { + compilerOptions: { target: ts.ScriptTarget.ESNext, module: ts.ModuleKind.ESNext, removeComments: false, sourceMap: true }, + fileName: file, + }) + return { code: result.outputText, position: sourcePosition(result.sourceMapText) } +} + +function sourcePosition(sourceMapText) { + if (!sourceMapText || typeof module.SourceMap !== 'function') return null + const map = new module.SourceMap(JSON.parse(sourceMapText)) + return ({ line, column }) => { + const entry = map.findEntry(line - 1, column) + if (typeof entry?.originalLine !== 'number') return { line, column } + return { line: entry.originalLine + 1, column: entry.originalColumn } + } +} + +export function parse(code) { + const options = { ecmaVersion: 'latest', locations: true, allowHashBang: true } + let comments = [] + try { + const ast = acorn.parse(code, { ...options, sourceType: 'module', onComment: comments }) + return { ast, comments } + } catch (err) { + comments = [] + try { + const ast = acorn.parse(code, { ...options, sourceType: 'script', allowReturnOutsideFunction: true, onComment: comments }) + return { ast, comments } + } catch { + throw err + } + } +} + +function suppressions(comments, position) { + const lineOf = loc => (position ? position(loc).line : loc.line) + const byLine = new Map() + const add = (line, ids) => { + if (!byLine.has(line)) byLine.set(line, new Set()) + ids.forEach(id => byLine.get(line).add(id)) + } + for (const comment of comments) { + const [directive, ...rest] = comment.value.trim().split(/[\s,]+/) + const ids = rest.filter(Boolean) + if (!ids.length) continue + if (directive === 'codeceptjs-lint-disable-line') add(lineOf(comment.loc.start), ids) + if (directive === 'codeceptjs-lint-disable-next-line') add(lineOf(comment.loc.end) + 1, ids) + } + return byLine +} + +function resolveLevel(rule, overrides) { + const configured = overrides?.[rule.id] + if (configured !== undefined) { + if (configured === false || configured === 'off' || configured === 0) return 'off' + if (configured === 'warn' || configured === 'warning' || configured === 1) return 'warn' + if (configured === 'error' || configured === true || configured === 2) return 'error' + } + return typeof rule.level === 'function' ? rule.level() : rule.level +} + +export function normalizeSource(text) { + return text.replace(/\s+/g, ' ').trim() +} + +export async function lintSource(code, file = 'file.js', options = {}) { + const js = await toJavaScript(code, file) + if (js.skipped) return { file, findings: [], skipped: js.skipped } + + const { ast, comments } = parse(js.code) + const suppressed = suppressions(comments, js.position) + const active = rules.map(rule => ({ rule, level: resolveLevel(rule, options.rules) })).filter(r => r.level !== 'off') + const findings = [] + const seen = new Set() + + const ctx = { + codeceptFile: usesCodeceptGlobals(ast), + source: node => normalizeSource(js.code.slice(node.start, node.end)), + } + + walk.fullAncestor(ast, (node, state, ancestors) => { + for (const { rule, level } of active) { + rule.check(node, ancestors, { + ...ctx, + report(target, message) { + const { line, column } = js.position ? js.position(target.loc.start) : target.loc.start + const key = `${rule.id}:${target.start}` + if (seen.has(key)) return + seen.add(key) + if (suppressed.get(line)?.has(rule.id)) return + findings.push({ file, line, column: column + 1, rule: rule.id, level, message, source: ctx.source(target) }) + }, + }) + } + }) + + findings.sort((a, b) => a.line - b.line || a.column - b.column) + return { file, findings } +} + +export async function lintFile(file, options = {}) { + const code = fs.readFileSync(file, 'utf8') + return lintSource(code, file, options) +} + +function isLintable(file) { + return LINT_EXTENSIONS.includes(path.extname(file)) +} + +function localFile(entry, root) { + if (typeof entry !== 'string') return null + if (!entry.startsWith('.') && !path.isAbsolute(entry)) return null + const resolved = path.resolve(root, entry) + const candidates = [resolved, ...LINT_EXTENSIONS.map(ext => resolved + ext)] + return candidates.find(f => fs.existsSync(f) && fs.statSync(f).isFile()) || null +} + +function expandPath(entry) { + const resolved = path.resolve(entry) + if (fs.existsSync(resolved)) { + if (fs.statSync(resolved).isDirectory()) { + return globSync(`**/*{${LINT_EXTENSIONS.join(',')}}`, { cwd: resolved, absolute: true, ignore: ['**/node_modules/**'] }) + } + return [resolved] + } + return globSync(entry, { absolute: true, ignore: ['**/node_modules/**'] }) +} + +export function collectFiles(config = {}, root = process.cwd(), paths = []) { + let files = [] + if (paths.length) { + for (const entry of paths) files.push(...expandPath(entry)) + } else { + const tests = [].concat(config.tests || []) + for (const pattern of tests) { + files.push(...globSync(pattern, { cwd: root, absolute: true, ignore: ['**/node_modules/**'] })) + } + for (const entry of Object.values(config.include || {})) { + const file = localFile(entry, root) + if (file) files.push(file) + } + for (const helper of Object.values(config.helpers || {})) { + const file = localFile(helper?.require, root) + if (file) files.push(file) + } + } + files = [...new Set(files.map(f => path.resolve(f)))].filter(isLintable) + return files.filter(f => !isIgnored(f, config, root)) +} + +export function isIgnored(file, config = {}, root = process.cwd()) { + const ignore = [].concat(config.lint?.ignore || []) + if (!ignore.length) return false + const target = path.resolve(file) + return withoutNodeWarnings(() => ignore.some(pattern => path.matchesGlob(target, path.resolve(root, pattern)))) +} + +export function lintOptions(config = {}) { + return { rules: config.lint?.rules || {} } +} + +export function findingKey(finding) { + return `${finding.rule}\u0000${finding.source}` +} + +export function newErrors(before, after) { + const counts = new Map() + for (const f of before) { + if (f.level !== 'error') continue + counts.set(findingKey(f), (counts.get(findingKey(f)) || 0) + 1) + } + const added = [] + for (const f of after) { + if (f.level !== 'error') continue + const key = findingKey(f) + const left = counts.get(key) || 0 + if (left > 0) counts.set(key, left - 1) + else added.push(f) + } + return added +} diff --git a/package.json b/package.json index d6771005d..5c25c2857 100644 --- a/package.json +++ b/package.json @@ -101,6 +101,7 @@ "@modelcontextprotocol/sdk": "^1.26.0", "@xmldom/xmldom": "0.9.10", "acorn": "8.15.0", + "acorn-walk": "8.3.5", "ai": "^6.0.43", "arrify": "3.0.0", "axios": "1.16.1", diff --git a/test/data/lint/await-grab.js b/test/data/lint/await-grab.js new file mode 100644 index 000000000..0de080279 --- /dev/null +++ b/test/data/lint/await-grab.js @@ -0,0 +1,16 @@ +Feature('grab') + +Scenario('grab', async ({ I }) => { + const title = I.grabTitle() + const text = await I.grabTextFrom('h1') + I.grabCurrentUrl() + I.say(I.grabValueFrom('#name')) + const [a, b] = await Promise.all([I.grabTitle(), I.grabCurrentUrl()]) + I.grabTitle().then(t => I.say(t)) +}) + +export default { + getTitle() { + return I.grabTitle() + }, +} diff --git a/test/data/lint/clean.js b/test/data/lint/clean.js new file mode 100644 index 000000000..8f8940c23 --- /dev/null +++ b/test/data/lint/clean.js @@ -0,0 +1,9 @@ +Feature('clean') + +Scenario('clean', async ({ I }) => { + I.amOnPage('/') + I.fillField('Password', secret('123456')) + I.waitForElement('#ok') + const title = await I.grabTitle() + I.see(title) +}) diff --git a/test/data/lint/no-actor-in-helper.js b/test/data/lint/no-actor-in-helper.js new file mode 100644 index 000000000..824f4e43b --- /dev/null +++ b/test/data/lint/no-actor-in-helper.js @@ -0,0 +1,22 @@ +import Helper from '@codeceptjs/helper' + +class MyHelper extends Helper { + async login() { + const { I } = inject() + I.amOnPage('/login') + } + + async open() { + const { Playwright } = this.helpers + await Playwright.amOnPage('/') + } +} + +class PageObject { + open() { + const { I } = inject() + I.amOnPage('/') + } +} + +export default MyHelper diff --git a/test/data/lint/no-fixed-wait.js b/test/data/lint/no-fixed-wait.js new file mode 100644 index 000000000..742415795 --- /dev/null +++ b/test/data/lint/no-fixed-wait.js @@ -0,0 +1,8 @@ +Feature('waits') + +Scenario('fixed wait', ({ I }) => { + I.amOnPage('/') + I.wait(5) + I.waitForElement('#ok', 5) + I.wait(waitTime) +}) diff --git a/test/data/lint/no-only.js b/test/data/lint/no-only.js new file mode 100644 index 000000000..f5fccf981 --- /dev/null +++ b/test/data/lint/no-only.js @@ -0,0 +1,17 @@ +Feature.only('focused') + +Scenario('normal', ({ I }) => { + I.see('ok') +}) + +Scenario.only('focused', ({ I }) => { + I.see('ok') +}) + +Data(['a', 'b']).only.Scenario('data', ({ I, current }) => { + I.see(current) +}) + +Scenario.skip('skipped', ({ I }) => { + I.see('ok') +}) diff --git a/test/data/lint/no-pause.js b/test/data/lint/no-pause.js new file mode 100644 index 000000000..afeeed7ec --- /dev/null +++ b/test/data/lint/no-pause.js @@ -0,0 +1,7 @@ +Feature('pause') + +Scenario('debug', ({ I }) => { + I.amOnPage('/') + pause() + I.see('Welcome') +}) diff --git a/test/data/lint/no-sleep-app.js b/test/data/lint/no-sleep-app.js new file mode 100644 index 000000000..3109ad384 --- /dev/null +++ b/test/data/lint/no-sleep-app.js @@ -0,0 +1,5 @@ +export default class Poller { + start() { + setTimeout(() => this.tick(), 100) + } +} diff --git a/test/data/lint/no-sleep.js b/test/data/lint/no-sleep.js new file mode 100644 index 000000000..4f5e98371 --- /dev/null +++ b/test/data/lint/no-sleep.js @@ -0,0 +1,19 @@ +const { I } = inject() + +Feature('sleep') + +Scenario('sleeps', async ({ I }) => { + await new Promise(resolve => setTimeout(resolve, 1000)) + I.waitForText('Done') +}) + +export default { + async open() { + I.amOnPage('/') + setTimeout(() => {}, 500) + }, +} + +function utility(fn) { + setTimeout(fn, 10) +} diff --git a/test/data/lint/project/checkout_test.js b/test/data/lint/project/checkout_test.js new file mode 100644 index 000000000..580c06630 --- /dev/null +++ b/test/data/lint/project/checkout_test.js @@ -0,0 +1,6 @@ +Feature('checkout') + +Scenario('pay', ({ I }) => { + I.wait(5) + I.executeScript(() => 1) +}) diff --git a/test/data/lint/project/codecept.conf.js b/test/data/lint/project/codecept.conf.js new file mode 100644 index 000000000..8f35d4a1b --- /dev/null +++ b/test/data/lint/project/codecept.conf.js @@ -0,0 +1,17 @@ +export const config = { + tests: './*_test.js', + include: { + I: './steps_file.js', + loginPage: './pages/login.js', + externalModule: 'some-package', + }, + helpers: { + Custom: { + require: './custom_helper.js', + }, + }, + lint: { + rules: { 'raw-browser-in-test': 'off' }, + ignore: ['legacy_test.js'], + }, +} diff --git a/test/data/lint/project/custom_helper.js b/test/data/lint/project/custom_helper.js new file mode 100644 index 000000000..744cb1f6e --- /dev/null +++ b/test/data/lint/project/custom_helper.js @@ -0,0 +1,9 @@ +import Helper from '@codeceptjs/helper' + +class Custom extends Helper { + hello() { + const { I } = inject() + } +} + +export default Custom diff --git a/test/data/lint/project/existing_test.js b/test/data/lint/project/existing_test.js new file mode 100644 index 000000000..b519ac020 --- /dev/null +++ b/test/data/lint/project/existing_test.js @@ -0,0 +1,7 @@ +Feature('existing') + +Scenario('existing', ({ I }) => { + I.amOnPage('/') + I.wait(5) + I.see('Welcome') +}) diff --git a/test/data/lint/project/legacy_test.js b/test/data/lint/project/legacy_test.js new file mode 100644 index 000000000..c140b231b --- /dev/null +++ b/test/data/lint/project/legacy_test.js @@ -0,0 +1,5 @@ +Feature('legacy') + +Scenario('old', ({ I }) => { + I.wait(10) +}) diff --git a/test/data/lint/project/pages/login.js b/test/data/lint/project/pages/login.js new file mode 100644 index 000000000..a50b43885 --- /dev/null +++ b/test/data/lint/project/pages/login.js @@ -0,0 +1,7 @@ +const { I } = inject() + +export default { + login() { + I.wait(1) + }, +} diff --git a/test/data/lint/project/steps_file.js b/test/data/lint/project/steps_file.js new file mode 100644 index 000000000..6e3d29dcd --- /dev/null +++ b/test/data/lint/project/steps_file.js @@ -0,0 +1,3 @@ +export default function () { + return actor({}) +} diff --git a/test/data/lint/raw-browser-in-test.js b/test/data/lint/raw-browser-in-test.js new file mode 100644 index 000000000..c6bb5b564 --- /dev/null +++ b/test/data/lint/raw-browser-in-test.js @@ -0,0 +1,17 @@ +Feature('raw') + +Scenario('raw', async ({ I }) => { + await I.usePlaywrightTo('click', async ({ page }) => page.click('#a')) + I.executeScript(() => window.scrollTo(0, 0)) + I.click('Login') +}) + +Before(({ I }) => { + I.executeScript(() => localStorage.clear()) +}) + +export const page = { + reset() { + I.useWebDriverTo('reset', async ({ browser }) => browser.reloadSession()) + }, +} diff --git a/test/data/lint/secret-credentials.js b/test/data/lint/secret-credentials.js new file mode 100644 index 000000000..254684602 --- /dev/null +++ b/test/data/lint/secret-credentials.js @@ -0,0 +1,11 @@ +Feature('login') + +Scenario('login', ({ I }) => { + I.fillField('Email', 'user@example.com') + I.fillField('Password', '123456') + I.fillField('Password', secret('123456')) + I.fillField('#api_key', process.env.API_KEY) + I.fillField('Token', secret(process.env.AUTH_TOKEN)) + I.sendPostRequest('/login', process.env.USER_PASSWORD) + I.fillField('Username', process.env.USERNAME) +}) diff --git a/test/data/lint/suppressed.js b/test/data/lint/suppressed.js new file mode 100644 index 000000000..53a2d1040 --- /dev/null +++ b/test/data/lint/suppressed.js @@ -0,0 +1,9 @@ +Feature('suppressed') + +Scenario('suppressed', ({ I }) => { + I.wait(1) // codeceptjs-lint-disable-line no-fixed-wait + // codeceptjs-lint-disable-next-line no-fixed-wait + I.wait(2) + I.wait(3) // codeceptjs-lint-disable-line + I.wait(4) // codeceptjs-lint-disable-line no-pause +}) diff --git a/test/data/lint/typescript-enum.ts b/test/data/lint/typescript-enum.ts new file mode 100644 index 000000000..b72ee1a49 --- /dev/null +++ b/test/data/lint/typescript-enum.ts @@ -0,0 +1,11 @@ +enum Role { + Admin, + User, +} + +Feature('enum') + +Scenario('enum', ({ I }) => { + I.wait(Role.Admin) + I.wait(3) +}) diff --git a/test/data/lint/typescript.ts b/test/data/lint/typescript.ts new file mode 100644 index 000000000..34f2371e3 --- /dev/null +++ b/test/data/lint/typescript.ts @@ -0,0 +1,15 @@ +interface Credentials { + email: string + password: string +} + +type Maybe = T | null + +const creds: Credentials = { email: 'a@b.c', password: 'x' } + +Feature('typescript') + +Scenario('ts', async ({ I }: { I: CodeceptJS.I }) => { + const title: string = I.grabTitle() as unknown as string + I.wait(5) +}) diff --git a/test/unit/command/lint_test.js b/test/unit/command/lint_test.js new file mode 100644 index 000000000..616b04fea --- /dev/null +++ b/test/unit/command/lint_test.js @@ -0,0 +1,300 @@ +import { expect } from 'chai' +import fs from 'fs' +import os from 'os' +import path from 'path' +import { spawnSync } from 'child_process' +import { fileURLToPath } from 'url' +import { lintFile, lintSource, collectFiles, newErrors } from '../../../lib/lint.js' +import { runHook } from '../../../lib/command/lint.js' + +const __dirname = path.dirname(fileURLToPath(import.meta.url)) +const fixtures = path.join(__dirname, '../../data/lint') +const bin = path.join(__dirname, '../../../bin/codecept.js') + +const fixture = name => path.join(fixtures, name) +const findings = async (name, options) => (await lintFile(fixture(name), options)).findings +const byRule = (list, rule) => list.filter(f => f.rule === rule).map(f => f.line) + +describe('lint command', () => { + const saved = {} + + before(() => { + for (const key of ['CI', 'CLAUDE_PROJECT_DIR']) { + saved[key] = process.env[key] + delete process.env[key] + } + }) + + after(() => { + for (const [key, value] of Object.entries(saved)) { + if (value === undefined) delete process.env[key] + else process.env[key] = value + } + }) + + describe('rules', () => { + it('no-fixed-wait flags I.wait with a number literal only', async () => { + const list = await findings('no-fixed-wait.js') + expect(byRule(list, 'no-fixed-wait')).to.deep.equal([5]) + expect(list[0].level).to.equal('error') + expect(list[0].column).to.equal(3) + expect(list[0].message).to.include('I.wait(5)') + }) + + it('no-sleep flags setTimeout in scenarios and page object methods', async () => { + const list = await findings('no-sleep.js') + expect(byRule(list, 'no-sleep')).to.deep.equal([6, 13]) + }) + + it('no-sleep ignores files without CodeceptJS code', async () => { + expect(await findings('no-sleep-app.js')).to.be.empty + }) + + it('no-only flags focused features, scenarios and data scenarios', async () => { + const list = await findings('no-only.js') + expect(byRule(list, 'no-only')).to.deep.equal([1, 7, 11]) + }) + + it('no-only and no-pause are warnings locally and errors on CI', async () => { + let list = [...(await findings('no-only.js')), ...(await findings('no-pause.js'))] + expect(list.map(f => f.level)).to.deep.equal(['warn', 'warn', 'warn', 'warn']) + process.env.CI = 'true' + try { + list = [...(await findings('no-only.js')), ...(await findings('no-pause.js'))] + } finally { + delete process.env.CI + } + expect(list.map(f => f.level)).to.deep.equal(['error', 'error', 'error', 'error']) + }) + + it('no-pause flags pause() calls', async () => { + expect(byRule(await findings('no-pause.js'), 'no-pause')).to.deep.equal([5]) + }) + + it('secret-credentials flags credentials not wrapped in secret()', async () => { + const list = await findings('secret-credentials.js') + expect(byRule(list, 'secret-credentials')).to.deep.equal([5, 7, 9]) + }) + + it('await-grab flags grab results used without await', async () => { + const list = await findings('await-grab.js') + expect(byRule(list, 'await-grab')).to.deep.equal([4, 7, 14]) + }) + + it('no-actor-in-helper flags I inside a Helper class only', async () => { + const list = await findings('no-actor-in-helper.js') + expect(byRule(list, 'no-actor-in-helper')).to.deep.equal([5, 6]) + }) + + it('raw-browser-in-test warns on use*To and executeScript inside a Scenario', async () => { + const list = await findings('raw-browser-in-test.js') + expect(byRule(list, 'raw-browser-in-test')).to.deep.equal([4, 5]) + expect(list.every(f => f.level === 'warn')).to.be.true + }) + + it('clean test has no findings', async () => { + expect(await findings('clean.js')).to.be.empty + }) + }) + + describe('engine', () => { + it('suppresses findings with disable-line and disable-next-line comments that name the rule', async () => { + expect(byRule(await findings('suppressed.js'), 'no-fixed-wait')).to.deep.equal([7, 8]) + }) + + it('applies rule levels from config', async () => { + const list = await findings('raw-browser-in-test.js', { rules: { 'raw-browser-in-test': 'off' } }) + expect(list).to.be.empty + const waits = await findings('no-fixed-wait.js', { rules: { 'no-fixed-wait': 'warn' } }) + expect(waits[0].level).to.equal('warn') + }) + + it('keeps TypeScript line numbers after stripping types', async () => { + const list = await findings('typescript.ts') + expect(list.map(f => [f.rule, f.line, f.column])).to.deep.equal([ + ['await-grab', 13, 25], + ['no-fixed-wait', 14, 3], + ]) + }) + + it('maps TypeScript syntax that needs transpiling back to source lines', async () => { + const list = await findings('typescript-enum.ts') + expect(list.map(f => [f.rule, f.line])).to.deep.equal([['no-fixed-wait', 10]]) + }) + + it('parses CommonJS files as scripts', async () => { + const code = 'const { I } = inject()\n\nmodule.exports = {\n open() {\n I.wait(2)\n },\n}\n\nreturn\n' + expect(byRule((await lintSource(code, 'page.js')).findings, 'no-fixed-wait')).to.deep.equal([5]) + }) + + it('throws on syntax errors', async () => { + let error + try { + await lintSource("Scenario('broken', ({ I }) => {\n I.see(\n", 'broken.js') + } catch (err) { + error = err + } + expect(error).to.be.instanceOf(SyntaxError) + }) + + it('collects tests, local includes and helpers from config, minus ignored files', () => { + const root = fixture('project') + const config = { + tests: './*_test.js', + include: { I: './steps_file.js', loginPage: './pages/login.js', externalModule: 'some-package' }, + helpers: { Custom: { require: './custom_helper.js' }, Playwright: {} }, + lint: { ignore: ['legacy_test.js'] }, + } + const files = collectFiles(config, root) + .map(f => path.relative(root, f)) + .sort() + expect(files).to.deep.equal(['checkout_test.js', 'custom_helper.js', 'existing_test.js', path.join('pages', 'login.js'), 'steps_file.js']) + }) + + it('treats repeated identical errors as new', async () => { + const before = (await lintSource("Scenario('a', ({ I }) => {\n I.wait(5)\n})\n")).findings + const after = (await lintSource("Scenario('a', ({ I }) => {\n I.wait(5)\n I.say('x')\n I.wait(5)\n})\n")).findings + const added = newErrors(before, after) + expect(added).to.have.length(1) + expect(added[0].line).to.equal(4) + }) + }) + + describe('CLI', () => { + const run = (args, opts = {}) => spawnSync(process.execPath, [bin, 'lint', ...args], { encoding: 'utf8', env: { ...process.env, CI: '' }, ...opts }) + + it('lints files from config and exits 1 on errors', () => { + const result = run(['-c', fixture('project/codecept.conf.js')]) + expect(result.status).to.equal(1) + expect(result.stdout).to.include('checkout_test.js:4:3') + expect(result.stdout).to.include('no-fixed-wait') + expect(result.stdout).to.include('custom_helper.js:5:13') + expect(result.stdout).to.include('login.js:5:5') + expect(result.stdout).not.to.include('legacy_test.js') + expect(result.stdout).not.to.include('raw-browser-in-test') + }) + + it('exits 0 when only warnings are found', () => { + const result = run([fixture('no-pause.js')]) + expect(result.status).to.equal(0) + expect(result.stdout).to.include('no-pause') + }) + + it('prints JSON', () => { + const result = run(['--json', fixture('no-fixed-wait.js')]) + expect(result.status).to.equal(1) + const json = JSON.parse(result.stdout) + expect(json.errors).to.equal(1) + expect(json.findings[0]).to.include({ rule: 'no-fixed-wait', line: 5, column: 3, level: 'error' }) + }) + + it('exits 2 on parse failure', () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codecept-lint-')) + const file = path.join(dir, 'broken_test.js') + fs.writeFileSync(file, "Scenario('broken', ({ I }) => {\n I.see(\n") + try { + const result = run([file]) + expect(result.status).to.equal(2) + expect(result.stdout).to.include('parse') + } finally { + fs.rmSync(dir, { recursive: true, force: true }) + } + }) + }) + + describe('hook', () => { + let dir + const existing = () => path.join(dir, 'existing_test.js') + + beforeEach(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codecept-lint-')) + fs.copyFileSync(fixture('project/existing_test.js'), existing()) + }) + + afterEach(() => { + fs.rmSync(dir, { recursive: true, force: true }) + }) + + const hook = payload => runHook({ cwd: dir, ...payload }, { agent: 'claude' }) + + it('blocks a Write that introduces an error', async () => { + const result = await hook({ tool_name: 'Write', tool_input: { file_path: path.join(dir, 'new_test.js'), content: fs.readFileSync(fixture('no-fixed-wait.js'), 'utf8') } }) + expect(result.code).to.equal(2) + expect(result.stderr).to.include('new_test.js:5:3') + expect(result.stderr).to.include('no-fixed-wait') + }) + + it('allows a Write that only produces warnings', async () => { + const result = await hook({ tool_name: 'Write', tool_input: { file_path: path.join(dir, 'stub_test.js'), content: fs.readFileSync(fixture('no-pause.js'), 'utf8') } }) + expect(result).to.deep.equal({ code: 0, stderr: '' }) + }) + + it('blocks an Edit that adds an error to a clean part of the file', async () => { + const result = await hook({ tool_name: 'Edit', tool_input: { file_path: existing(), old_string: "I.see('Welcome')", new_string: "I.see('Welcome')\n I.wait(3)" } }) + expect(result.code).to.equal(2) + expect(result.stderr).to.include('I.wait(3)') + expect(result.stderr).not.to.include('I.wait(5)') + }) + + it('allows an Edit on a file with pre-existing violations', async () => { + const result = await hook({ tool_name: 'Edit', tool_input: { file_path: existing(), old_string: "I.see('Welcome')", new_string: "I.see('Welcome')\n I.waitForElement('#ok')" } }) + expect(result.code).to.equal(0) + }) + + it('blocks a second identical violation', async () => { + const result = await hook({ tool_name: 'Edit', tool_input: { file_path: existing(), old_string: "I.see('Welcome')", new_string: "I.see('Welcome')\n I.wait(5)" } }) + expect(result.code).to.equal(2) + expect(result.stderr).to.include('existing_test.js:7:3') + }) + + it('applies MultiEdit edits in order', async () => { + const result = await hook({ + tool_name: 'MultiEdit', + tool_input: { + file_path: existing(), + edits: [ + { old_string: 'I.wait(5)', new_string: "I.waitForText('Welcome')" }, + { old_string: "I.see('Welcome')", new_string: "I.see('Welcome')\n I.wait(1)" }, + ], + }, + }) + expect(result.code).to.equal(2) + expect(result.stderr).to.include('I.wait(1)') + }) + + it('ignores other tools, other files and files outside the project', async () => { + expect((await hook({ tool_name: 'Read', tool_input: { file_path: existing() } })).code).to.equal(0) + expect((await hook({ tool_name: 'Write', tool_input: { file_path: path.join(dir, 'notes.md'), content: 'I.wait(5)' } })).code).to.equal(0) + const outside = path.join(os.tmpdir(), 'outside_test.js') + expect((await hook({ tool_name: 'Write', tool_input: { file_path: outside, content: "Scenario('a', ({ I }) => { I.wait(5) })" } })).code).to.equal(0) + }) + + it('respects lint config from the project', async () => { + fs.writeFileSync(path.join(dir, 'codecept.conf.js'), "exports.config = { tests: './*_test.js', lint: { ignore: ['legacy/**'], rules: { 'await-grab': 'warn' } } }\n") + const legacy = await hook({ tool_name: 'Write', tool_input: { file_path: path.join(dir, 'legacy', 'old_test.js'), content: "Scenario('a', ({ I }) => {\n I.wait(5)\n})\n" } }) + expect(legacy.code).to.equal(0) + const grab = await hook({ tool_name: 'Write', tool_input: { file_path: path.join(dir, 'grab_test.js'), content: "Scenario('a', ({ I }) => {\n const t = I.grabTitle()\n})\n" } }) + expect(grab.code).to.equal(0) + }) + + it('allows edits that leave the file unparseable', async () => { + const result = await hook({ tool_name: 'Write', tool_input: { file_path: path.join(dir, 'broken_test.js'), content: 'Scenario((' } }) + expect(result.code).to.equal(0) + expect(result.stderr).to.include('could not be parsed') + }) + + it('reads the payload from stdin and exits 2 with findings on stderr', () => { + const payload = { cwd: dir, tool_name: 'Edit', tool_input: { file_path: existing(), old_string: "I.see('Welcome')", new_string: "I.see('Welcome')\n I.wait(5)" } } + const result = spawnSync(process.execPath, [bin, 'lint', '--hook', 'claude'], { cwd: dir, input: JSON.stringify(payload), encoding: 'utf8', env: { ...process.env, CLAUDE_PROJECT_DIR: dir } }) + expect(result.status).to.equal(2) + expect(result.stdout).to.equal('') + expect(result.stderr).to.include('no-fixed-wait') + }) + + it('exits 0 on invalid stdin', () => { + const result = spawnSync(process.execPath, [bin, 'lint', '--hook', 'claude'], { cwd: dir, input: 'not json', encoding: 'utf8', env: { ...process.env, CLAUDE_PROJECT_DIR: dir } }) + expect(result.status).to.equal(0) + expect(result.stdout).to.equal('') + }) + }) +}) From 7cfdc4cec9c0234f9a747744a8ec50044f220c69 Mon Sep 17 00:00:00 2001 From: DavertMik Date: Thu, 8 Oct 2026 00:49:20 +0300 Subject: [PATCH 2/2] refactor: simplify lint into Linter and rule classes One Linter class with a class per rule. Drop the typescript fallback, source maps, ignore config, warning suppression and per-rule fixtures. The hook only checks files the config lints. return I.grab*() is no longer flagged. Co-Authored-By: Claude Opus 5.5 --- docs/lint.md | 11 +- lib/command/lint.js | 221 +++------- lib/lint.js | 559 +++++++----------------- test/data/lint/await-grab.js | 16 - test/data/lint/clean.js | 9 - test/data/lint/no-actor-in-helper.js | 22 - test/data/lint/no-fixed-wait.js | 8 - test/data/lint/no-only.js | 17 - test/data/lint/no-pause.js | 7 - test/data/lint/no-sleep-app.js | 5 - test/data/lint/no-sleep.js | 19 - test/data/lint/project/codecept.conf.js | 1 - test/data/lint/project/legacy_test.js | 5 - test/data/lint/raw-browser-in-test.js | 17 - test/data/lint/secret-credentials.js | 11 - test/data/lint/suppressed.js | 9 - test/data/lint/typescript-enum.ts | 11 - test/data/lint/typescript.ts | 15 - test/unit/command/lint_test.js | 335 ++++---------- 19 files changed, 317 insertions(+), 981 deletions(-) delete mode 100644 test/data/lint/await-grab.js delete mode 100644 test/data/lint/clean.js delete mode 100644 test/data/lint/no-actor-in-helper.js delete mode 100644 test/data/lint/no-fixed-wait.js delete mode 100644 test/data/lint/no-only.js delete mode 100644 test/data/lint/no-pause.js delete mode 100644 test/data/lint/no-sleep-app.js delete mode 100644 test/data/lint/no-sleep.js delete mode 100644 test/data/lint/project/legacy_test.js delete mode 100644 test/data/lint/raw-browser-in-test.js delete mode 100644 test/data/lint/secret-credentials.js delete mode 100644 test/data/lint/suppressed.js delete mode 100644 test/data/lint/typescript-enum.ts delete mode 100644 test/data/lint/typescript.ts diff --git a/docs/lint.md b/docs/lint.md index 35c2d285a..dc820d182 100644 --- a/docs/lint.md +++ b/docs/lint.md @@ -20,7 +20,7 @@ Each finding is printed on one line: tests/checkout_test.js:14:3 error no-fixed-wait I.wait(5) sleeps unconditionally. Wait for a condition: I.waitForElement / I.waitForText / I.see ``` -Without paths, lint checks files matched by `tests`, local files from `include` (page objects, steps file) and custom helpers loaded with `require`. JavaScript and TypeScript files are supported. +Without paths, lint checks files matched by `tests`, local files from `include` (page objects, steps file) and custom helpers loaded with `require`. JavaScript and TypeScript files are supported. TypeScript is read with Node type stripping, so files using `enum` or `namespace` are reported as parse errors. Exit codes: `0` no errors (warnings allowed), `1` errors found, `2` bad input or a file that can't be parsed. @@ -29,11 +29,11 @@ Exit codes: `0` no errors (warnings allowed), `1` errors found, `2` bad input or | Rule | Default | Detects | | --- | --- | --- | | `no-fixed-wait` | error | `I.wait(5)` with a number. Use `I.waitForElement`, `I.waitForText`, `I.see` | -| `no-sleep` | error | `setTimeout` (including `new Promise(r => setTimeout(r, ms))`) in a Scenario, hook or page object method | +| `no-sleep` | error | `setTimeout` (including `new Promise(r => setTimeout(r, ms))`) outside helper classes | | `no-only` | error on CI, warning locally | `Scenario.only`, `Feature.only`, `Data(...).only.Scenario` | | `no-pause` | error on CI, warning locally | `pause()` | | `secret-credentials` | error | `I.fillField` on a password, token, secret or API key field without `secret()`; `process.env.*` with such a name passed to an `I.*` call without `secret()` | -| `await-grab` | error | `I.grab*()` result assigned, returned or passed on without `await` | +| `await-grab` | error | `I.grab*()` result assigned or used without `await` | | `no-actor-in-helper` | error | `I` (including `const { I } = inject()`) inside a class extending `Helper`. Use `this.helpers[...]` | | `raw-browser-in-test` | warning | `I.usePlaywrightTo`, `I.usePuppeteerTo`, `I.useWebDriverTo` and other `use*To`, `I.executeScript` in a Scenario body. Move it into a helper or page object | @@ -46,11 +46,10 @@ Add an optional `lint` section to `codecept.conf.js`: ```js lint: { rules: { 'raw-browser-in-test': 'off', 'no-fixed-wait': 'warn' }, - ignore: ['tests/legacy/**'], } ``` -Rule levels are `error`, `warn` or `off`. `ignore` takes glob patterns relative to the config file. +Rule levels are `error`, `warn` or `off`. To allow a single case, suppress it inline. The rule id is required: @@ -87,4 +86,4 @@ Lint can block a bad edit before an agent writes it. Add a `PreToolUse` hook to } ``` -The hook builds the file as it would look after the edit and lints it. The edit is blocked only when it adds a new error. The agent receives the findings and rewrites the edit. Errors already in the file and warnings never block, so an agent can still add a `pause()` stub or touch a legacy test. If the hook fails or the resulting file can't be parsed, the edit is allowed. +The hook checks only files that `codeceptjs lint` would check: tests, `include` files and helpers from the config. It builds the file as it would look after the edit and lints it. The edit is blocked only when it adds a new error. The agent receives the findings and rewrites the edit. Errors already in the file and warnings never block, so an agent can still add a `pause()` stub or touch a legacy test. If the hook fails or the resulting file can't be parsed, the edit is allowed. diff --git a/lib/command/lint.js b/lib/command/lint.js index 2cc56e62e..1caf3a0c3 100644 --- a/lib/command/lint.js +++ b/lib/command/lint.js @@ -2,195 +2,90 @@ import fs from 'fs' import path from 'path' import output from '../output.js' import Config from '../config.js' -import { captureStream } from './utils.js' -import { LINT_EXTENSIONS, collectFiles, isIgnored, lintFile, lintOptions, lintSource, newErrors } from '../lint.js' +import Linter from '../lint.js' +import { getTestRoot } from './utils.js' -const HOOK_AGENTS = ['claude'] -const CONFIG_NAMES = ['codecept.config.js', 'codecept.conf.js', 'codecept.js', 'codecept.config.cjs', 'codecept.conf.cjs', 'codecept.config.ts', 'codecept.conf.ts'] - -function findConfig(dir) { - return CONFIG_NAMES.map(name => path.join(dir, name)).find(f => fs.existsSync(f)) || null -} - -async function loadConfig(configPath, dir) { - const file = configPath ? path.resolve(configPath) : findConfig(dir) - if (!file) return { config: null, root: dir } - const root = fs.existsSync(file) && fs.statSync(file).isDirectory() ? file : path.dirname(file) - return { config: await Config.load(file), root } -} - -function relative(file) { - const rel = path.relative(process.cwd(), file) - return rel.startsWith('..') ? file : rel -} - -function formatFinding(f, colors = true) { - const level = f.level === 'error' ? 'error' : 'warn ' - const levelText = colors ? (f.level === 'error' ? output.colors.red(level) : output.colors.yellow(level)) : level - const location = `${relative(f.file)}:${f.line}:${f.column}` - return `${colors ? output.colors.bold(location) : location} ${levelText} ${colors ? output.colors.grey(f.rule) : f.rule} ${f.message}` -} - -export default async function lint(paths = [], options = {}) { - if (options.hook) return runHookCommand(options) - - let loaded +export default async function (paths = [], options = {}) { + let config = {} try { - loaded = await loadConfig(options.config, process.cwd()) + config = await Config.load(options.config) } catch (err) { - output.error(`Can't load config: ${err.message}`) - process.exitCode = 2 - return - } - const { config, root } = loaded - if (!config && !paths.length) { - output.error('No codecept config found. Pass files to lint or use -c to point to a config') - process.exitCode = 2 - return + if (options.hook) return + if (!paths.length || options.config) { + output.error(err.message) + process.exitCode = 2 + return + } } + const linter = new Linter(config, getTestRoot(options.config)) - const files = collectFiles(config || {}, root, paths) - const opts = lintOptions(config || {}) - const findings = [] - const failures = [] - const skipped = [] + if (options.hook) return hook(linter) + const findings = [] + let failed = false + const files = linter.files(paths) for (const file of files) { try { - const result = await lintFile(file, opts) - if (result.skipped) skipped.push({ file, reason: result.skipped }) - findings.push(...result.findings) + findings.push(...linter.lintFile(file)) } catch (err) { - failures.push({ file, message: err.message }) + failed = true + output.print(`${path.relative(process.cwd(), file)} ${output.colors.red('parse')} ${err.message}`) } } const errors = findings.filter(f => f.level === 'error').length - const warnings = findings.length - errors - if (options.json) { - process.stdout.write(`${JSON.stringify({ files: files.length, errors, warnings, findings, failures, skipped }, null, 2)}\n`) + output.print(JSON.stringify(findings, null, 2)) } else { - for (const f of findings) output.print(formatFinding(f)) - for (const s of skipped) output.print(`${output.colors.bold(relative(s.file))} ${output.colors.yellow('skip ')} ${s.reason}`) - for (const e of failures) output.print(`${output.colors.bold(relative(e.file))} ${output.colors.red('parse')} ${e.message}`) - const summary = `${files.length} file(s) checked, ${errors} error(s), ${warnings} warning(s)` - output.print(errors || failures.length ? output.colors.red(summary) : output.colors.green(summary)) + for (const finding of findings) output.print(format(finding)) + output.print(`${files.length} file(s) checked, ${errors} error(s), ${findings.length - errors} warning(s)`) } - if (failures.length || (!files.length && paths.length)) process.exitCode = 2 - else if (errors) process.exitCode = 1 + if (errors) process.exitCode = 1 + if (failed) process.exitCode = 2 } -function readStdin() { - return new Promise((resolve, reject) => { - let data = '' - process.stdin.setEncoding('utf8') - process.stdin.on('data', chunk => (data += chunk)) - process.stdin.on('end', () => resolve(data)) - process.stdin.on('error', reject) - }) -} - -function applyEdit(content, oldString, newString, replaceAll) { - if (typeof oldString !== 'string' || typeof newString !== 'string') return null - if (oldString === '') return content === '' ? newString : null - if (!content.includes(oldString)) return null - return replaceAll ? content.split(oldString).join(newString) : content.replace(oldString, () => newString) -} +async function hook(linter) { + let data = '' + for await (const chunk of process.stdin) data += chunk -function resultingContent(toolName, input, current) { - if (toolName === 'Write') return typeof input.content === 'string' ? input.content : null - if (toolName === 'Edit') return applyEdit(current, input.old_string, input.new_string, input.replace_all) - if (toolName === 'MultiEdit') { - let content = current - for (const edit of input.edits || []) { - content = applyEdit(content, edit.old_string, edit.new_string, edit.replace_all) - if (content === null) return null - } - return content - } - return null -} - -export async function runHook(payload, { agent = 'claude', config: configPath } = {}) { - const allow = { code: 0, stderr: '' } - if (!HOOK_AGENTS.includes(agent)) return { code: 0, stderr: `codeceptjs lint: unsupported hook agent "${agent}", supported: ${HOOK_AGENTS.join(', ')}\n` } - if (!payload || typeof payload !== 'object') return allow - - const toolName = payload.tool_name - const input = payload.tool_input || {} - if (!['Write', 'Edit', 'MultiEdit'].includes(toolName) || typeof input.file_path !== 'string') return allow - - const projectDir = path.resolve(process.env.CLAUDE_PROJECT_DIR || payload.cwd || process.cwd()) - const file = path.resolve(payload.cwd || projectDir, input.file_path) - const rel = path.relative(projectDir, file) - if (rel.startsWith('..') || path.isAbsolute(rel) || rel.split(path.sep).includes('node_modules')) return allow - if (!LINT_EXTENSIONS.includes(path.extname(file))) return allow - - let config = {} - let root = projectDir - const stdout = captureStream(process.stdout) - stdout.startCapture() try { - const loaded = await loadConfig(configPath, projectDir) - if (loaded.config) { - config = loaded.config - root = loaded.root + const { tool_name: tool, tool_input: input } = JSON.parse(data) + const file = path.resolve(input.file_path) + if (!linter.includes(file)) return + + let before = '' + if (fs.existsSync(file)) before = fs.readFileSync(file, 'utf8') + + let after = input.content + if (tool !== 'Write') { + after = before + for (const edit of input.edits || [input]) { + if (edit.replace_all) after = after.replaceAll(edit.old_string, () => edit.new_string) + else after = after.replace(edit.old_string, () => edit.new_string) + } } - } catch { - config = {} - } finally { - stdout.stopCapture() - } - if (isIgnored(file, config, root)) return allow - - const current = fs.existsSync(file) ? fs.readFileSync(file, 'utf8') : '' - const after = resultingContent(toolName, input, current) - if (after === null) return allow - - const opts = lintOptions(config) - let afterResult - try { - afterResult = await lintSource(after, file, opts) - } catch (err) { - return { code: 0, stderr: `codeceptjs lint: ${relative(file)} could not be parsed after this edit (${err.message})\n` } - } - if (afterResult.skipped) return allow - let beforeFindings = [] - if (current) { - try { - beforeFindings = (await lintSource(current, file, opts)).findings - } catch { - beforeFindings = [] + const existing = linter + .lint(before, file) + .filter(f => f.level === 'error') + .map(f => `${f.rule}:${f.source}`) + const added = [] + for (const finding of linter.lint(after, file)) { + if (finding.level !== 'error') continue + const index = existing.indexOf(`${finding.rule}:${finding.source}`) + if (index >= 0) existing.splice(index, 1) + else added.push(finding) } - } + if (!added.length) return - const added = newErrors(beforeFindings, afterResult.findings) - if (!added.length) return allow - - const lines = added.map(f => formatFinding(f, false)) - return { - code: 2, - stderr: `codeceptjs lint blocked this edit, ${added.length} new error(s):\n${lines.join('\n')}\nFix the code and retry.\n`, + process.stderr.write(`codeceptjs lint blocked this edit:\n${added.map(format).join('\n')}\nFix the code and retry.\n`) + process.exitCode = 2 + } catch (err) { + process.exitCode = 0 } } -async function runHookCommand(options) { - let result = { code: 0, stderr: '' } - try { - const raw = await readStdin() - let payload = null - try { - payload = JSON.parse(raw) - } catch { - payload = null - } - result = await runHook(payload, { agent: options.hook, config: options.config }) - } catch (err) { - result = { code: 0, stderr: `codeceptjs lint: hook failed (${err.message})\n` } - } - process.exitCode = result.code - process.stderr.write(result.stderr, () => process.exit(result.code)) +function format(finding) { + return `${path.relative(process.cwd(), finding.file)}:${finding.line}:${finding.column} ${finding.level} ${finding.rule} ${finding.message}` } diff --git a/lib/lint.js b/lib/lint.js index ed079c8ff..65c89f8a8 100644 --- a/lib/lint.js +++ b/lib/lint.js @@ -5,456 +5,225 @@ import * as acorn from 'acorn' import * as walk from 'acorn-walk' import { globSync } from 'glob' -export const LINT_EXTENSIONS = ['.js', '.ts', '.mjs', '.cjs'] - -const TEST_BLOCKS = new Set(['Scenario', 'Before', 'After', 'BeforeSuite', 'AfterSuite', 'Background']) -const CREDENTIAL_WORDS = /pass(word|wd)|token|secret|api[\s_-]?key/i -const CREDENTIAL_ENV = /pass(word|wd)|(^|_)pass($|_)|token|secret|api_?key/i -const RAW_BROWSER_METHOD = /^use[A-Z]\w*To$/ -const PROMISE_COMBINATORS = new Set(['all', 'allSettled', 'race', 'any']) - -const isCI = () => !!process.env.CI - -function isIdentifier(node, name) { - return node?.type === 'Identifier' && (name === undefined || node.name === name) -} - -function propertyName(member) { - if (member?.type !== 'MemberExpression') return null - if (!member.computed && member.property.type === 'Identifier') return member.property.name - if (member.computed && member.property.type === 'Literal' && typeof member.property.value === 'string') return member.property.value - return null -} +const EXTENSIONS = ['.js', '.ts', '.mjs', '.cjs'] +const TEST_BLOCKS = ['Scenario', 'Before', 'After', 'BeforeSuite', 'AfterSuite'] +const CREDENTIALS = /pass(word|wd)|(^|_)pass(_|$)|token|secret|api[\s_-]?key/i function actorMethod(node) { - if (node?.type !== 'CallExpression') return null - const callee = node.callee - if (callee.type !== 'MemberExpression' || !isIdentifier(callee.object, 'I')) return null - return propertyName(callee) -} - -function isFunction(node) { - return node?.type === 'FunctionExpression' || node?.type === 'ArrowFunctionExpression' || node?.type === 'FunctionDeclaration' + if (node.type !== 'CallExpression') return + if (node.callee.type !== 'MemberExpression') return + if (node.callee.object.name !== 'I') return + return node.callee.property.name +} + +function insideTestBlock(ancestors) { + return ancestors.some(node => { + if (node.type !== 'CallExpression') return false + let callee = node.callee + if (callee.type === 'MemberExpression') callee = callee.object + return TEST_BLOCKS.includes(callee.name) + }) } -function isDataCall(node) { - return node?.type === 'CallExpression' && isIdentifier(node.callee, 'Data') +function insideHelper(ancestors) { + return ancestors.some(node => node.superClass && node.superClass.name === 'Helper') } -function testBlockName(callee) { - if (callee.type === 'Identifier' && TEST_BLOCKS.has(callee.name)) return callee.name - if (callee.type !== 'MemberExpression') return null - const prop = propertyName(callee) - if (isIdentifier(callee.object, 'Scenario') && ['only', 'skip', 'todo'].includes(prop)) return 'Scenario' - if (prop === 'Scenario') { - const obj = callee.object - if (isDataCall(obj)) return 'Scenario' - if (obj.type === 'MemberExpression' && isDataCall(obj.object)) return 'Scenario' +class Rule { + get level() { + return 'error' } - return null } -function enclosingTestBlock(ancestors) { - for (let i = ancestors.length - 2; i >= 0; i--) { - const node = ancestors[i] - if (node.type !== 'CallExpression' || !isFunction(ancestors[i + 1])) continue - if (!node.arguments.includes(ancestors[i + 1])) continue - const name = testBlockName(node.callee) - if (name) return name - } - return null -} +class NoFixedWait extends Rule { + id = 'no-fixed-wait' -function isHelperClass(node) { - if (node.type !== 'ClassDeclaration' && node.type !== 'ClassExpression') return false - const sup = node.superClass - if (!sup) return false - return isIdentifier(sup, 'Helper') || propertyName(sup) === 'Helper' + check(node) { + if (actorMethod(node) !== 'wait') return + if (typeof node.arguments[0]?.value !== 'number') return + return 'I.wait() sleeps unconditionally. Wait for a condition: I.waitForElement / I.waitForText / I.see' + } } -function insideHelperClass(ancestors) { - return ancestors.some(isHelperClass) -} +class NoSleep extends Rule { + id = 'no-sleep' -function insideMethod(ancestors) { - for (let i = ancestors.length - 2; i > 0; i--) { - const node = ancestors[i] - if (!isFunction(node)) continue - const parent = ancestors[i - 1] - if (parent.type === 'MethodDefinition') return true - if (parent.type === 'Property' && parent.value === node) return true - if (parent.type === 'PropertyDefinition' && parent.value === node) return true + check(node, ancestors) { + if (node.type !== 'CallExpression' || node.callee.name !== 'setTimeout') return + if (insideHelper(ancestors)) return + return 'setTimeout pauses for a fixed time. Wait for a condition: I.waitForElement / I.waitForText / I.waitForFunction' } - return false } -function isSecretCall(node) { - return node?.type === 'CallExpression' && isIdentifier(node.callee, 'secret') -} +class NoOnly extends Rule { + id = 'no-only' -function stringValue(node) { - if (node?.type === 'Literal' && typeof node.value === 'string') return node.value - if (node?.type === 'TemplateLiteral' && node.expressions.length === 0) return node.quasis[0].value.cooked - return null -} + get level() { + if (process.env.CI) return 'error' + return 'warn' + } -function envName(node) { - if (node?.type !== 'MemberExpression') return null - const obj = node.object - if (obj.type !== 'MemberExpression' || !isIdentifier(obj.object, 'process') || propertyName(obj) !== 'env') return null - return propertyName(node) + check(node) { + if (node.type !== 'MemberExpression' || node.property.name !== 'only') return + const target = node.object.name || node.object.callee?.name + if (!['Scenario', 'Feature', 'Data'].includes(target)) return + return `${target}.only limits the run to focused tests. Remove it before commit` + } } -function isPromiseCombinator(node) { - return node?.type === 'CallExpression' && node.callee.type === 'MemberExpression' && isIdentifier(node.callee.object, 'Promise') && PROMISE_COMBINATORS.has(propertyName(node.callee)) -} +class NoPause extends Rule { + id = 'no-pause' -function grabResultUsed(ancestors) { - let i = ancestors.length - 1 - let child = ancestors[i] - let parent = ancestors[i - 1] - while (parent && parent.type === 'ChainExpression') { - child = parent - parent = ancestors[--i - 1] + get level() { + if (process.env.CI) return 'error' + return 'warn' } - if (!parent) return false - switch (parent.type) { - case 'AwaitExpression': - case 'ExpressionStatement': - case 'YieldExpression': - case 'SequenceExpression': - return false - case 'MemberExpression': - return parent.object !== child - case 'ArrayExpression': - return !isPromiseCombinator(ancestors[i - 2]) - case 'ArrowFunctionExpression': - return parent.body === child - case 'UnaryExpression': - return parent.operator !== 'void' - default: - return true + + check(node) { + if (node.type !== 'CallExpression' || node.callee.name !== 'pause') return + return 'pause() stops the test for debugging. Remove it before commit' } } -function usesCodeceptGlobals(ast) { - let found = false - walk.full(ast, node => { - if (found) return - if (node.type === 'CallExpression' && (isIdentifier(node.callee, 'inject') || isIdentifier(node.callee, 'actor') || isIdentifier(node.callee, 'Feature') || testBlockName(node.callee))) found = true - }) - return found -} +class SecretCredentials extends Rule { + id = 'secret-credentials' -export const rules = [ - { - id: 'no-fixed-wait', - level: 'error', - check(node, ancestors, ctx) { - if (actorMethod(node) !== 'wait') return - const arg = node.arguments[0] - if (arg?.type !== 'Literal' || typeof arg.value !== 'number') return - ctx.report(node, `${ctx.source(node)} sleeps unconditionally. Wait for a condition: I.waitForElement / I.waitForText / I.see`) - }, - }, - { - id: 'no-sleep', - level: 'error', - check(node, ancestors, ctx) { - if (node.type !== 'CallExpression' || !isIdentifier(node.callee, 'setTimeout')) return - if (insideHelperClass(ancestors)) return - const inTest = enclosingTestBlock(ancestors) - if (!inTest && !(ctx.codeceptFile && insideMethod(ancestors))) return - ctx.report(node, 'setTimeout pauses for a fixed time. Wait for a condition: I.waitForElement / I.waitForText / I.waitForFunction') - }, - }, - { - id: 'no-only', - level: () => (isCI() ? 'error' : 'warn'), - check(node, ancestors, ctx) { - if (node.type !== 'MemberExpression' || propertyName(node) !== 'only') return - const obj = node.object - if (!isIdentifier(obj, 'Scenario') && !isIdentifier(obj, 'Feature') && !isDataCall(obj)) return - const name = isDataCall(obj) ? 'Data(...).only' : `${obj.name}.only` - ctx.report(node, `${name} limits the run to the focused tests. Remove .only before commit`) - }, - }, - { - id: 'no-pause', - level: () => (isCI() ? 'error' : 'warn'), - check(node, ancestors, ctx) { - if (node.type !== 'CallExpression' || !isIdentifier(node.callee, 'pause')) return - ctx.report(node, 'pause() stops the test for interactive debugging. Remove it before commit') - }, - }, - { - id: 'secret-credentials', - level: 'error', - check(node, ancestors, ctx) { - const method = actorMethod(node) - if (!method) return - let envReported = false - for (const arg of node.arguments) { - const name = envName(arg) - if (name && CREDENTIAL_ENV.test(name)) { - envReported = true - ctx.report(arg, `process.env.${name} is passed to I.${method} in plain text and will be printed in logs. Wrap it: secret(process.env.${name})`) - } - } - if (method !== 'fillField' || envReported) return - const [locator, value] = node.arguments - const text = stringValue(locator) - if (!text || !CREDENTIAL_WORDS.test(text) || !value || isSecretCall(value)) return - ctx.report(node, `I.fillField('${text}', ...) types a credential in plain text and it will be printed in logs. Wrap the value: secret(...)`) - }, - }, - { - id: 'await-grab', - level: 'error', - check(node, ancestors, ctx) { - const method = actorMethod(node) - if (!method || !method.startsWith('grab')) return - if (!grabResultUsed(ancestors)) return - ctx.report(node, `I.${method}() returns a promise. Use: await I.${method}(...)`) - }, - }, - { - id: 'no-actor-in-helper', - level: 'error', - check(node, ancestors, ctx) { - const isActorRef = - (node.type === 'Identifier' && node.name === 'I') || (node.type === 'MemberExpression' && propertyName(node) === 'I' && node.object.type === 'CallExpression' && isIdentifier(node.object.callee, 'inject')) - if (!isActorRef || !insideHelperClass(ancestors)) return - ctx.report(node, 'The I actor is not available inside a helper. Call other helpers via this.helpers[...]') - }, - }, - { - id: 'raw-browser-in-test', - level: 'warn', - check(node, ancestors, ctx) { - const method = actorMethod(node) - if (!method || !(RAW_BROWSER_METHOD.test(method) || method === 'executeScript')) return - if (enclosingTestBlock(ancestors) !== 'Scenario') return - ctx.report(node, `I.${method} runs raw browser code inside a test. Move it into a helper or page object`) - }, - }, -] - -export const ruleIds = rules.map(r => r.id) - -function withoutNodeWarnings(fn) { - const original = process.emitWarning - process.emitWarning = (warning, ...args) => { - const type = (typeof args[0] === 'string' ? args[0] : args[0]?.type) || warning?.name - if (type === 'ExperimentalWarning' || type === 'DeprecationWarning') return - return original.call(process, warning, ...args) - } - try { - return fn() - } finally { - process.emitWarning = original + check(node) { + const method = actorMethod(node) + if (!method) return + for (const arg of node.arguments) { + const env = arg.type === 'MemberExpression' && arg.object.property?.name === 'env' && arg.property.name + if (env && CREDENTIALS.test(env)) return `process.env.${env} is printed in logs. Wrap it: secret(process.env.${env})` + } + if (method !== 'fillField') return + const [locator, value] = node.arguments + if (typeof locator?.value !== 'string' || !CREDENTIALS.test(locator.value)) return + if (!value || value.callee?.name === 'secret') return + return `I.fillField('${locator.value}', ...) types a credential that is printed in logs. Wrap the value: secret(...)` } } -let typescriptModule -async function loadTypeScript() { - if (typescriptModule !== undefined) return typescriptModule - try { - const mod = await import('typescript') - typescriptModule = mod.default || mod - } catch { - typescriptModule = null - } - return typescriptModule -} +class AwaitGrab extends Rule { + id = 'await-grab' -export async function toJavaScript(code, file) { - if (path.extname(file) !== '.ts') return { code } - if (typeof module.stripTypeScriptTypes === 'function') { - try { - return { code: withoutNodeWarnings(() => module.stripTypeScriptTypes(code, { mode: 'strip' })) } - } catch {} + check(node, ancestors) { + const method = actorMethod(node) + if (!method || !method.startsWith('grab')) return + const parent = ancestors[ancestors.length - 2] + if (!['VariableDeclarator', 'AssignmentExpression', 'BinaryExpression', 'TemplateLiteral', 'MemberExpression'].includes(parent.type)) return + return `I.${method}() returns a promise. Use: await I.${method}()` } - const ts = await loadTypeScript() - if (!ts) return { skipped: 'TypeScript file skipped: type stripping is not supported by this Node.js version and the "typescript" package is not installed' } - const result = ts.transpileModule(code, { - compilerOptions: { target: ts.ScriptTarget.ESNext, module: ts.ModuleKind.ESNext, removeComments: false, sourceMap: true }, - fileName: file, - }) - return { code: result.outputText, position: sourcePosition(result.sourceMapText) } } -function sourcePosition(sourceMapText) { - if (!sourceMapText || typeof module.SourceMap !== 'function') return null - const map = new module.SourceMap(JSON.parse(sourceMapText)) - return ({ line, column }) => { - const entry = map.findEntry(line - 1, column) - if (typeof entry?.originalLine !== 'number') return { line, column } - return { line: entry.originalLine + 1, column: entry.originalColumn } - } -} +class NoActorInHelper extends Rule { + id = 'no-actor-in-helper' -export function parse(code) { - const options = { ecmaVersion: 'latest', locations: true, allowHashBang: true } - let comments = [] - try { - const ast = acorn.parse(code, { ...options, sourceType: 'module', onComment: comments }) - return { ast, comments } - } catch (err) { - comments = [] - try { - const ast = acorn.parse(code, { ...options, sourceType: 'script', allowReturnOutsideFunction: true, onComment: comments }) - return { ast, comments } - } catch { - throw err - } + check(node, ancestors) { + if (node.type !== 'Identifier' || node.name !== 'I') return + if (!insideHelper(ancestors)) return + return 'I is not available inside a helper. Call other helpers via this.helpers[...]' } } -function suppressions(comments, position) { - const lineOf = loc => (position ? position(loc).line : loc.line) - const byLine = new Map() - const add = (line, ids) => { - if (!byLine.has(line)) byLine.set(line, new Set()) - ids.forEach(id => byLine.get(line).add(id)) - } - for (const comment of comments) { - const [directive, ...rest] = comment.value.trim().split(/[\s,]+/) - const ids = rest.filter(Boolean) - if (!ids.length) continue - if (directive === 'codeceptjs-lint-disable-line') add(lineOf(comment.loc.start), ids) - if (directive === 'codeceptjs-lint-disable-next-line') add(lineOf(comment.loc.end) + 1, ids) - } - return byLine -} +class RawBrowserInTest extends Rule { + id = 'raw-browser-in-test' -function resolveLevel(rule, overrides) { - const configured = overrides?.[rule.id] - if (configured !== undefined) { - if (configured === false || configured === 'off' || configured === 0) return 'off' - if (configured === 'warn' || configured === 'warning' || configured === 1) return 'warn' - if (configured === 'error' || configured === true || configured === 2) return 'error' + get level() { + return 'warn' } - return typeof rule.level === 'function' ? rule.level() : rule.level -} -export function normalizeSource(text) { - return text.replace(/\s+/g, ' ').trim() + check(node, ancestors) { + const method = actorMethod(node) + if (!method) return + if (method !== 'executeScript' && !/^use\w+To$/.test(method)) return + if (!insideTestBlock(ancestors)) return + return `I.${method} runs raw browser code in a test. Move it into a helper or page object` + } } -export async function lintSource(code, file = 'file.js', options = {}) { - const js = await toJavaScript(code, file) - if (js.skipped) return { file, findings: [], skipped: js.skipped } - - const { ast, comments } = parse(js.code) - const suppressed = suppressions(comments, js.position) - const active = rules.map(rule => ({ rule, level: resolveLevel(rule, options.rules) })).filter(r => r.level !== 'off') - const findings = [] - const seen = new Set() +export const rules = [new NoFixedWait(), new NoSleep(), new NoOnly(), new NoPause(), new SecretCredentials(), new AwaitGrab(), new NoActorInHelper(), new RawBrowserInTest()] - const ctx = { - codeceptFile: usesCodeceptGlobals(ast), - source: node => normalizeSource(js.code.slice(node.start, node.end)), +export default class Linter { + constructor(config = {}, root = process.cwd()) { + this.config = config + this.root = root + this.levels = config.lint?.rules || {} } - walk.fullAncestor(ast, (node, state, ancestors) => { - for (const { rule, level } of active) { - rule.check(node, ancestors, { - ...ctx, - report(target, message) { - const { line, column } = js.position ? js.position(target.loc.start) : target.loc.start - const key = `${rule.id}:${target.start}` - if (seen.has(key)) return - seen.add(key) - if (suppressed.get(line)?.has(rule.id)) return - findings.push({ file, line, column: column + 1, rule: rule.id, level, message, source: ctx.source(target) }) - }, - }) + files(paths = []) { + const files = [] + for (const pattern of paths) { + if (fs.existsSync(pattern) && fs.statSync(pattern).isDirectory()) { + files.push(...globSync(`${pattern}/**/*.{js,ts,mjs,cjs}`, { absolute: true, ignore: '**/node_modules/**' })) + } else { + files.push(...globSync(pattern, { absolute: true })) + } } - }) - - findings.sort((a, b) => a.line - b.line || a.column - b.column) - return { file, findings } -} - -export async function lintFile(file, options = {}) { - const code = fs.readFileSync(file, 'utf8') - return lintSource(code, file, options) -} - -function isLintable(file) { - return LINT_EXTENSIONS.includes(path.extname(file)) -} - -function localFile(entry, root) { - if (typeof entry !== 'string') return null - if (!entry.startsWith('.') && !path.isAbsolute(entry)) return null - const resolved = path.resolve(root, entry) - const candidates = [resolved, ...LINT_EXTENSIONS.map(ext => resolved + ext)] - return candidates.find(f => fs.existsSync(f) && fs.statSync(f).isFile()) || null -} - -function expandPath(entry) { - const resolved = path.resolve(entry) - if (fs.existsSync(resolved)) { - if (fs.statSync(resolved).isDirectory()) { - return globSync(`**/*{${LINT_EXTENSIONS.join(',')}}`, { cwd: resolved, absolute: true, ignore: ['**/node_modules/**'] }) + if (!paths.length) { + for (const pattern of [].concat(this.config.tests || [])) { + files.push(...globSync(pattern, { cwd: this.root, absolute: true })) + } + files.push(...this.supportFiles()) } - return [resolved] + return [...new Set(files)].filter(file => EXTENSIONS.includes(path.extname(file))) } - return globSync(entry, { absolute: true, ignore: ['**/node_modules/**'] }) -} -export function collectFiles(config = {}, root = process.cwd(), paths = []) { - let files = [] - if (paths.length) { - for (const entry of paths) files.push(...expandPath(entry)) - } else { - const tests = [].concat(config.tests || []) - for (const pattern of tests) { - files.push(...globSync(pattern, { cwd: root, absolute: true, ignore: ['**/node_modules/**'] })) - } - for (const entry of Object.values(config.include || {})) { - const file = localFile(entry, root) - if (file) files.push(file) + includes(file) { + for (const pattern of [].concat(this.config.tests || [])) { + if (path.matchesGlob(file, path.resolve(this.root, pattern))) return true } - for (const helper of Object.values(config.helpers || {})) { - const file = localFile(helper?.require, root) - if (file) files.push(file) + return this.supportFiles().includes(file) + } + + supportFiles() { + const entries = Object.values(this.config.include || {}) + for (const helper of Object.values(this.config.helpers || {})) entries.push(helper.require) + const files = [] + for (const entry of entries) { + if (typeof entry !== 'string' || !entry.startsWith('.')) continue + const file = path.resolve(this.root, entry) + for (const candidate of [file, ...EXTENSIONS.map(ext => file + ext)]) { + if (fs.existsSync(candidate) && fs.statSync(candidate).isFile()) files.push(candidate) + } } + return files } - files = [...new Set(files.map(f => path.resolve(f)))].filter(isLintable) - return files.filter(f => !isIgnored(f, config, root)) -} -export function isIgnored(file, config = {}, root = process.cwd()) { - const ignore = [].concat(config.lint?.ignore || []) - if (!ignore.length) return false - const target = path.resolve(file) - return withoutNodeWarnings(() => ignore.some(pattern => path.matchesGlob(target, path.resolve(root, pattern)))) -} + lintFile(file) { + return this.lint(fs.readFileSync(file, 'utf8'), file) + } -export function lintOptions(config = {}) { - return { rules: config.lint?.rules || {} } -} + lint(code, file) { + if (file.endsWith('.ts')) code = module.stripTypeScriptTypes(code) -export function findingKey(finding) { - return `${finding.rule}\u0000${finding.source}` -} + const comments = [] + let ast + try { + ast = acorn.parse(code, { ecmaVersion: 'latest', sourceType: 'module', locations: true, onComment: comments }) + } catch (err) { + comments.length = 0 + ast = acorn.parse(code, { ecmaVersion: 'latest', sourceType: 'script', locations: true, allowReturnOutsideFunction: true, onComment: comments }) + } -export function newErrors(before, after) { - const counts = new Map() - for (const f of before) { - if (f.level !== 'error') continue - counts.set(findingKey(f), (counts.get(findingKey(f)) || 0) + 1) - } - const added = [] - for (const f of after) { - if (f.level !== 'error') continue - const key = findingKey(f) - const left = counts.get(key) || 0 - if (left > 0) counts.set(key, left - 1) - else added.push(f) + const disabled = [] + for (const comment of comments) { + const [directive, rule] = comment.value.trim().split(/\s+/) + if (directive === 'codeceptjs-lint-disable-line') disabled.push(`${comment.loc.start.line}:${rule}`) + if (directive === 'codeceptjs-lint-disable-next-line') disabled.push(`${comment.loc.end.line + 1}:${rule}`) + } + + const findings = [] + walk.fullAncestor(ast, (node, state, ancestors) => { + for (const rule of rules) { + const level = this.levels[rule.id] || rule.level + if (level === 'off') continue + const message = rule.check(node, ancestors) + if (!message) continue + const { line, column } = node.loc.start + if (disabled.includes(`${line}:${rule.id}`)) continue + findings.push({ file, line, column: column + 1, rule: rule.id, level, message, source: code.slice(node.start, node.end) }) + } + }) + return findings.sort((a, b) => a.line - b.line || a.column - b.column) } - return added } diff --git a/test/data/lint/await-grab.js b/test/data/lint/await-grab.js deleted file mode 100644 index 0de080279..000000000 --- a/test/data/lint/await-grab.js +++ /dev/null @@ -1,16 +0,0 @@ -Feature('grab') - -Scenario('grab', async ({ I }) => { - const title = I.grabTitle() - const text = await I.grabTextFrom('h1') - I.grabCurrentUrl() - I.say(I.grabValueFrom('#name')) - const [a, b] = await Promise.all([I.grabTitle(), I.grabCurrentUrl()]) - I.grabTitle().then(t => I.say(t)) -}) - -export default { - getTitle() { - return I.grabTitle() - }, -} diff --git a/test/data/lint/clean.js b/test/data/lint/clean.js deleted file mode 100644 index 8f8940c23..000000000 --- a/test/data/lint/clean.js +++ /dev/null @@ -1,9 +0,0 @@ -Feature('clean') - -Scenario('clean', async ({ I }) => { - I.amOnPage('/') - I.fillField('Password', secret('123456')) - I.waitForElement('#ok') - const title = await I.grabTitle() - I.see(title) -}) diff --git a/test/data/lint/no-actor-in-helper.js b/test/data/lint/no-actor-in-helper.js deleted file mode 100644 index 824f4e43b..000000000 --- a/test/data/lint/no-actor-in-helper.js +++ /dev/null @@ -1,22 +0,0 @@ -import Helper from '@codeceptjs/helper' - -class MyHelper extends Helper { - async login() { - const { I } = inject() - I.amOnPage('/login') - } - - async open() { - const { Playwright } = this.helpers - await Playwright.amOnPage('/') - } -} - -class PageObject { - open() { - const { I } = inject() - I.amOnPage('/') - } -} - -export default MyHelper diff --git a/test/data/lint/no-fixed-wait.js b/test/data/lint/no-fixed-wait.js deleted file mode 100644 index 742415795..000000000 --- a/test/data/lint/no-fixed-wait.js +++ /dev/null @@ -1,8 +0,0 @@ -Feature('waits') - -Scenario('fixed wait', ({ I }) => { - I.amOnPage('/') - I.wait(5) - I.waitForElement('#ok', 5) - I.wait(waitTime) -}) diff --git a/test/data/lint/no-only.js b/test/data/lint/no-only.js deleted file mode 100644 index f5fccf981..000000000 --- a/test/data/lint/no-only.js +++ /dev/null @@ -1,17 +0,0 @@ -Feature.only('focused') - -Scenario('normal', ({ I }) => { - I.see('ok') -}) - -Scenario.only('focused', ({ I }) => { - I.see('ok') -}) - -Data(['a', 'b']).only.Scenario('data', ({ I, current }) => { - I.see(current) -}) - -Scenario.skip('skipped', ({ I }) => { - I.see('ok') -}) diff --git a/test/data/lint/no-pause.js b/test/data/lint/no-pause.js deleted file mode 100644 index afeeed7ec..000000000 --- a/test/data/lint/no-pause.js +++ /dev/null @@ -1,7 +0,0 @@ -Feature('pause') - -Scenario('debug', ({ I }) => { - I.amOnPage('/') - pause() - I.see('Welcome') -}) diff --git a/test/data/lint/no-sleep-app.js b/test/data/lint/no-sleep-app.js deleted file mode 100644 index 3109ad384..000000000 --- a/test/data/lint/no-sleep-app.js +++ /dev/null @@ -1,5 +0,0 @@ -export default class Poller { - start() { - setTimeout(() => this.tick(), 100) - } -} diff --git a/test/data/lint/no-sleep.js b/test/data/lint/no-sleep.js deleted file mode 100644 index 4f5e98371..000000000 --- a/test/data/lint/no-sleep.js +++ /dev/null @@ -1,19 +0,0 @@ -const { I } = inject() - -Feature('sleep') - -Scenario('sleeps', async ({ I }) => { - await new Promise(resolve => setTimeout(resolve, 1000)) - I.waitForText('Done') -}) - -export default { - async open() { - I.amOnPage('/') - setTimeout(() => {}, 500) - }, -} - -function utility(fn) { - setTimeout(fn, 10) -} diff --git a/test/data/lint/project/codecept.conf.js b/test/data/lint/project/codecept.conf.js index 8f35d4a1b..cd1276f24 100644 --- a/test/data/lint/project/codecept.conf.js +++ b/test/data/lint/project/codecept.conf.js @@ -12,6 +12,5 @@ export const config = { }, lint: { rules: { 'raw-browser-in-test': 'off' }, - ignore: ['legacy_test.js'], }, } diff --git a/test/data/lint/project/legacy_test.js b/test/data/lint/project/legacy_test.js deleted file mode 100644 index c140b231b..000000000 --- a/test/data/lint/project/legacy_test.js +++ /dev/null @@ -1,5 +0,0 @@ -Feature('legacy') - -Scenario('old', ({ I }) => { - I.wait(10) -}) diff --git a/test/data/lint/raw-browser-in-test.js b/test/data/lint/raw-browser-in-test.js deleted file mode 100644 index c6bb5b564..000000000 --- a/test/data/lint/raw-browser-in-test.js +++ /dev/null @@ -1,17 +0,0 @@ -Feature('raw') - -Scenario('raw', async ({ I }) => { - await I.usePlaywrightTo('click', async ({ page }) => page.click('#a')) - I.executeScript(() => window.scrollTo(0, 0)) - I.click('Login') -}) - -Before(({ I }) => { - I.executeScript(() => localStorage.clear()) -}) - -export const page = { - reset() { - I.useWebDriverTo('reset', async ({ browser }) => browser.reloadSession()) - }, -} diff --git a/test/data/lint/secret-credentials.js b/test/data/lint/secret-credentials.js deleted file mode 100644 index 254684602..000000000 --- a/test/data/lint/secret-credentials.js +++ /dev/null @@ -1,11 +0,0 @@ -Feature('login') - -Scenario('login', ({ I }) => { - I.fillField('Email', 'user@example.com') - I.fillField('Password', '123456') - I.fillField('Password', secret('123456')) - I.fillField('#api_key', process.env.API_KEY) - I.fillField('Token', secret(process.env.AUTH_TOKEN)) - I.sendPostRequest('/login', process.env.USER_PASSWORD) - I.fillField('Username', process.env.USERNAME) -}) diff --git a/test/data/lint/suppressed.js b/test/data/lint/suppressed.js deleted file mode 100644 index 53a2d1040..000000000 --- a/test/data/lint/suppressed.js +++ /dev/null @@ -1,9 +0,0 @@ -Feature('suppressed') - -Scenario('suppressed', ({ I }) => { - I.wait(1) // codeceptjs-lint-disable-line no-fixed-wait - // codeceptjs-lint-disable-next-line no-fixed-wait - I.wait(2) - I.wait(3) // codeceptjs-lint-disable-line - I.wait(4) // codeceptjs-lint-disable-line no-pause -}) diff --git a/test/data/lint/typescript-enum.ts b/test/data/lint/typescript-enum.ts deleted file mode 100644 index b72ee1a49..000000000 --- a/test/data/lint/typescript-enum.ts +++ /dev/null @@ -1,11 +0,0 @@ -enum Role { - Admin, - User, -} - -Feature('enum') - -Scenario('enum', ({ I }) => { - I.wait(Role.Admin) - I.wait(3) -}) diff --git a/test/data/lint/typescript.ts b/test/data/lint/typescript.ts deleted file mode 100644 index 34f2371e3..000000000 --- a/test/data/lint/typescript.ts +++ /dev/null @@ -1,15 +0,0 @@ -interface Credentials { - email: string - password: string -} - -type Maybe = T | null - -const creds: Credentials = { email: 'a@b.c', password: 'x' } - -Feature('typescript') - -Scenario('ts', async ({ I }: { I: CodeceptJS.I }) => { - const title: string = I.grabTitle() as unknown as string - I.wait(5) -}) diff --git a/test/unit/command/lint_test.js b/test/unit/command/lint_test.js index 616b04fea..e6e57d449 100644 --- a/test/unit/command/lint_test.js +++ b/test/unit/command/lint_test.js @@ -1,300 +1,145 @@ import { expect } from 'chai' import fs from 'fs' -import os from 'os' import path from 'path' import { spawnSync } from 'child_process' import { fileURLToPath } from 'url' -import { lintFile, lintSource, collectFiles, newErrors } from '../../../lib/lint.js' -import { runHook } from '../../../lib/command/lint.js' +import Linter from '../../../lib/lint.js' const __dirname = path.dirname(fileURLToPath(import.meta.url)) -const fixtures = path.join(__dirname, '../../data/lint') +const project = path.join(__dirname, '../../data/lint/project') const bin = path.join(__dirname, '../../../bin/codecept.js') -const fixture = name => path.join(fixtures, name) -const findings = async (name, options) => (await lintFile(fixture(name), options)).findings -const byRule = (list, rule) => list.filter(f => f.rule === rule).map(f => f.line) +const lint = (code, file = 'test.js') => new Linter().lint(code, file) +const rules = code => lint(code).map(f => `${f.line}:${f.rule}`) -describe('lint command', () => { - const saved = {} +const runHook = payload => + spawnSync(process.execPath, [bin, 'lint', '--hook', 'claude'], { + cwd: project, + input: JSON.stringify(payload), + encoding: 'utf8', + env: { ...process.env, CI: '' }, + }) + +describe('lint', () => { + let ci - before(() => { - for (const key of ['CI', 'CLAUDE_PROJECT_DIR']) { - saved[key] = process.env[key] - delete process.env[key] - } + beforeEach(() => { + ci = process.env.CI + delete process.env.CI }) - after(() => { - for (const [key, value] of Object.entries(saved)) { - if (value === undefined) delete process.env[key] - else process.env[key] = value - } + afterEach(() => { + if (ci !== undefined) process.env.CI = ci }) describe('rules', () => { - it('no-fixed-wait flags I.wait with a number literal only', async () => { - const list = await findings('no-fixed-wait.js') - expect(byRule(list, 'no-fixed-wait')).to.deep.equal([5]) - expect(list[0].level).to.equal('error') - expect(list[0].column).to.equal(3) - expect(list[0].message).to.include('I.wait(5)') - }) - - it('no-sleep flags setTimeout in scenarios and page object methods', async () => { - const list = await findings('no-sleep.js') - expect(byRule(list, 'no-sleep')).to.deep.equal([6, 13]) + it('no-fixed-wait', () => { + expect(rules("I.wait(5)\nI.waitForElement('#a', 5)\nI.wait(timeout)")).to.deep.equal(['1:no-fixed-wait']) }) - it('no-sleep ignores files without CodeceptJS code', async () => { - expect(await findings('no-sleep-app.js')).to.be.empty + it('no-sleep', () => { + expect(rules("Scenario('a', async ({ I }) => {\n await new Promise(r => setTimeout(r, 100))\n})")).to.deep.equal(['2:no-sleep']) + expect(rules('class X extends Helper {\n m() { setTimeout(() => {}, 1) }\n}')).to.deep.equal([]) }) - it('no-only flags focused features, scenarios and data scenarios', async () => { - const list = await findings('no-only.js') - expect(byRule(list, 'no-only')).to.deep.equal([1, 7, 11]) - }) - - it('no-only and no-pause are warnings locally and errors on CI', async () => { - let list = [...(await findings('no-only.js')), ...(await findings('no-pause.js'))] - expect(list.map(f => f.level)).to.deep.equal(['warn', 'warn', 'warn', 'warn']) + it('no-only and no-pause are warnings locally and errors on CI', () => { + const code = "Scenario.only('a', () => {})\nFeature.only('f')\nData([]).only.Scenario('d', () => {})\npause()" + expect(lint(code).map(f => `${f.rule}:${f.level}`)).to.deep.equal(['no-only:warn', 'no-only:warn', 'no-only:warn', 'no-pause:warn']) process.env.CI = 'true' - try { - list = [...(await findings('no-only.js')), ...(await findings('no-pause.js'))] - } finally { - delete process.env.CI - } - expect(list.map(f => f.level)).to.deep.equal(['error', 'error', 'error', 'error']) - }) - - it('no-pause flags pause() calls', async () => { - expect(byRule(await findings('no-pause.js'), 'no-pause')).to.deep.equal([5]) + expect(lint(code).map(f => f.level)).to.deep.equal(['error', 'error', 'error', 'error']) }) - it('secret-credentials flags credentials not wrapped in secret()', async () => { - const list = await findings('secret-credentials.js') - expect(byRule(list, 'secret-credentials')).to.deep.equal([5, 7, 9]) + it('secret-credentials', () => { + const code = [ + "I.fillField('Password', '123456')", + "I.fillField('Password', secret('123456'))", + "I.fillField('Email', 'a@b.c')", + 'I.fillField(loc, process.env.API_TOKEN)', + 'I.fillField(loc, secret(process.env.API_TOKEN))', + ].join('\n') + expect(rules(code)).to.deep.equal(['1:secret-credentials', '4:secret-credentials']) }) - it('await-grab flags grab results used without await', async () => { - const list = await findings('await-grab.js') - expect(byRule(list, 'await-grab')).to.deep.equal([4, 7, 14]) + it('await-grab', () => { + const code = ['const a = I.grabTextFrom("h1")', 'const b = await I.grabTextFrom("h1")', 'return I.grabTextFrom("h1")', 'I.grabTextFrom("h1").length', 'await Promise.all([I.grabTitle()])'].join('\n') + expect(new Linter().lint(`async function f() {\n${code}\n}`, 'test.js').map(f => `${f.line}:${f.rule}`)).to.deep.equal(['2:await-grab', '5:await-grab']) }) - it('no-actor-in-helper flags I inside a Helper class only', async () => { - const list = await findings('no-actor-in-helper.js') - expect(byRule(list, 'no-actor-in-helper')).to.deep.equal([5, 6]) + it('no-actor-in-helper', () => { + const code = 'class X extends Helper {\n m() {\n I.click("a")\n }\n}\nI.click("b")' + expect(rules(code)).to.deep.equal(['3:no-actor-in-helper']) }) - it('raw-browser-in-test warns on use*To and executeScript inside a Scenario', async () => { - const list = await findings('raw-browser-in-test.js') - expect(byRule(list, 'raw-browser-in-test')).to.deep.equal([4, 5]) - expect(list.every(f => f.level === 'warn')).to.be.true - }) - - it('clean test has no findings', async () => { - expect(await findings('clean.js')).to.be.empty + it('raw-browser-in-test', () => { + const code = "Scenario('a', ({ I }) => {\n I.usePlaywrightTo('x', () => {})\n I.executeScript(() => 1)\n})\nI.executeScript(() => 1)" + expect(lint(code).map(f => `${f.line}:${f.rule}:${f.level}`)).to.deep.equal(['2:raw-browser-in-test:warn', '3:raw-browser-in-test:warn']) }) }) - describe('engine', () => { - it('suppresses findings with disable-line and disable-next-line comments that name the rule', async () => { - expect(byRule(await findings('suppressed.js'), 'no-fixed-wait')).to.deep.equal([7, 8]) - }) - - it('applies rule levels from config', async () => { - const list = await findings('raw-browser-in-test.js', { rules: { 'raw-browser-in-test': 'off' } }) - expect(list).to.be.empty - const waits = await findings('no-fixed-wait.js', { rules: { 'no-fixed-wait': 'warn' } }) - expect(waits[0].level).to.equal('warn') - }) - - it('keeps TypeScript line numbers after stripping types', async () => { - const list = await findings('typescript.ts') - expect(list.map(f => [f.rule, f.line, f.column])).to.deep.equal([ - ['await-grab', 13, 25], - ['no-fixed-wait', 14, 3], - ]) - }) - - it('maps TypeScript syntax that needs transpiling back to source lines', async () => { - const list = await findings('typescript-enum.ts') - expect(list.map(f => [f.rule, f.line])).to.deep.equal([['no-fixed-wait', 10]]) - }) + it('suppresses a rule with a comment', () => { + const code = 'I.wait(1) // codeceptjs-lint-disable-line no-fixed-wait\n// codeceptjs-lint-disable-next-line no-fixed-wait\nI.wait(2)\nI.wait(3)' + expect(rules(code)).to.deep.equal(['4:no-fixed-wait']) + }) - it('parses CommonJS files as scripts', async () => { - const code = 'const { I } = inject()\n\nmodule.exports = {\n open() {\n I.wait(2)\n },\n}\n\nreturn\n' - expect(byRule((await lintSource(code, 'page.js')).findings, 'no-fixed-wait')).to.deep.equal([5]) - }) + it('turns rules off from config', () => { + const linter = new Linter({ lint: { rules: { 'no-fixed-wait': 'off', 'no-pause': 'error' } } }) + expect(linter.lint('I.wait(1)\npause()', 'test.js').map(f => `${f.rule}:${f.level}`)).to.deep.equal(['no-pause:error']) + }) - it('throws on syntax errors', async () => { - let error - try { - await lintSource("Scenario('broken', ({ I }) => {\n I.see(\n", 'broken.js') - } catch (err) { - error = err - } - expect(error).to.be.instanceOf(SyntaxError) - }) + it('lints TypeScript with correct lines', () => { + expect(lint('const n: number = 5\n\nI.wait(n as number)\nI.wait(5)', 'test.ts').map(f => f.line)).to.deep.equal([4]) + }) - it('collects tests, local includes and helpers from config, minus ignored files', () => { - const root = fixture('project') - const config = { - tests: './*_test.js', - include: { I: './steps_file.js', loginPage: './pages/login.js', externalModule: 'some-package' }, - helpers: { Custom: { require: './custom_helper.js' }, Playwright: {} }, - lint: { ignore: ['legacy_test.js'] }, - } - const files = collectFiles(config, root) - .map(f => path.relative(root, f)) - .sort() - expect(files).to.deep.equal(['checkout_test.js', 'custom_helper.js', 'existing_test.js', path.join('pages', 'login.js'), 'steps_file.js']) - }) + it('parses CommonJS files', () => { + expect(rules("const x = require('x')\nreturn I.wait(1)")).to.deep.equal(['2:no-fixed-wait']) + }) - it('treats repeated identical errors as new', async () => { - const before = (await lintSource("Scenario('a', ({ I }) => {\n I.wait(5)\n})\n")).findings - const after = (await lintSource("Scenario('a', ({ I }) => {\n I.wait(5)\n I.say('x')\n I.wait(5)\n})\n")).findings - const added = newErrors(before, after) - expect(added).to.have.length(1) - expect(added[0].line).to.equal(4) - }) + it('collects tests, include and helper files from config', () => { + const linter = new Linter({ tests: './*_test.js', include: { I: './steps_file.js', page: './pages/login.js', other: 'some-package' }, helpers: { Custom: { require: './custom_helper.js' } } }, project) + const files = linter + .files() + .map(f => path.relative(project, f)) + .sort() + expect(files).to.deep.equal(['checkout_test.js', 'custom_helper.js', 'existing_test.js', 'pages/login.js', 'steps_file.js']) + expect(linter.includes(path.join(project, 'new_test.js'))).to.be.true + expect(linter.includes(path.join(project, 'app.js'))).to.be.false }) describe('CLI', () => { - const run = (args, opts = {}) => spawnSync(process.execPath, [bin, 'lint', ...args], { encoding: 'utf8', env: { ...process.env, CI: '' }, ...opts }) - - it('lints files from config and exits 1 on errors', () => { - const result = run(['-c', fixture('project/codecept.conf.js')]) - expect(result.status).to.equal(1) - expect(result.stdout).to.include('checkout_test.js:4:3') - expect(result.stdout).to.include('no-fixed-wait') - expect(result.stdout).to.include('custom_helper.js:5:13') - expect(result.stdout).to.include('login.js:5:5') - expect(result.stdout).not.to.include('legacy_test.js') + it('prints findings and exits 1 on errors', () => { + const result = spawnSync(process.execPath, [bin, 'lint'], { cwd: project, encoding: 'utf8', env: { ...process.env, CI: '' } }) + expect(result.stdout).to.include('checkout_test.js:4:3 error no-fixed-wait') + expect(result.stdout).to.include('custom_helper.js:5:13 error no-actor-in-helper') expect(result.stdout).not.to.include('raw-browser-in-test') - }) - - it('exits 0 when only warnings are found', () => { - const result = run([fixture('no-pause.js')]) - expect(result.status).to.equal(0) - expect(result.stdout).to.include('no-pause') - }) - - it('prints JSON', () => { - const result = run(['--json', fixture('no-fixed-wait.js')]) expect(result.status).to.equal(1) - const json = JSON.parse(result.stdout) - expect(json.errors).to.equal(1) - expect(json.findings[0]).to.include({ rule: 'no-fixed-wait', line: 5, column: 3, level: 'error' }) - }) - - it('exits 2 on parse failure', () => { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codecept-lint-')) - const file = path.join(dir, 'broken_test.js') - fs.writeFileSync(file, "Scenario('broken', ({ I }) => {\n I.see(\n") - try { - const result = run([file]) - expect(result.status).to.equal(2) - expect(result.stdout).to.include('parse') - } finally { - fs.rmSync(dir, { recursive: true, force: true }) - } }) }) describe('hook', () => { - let dir - const existing = () => path.join(dir, 'existing_test.js') - - beforeEach(() => { - dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codecept-lint-')) - fs.copyFileSync(fixture('project/existing_test.js'), existing()) - }) - - afterEach(() => { - fs.rmSync(dir, { recursive: true, force: true }) - }) - - const hook = payload => runHook({ cwd: dir, ...payload }, { agent: 'claude' }) - - it('blocks a Write that introduces an error', async () => { - const result = await hook({ tool_name: 'Write', tool_input: { file_path: path.join(dir, 'new_test.js'), content: fs.readFileSync(fixture('no-fixed-wait.js'), 'utf8') } }) - expect(result.code).to.equal(2) - expect(result.stderr).to.include('new_test.js:5:3') - expect(result.stderr).to.include('no-fixed-wait') - }) - - it('allows a Write that only produces warnings', async () => { - const result = await hook({ tool_name: 'Write', tool_input: { file_path: path.join(dir, 'stub_test.js'), content: fs.readFileSync(fixture('no-pause.js'), 'utf8') } }) - expect(result).to.deep.equal({ code: 0, stderr: '' }) - }) - - it('blocks an Edit that adds an error to a clean part of the file', async () => { - const result = await hook({ tool_name: 'Edit', tool_input: { file_path: existing(), old_string: "I.see('Welcome')", new_string: "I.see('Welcome')\n I.wait(3)" } }) - expect(result.code).to.equal(2) - expect(result.stderr).to.include('I.wait(3)') - expect(result.stderr).not.to.include('I.wait(5)') - }) - - it('allows an Edit on a file with pre-existing violations', async () => { - const result = await hook({ tool_name: 'Edit', tool_input: { file_path: existing(), old_string: "I.see('Welcome')", new_string: "I.see('Welcome')\n I.waitForElement('#ok')" } }) - expect(result.code).to.equal(0) - }) - - it('blocks a second identical violation', async () => { - const result = await hook({ tool_name: 'Edit', tool_input: { file_path: existing(), old_string: "I.see('Welcome')", new_string: "I.see('Welcome')\n I.wait(5)" } }) - expect(result.code).to.equal(2) - expect(result.stderr).to.include('existing_test.js:7:3') - }) - - it('applies MultiEdit edits in order', async () => { - const result = await hook({ - tool_name: 'MultiEdit', - tool_input: { - file_path: existing(), - edits: [ - { old_string: 'I.wait(5)', new_string: "I.waitForText('Welcome')" }, - { old_string: "I.see('Welcome')", new_string: "I.see('Welcome')\n I.wait(1)" }, - ], - }, - }) - expect(result.code).to.equal(2) - expect(result.stderr).to.include('I.wait(1)') - }) - - it('ignores other tools, other files and files outside the project', async () => { - expect((await hook({ tool_name: 'Read', tool_input: { file_path: existing() } })).code).to.equal(0) - expect((await hook({ tool_name: 'Write', tool_input: { file_path: path.join(dir, 'notes.md'), content: 'I.wait(5)' } })).code).to.equal(0) - const outside = path.join(os.tmpdir(), 'outside_test.js') - expect((await hook({ tool_name: 'Write', tool_input: { file_path: outside, content: "Scenario('a', ({ I }) => { I.wait(5) })" } })).code).to.equal(0) - }) - - it('respects lint config from the project', async () => { - fs.writeFileSync(path.join(dir, 'codecept.conf.js'), "exports.config = { tests: './*_test.js', lint: { ignore: ['legacy/**'], rules: { 'await-grab': 'warn' } } }\n") - const legacy = await hook({ tool_name: 'Write', tool_input: { file_path: path.join(dir, 'legacy', 'old_test.js'), content: "Scenario('a', ({ I }) => {\n I.wait(5)\n})\n" } }) - expect(legacy.code).to.equal(0) - const grab = await hook({ tool_name: 'Write', tool_input: { file_path: path.join(dir, 'grab_test.js'), content: "Scenario('a', ({ I }) => {\n const t = I.grabTitle()\n})\n" } }) - expect(grab.code).to.equal(0) + it('blocks a write that adds an error', () => { + const result = runHook({ tool_name: 'Write', tool_input: { file_path: path.join(project, 'new_test.js'), content: "Scenario('a', ({ I }) => {\n I.wait(3)\n})\n" } }) + expect(result.status).to.equal(2) + expect(result.stderr).to.include('new_test.js:2:3 error no-fixed-wait') }) - it('allows edits that leave the file unparseable', async () => { - const result = await hook({ tool_name: 'Write', tool_input: { file_path: path.join(dir, 'broken_test.js'), content: 'Scenario((' } }) - expect(result.code).to.equal(0) - expect(result.stderr).to.include('could not be parsed') + it('allows an edit that keeps existing errors', () => { + const result = runHook({ tool_name: 'Edit', tool_input: { file_path: path.join(project, 'existing_test.js'), old_string: "I.see('Welcome')", new_string: "I.see('Hello')" } }) + expect(result.status).to.equal(0) }) - it('reads the payload from stdin and exits 2 with findings on stderr', () => { - const payload = { cwd: dir, tool_name: 'Edit', tool_input: { file_path: existing(), old_string: "I.see('Welcome')", new_string: "I.see('Welcome')\n I.wait(5)" } } - const result = spawnSync(process.execPath, [bin, 'lint', '--hook', 'claude'], { cwd: dir, input: JSON.stringify(payload), encoding: 'utf8', env: { ...process.env, CLAUDE_PROJECT_DIR: dir } }) + it('blocks an edit that adds a second identical error', () => { + const result = runHook({ tool_name: 'Edit', tool_input: { file_path: path.join(project, 'existing_test.js'), old_string: "I.see('Welcome')", new_string: "I.wait(5)\n I.see('Welcome')" } }) expect(result.status).to.equal(2) - expect(result.stdout).to.equal('') - expect(result.stderr).to.include('no-fixed-wait') }) - it('exits 0 on invalid stdin', () => { - const result = spawnSync(process.execPath, [bin, 'lint', '--hook', 'claude'], { cwd: dir, input: 'not json', encoding: 'utf8', env: { ...process.env, CLAUDE_PROJECT_DIR: dir } }) - expect(result.status).to.equal(0) - expect(result.stdout).to.equal('') + it('allows warnings, files outside the project config and bad payloads', () => { + expect(runHook({ tool_name: 'Write', tool_input: { file_path: path.join(project, 'new_test.js'), content: 'pause()' } }).status).to.equal(0) + expect(runHook({ tool_name: 'Write', tool_input: { file_path: path.join(project, 'app.js'), content: 'I.wait(1)' } }).status).to.equal(0) + expect(runHook({ nonsense: true }).status).to.equal(0) }) }) + + it('does not touch fixture files', () => { + expect(fs.existsSync(path.join(project, 'new_test.js'))).to.be.false + }) })