multi-line colored output with explanation and suggestion

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.

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')
 }

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)),
         })
       }

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}()` }
   }
 }

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 {

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()
   }
 }

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()
   }
 }

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()
   }
 }

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()
   }
 }

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()
   }
 }

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' }
   }
 }

lib/rules/no-pause.js

@@ -3,14 +3,15 @@ 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
     const callee = node.callee
     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()
   }
 }

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' }
   }
 }

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()
   }
 }

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' }
   }
 }

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(...)' }
   }
 }

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', () => {