From db4f6d3859a52065d14d133f864cfd5b7df9cbf6 Mon Sep 17 00:00:00 2001 From: Jonathan Grimes Date: Fri, 20 Nov 2020 08:30:09 -0600 Subject: [PATCH 1/4] feat: add failing test --- test/watch.test.js | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/test/watch.test.js b/test/watch.test.js index cadc5cb..b8921ba 100644 --- a/test/watch.test.js +++ b/test/watch.test.js @@ -6,20 +6,25 @@ import { removeSync } from 'fs-extra'; import pack from './utils/pack'; const target = join(__dirname, 'fixtures', 'watch-entry.js'); +const target2 = join(__dirname, 'fixtures', 'watch-leaf.js'); const targetExpectedPattern = expect.stringMatching( target.replace(/\\/g, '\\\\') ); describe('watch', () => { + let watch; afterEach(() => { + if (watch) { + watch.close(); + } removeSync(target); + removeSync(target2); }); it('should watch', (done) => { const compiler = pack('good'); - const watch = compiler.watch({}, (err, stats) => { - watch.close(); + watch = compiler.watch({}, (err, stats) => { expect(err).toBeNull(); expect(stats.hasWarnings()).toBe(false); expect(stats.hasErrors()).toBe(false); @@ -32,7 +37,7 @@ describe('watch', () => { let next = firstPass; const compiler = pack('watch'); - const watch = compiler.watch({}, (err, stats) => next(err, stats)); + watch = compiler.watch({}, (err, stats) => next(err, stats)); function firstPass(err, stats) { expect(err).toBeNull(); @@ -46,7 +51,11 @@ describe('watch', () => { next = secondPass; - writeFileSync(target, 'const foo = false;\n'); + writeFileSync(target2, 'let bar = false;\n'); + writeFileSync( + target, + "const x = require('./watch-leaf.js')\n\nconst foo = false;\n" + ); } function secondPass(err, stats) { @@ -58,11 +67,16 @@ describe('watch', () => { const [{ message }] = errors; expect(message).toEqual(targetExpectedPattern); expect(message).toEqual(expect.stringMatching('no-unused-vars')); - expect(message).toEqual(expect.stringMatching('\\(1 error,')); + // `prefer-const` passes here + expect(message).toEqual(expect.stringMatching('prefer-const')); + expect(message).toEqual(expect.stringMatching('\\(4 errors,')); next = thirdPass; - writeFileSync(target, 'const foo = 0\n'); + writeFileSync( + target, + "const x = require('./watch-leaf.js')\nconst foo = 0\n" + ); } function thirdPass(err, stats) { @@ -74,6 +88,8 @@ describe('watch', () => { const [{ message }] = errors; expect(message).toEqual(targetExpectedPattern); expect(message).toEqual(expect.stringMatching('no-unused-vars')); + // `prefer-const` fails here + expect(message).toEqual(expect.stringMatching('prefer-const')); expect(message).toEqual(expect.stringMatching('\\(1 error,')); next = finish; @@ -85,7 +101,6 @@ describe('watch', () => { } function finish(err, stats) { - watch.close(); expect(err).toBeNull(); expect(stats.hasWarnings()).toBe(false); expect(stats.hasErrors()).toBe(false); From 9c48a8f95220a086f5b4ec4d799b5a0a3b5d5a75 Mon Sep 17 00:00:00 2001 From: Jonathan Grimes Date: Fri, 20 Nov 2020 09:30:07 -0600 Subject: [PATCH 2/4] fix: #44 keep all results across runs --- declarations/linter.d.ts | 15 +++------------ src/linter.js | 38 +++++++++++++++++++++++++++++++++++++- test/watch.test.js | 2 +- 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/declarations/linter.d.ts b/declarations/linter.d.ts index db852cf..e29bab9 100644 --- a/declarations/linter.d.ts +++ b/declarations/linter.d.ts @@ -1,15 +1,3 @@ -/** @typedef {import('eslint').ESLint} ESLint */ -/** @typedef {import('eslint').ESLint.Formatter} Formatter */ -/** @typedef {import('eslint').ESLint.LintResult} LintResult */ -/** @typedef {import('webpack').Compiler} Compiler */ -/** @typedef {import('webpack').Compilation} Compilation */ -/** @typedef {import('webpack-sources').Source} Source */ -/** @typedef {import('./options').Options} Options */ -/** @typedef {import('./options').FormatterFunction} FormatterFunction */ -/** @typedef {(compilation: Compilation) => Promise} GenerateReport */ -/** @typedef {{errors?: ESLintError, warnings?: ESLintError, generateReportAsset?: GenerateReport}} Report */ -/** @typedef {() => Promise} Reporter */ -/** @typedef {(files: string|string[]) => void} Linter */ /** * @param {Options} options * @param {Compilation} compilation @@ -59,4 +47,7 @@ export type Report = { }; export type Reporter = () => Promise; export type Linter = (files: string | string[]) => void; +export type LintResultMap = { + [files: string]: import('eslint').ESLint.LintResult; +}; import ESLintError from './ESLintError'; diff --git a/src/linter.js b/src/linter.js index 469ffa4..ac6a986 100644 --- a/src/linter.js +++ b/src/linter.js @@ -15,6 +15,11 @@ import getESLint from './getESLint'; /** @typedef {{errors?: ESLintError, warnings?: ESLintError, generateReportAsset?: GenerateReport}} Report */ /** @typedef {() => Promise} Reporter */ /** @typedef {(files: string|string[]) => void} Linter */ +/** @typedef {{[files: string]: LintResult}} LintResultMap */ + +/** @type {WeakMap} */ +const resultStorage = new WeakMap(); + /** * @param {Options} options * @param {Compilation} compilation @@ -36,6 +41,8 @@ export default function linter(options, compilation) { /** @type {Promise[]} */ const rawResults = []; + const crossRunResultStorage = getResultStorage(compilation); + try { ({ ESLint, eslint, lintFiles, cleanup } = getESLint(options)); } catch (e) { @@ -51,6 +58,9 @@ export default function linter(options, compilation) { * @param {string | string[]} files */ function lint(files) { + for (const file of asList(files)) { + delete crossRunResultStorage[file]; + } rawResults.push( lintFiles(files).catch((e) => { compilation.errors.push(e); @@ -61,7 +71,7 @@ export default function linter(options, compilation) { async function report() { // Filter out ignored files. - const results = await removeIgnoredWarnings( + let results = await removeIgnoredWarnings( eslint, // Get the current results, resetting the rawResults to empty await flatten(rawResults.splice(0, rawResults.length)) @@ -80,6 +90,12 @@ export default function linter(options, compilation) { await ESLint.outputFixes(results); } + for (const result of results) { + crossRunResultStorage[result.filePath] = result; + } + + results = Object.values(crossRunResultStorage); + const formatter = await loadFormatter(eslint, options.formatter); const { errors, warnings } = formatResults( formatter, @@ -276,3 +292,23 @@ async function flatten(results) { const flat = (acc, list) => [...acc, ...list]; return (await Promise.all(results)).reduce(flat, []); } + +/** + * @param {Compilation} compilation + * @returns {LintResultMap} + */ +function getResultStorage({ compiler }) { + let storage = resultStorage.get(compiler); + if (!storage) { + resultStorage.set(compiler, (storage = {})); + } + return storage; +} + +/** + * @param {string | string[]} x + */ +function asList(x) { + /* istanbul ignore next */ + return Array.isArray(x) ? x : [x]; +} diff --git a/test/watch.test.js b/test/watch.test.js index b8921ba..c5a6054 100644 --- a/test/watch.test.js +++ b/test/watch.test.js @@ -90,7 +90,7 @@ describe('watch', () => { expect(message).toEqual(expect.stringMatching('no-unused-vars')); // `prefer-const` fails here expect(message).toEqual(expect.stringMatching('prefer-const')); - expect(message).toEqual(expect.stringMatching('\\(1 error,')); + expect(message).toEqual(expect.stringMatching('\\(5 errors,')); next = finish; From 9883b78ead88b2e079b0c473e7646d872f701ec9 Mon Sep 17 00:00:00 2001 From: Jonathan Grimes Date: Fri, 20 Nov 2020 09:59:03 -0600 Subject: [PATCH 3/4] fix: #45 - include eslint options in types --- declarations/getESLint.d.ts | 19 ++----------------- declarations/index.d.ts | 21 +++------------------ declarations/linter.d.ts | 19 ++----------------- declarations/options.d.ts | 10 ++++++---- src/options.js | 6 ++++-- 5 files changed, 17 insertions(+), 58 deletions(-) diff --git a/declarations/getESLint.d.ts b/declarations/getESLint.d.ts index 888e803..f555f3c 100644 --- a/declarations/getESLint.d.ts +++ b/declarations/getESLint.d.ts @@ -23,23 +23,8 @@ export function loadESLintThreaded(poolSize: number, options: Options): Linter; export default function getESLint({ threads, ...options }: Options): Linter; export type ESLint = import('eslint').ESLint; export type LintResult = import('eslint').ESLint.LintResult; -export type Options = { - context?: string | undefined; - emitError?: boolean | undefined; - emitWarning?: boolean | undefined; - eslintPath?: string | undefined; - exclude?: string | string[] | undefined; - extensions?: string | string[] | undefined; - failOnError?: boolean | undefined; - failOnWarning?: boolean | undefined; - files?: string | string[] | undefined; - fix?: boolean | undefined; - formatter?: string | import('./options').FormatterFunction | undefined; - lintDirtyModulesOnly?: boolean | undefined; - quiet?: boolean | undefined; - outputReport?: import('./options').OutputReport | undefined; - threads?: number | boolean | undefined; -}; +export type Options = import('./options').PluginOptions & + import('eslint').ESLint.Options; export type AsyncTask = () => Promise; export type LintTask = (files: string | string[]) => Promise; export type Worker = JestWorker & { diff --git a/declarations/index.d.ts b/declarations/index.d.ts index b4c44d5..7c16f5e 100644 --- a/declarations/index.d.ts +++ b/declarations/index.d.ts @@ -3,7 +3,7 @@ export class ESLintWebpackPlugin { * @param {Options} options */ constructor(options?: Options); - options: import('./options').Options; + options: import('./options').PluginOptions; /** * @param {Compiler} compiler */ @@ -22,20 +22,5 @@ export class ESLintWebpackPlugin { } export default ESLintWebpackPlugin; export type Compiler = import('webpack').Compiler; -export type Options = { - context?: string | undefined; - emitError?: boolean | undefined; - emitWarning?: boolean | undefined; - eslintPath?: string | undefined; - exclude?: string | string[] | undefined; - extensions?: string | string[] | undefined; - failOnError?: boolean | undefined; - failOnWarning?: boolean | undefined; - files?: string | string[] | undefined; - fix?: boolean | undefined; - formatter?: string | import('./options').FormatterFunction | undefined; - lintDirtyModulesOnly?: boolean | undefined; - quiet?: boolean | undefined; - outputReport?: import('./options').OutputReport | undefined; - threads?: number | boolean | undefined; -}; +export type Options = import('./options').PluginOptions & + import('eslint').ESLint.Options; diff --git a/declarations/linter.d.ts b/declarations/linter.d.ts index e29bab9..3368c35 100644 --- a/declarations/linter.d.ts +++ b/declarations/linter.d.ts @@ -16,23 +16,8 @@ export type LintResult = import('eslint').ESLint.LintResult; export type Compiler = import('webpack').Compiler; export type Compilation = import('webpack').Compilation; export type Source = import('webpack-sources/lib/Source'); -export type Options = { - context?: string | undefined; - emitError?: boolean | undefined; - emitWarning?: boolean | undefined; - eslintPath?: string | undefined; - exclude?: string | string[] | undefined; - extensions?: string | string[] | undefined; - failOnError?: boolean | undefined; - failOnWarning?: boolean | undefined; - files?: string | string[] | undefined; - fix?: boolean | undefined; - formatter?: string | import('./options').FormatterFunction | undefined; - lintDirtyModulesOnly?: boolean | undefined; - quiet?: boolean | undefined; - outputReport?: import('./options').OutputReport | undefined; - threads?: number | boolean | undefined; -}; +export type Options = import('./options').PluginOptions & + import('eslint').ESLint.Options; export type FormatterFunction = ( results: import('eslint').ESLint.LintResult[], data?: import('eslint').ESLint.LintResultData | undefined diff --git a/declarations/options.d.ts b/declarations/options.d.ts index fde2613..7be091d 100644 --- a/declarations/options.d.ts +++ b/declarations/options.d.ts @@ -13,7 +13,7 @@ * @property {string|FormatterFunction=} formatter */ /** - * @typedef {Object} Options + * @typedef {Object} PluginOptions * @property {string=} context * @property {boolean=} emitError * @property {boolean=} emitWarning @@ -30,11 +30,12 @@ * @property {OutputReport=} outputReport * @property {number|boolean=} threads */ +/** @typedef {PluginOptions & ESLintOptions} Options */ /** * @param {Options} pluginOptions - * @returns {Options} + * @returns {PluginOptions} */ -export function getOptions(pluginOptions: Options): Options; +export function getOptions(pluginOptions: Options): PluginOptions; /** * @param {Options} loaderOptions * @returns {ESLintOptions} @@ -51,7 +52,7 @@ export type OutputReport = { filePath?: string | undefined; formatter?: (string | FormatterFunction) | undefined; }; -export type Options = { +export type PluginOptions = { context?: string | undefined; emitError?: boolean | undefined; emitWarning?: boolean | undefined; @@ -68,3 +69,4 @@ export type Options = { outputReport?: OutputReport | undefined; threads?: (number | boolean) | undefined; }; +export type Options = PluginOptions & import('eslint').ESLint.Options; diff --git a/src/options.js b/src/options.js index 397d489..0763850 100644 --- a/src/options.js +++ b/src/options.js @@ -20,7 +20,7 @@ import schema from './options.json'; */ /** - * @typedef {Object} Options + * @typedef {Object} PluginOptions * @property {string=} context * @property {boolean=} emitError * @property {boolean=} emitWarning @@ -38,9 +38,11 @@ import schema from './options.json'; * @property {number|boolean=} threads */ +/** @typedef {PluginOptions & ESLintOptions} Options */ + /** * @param {Options} pluginOptions - * @returns {Options} + * @returns {PluginOptions} */ export function getOptions(pluginOptions) { const options = { From 85df34530a77d3ac463ee3bf3dc951ebaac1f1bc Mon Sep 17 00:00:00 2001 From: Jonathan Grimes Date: Mon, 30 Nov 2020 01:48:22 -0600 Subject: [PATCH 4/4] fix: 43 --- src/getESLint.js | 10 ++++++++-- src/linter.js | 11 +---------- src/worker.js | 18 ++++++++++++++--- test/autofix.test.js | 46 ++++++++++++++++++++++++++++---------------- test/threads.test.js | 13 ++++++++++--- 5 files changed, 63 insertions(+), 35 deletions(-) diff --git a/src/getESLint.js b/src/getESLint.js index f90ae80..faa1b9c 100644 --- a/src/getESLint.js +++ b/src/getESLint.js @@ -23,13 +23,19 @@ export function loadESLint(options) { // Filter out loader options before passing the options to ESLint. const eslint = new ESLint(getESLintOptions(options)); - const lintFiles = eslint.lintFiles.bind(eslint); return { threads: 1, ESLint, eslint, - lintFiles, + lintFiles: async (files) => { + const results = await eslint.lintFiles(files); + // istanbul ignore else + if (options.fix) { + await ESLint.outputFixes(results); + } + return results; + }, // no-op for non-threaded cleanup: async () => {}, }; diff --git a/src/linter.js b/src/linter.js index ac6a986..07b6c93 100644 --- a/src/linter.js +++ b/src/linter.js @@ -26,9 +26,6 @@ const resultStorage = new WeakMap(); * @returns {{lint: Linter, report: Reporter}} */ export default function linter(options, compilation) { - /** @type {ESLint} */ - let ESLint; - /** @type {ESLint} */ let eslint; @@ -44,7 +41,7 @@ export default function linter(options, compilation) { const crossRunResultStorage = getResultStorage(compilation); try { - ({ ESLint, eslint, lintFiles, cleanup } = getESLint(options)); + ({ eslint, lintFiles, cleanup } = getESLint(options)); } catch (e) { throw new ESLintError(e.message); } @@ -84,12 +81,6 @@ export default function linter(options, compilation) { return {}; } - // if enabled, use eslint autofixing where possible - if (options.fix) { - // @ts-ignore - await ESLint.outputFixes(results); - } - for (const result of results) { crossRunResultStorage[result.filePath] = result; } diff --git a/src/worker.js b/src/worker.js index 8aba7cf..6e5f842 100644 --- a/src/worker.js +++ b/src/worker.js @@ -6,9 +6,15 @@ Object.assign(module.exports, { setup, }); +/** @type {{ new (arg0: import("eslint").ESLint.Options): import("eslint").ESLint; outputFixes: (arg0: import("eslint").ESLint.LintResult[]) => any; }} */ +let ESLint; + /** @type {ESLint} */ let eslint; +/** @type {boolean} */ +let fix; + /** * @typedef {object} setupOptions * @property {string=} eslintPath - import path of eslint @@ -16,8 +22,9 @@ let eslint; * * @param {setupOptions} arg0 - setup worker */ -function setup({ eslintPath, eslintOptions }) { - const { ESLint } = require(eslintPath || 'eslint'); +function setup({ eslintPath, eslintOptions = {} }) { + fix = !!(eslintOptions && eslintOptions.fix); + ({ ESLint } = require(eslintPath || 'eslint')); eslint = new ESLint(eslintOptions); } @@ -25,5 +32,10 @@ function setup({ eslintPath, eslintOptions }) { * @param {string | string[]} files */ async function lintFiles(files) { - return eslint.lintFiles(files); + const result = await eslint.lintFiles(files); + // if enabled, use eslint autofixing where possible + if (fix) { + await ESLint.outputFixes(result); + } + return result; } diff --git a/test/autofix.test.js b/test/autofix.test.js index 6b286f1..8d4c45d 100644 --- a/test/autofix.test.js +++ b/test/autofix.test.js @@ -1,6 +1,6 @@ import { join } from 'path'; -import { copySync, removeSync } from 'fs-extra'; +import { copySync, removeSync, readFileSync } from 'fs-extra'; import pack from './utils/pack'; @@ -15,20 +15,32 @@ describe('autofix stop', () => { removeSync(entry); }); - it('should not throw error if file ok after auto-fixing', (done) => { - const compiler = pack('fixable-clone', { - fix: true, - extensions: ['js', 'cjs', 'mjs'], - overrideConfig: { - rules: { semi: ['error', 'always'] }, - }, - }); - - compiler.run((err, stats) => { - expect(err).toBeNull(); - expect(stats.hasWarnings()).toBe(false); - expect(stats.hasErrors()).toBe(false); - done(); - }); - }); + test.each([[{}], [{ threads: false }]])( + 'should not throw error if file ok after auto-fixing', + (cfg, done) => { + const compiler = pack('fixable-clone', { + ...cfg, + fix: true, + extensions: ['js', 'cjs', 'mjs'], + overrideConfig: { + rules: { semi: ['error', 'always'] }, + }, + }); + + compiler.run((err, stats) => { + expect(err).toBeNull(); + expect(stats.hasWarnings()).toBe(false); + expect(stats.hasErrors()).toBe(false); + expect(readFileSync(entry).toString('utf8')).toMatchInlineSnapshot(` + "function foo() { + return true; + } + + foo(); + " + `); + done(); + }); + } + ); }); diff --git a/test/threads.test.js b/test/threads.test.js index 83f0fa7..820ec34 100644 --- a/test/threads.test.js +++ b/test/threads.test.js @@ -45,9 +45,14 @@ describe('Threading', () => { const mockThread = { parentPort: { on: jest.fn() }, workerData: {} }; const mockLintFiles = jest.fn(); jest.mock('eslint', () => ({ - ESLint: function ESLint() { - this.lintFiles = mockLintFiles; - }, + ESLint: Object.assign( + function ESLint() { + this.lintFiles = mockLintFiles; + }, + { + outputFixes: jest.fn(), + } + ), })); jest.mock('worker_threads', () => mockThread); const { setup, lintFiles } = require('../src/worker'); @@ -55,6 +60,8 @@ describe('Threading', () => { await setup({}); await lintFiles('foo'); expect(mockLintFiles).toHaveBeenCalledWith('foo'); + await setup({ eslintOptions: { fix: true } }); + await lintFiles('foo'); }); }); });