Skip to content

Commit

Permalink
Process and log results from ESLint 🦎 (#12)
Browse files Browse the repository at this point in the history
* Do not truncate the error message 🐘

* Improve error logging πŸ‘Ί

* Removed cwd from the file when logging πŸ—ΊοΈ

* Ignore *.min.* files from being linted πŸ‘¨β€πŸŽ€

* Process and log results from eslint πŸ—

* Code improvements πŸ™Œ

* Improved testing process.exit calls πŸšͺ

* Added tests for eslint β˜€οΈ

* Fixed unit tests πŸ«–

* Test improvements 🫑

* Rollback Node 22 version in test.yml πŸ’”

GitHub actions cannot run executables on Node 22.5.0.
  • Loading branch information
01taylop authored Jul 19, 2024
1 parent 589a2a6 commit 4749797
Show file tree
Hide file tree
Showing 16 changed files with 354 additions and 86 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:

strategy:
matrix:
node-version: [18.x, 20.x, 22.x]
node-version: [18.x, 20.x, 22.4]

steps:
- name: Checkout Repository
Expand Down
6 changes: 6 additions & 0 deletions eslint.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export default [{
rules: {
quotes: [2, 'single'],
semi: [2, 'never'],
},
}]
4 changes: 4 additions & 0 deletions jest-config/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ jest.mock('log-symbols', () => ({
error: 'X',
}))

jest.spyOn(process, 'exit').mockImplementation(code => {
throw new Error(`process.exit(${code})`)
})

beforeEach(() => {
global.debug = false
})
Expand Down
2 changes: 1 addition & 1 deletion jest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const config: JestConfigWithTsJest = {
'src/**/*.ts',
// TODO: Write tests for these files when they are less likely to change
'!src/index.ts',
'!src/linters/(eslint|index|stylelint).ts',
'!src/linters/(index|stylelint).ts',
],
coverageDirectory: 'coverage',
coverageThreshold: {
Expand Down
13 changes: 8 additions & 5 deletions src/__tests__/sourceFiles.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ describe('sourceFiles', () => {

jest.spyOn(colourLog, 'configDebug').mockImplementation(() => {})
jest.spyOn(colourLog, 'error').mockImplementation(() => {})
jest.spyOn(process, 'exit').mockImplementation(() => null as never)

const commonArgs = {
filePattern: '*.ts',
Expand Down Expand Up @@ -54,14 +53,18 @@ describe('sourceFiles', () => {
})

it('catches any errors and exists the process', async () => {
expect.assertions(2)

const error = new Error('Test error')

jest.mocked(glob).mockRejectedValue(error)

await sourceFiles(commonArgs)

expect(colourLog.error).toHaveBeenCalledWith('An error occurred while trying to source files matching *.ts', error)
expect(process.exit).toHaveBeenCalledWith(1)
try {
await sourceFiles(commonArgs)
} catch {
expect(colourLog.error).toHaveBeenCalledWith('An error occurred while trying to source files matching *.ts', error)
expect(process.exit).toHaveBeenCalledWith(1)
}
})

})
14 changes: 12 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ const runLinter = async ({ filePattern, linter }: RunLinter) => {

const files = await sourceFiles({
filePattern,
ignore: '**/+(coverage|node_modules)/**',
ignore: [
'**/*.min.*',
'**/+(coverage|node_modules)/**',
],
linter,
})

Expand Down Expand Up @@ -93,7 +96,10 @@ program
'**/*.{md,mdx}',
'**/*.{css,scss,less,sass,styl,stylus}',
],
ignorePatterns: ['**/+(coverage|node_modules)/**'],
ignorePatterns: [
'**/*.min.*',
'**/+(coverage|node_modules)/**',
],
})

fileChangeEvent.on(Events.FILE_CHANGED, ({ message }) => {
Expand All @@ -105,4 +111,8 @@ program
}
})

process.on('exit', () => {
console.log()
})

program.parse(process.argv)
217 changes: 217 additions & 0 deletions src/linters/__tests__/eslint.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
import { ESLint } from 'eslint'

import colourLog from '@Utils/colourLog'

