From 388ee7e2aeb4deee2de785f70c24ed6681e5a952 Mon Sep 17 00:00:00 2001 From: Everton Nunes Date: Sun, 24 Apr 2022 19:56:32 -0300 Subject: [PATCH 1/4] adds --output-file flag --- packages/next/cli/next-lint.ts | 5 +++ packages/next/lib/eslint/customFormatter.ts | 4 +- packages/next/lib/eslint/runLintCheck.ts | 14 +++++-- packages/next/lib/eslint/writeOutputFile.ts | 45 +++++++++++++++++++++ 4 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 packages/next/lib/eslint/writeOutputFile.ts diff --git a/packages/next/cli/next-lint.ts b/packages/next/cli/next-lint.ts index 439805dcfd3d..bb5682e909c6 100755 --- a/packages/next/cli/next-lint.ts +++ b/packages/next/cli/next-lint.ts @@ -71,10 +71,12 @@ const nextLint: cliCommand = async (argv) => { '--cache-strategy': String, '--error-on-unmatched-pattern': Boolean, '--format': String, + '--output-file': String, // Aliases '-c': '--config', '-f': '--format', + '-o': '--output-file', } let args: arg.Result @@ -127,6 +129,7 @@ const nextLint: cliCommand = async (argv) => { --max-warnings Int Number of warnings to trigger nonzero exit code - default: -1 Output: + -o, --output-file path::String Specify file to write report to -f, --format String Use a specific output format - default: Next.js custom formatter Inline configuration comments: @@ -171,6 +174,7 @@ const nextLint: cliCommand = async (argv) => { const maxWarnings = args['--max-warnings'] ?? -1 const formatter = args['--format'] || null const strict = Boolean(args['--strict']) + const outputFile = args['--output-file'] || null const distDir = join(baseDir, nextConfig.distDir) const defaultCacheLocation = join(distDir, 'cache', 'eslint/') @@ -183,6 +187,7 @@ const nextLint: cliCommand = async (argv) => { reportErrorsOnly, maxWarnings, formatter, + outputFile, strict ) .then(async (lintResults) => { diff --git a/packages/next/lib/eslint/customFormatter.ts b/packages/next/lib/eslint/customFormatter.ts index c3d1a7bbfa16..d3fb9db906c7 100644 --- a/packages/next/lib/eslint/customFormatter.ts +++ b/packages/next/lib/eslint/customFormatter.ts @@ -100,6 +100,7 @@ export function formatResults( format: (r: LintResult[]) => string ): { output: string + outputWithMessages: string totalNextPluginErrorCount: number totalNextPluginWarningCount: number } { @@ -124,7 +125,8 @@ export function formatResults( .join('\n') return { - output: + output: output, + outputWithMessages: resultsWithMessages.length > 0 ? output + `\n\n${chalk.cyan( diff --git a/packages/next/lib/eslint/runLintCheck.ts b/packages/next/lib/eslint/runLintCheck.ts index e994f8528680..51e99ca2499d 100644 --- a/packages/next/lib/eslint/runLintCheck.ts +++ b/packages/next/lib/eslint/runLintCheck.ts @@ -1,4 +1,4 @@ -import { promises as fs } from 'fs' +import { promises as fs, promises, statSync } from 'fs' import chalk from 'next/dist/compiled/chalk' import path from 'path' @@ -9,6 +9,7 @@ import * as CommentJson from 'next/dist/compiled/comment-json' import { LintResult, formatResults } from './customFormatter' import { writeDefaultConfig } from './writeDefaultConfig' import { hasEslintConfiguration } from './hasEslintConfiguration' +import { writeOutputFile } from './writeOutputFile' import { ESLINT_PROMPT_VALUES } from '../constants' import { existsSync, findPagesDir } from '../find-pages-dir' @@ -86,7 +87,8 @@ async function lint( eslintOptions: any = null, reportErrorsOnly: boolean = false, maxWarnings: number = -1, - formatter: string | null = null + formatter: string | null = null, + outputFile: string | null = null ): Promise< | string | null @@ -221,8 +223,10 @@ async function lint( 0 ) + if (outputFile) await writeOutputFile(outputFile, formattedResult.output) + return { - output: formattedResult.output, + output: formattedResult.outputWithMessages, isError: ESLint.getErrorResults(results)?.length > 0 || (maxWarnings >= 0 && totalWarnings > maxWarnings), @@ -266,6 +270,7 @@ export async function runLintCheck( reportErrorsOnly: boolean = false, maxWarnings: number = -1, formatter: string | null = null, + outputFile: string | null = null, strict: boolean = false ): ReturnType { try { @@ -309,7 +314,8 @@ export async function runLintCheck( eslintOptions, reportErrorsOnly, maxWarnings, - formatter + formatter, + outputFile ) } else { // Display warning if no ESLint configuration is present during "next build" diff --git a/packages/next/lib/eslint/writeOutputFile.ts b/packages/next/lib/eslint/writeOutputFile.ts new file mode 100644 index 000000000000..25c8a6349609 --- /dev/null +++ b/packages/next/lib/eslint/writeOutputFile.ts @@ -0,0 +1,45 @@ +import { promises as fs } from 'fs' +import path from 'path' +import * as Log from '../../build/output/log' +import isError from '../../lib/is-error' + +/** + * Check if a given file path is a directory or not. + * @param {string} filePath The path to a file to check. + * @returns {Promise} `true` if the path is a directory. + */ +async function isDirectory(filePath: string): Promise { + try { + return (await fs.stat(filePath)).isDirectory() + } catch (error) { + if ( + isError(error) && + (error.code === 'ENOENT' || error.code === 'ENOTDIR') + ) { + return false + } + throw error + } +} +/** + * Create a file with eslint output data + * @param {string} outputFile The name file that needs to be created + * @param {string} outputData The data that needs to be inserted into the file + */ +export async function writeOutputFile(outputFile: string, outputData: string) { + const filePath = path.resolve(process.cwd(), outputFile) + + if (await isDirectory(filePath)) { + Log.error( + `Cannot write to output file path, it is a directory: ${filePath}` + ) + } else { + try { + await fs.mkdir(path.dirname(filePath), { recursive: true }) + await fs.writeFile(filePath, outputData) + Log.info(`The output file has been created: ${filePath}`) + } catch { + Log.error(`There was a problem writing the output file: ${filePath}`) + } + } +} From 7362f7b178f65dfb3dfb33a13ac4d82c1f8b6270 Mon Sep 17 00:00:00 2001 From: Everton Nunes Date: Sat, 30 Apr 2022 00:15:49 -0300 Subject: [PATCH 2/4] add some tests --- packages/next/lib/eslint/runLintCheck.ts | 2 +- test/integration/eslint/test/index.test.js | 103 +++++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/packages/next/lib/eslint/runLintCheck.ts b/packages/next/lib/eslint/runLintCheck.ts index 51e99ca2499d..1266a12997cf 100644 --- a/packages/next/lib/eslint/runLintCheck.ts +++ b/packages/next/lib/eslint/runLintCheck.ts @@ -1,4 +1,4 @@ -import { promises as fs, promises, statSync } from 'fs' +import { promises as fs } from 'fs' import chalk from 'next/dist/compiled/chalk' import path from 'path' diff --git a/test/integration/eslint/test/index.test.js b/test/integration/eslint/test/index.test.js index 31c7be989885..5e8ec6fd531d 100644 --- a/test/integration/eslint/test/index.test.js +++ b/test/integration/eslint/test/index.test.js @@ -604,5 +604,108 @@ describe('ESLint', () => { expect(output).not.toContain('pages/index.js') expect(output).not.toContain('External synchronous scripts are forbidden') }) + + test('output flag create a file respecting the chosen format', async () => { + const filePath = `${__dirname}/output/output.json` + const { stdout, stderr } = await nextLint( + dirFileLinting, + ['--format', 'json', '--output-file', filePath], + { + stdout: true, + stderr: true, + } + ) + + const cliOutput = stdout + stderr + const fileOutput = await fs.readJSON(filePath) + + expect(cliOutput).toContain( + `The output file has been created: ${filePath}` + ) + + if (fileOutput && fileOutput.length) { + fileOutput.forEach((file) => { + expect(file).toHaveProperty('filePath') + expect(file).toHaveProperty('messages') + expect(file).toHaveProperty('errorCount') + expect(file).toHaveProperty('warningCount') + expect(file).toHaveProperty('fixableErrorCount') + expect(file).toHaveProperty('fixableWarningCount') + expect(file).toHaveProperty('source') + expect(file).toHaveProperty('usedDeprecatedRules') + }) + + expect(fileOutput[0].messages).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + message: + 'img elements must have an alt prop, either with meaningful text, or an empty string for decorative images.', + }), + expect.objectContaining({ + message: `Do not use . Use Image from 'next/image' instead. See: https://nextjs.org/docs/messages/no-img-element`, + }), + ]) + ) + + expect(fileOutput[1].messages).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + message: + 'External synchronous scripts are forbidden. See: https://nextjs.org/docs/messages/no-sync-scripts', + }), + ]) + ) + } + }) + + test('output flag create a file respecting the chosen format', async () => { + const filePath = `${__dirname}/output/output.txt` + const { stdout, stderr } = await nextLint( + dirFileLinting, + ['--format', 'compact', '--output-file', filePath], + { + stdout: true, + stderr: true, + } + ) + + const cliOutput = stdout + stderr + const fileOutput = fs.readFileSync(filePath, 'utf8') + + expect(cliOutput).toContain( + `The output file has been created: ${filePath}` + ) + + expect(fileOutput).toContain('file-linting/pages/bar.js') + expect(fileOutput).toContain( + 'img elements must have an alt prop, either with meaningful text, or an empty string for decorative images.' + ) + expect(fileOutput).toContain( + `Do not use . Use Image from 'next/image' instead. See: https://nextjs.org/docs/messages/no-img-element` + ) + + expect(fileOutput).toContain('file-linting/pages/index.js') + expect(fileOutput).toContain( + 'External synchronous scripts are forbidden. See: https://nextjs.org/docs/messages/no-sync-scripts' + ) + }) + + test('show error message when the file path is a directory', async () => { + const filePath = `${__dirname}` + const { stdout, stderr } = await nextLint( + dirFileLinting, + ['--format', 'compact', '--output-file', filePath], + { + stdout: true, + stderr: true, + } + ) + + const cliOutput = stdout + stderr + + expect(cliOutput).toContain( + `Cannot write to output file path, it is a directory: ${filePath}` + ) + }) }) }) From 74e286a735a4c2dda9206e88f08fe7eeb4fc82f8 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Sun, 7 Aug 2022 23:06:24 -0500 Subject: [PATCH 3/4] Apply suggestions from code review --- packages/next/lib/eslint/writeOutputFile.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/next/lib/eslint/writeOutputFile.ts b/packages/next/lib/eslint/writeOutputFile.ts index 25c8a6349609..774ba5a9d32d 100644 --- a/packages/next/lib/eslint/writeOutputFile.ts +++ b/packages/next/lib/eslint/writeOutputFile.ts @@ -38,8 +38,9 @@ export async function writeOutputFile(outputFile: string, outputData: string) { await fs.mkdir(path.dirname(filePath), { recursive: true }) await fs.writeFile(filePath, outputData) Log.info(`The output file has been created: ${filePath}`) - } catch { + } catch (err) { Log.error(`There was a problem writing the output file: ${filePath}`) + console.error(err) } } } From 1a272a75910a5d521ef5ae35ef5cd6a713e54ffc Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Sun, 7 Aug 2022 23:30:12 -0500 Subject: [PATCH 4/4] update test --- test/integration/eslint/test/index.test.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/integration/eslint/test/index.test.js b/test/integration/eslint/test/index.test.js index a0f3e373dddb..2097d0b43bb7 100644 --- a/test/integration/eslint/test/index.test.js +++ b/test/integration/eslint/test/index.test.js @@ -701,7 +701,8 @@ describe('ESLint', () => { 'img elements must have an alt prop, either with meaningful text, or an empty string for decorative images.', }), expect.objectContaining({ - message: `Do not use . Use Image from 'next/image' instead. See: https://nextjs.org/docs/messages/no-img-element`, + message: + 'Do not use `` element. Use `` from `next/image` instead. See: https://nextjs.org/docs/messages/no-img-element', }), ]) ) @@ -710,7 +711,7 @@ describe('ESLint', () => { expect.arrayContaining([ expect.objectContaining({ message: - 'External synchronous scripts are forbidden. See: https://nextjs.org/docs/messages/no-sync-scripts', + 'Synchronous scripts should not be used. See: https://nextjs.org/docs/messages/no-sync-scripts', }), ]) ) @@ -740,12 +741,12 @@ describe('ESLint', () => { 'img elements must have an alt prop, either with meaningful text, or an empty string for decorative images.' ) expect(fileOutput).toContain( - `Do not use . Use Image from 'next/image' instead. See: https://nextjs.org/docs/messages/no-img-element` + 'Do not use `` element. Use `` from `next/image` instead. See: https://nextjs.org/docs/messages/no-img-element' ) expect(fileOutput).toContain('file-linting/pages/index.js') expect(fileOutput).toContain( - 'External synchronous scripts are forbidden. See: https://nextjs.org/docs/messages/no-sync-scripts' + 'Synchronous scripts should not be used. See: https://nextjs.org/docs/messages/no-sync-scripts' ) })