From 7999f475ff69c17ffb2b93c880b376ef6785c6fa Mon Sep 17 00:00:00 2001 From: Denys Kuchma Date: Thu, 15 Oct 2026 15:12:30 +0200 Subject: [PATCH] multi-line colored output with explanation and suggestion --- README.md | 6 ++++-- bin/lint.js | 22 ++++++++++++++++++++-- lib/Linter.js | 8 +++++--- lib/rules/await-grab.js | 2 +- lib/rules/base.js | 4 ++++ lib/rules/no-actor-in-helper.js | 5 +++-- lib/rules/no-console.js | 5 +++-- lib/rules/no-fetch.js | 5 +++-- lib/rules/no-fixed-wait.js | 5 +++-- lib/rules/no-if-in-test.js | 5 +++-- lib/rules/no-only.js | 2 +- lib/rules/no-pause.js | 5 +++-- lib/rules/no-skip.js | 4 ++-- lib/rules/no-sleep.js | 5 +++-- lib/rules/raw-browser-in-test.js | 2 +- lib/rules/secret-credentials.js | 4 ++-- test/lint_test.js | 19 ++++++++++++++----- 17 files changed, 75 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index be718bc..87e8b32 100644 --- a/README.md +++ b/README.md @@ -14,10 +14,12 @@ npx @codeceptjs/lint -c e2e/codecept.conf.js Without paths, the linter reads `codecept.conf.js` (or `.ts`, `.cjs`, `.mjs`) from the current directory. It lints files matched by `tests`, local files from `include` (page objects, steps file) and helpers loaded with `require`. -Each finding is printed on one line: +Each finding is printed with the type, location and rule on one line, then the explanation and the suggestion indented below (colored when stdout is a TTY; set `NO_COLOR` to disable): ``` -checkout_test.js:4:3 error no-fixed-wait I.wait() sleeps unconditionally. Wait for a condition: I.waitForElement / I.waitForText / I.see +error checkout_test.js:4 no-fixed-wait + I.wait() sleeps unconditionally + wait for a condition: I.waitForElement / I.waitForText / I.see ``` Exit codes: `0` no errors (warnings allowed), `1` errors found, `2` bad input or a file that can't be parsed. diff --git a/bin/lint.js b/bin/lint.js index 0d3600c..0d5d0b7 100755 --- a/bin/lint.js +++ b/bin/lint.js @@ -5,6 +5,16 @@ import { parseArgs } from 'util' import Linter from '../lib/Linter.js' import rules from '../lib/rules/index.js' +const paint = (process.stdout.isTTY || process.env.FORCE_COLOR) && !process.env.NO_COLOR +const style = { + error: text => (paint ? `\x1b[1;31m${text}\x1b[0m` : text), + warn: text => (paint ? `\x1b[33m${text}\x1b[0m` : text), + file: text => (paint ? `\x1b[36m${text}\x1b[0m` : text), + suggestion: text => (paint ? `\x1b[32m${text}\x1b[0m` : text), + dim: text => (paint ? `\x1b[2m${text}\x1b[0m` : text), + ok: text => (paint ? `\x1b[32m${text}\x1b[0m` : text), +} + const { values: options, positionals: paths } = parseArgs({ allowPositionals: true, options: { @@ -78,7 +88,12 @@ if (options.json) { console.log(JSON.stringify(findings, null, 2)) } else { for (const finding of findings) console.log(format(finding)) - console.log(`${files.length} file(s) checked, ${errors} error(s), ${findings.length - errors} warning(s)`) + const warnings = findings.length - errors + if (!findings.length) { + console.log(style.ok(`✔ ${files.length} file(s) checked, no problems found`)) + } else { + console.log(`${files.length} file(s) checked, ${style.error(`${errors} error(s)`)}, ${style.warn(`${warnings} warning(s)`)}`) + } } if (errors && !process.exitCode) process.exitCode = 1 @@ -124,5 +139,8 @@ async function hook(linter) { } function format(finding) { - return `${path.relative(process.cwd(), finding.file).replaceAll('\\', '/')}:${finding.line}:${finding.column} ${finding.level} ${finding.rule} ${finding.message}` + const type = style[finding.level](finding.level.padEnd(5)) + const location = style.file(`${path.relative(process.cwd(), finding.file).replaceAll('\\', '/')}:${finding.line}`) + const rule = style.dim(finding.rule) + return [`${type} ${location} ${rule}`, ` ${finding.explanation}`, ` ${style.suggestion(finding.suggestion)}`].join('\n') } diff --git a/lib/Linter.js b/lib/Linter.js index 69a0418..23f565d 100644 --- a/lib/Linter.js +++ b/lib/Linter.js @@ -110,8 +110,8 @@ export default class Linter { for (const rule of this.rules) { const level = this.levels[rule.id] || rule.level if (level === 'off') continue - const message = rule.check(node, ancestors) - if (!message) continue + const issue = rule.check(node, ancestors) + if (!issue) continue const { line, column } = node.loc.start if (disabled.includes(`${line}:${rule.id}`)) continue findings.push({ @@ -120,7 +120,9 @@ export default class Linter { column: column + 1, rule: rule.id, level, - message, + explanation: issue.explanation, + suggestion: issue.suggestion, + message: `${issue.explanation}. ${issue.suggestion[0].toUpperCase()}${issue.suggestion.slice(1)}`, source: truncate(code.slice(node.start, node.end)), }) } diff --git a/lib/rules/await-grab.js b/lib/rules/await-grab.js index 40dfb75..8f1d71f 100644 --- a/lib/rules/await-grab.js +++ b/lib/rules/await-grab.js @@ -13,6 +13,6 @@ export class AwaitGrab extends Rule { const parent = ancestors[ancestors.length - 2] if (parent.type === 'MemberExpression' && parent.object === node && ['then', 'catch', 'finally'].includes(parent.property?.name)) return if (!FLAGGED_PARENTS.includes(parent.type)) return - return `I.${method}() returns a promise. Use: await I.${method}()` + return { explanation: `I.${method}() returns a promise`, suggestion: `use: await I.${method}()` } } } diff --git a/lib/rules/base.js b/lib/rules/base.js index 8519dc7..836837d 100644 --- a/lib/rules/base.js +++ b/lib/rules/base.js @@ -2,6 +2,10 @@ export class Rule { get level() { return 'error' } + + issue() { + return { explanation: this.explanation, suggestion: this.suggestion } + } } export class CommitRule extends Rule { diff --git a/lib/rules/no-actor-in-helper.js b/lib/rules/no-actor-in-helper.js index 69b132b..4c48d4b 100644 --- a/lib/rules/no-actor-in-helper.js +++ b/lib/rules/no-actor-in-helper.js @@ -4,11 +4,12 @@ import { insideHelper } from './utils.js' export class NoActorInHelper extends Rule { id = 'no-actor-in-helper' description = 'I is not available inside a helper. Call other helpers via this.helpers[...]' - message = 'I is not available inside a helper. Call other helpers via this.helpers[...]' + explanation = 'I is not available inside a helper' + suggestion = 'call other helpers via this.helpers[...]' check(node, ancestors) { if (node.type !== 'Identifier' || node.name !== 'I') return if (!insideHelper(ancestors)) return - return this.message + return this.issue() } } diff --git a/lib/rules/no-console.js b/lib/rules/no-console.js index 8d15e4d..acd40d6 100644 --- a/lib/rules/no-console.js +++ b/lib/rules/no-console.js @@ -4,7 +4,8 @@ import { insideHelper } from './utils.js' export class NoConsole extends Rule { id = 'no-console' description = 'console.log in tests or page objects is not part of the test report. Use I.say()' - message = 'console output is not part of the test report. Use I.say()' + explanation = 'console output is not part of the test report' + suggestion = 'use I.say()' get level() { return 'warn' @@ -13,6 +14,6 @@ export class NoConsole extends Rule { check(node, ancestors) { if (node.type !== 'CallExpression' || node.callee.object?.name !== 'console') return if (insideHelper(ancestors)) return - return this.message + return this.issue() } } diff --git a/lib/rules/no-fetch.js b/lib/rules/no-fetch.js index f1c172c..4b63d61 100644 --- a/lib/rules/no-fetch.js +++ b/lib/rules/no-fetch.js @@ -4,11 +4,12 @@ import { insideHelper, isCalleeName } from './utils.js' export class NoFetch extends Rule { id = 'no-fetch' description = 'fetch() in tests or page objects, including window.fetch and globalThis.fetch, bypasses CodeceptJS steps and logs. Use the REST helper: I.sendGetRequest, I.sendPostRequest' - message = 'fetch() bypasses CodeceptJS steps and logs. Use the REST helper: I.sendGetRequest / I.sendPostRequest' + explanation = 'fetch() bypasses CodeceptJS steps and logs' + suggestion = 'use the REST helper: I.sendGetRequest / I.sendPostRequest' check(node, ancestors) { if (node.type !== 'CallExpression' || !isCalleeName(node.callee, 'fetch')) return if (insideHelper(ancestors)) return - return this.message + return this.issue() } } diff --git a/lib/rules/no-fixed-wait.js b/lib/rules/no-fixed-wait.js index 30f7625..07ab29b 100644 --- a/lib/rules/no-fixed-wait.js +++ b/lib/rules/no-fixed-wait.js @@ -4,11 +4,12 @@ import { actorMethod } from './utils.js' export class NoFixedWait extends Rule { id = 'no-fixed-wait' description = 'I.wait(5) sleeps for a fixed time. Wait for a condition with I.waitForElement, I.waitForText or I.see' - message = 'I.wait() sleeps unconditionally. Wait for a condition: I.waitForElement / I.waitForText / I.see' + explanation = 'I.wait() sleeps unconditionally' + suggestion = 'wait for a condition: I.waitForElement / I.waitForText / I.see' check(node) { if (actorMethod(node) !== 'wait') return if (typeof node.arguments[0]?.value !== 'number') return - return this.message + return this.issue() } } diff --git a/lib/rules/no-if-in-test.js b/lib/rules/no-if-in-test.js index 2449179..d2fb4f8 100644 --- a/lib/rules/no-if-in-test.js +++ b/lib/rules/no-if-in-test.js @@ -4,11 +4,12 @@ import { insideTest } from './utils.js' export class NoIfInTest extends Rule { id = 'no-if-in-test' description = 'if inside a Scenario or hook makes the test non-deterministic. Use tryTo() from codeceptjs/effects for optional steps' - message = 'if makes the test non-deterministic. Use tryTo() from codeceptjs/effects for optional steps' + explanation = 'if makes the test non-deterministic' + suggestion = 'use tryTo() from codeceptjs/effects for optional steps' check(node, ancestors) { if (node.type !== 'IfStatement') return if (!insideTest(ancestors)) return - return this.message + return this.issue() } } diff --git a/lib/rules/no-only.js b/lib/rules/no-only.js index 687e244..e01d7e7 100644 --- a/lib/rules/no-only.js +++ b/lib/rules/no-only.js @@ -8,6 +8,6 @@ export class NoOnly extends CommitRule { 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` + return { explanation: `${target}.only limits the run to focused tests`, suggestion: 'remove it before commit' } } } diff --git a/lib/rules/no-pause.js b/lib/rules/no-pause.js index 3db26c4..764c013 100644 --- a/lib/rules/no-pause.js +++ b/lib/rules/no-pause.js @@ -3,7 +3,8 @@ import { CommitRule } from './base.js' export class NoPause extends CommitRule { id = 'no-pause' description = 'pause() and this.pause() stop the test for interactive debugging. Error on CI, warning locally' - message = 'pause() stops the test for debugging. Remove it before commit' + explanation = 'pause() stops the test for interactive debugging' + suggestion = 'remove it before commit' check(node) { if (node.type !== 'CallExpression') return @@ -11,6 +12,6 @@ export class NoPause extends CommitRule { const bare = callee.type === 'Identifier' && callee.name === 'pause' const thisPause = callee.type === 'MemberExpression' && callee.object.type === 'ThisExpression' && callee.property?.name === 'pause' if (!bare && !thisPause) return - return this.message + return this.issue() } } diff --git a/lib/rules/no-skip.js b/lib/rules/no-skip.js index b9e9444..eca997c 100644 --- a/lib/rules/no-skip.js +++ b/lib/rules/no-skip.js @@ -11,9 +11,9 @@ export class NoSkip extends Rule { check(node) { if (node.type !== 'CallExpression') return const callee = node.callee - if (callee.name === 'xScenario' || callee.name === 'xFeature') return `${callee.name} skips the test` + if (callee.name === 'xScenario' || callee.name === 'xFeature') return { explanation: `${callee.name} skips the test`, suggestion: 'remove it, or suppress with a comment if intentional' } if (callee.property?.name !== 'skip') return if (!['Scenario', 'Feature'].includes(callee.object.name)) return - return `${callee.object.name}.skip skips the test` + return { explanation: `${callee.object.name}.skip skips the test`, suggestion: 'remove it, or suppress with a comment if intentional' } } } diff --git a/lib/rules/no-sleep.js b/lib/rules/no-sleep.js index d7619d3..b79ebc4 100644 --- a/lib/rules/no-sleep.js +++ b/lib/rules/no-sleep.js @@ -4,11 +4,12 @@ import { insideHelper, isCalleeName } from './utils.js' export class NoSleep extends Rule { id = 'no-sleep' description = 'setTimeout, including window.setTimeout, await new Promise(r => setTimeout(r, ms)) and timers/promises, sleeps for a fixed time. Wait for a condition instead' - message = 'setTimeout pauses for a fixed time. Wait for a condition: I.waitForElement / I.waitForText / I.waitForFunction' + explanation = 'setTimeout pauses for a fixed time' + suggestion = 'wait for a condition: I.waitForElement / I.waitForText / I.waitForFunction' check(node, ancestors) { if (node.type !== 'CallExpression' || !isCalleeName(node.callee, 'setTimeout')) return if (insideHelper(ancestors)) return - return this.message + return this.issue() } } diff --git a/lib/rules/raw-browser-in-test.js b/lib/rules/raw-browser-in-test.js index 0c66e86..450d977 100644 --- a/lib/rules/raw-browser-in-test.js +++ b/lib/rules/raw-browser-in-test.js @@ -14,6 +14,6 @@ export class RawBrowserInTest extends Rule { if (!method) return if (method !== 'executeScript' && !/^use\w+To$/.test(method)) return if (!insideTest(ancestors)) return - return `I.${method} runs raw browser code in a test. Move it into a helper or page object` + return { explanation: `I.${method} runs raw browser code in a test`, suggestion: 'move it into a helper or page object' } } } diff --git a/lib/rules/secret-credentials.js b/lib/rules/secret-credentials.js index 0660f3e..f00c6d0 100644 --- a/lib/rules/secret-credentials.js +++ b/lib/rules/secret-credentials.js @@ -10,12 +10,12 @@ export class SecretCredentials extends Rule { if (!method) return for (const arg of node.arguments) { const env = envPropertyName(arg) - if (env && CREDENTIALS.test(env)) return `process.env.${env} is printed in logs. Wrap it: secret(process.env.${env})` + if (env && CREDENTIALS.test(env)) return { explanation: `process.env.${env} is printed in logs`, suggestion: `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(...)` + return { explanation: `I.fillField('${locator.value}', ...) types a credential that is printed in logs`, suggestion: 'wrap the value: secret(...)' } } } diff --git a/test/lint_test.js b/test/lint_test.js index ebf4f30..e345aad 100644 --- a/test/lint_test.js +++ b/test/lint_test.js @@ -196,9 +196,9 @@ describe('@codeceptjs/lint', () => { describe('CLI', () => { it('lints files from codecept config', () => { const result = run([]) - 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).to.include('pages/login.js:5:11 error no-fetch') + expect(result.stdout).to.include('error checkout_test.js:4 no-fixed-wait') + expect(result.stdout).to.include('custom_helper.js:5 no-actor-in-helper') + expect(result.stdout).to.include('pages/login.js:5 no-fetch') expect(result.stdout).not.to.include('raw-browser-in-test') expect(result.stdout).not.to.include('no-sleep') expect(result.status).to.equal(1) @@ -221,7 +221,7 @@ describe('@codeceptjs/lint', () => { const result = run(['--json']) const findings = JSON.parse(result.stdout) expect(findings).to.be.an('array').with.length.greaterThan(0) - for (const key of ['file', 'line', 'column', 'rule', 'level', 'message', 'source']) { + for (const key of ['file', 'line', 'column', 'rule', 'level', 'explanation', 'suggestion', 'message', 'source']) { expect(findings[0]).to.have.property(key) } for (const finding of findings) { @@ -230,6 +230,13 @@ describe('@codeceptjs/lint', () => { expect(result.status).to.equal(1) }) + it('prints one finding as type, location and rule followed by explanation and suggestion', () => { + const result = run(['-r', 'no-fixed-wait', 'checkout_test.js']) + expect(result.stdout).to.include('error checkout_test.js:4 no-fixed-wait\n I.wait() sleeps unconditionally\n wait for a condition: I.waitForElement / I.waitForText / I.see') + expect(result.stdout).to.include('1 file(s) checked, 1 error(s), 0 warning(s)') + expect(result.stdout).to.not.include('\x1b[') + }) + it('lists rules', () => { const result = run(['rules']) expect(result.stdout).to.include('no-if-in-test') @@ -253,7 +260,9 @@ describe('@codeceptjs/lint', () => { }, }) expect(result.status).to.equal(2) - expect(result.stderr).to.include('new_test.js:2:3 error no-fixed-wait') + expect(result.stderr).to.include('error new_test.js:2 no-fixed-wait') + expect(result.stderr).to.include('I.wait() sleeps unconditionally') + expect(result.stderr).to.include('wait for a condition: I.waitForElement') }) it('allows an edit that keeps existing errors', () => {