import eslintLib from '../eslint'

jest.mock('eslint')
jest.mock('@Utils/colourLog')

describe('eslint', () => {

jest.spyOn(colourLog, 'error').mockImplementation(() => {})

const testFiles = ['index.ts']
const lintFilesMock = ESLint.prototype.lintFiles as jest.Mock

it('creates a new ESLint instance', async () => {
lintFilesMock.mockImplementationOnce(() => [])

await eslintLib.lintFiles(testFiles)

expect(ESLint).toHaveBeenCalledOnceWith({
cache: false,
fix: false,
})
})

it('calls ESLint.lintFiles with the files', async () => {
lintFilesMock.mockImplementationOnce(() => [])

await eslintLib.lintFiles(testFiles)

expect(ESLint.prototype.lintFiles).toHaveBeenCalledOnceWith(testFiles)
})

it('exists the process when eslint throws an error', async () => {
expect.assertions(2)

const error = new Error('Test error')

lintFilesMock.mockImplementationOnce(() => {
throw error
})

try {
await eslintLib.lintFiles(testFiles)
} catch {
expect(colourLog.error).toHaveBeenCalledOnceWith('An error occurred while running eslint', error)
expect(process.exit).toHaveBeenCalledWith(1)
}
})

it('resolves with results and a summary when eslint successfully lints (no files)', async () => {
const lintResults: Array<ESLint.LintResult> = []

lintFilesMock.mockImplementationOnce(() => lintResults)

expect(await eslintLib.lintFiles(testFiles)).toStrictEqual({
results: {},
summary: {
deprecatedRules: [],
errorCount: 0,
fileCount: 0,
fixableErrorCount: 0,
fixableWarningCount: 0,
linter: 'ESLint',
warningCount: 0,
},
})
})

it('resolves with results and a summary when eslint successfully lints (no errors)', async () => {
const lintResults: Array<ESLint.LintResult> = [{
errorCount: 0,
fatalErrorCount: 0,
filePath: `${process.cwd()}/index.ts`,
fixableErrorCount: 0,
fixableWarningCount: 0,
messages: [],
suppressedMessages: [],
usedDeprecatedRules: [],
warningCount: 0,
}]

lintFilesMock.mockImplementationOnce(() => lintResults)

expect(await eslintLib.lintFiles(testFiles)).toStrictEqual({
results: {},
summary: {
deprecatedRules: [],
errorCount: 0,
fileCount: 1,
fixableErrorCount: 0,
fixableWarningCount: 0,
linter: 'ESLint',
warningCount: 0,
},
})
})

it('resolves with results and a summary when eslint successfully lints (with errors, warnings, and deprecations)', async () => {
const eslintResult: Array<ESLint.LintResult> = [{
errorCount: 0,
fatalErrorCount: 0,
filePath: `${process.cwd()}/constants.ts`,
fixableErrorCount: 0,
fixableWarningCount: 0,
messages: [],
suppressedMessages: [],
usedDeprecatedRules: [],
warningCount: 0,
}, {
errorCount: 4,
fatalErrorCount: 0,
filePath: `${process.cwd()}/index.ts`,
fixableErrorCount: 3,
fixableWarningCount: 1,
messages: [{
column: 1,
line: 1,
message: 'Normal rule error',
ruleId: 'normal-error',
severity: 2,
}, {
column: 1,
line: 2,
message: 'Normal rule warning',
ruleId: 'normal-warning',
severity: 1,
}, {
column: 1,
line: 3,
message: 'Error with trailing whitespace ',
ruleId: 'spaced-error',
severity: 2,
}],
suppressedMessages: [],
usedDeprecatedRules: [{
ruleId: 'deprecated-rule-1',
replacedBy: ['new-rule-1'],
}],
warningCount: 2,
}, {
errorCount: 1,
fatalErrorCount: 0,
filePath: `${process.cwd()}/utils.ts`,
fixableErrorCount: 0,
fixableWarningCount: 0,
// @ts-expect-error - line is required, but for ignored files it is not provided
messages: [{
column: 1,
message: 'Test core error',
ruleId: null,
severity: 1,
}],
suppressedMessages: [],
usedDeprecatedRules: [{
ruleId: 'deprecated-rule-1',
replacedBy: ['new-rule-1'],
}, {
ruleId: 'deprecated-rule-2',
replacedBy: ['new-rule-2'],
}],
warningCount: 0,
}]

const commonResult = {
messageTheme: expect.any(Function),
positionTheme: expect.any(Function),
ruleTheme: expect.any(Function),
}

lintFilesMock.mockImplementationOnce(() => eslintResult)

expect(await eslintLib.lintFiles(testFiles)).toStrictEqual({
results: {
'index.ts': [{
...commonResult,
position: '1:1',
message: 'Normal rule error',
rule: 'normal-error',
severity: 'X',
}, {
...commonResult,
position: '2:1',
message: 'Normal rule warning',
rule: 'normal-warning',
severity: '!',
}, {
...commonResult,
position: '3:1',
message: 'Error with trailing whitespace',
rule: 'spaced-error',
severity: 'X',
}],
'utils.ts': [{
...commonResult,
position: '0',
message: 'Test core error',
rule: 'core-error',
severity: '!',
}],
},
summary: {
deprecatedRules: ['deprecated-rule-1', 'deprecated-rule-2'],
errorCount: 5,
fileCount: 3,
fixableErrorCount: 3,
fixableWarningCount: 1,
linter: 'ESLint',
warningCount: 2,
},
})

})

})
48 changes: 31 additions & 17 deletions src/linters/eslint.ts
Original file line number Diff line number Diff line change
@@ -1,49 +1,63 @@
import { ESLint } from 'eslint'

