From 19b4761f405da2be4fbd9043fea4f438cd740c49 Mon Sep 17 00:00:00 2001 From: Simeon Cheeseman <1085899+SimeonC@users.noreply.github.com> Date: Wed, 26 Jul 2023 16:04:52 +0900 Subject: [PATCH] feat(linter): add error and warning statistics Outputs the error and warning count statistics including how many can be auto-fixed --- e2e/angular-core/src/ng-add.test.ts | 4 +- e2e/expo/src/expo.test.ts | 4 +- e2e/js/src/js-generators.ts | 2 +- e2e/next-core/src/utils.ts | 2 +- e2e/next-extensions/src/utils.ts | 2 +- e2e/node/src/node.test.ts | 16 ++-- e2e/plugin/src/nx-plugin.test.ts | 8 +- e2e/react-core/src/react.test.ts | 6 +- e2e/react-native/src/react-native.test.ts | 4 +- e2e/web/src/web-vite.test.ts | 4 +- e2e/web/src/web.test.ts | 6 +- .../src/executors/lint/lint.impl.spec.ts | 66 ++++++++++----- .../eslint/src/executors/lint/lint.impl.ts | 80 ++++++++++++++----- 13 files changed, 135 insertions(+), 69 deletions(-) diff --git a/e2e/angular-core/src/ng-add.test.ts b/e2e/angular-core/src/ng-add.test.ts index 30bc5ef938fd4..cef8af5613a72 100644 --- a/e2e/angular-core/src/ng-add.test.ts +++ b/e2e/angular-core/src/ng-add.test.ts @@ -411,14 +411,14 @@ describe('convert Angular CLI workspace to an Nx workspace', () => { let output = runCLI(`lint ${project}`); expect(output).toContain(`> nx run ${project}:lint`); - expect(output).toContain('All files pass linting.'); + expect(output).toContain('All files pass linting'); expect(output).toContain( `Successfully ran target lint for project ${project}` ); output = runCLI(`lint ${project}`); expect(output).toContain(`> nx run ${project}:lint [local cache]`); - expect(output).toContain('All files pass linting.'); + expect(output).toContain('All files pass linting'); expect(output).toContain( `Successfully ran target lint for project ${project}` ); diff --git a/e2e/expo/src/expo.test.ts b/e2e/expo/src/expo.test.ts index 53fe9d79e6b39..609a0323a0b18 100644 --- a/e2e/expo/src/expo.test.ts +++ b/e2e/expo/src/expo.test.ts @@ -61,10 +61,10 @@ describe('expo', () => { expectTestsPass(await runCLIAsync(`test ${libName}`)); const appLintResults = await runCLIAsync(`lint ${appName}`); - expect(appLintResults.combinedOutput).toContain('All files pass linting.'); + expect(appLintResults.combinedOutput).toContain('All files pass linting'); const libLintResults = await runCLIAsync(`lint ${libName}`); - expect(libLintResults.combinedOutput).toContain('All files pass linting.'); + expect(libLintResults.combinedOutput).toContain('All files pass linting'); }); it('should serve with metro', async () => { diff --git a/e2e/js/src/js-generators.ts b/e2e/js/src/js-generators.ts index 70737b4699377..99c1ab603aac0 100644 --- a/e2e/js/src/js-generators.ts +++ b/e2e/js/src/js-generators.ts @@ -54,7 +54,7 @@ describe('js e2e', () => { const result = runCLI(`lint ${dirName}-${libName}`); expect(result).toContain(`Linting "${dirName}-${libName}"...`); - expect(result).toContain('All files pass linting.'); + expect(result).toContain('All files pass linting'); // Test const testResult = await runCLIAsync(`test ${dirName}-${libName}`); diff --git a/e2e/next-core/src/utils.ts b/e2e/next-core/src/utils.ts index 028fcbbb8e66f..6b093936847f8 100644 --- a/e2e/next-core/src/utils.ts +++ b/e2e/next-core/src/utils.ts @@ -21,7 +21,7 @@ export async function checkApp( if (opts.checkLint) { const lintResults = runCLI(`lint ${appName}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); } if (opts.checkUnitTest) { diff --git a/e2e/next-extensions/src/utils.ts b/e2e/next-extensions/src/utils.ts index 028fcbbb8e66f..6b093936847f8 100644 --- a/e2e/next-extensions/src/utils.ts +++ b/e2e/next-extensions/src/utils.ts @@ -21,7 +21,7 @@ export async function checkApp( if (opts.checkLint) { const lintResults = runCLI(`lint ${appName}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); } if (opts.checkUnitTest) { diff --git a/e2e/node/src/node.test.ts b/e2e/node/src/node.test.ts index 4a2d2baf45122..ad98699e255b5 100644 --- a/e2e/node/src/node.test.ts +++ b/e2e/node/src/node.test.ts @@ -65,7 +65,7 @@ describe('Node Applications', () => { setMaxWorkers(join('apps', nodeapp, 'project.json')); const lintResults = runCLI(`lint ${nodeapp}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); updateFile(`apps/${nodeapp}/src/main.ts`, `console.log('Hello World!');`); await runCLIAsync(`build ${nodeapp}`); @@ -100,7 +100,7 @@ describe('Node Applications', () => { setMaxWorkers(join('apps', nodeapp, 'project.json')); const lintResults = runCLI(`lint ${nodeapp}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); updateJson(join('apps', nodeapp, 'project.json'), (config) => { config.targets.build.options.additionalEntryPoints = [ @@ -194,7 +194,7 @@ describe('Node Applications', () => { runCLI(`generate @nx/express:app ${nodeapp} --linter=eslint`); setMaxWorkers(join('apps', nodeapp, 'project.json')); const lintResults = runCLI(`lint ${nodeapp}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); updateFile( `apps/${nodeapp}/src/app/test.spec.ts`, @@ -237,7 +237,7 @@ describe('Node Applications', () => { setMaxWorkers(join('apps', nestapp, 'project.json')); const lintResults = runCLI(`lint ${nestapp}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); updateFile(`apps/${nestapp}/src/assets/file.txt`, ``); const jestResult = await runCLIAsync(`test ${nestapp}`); @@ -590,7 +590,7 @@ ${jslib}(); runCLI(`generate @nx/nest:lib ${nestlib}`); const lintResults = runCLI(`lint ${nestlib}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); const testResults = runCLI(`test ${nestlib}`); expect(testResults).toContain( @@ -604,7 +604,7 @@ ${jslib}(); runCLI(`generate @nx/nest:lib ${nestlib} --service`); const lintResults = runCLI(`lint ${nestlib}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); const jestResult = await runCLIAsync(`test ${nestlib}`); expect(jestResult.combinedOutput).toContain( @@ -618,7 +618,7 @@ ${jslib}(); runCLI(`generate @nx/nest:lib ${nestlib} --controller`); const lintResults = runCLI(`lint ${nestlib}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); const jestResult = await runCLIAsync(`test ${nestlib}`); expect(jestResult.combinedOutput).toContain( @@ -632,7 +632,7 @@ ${jslib}(); runCLI(`generate @nx/nest:lib ${nestlib} --controller --service`); const lintResults = runCLI(`lint ${nestlib}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); const jestResult = await runCLIAsync(`test ${nestlib}`); expect(jestResult.combinedOutput).toContain( diff --git a/e2e/plugin/src/nx-plugin.test.ts b/e2e/plugin/src/nx-plugin.test.ts index 44cde5e6fe1aa..6b17ca7500487 100644 --- a/e2e/plugin/src/nx-plugin.test.ts +++ b/e2e/plugin/src/nx-plugin.test.ts @@ -38,7 +38,7 @@ describe('Nx Plugin', () => { `generate @nx/plugin:plugin ${plugin} --linter=eslint --e2eTestRunner=jest --publishable` ); const lintResults = runCLI(`lint ${plugin}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); const buildResults = runCLI(`build ${plugin}`); expect(buildResults).toContain('Done compiling TypeScript files'); @@ -63,7 +63,7 @@ describe('Nx Plugin', () => { ); const lintResults = runCLI(`lint ${plugin}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); expectTestsPass(await runCLIAsync(`test ${plugin}`)); @@ -93,7 +93,7 @@ describe('Nx Plugin', () => { runCLI(`generate @nx/plugin:generator ${generator} --project=${plugin}`); const lintResults = runCLI(`lint ${plugin}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); expectTestsPass(await runCLIAsync(`test ${plugin}`)); @@ -130,7 +130,7 @@ describe('Nx Plugin', () => { ); const lintResults = runCLI(`lint ${plugin}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); expectTestsPass(await runCLIAsync(`test ${plugin}`)); diff --git a/e2e/react-core/src/react.test.ts b/e2e/react-core/src/react.test.ts index 9d050823883f6..e99c6b7100990 100644 --- a/e2e/react-core/src/react.test.ts +++ b/e2e/react-core/src/react.test.ts @@ -218,14 +218,14 @@ describe('React Applications', () => { runCLI(`g @nx/react:redux orange --project=${libName} --skipFormat`); let lintResults = runCLI(`lint ${appName}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); const appTestResults = await runCLIAsync(`test ${appName}`); expect(appTestResults.combinedOutput).toContain( 'Test Suites: 2 passed, 2 total' ); lintResults = runCLI(`lint ${libName}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); const libTestResults = await runCLIAsync(`test ${libName}`); expect(libTestResults.combinedOutput).toContain( 'Test Suites: 2 passed, 2 total' @@ -443,7 +443,7 @@ async function testGeneratedApp( ) { if (opts.checkLinter) { const lintResults = runCLI(`lint ${appName}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); } runCLI( diff --git a/e2e/react-native/src/react-native.test.ts b/e2e/react-native/src/react-native.test.ts index 32a15178d1979..a75818cd05ca8 100644 --- a/e2e/react-native/src/react-native.test.ts +++ b/e2e/react-native/src/react-native.test.ts @@ -65,10 +65,10 @@ describe('react native', () => { expectTestsPass(await runCLIAsync(`test ${libName}`)); const appLintResults = await runCLIAsync(`lint ${appName}`); - expect(appLintResults.combinedOutput).toContain('All files pass linting.'); + expect(appLintResults.combinedOutput).toContain('All files pass linting'); const libLintResults = await runCLIAsync(`lint ${libName}`); - expect(libLintResults.combinedOutput).toContain('All files pass linting.'); + expect(libLintResults.combinedOutput).toContain('All files pass linting'); }); it('should run e2e for cypress', async () => { diff --git a/e2e/web/src/web-vite.test.ts b/e2e/web/src/web-vite.test.ts index 4145a59563343..39151a0bceb84 100644 --- a/e2e/web/src/web-vite.test.ts +++ b/e2e/web/src/web-vite.test.ts @@ -24,7 +24,7 @@ describe('Web Components Applications with bundler set as vite', () => { setMaxWorkers(join('apps', appName, 'project.json')); const lintResults = runCLI(`lint ${appName}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); runCLI(`build ${appName}`); checkFilesExist(`dist/apps/${appName}/index.html`); @@ -35,7 +35,7 @@ describe('Web Components Applications with bundler set as vite', () => { const lintE2eResults = runCLI(`lint ${appName}-e2e`); - expect(lintE2eResults).toContain('All files pass linting.'); + expect(lintE2eResults).toContain('All files pass linting'); if (isNotWindows() && runE2ETests()) { const e2eResults = runCLI(`e2e ${appName}-e2e --no-watch`); diff --git a/e2e/web/src/web.test.ts b/e2e/web/src/web.test.ts index 3d8518b02d3ad..5dfba4abf8b51 100644 --- a/e2e/web/src/web.test.ts +++ b/e2e/web/src/web.test.ts @@ -33,7 +33,7 @@ describe('Web Components Applications', () => { setMaxWorkers(join('apps', appName, 'project.json')); const lintResults = runCLI(`lint ${appName}`); - expect(lintResults).toContain('All files pass linting.'); + expect(lintResults).toContain('All files pass linting'); const testResults = await runCLIAsync(`test ${appName}`); @@ -42,7 +42,7 @@ describe('Web Components Applications', () => { ); const lintE2eResults = runCLI(`lint ${appName}-e2e`); - expect(lintE2eResults).toContain('All files pass linting.'); + expect(lintE2eResults).toContain('All files pass linting'); if (isNotWindows() && runE2ETests()) { const e2eResults = runCLI(`e2e ${appName}-e2e --no-watch`); @@ -110,7 +110,7 @@ describe('Web Components Applications', () => { const lintE2eResults = runCLI(`lint ${appName}-e2e`); - expect(lintE2eResults).toContain('All files pass linting.'); + expect(lintE2eResults).toContain('All files pass linting'); if (isNotWindows() && runE2ETests()) { ensurePlaywrightBrowsersInstallation(); diff --git a/packages/eslint/src/executors/lint/lint.impl.spec.ts b/packages/eslint/src/executors/lint/lint.impl.spec.ts index 1167155779176..3660c47174772 100644 --- a/packages/eslint/src/executors/lint/lint.impl.spec.ts +++ b/packages/eslint/src/executors/lint/lint.impl.spec.ts @@ -316,11 +316,45 @@ describe('Linter Builder', () => { }), mockContext ); - expect(console.error).toHaveBeenCalledWith( - 'Lint errors found in the listed files.\n' + expect(console.info).toHaveBeenCalledWith( + '✖ 14 problems (4 errors, 10 warnings)\n' + ); + }); + + it('should log fixable errors or warnings', async () => { + mockReports = [ + { + errorCount: 2, + warningCount: 4, + fixableErrorCount: 1, + fixableWarningCount: 2, + results: [], + usedDeprecatedRules: [], + }, + { + errorCount: 3, + warningCount: 6, + fixableErrorCount: 2, + fixableWarningCount: 4, + results: [], + usedDeprecatedRules: [], + }, + ]; + setupMocks(); + await lintExecutor( + createValidRunBuilderOptions({ + eslintConfig: './.eslintrc.json', + lintFilePatterns: ['includedFile1'], + format: 'json', + silent: false, + }), + mockContext + ); + expect(console.info).toHaveBeenCalledWith( + '✖ 15 problems (5 errors, 10 warnings)\n' ); - expect(console.warn).toHaveBeenCalledWith( - 'Lint warnings found in the listed files.\n' + expect(console.info).toHaveBeenCalledWith( + ' 3 errors and 6 warnings are potentially fixable with the `--fix` option.\n' ); }); @@ -375,13 +409,13 @@ Please see https://nx.dev/guides/eslint for full guidance on how to resolve this }), mockContext ); - expect(console.error).not.toHaveBeenCalledWith( - 'Lint errors found in the listed files.\n' + expect(console.info).not.toHaveBeenCalledWith( + '✖ 0 problems (0 errors, 0 warnings)\n' ); - expect(console.warn).not.toHaveBeenCalledWith( - 'Lint warnings found in the listed files.\n' + expect(console.info).not.toHaveBeenCalledWith( + ' 0 errors and 0 warnings are potentially fixable with the `--fix` option.\n' ); - expect(console.info).toHaveBeenCalledWith('All files pass linting.\n'); + expect(console.info).toHaveBeenCalledWith('✔ All files pass linting\n'); }); it('should not log warnings if the quiet flag was passed', async () => { @@ -486,11 +520,8 @@ Please see https://nx.dev/guides/eslint for full guidance on how to resolve this }), mockContext ); - expect(console.error).toHaveBeenCalledWith( - 'Lint errors found in the listed files.\n' - ); - expect(console.warn).not.toHaveBeenCalledWith( - 'Lint warnings found in the listed files.\n' + expect(console.info).toHaveBeenCalledWith( + '✖ 4 problems (4 errors, 0 warnings)\n' ); }); it('should not log if the silent flag was passed', async () => { @@ -518,11 +549,8 @@ Please see https://nx.dev/guides/eslint for full guidance on how to resolve this }), mockContext ); - expect(console.error).not.toHaveBeenCalledWith( - 'Lint errors found in the listed files.\n' - ); - expect(console.warn).not.toHaveBeenCalledWith( - 'Lint warnings found in the listed files.\n' + expect(console.info).not.toHaveBeenCalledWith( + '✖ 14 problems (4 errors, 10 warnings)\n' ); }); }); diff --git a/packages/eslint/src/executors/lint/lint.impl.ts b/packages/eslint/src/executors/lint/lint.impl.ts index 62598177bec8a..d583098d309c2 100644 --- a/packages/eslint/src/executors/lint/lint.impl.ts +++ b/packages/eslint/src/executors/lint/lint.impl.ts @@ -174,16 +174,6 @@ Please see https://nx.dev/guides/eslint for full guidance on how to resolve this const formatter = await eslint.loadFormatter(normalizedOptions.format); - let totalErrors = 0; - let totalWarnings = 0; - - for (const result of lintResults) { - if (result.errorCount || result.warningCount) { - totalErrors += result.errorCount; - totalWarnings += result.warningCount; - } - } - const formattedResults = await formatter.format(lintResults); if (normalizedOptions.outputFile) { @@ -197,23 +187,71 @@ Please see https://nx.dev/guides/eslint for full guidance on how to resolve this console.info(formattedResults); } - if (totalWarnings > 0 && printInfo) { - console.warn('Lint warnings found in the listed files.\n'); - } + const totals = getTotals(lintResults); - if (totalErrors > 0 && printInfo) { - console.error('Lint errors found in the listed files.\n'); - } - - if (totalWarnings === 0 && totalErrors === 0 && printInfo) { - console.info('All files pass linting.\n'); + if (printInfo) { + outputPrintInfo(totals); } return { success: normalizedOptions.force || - (totalErrors === 0 && + (totals.errors === 0 && (normalizedOptions.maxWarnings === -1 || - totalWarnings <= normalizedOptions.maxWarnings)), + totals.warnings <= normalizedOptions.maxWarnings)), + }; +} + +function getTotals(lintResults: ESLint.LintResult[]) { + let errors = 0; + let warnings = 0; + let fixableErrors = 0; + let fixableWarnings = 0; + + for (const result of lintResults) { + errors += result.errorCount || 0; + warnings += result.warningCount || 0; + fixableErrors += result.fixableErrorCount || 0; + fixableWarnings += result.fixableWarningCount || 0; + } + + return { + errors, + warnings, + fixableErrors, + fixableWarnings, }; } + +function pluralizedOutput(word: string, count: number) { + return `${count} ${word}${count === 1 ? '' : 's'}`; +} + +function outputPrintInfo({ + errors, + warnings, + fixableErrors, + fixableWarnings, +}: ReturnType) { + const total = warnings + errors; + const totalFixable = fixableErrors + fixableWarnings; + + if (total <= 0) { + console.info('\u2714 All files pass linting\n'); + return; + } + + console.info( + `\u2716 ${pluralizedOutput('problem', total)} (${pluralizedOutput( + 'error', + errors + )}, ${pluralizedOutput('warning', warnings)})\n` + ); + if (totalFixable <= 0) return; + console.info( + ` ${pluralizedOutput('error', fixableErrors)} and ${pluralizedOutput( + 'warning', + fixableWarnings + )} are potentially fixable with the \`--fix\` option.\n` + ); +}