import { Linter, type LintReport, type ReportSummary } from '@Types'
import { Linter, RuleSeverity } from '@Types'
import colourLog from '@Utils/colourLog'
import { formatResult } from '@Utils/transform'

import type { LintReport, ReportResults, ReportSummary } from '@Types'

const lintFiles = async (files: Array<string>): Promise<LintReport> => {
try {
const eslint = new ESLint({
// @ts-expect-error
overrideConfigFile: true,
overrideConfig: {
rules: {
'quotes': [2, 'single'],
'no-console': 2,
'no-ternary': 1,
'no-unused-vars': 2,
},
},
cache: false,
fix: false,
})

const results = await eslint.lintFiles(files)
const lintResults: Array<ESLint.LintResult> = await eslint.lintFiles(files)

const reportResults: ReportResults = {}

const reportSummary: ReportSummary = {
deprecatedRules: [],
errorCount: 0,
fileCount: results.length,
fileCount: lintResults.length,
fixableErrorCount: 0,
fixableWarningCount: 0,
linter: Linter.ESLint,
warningCount: 0,
}

results.forEach(({ errorCount, fixableErrorCount, fixableWarningCount, usedDeprecatedRules, warningCount }) => {
lintResults.forEach(({ errorCount, filePath, fixableErrorCount, fixableWarningCount, messages, usedDeprecatedRules, warningCount }) => {
const file = filePath.replace(`${process.cwd()}/`, '')

reportSummary.deprecatedRules = [...new Set([...reportSummary.deprecatedRules, ...usedDeprecatedRules.map(({ ruleId }) => ruleId)])]
reportSummary.errorCount += errorCount
reportSummary.fixableErrorCount += fixableErrorCount
reportSummary.fixableWarningCount += fixableWarningCount
reportSummary.warningCount += warningCount

messages.forEach(({ column, line, message, ruleId, severity }) => {
if (!reportResults[file]) {
reportResults[file] = []
}

reportResults[file].push(formatResult({
column: line ? column : undefined,
lineNumber: line || 0,
message: message.trim(),
rule: ruleId || 'core-error',
severity: severity === 1 ? RuleSeverity.WARNING : RuleSeverity.ERROR,
}))
})
})

return {
results: {},
results: reportResults,
summary: reportSummary,
}
} catch (error: any) {
console.error(error.stack)
throw error
colourLog.error('An error occurred while running eslint', error)
process.exit(1)
}
}

Expand Down
Loading

0 comments on commit 4749797

Please sign in to comment.