From b598b6e6a673da1c3d2c83824c0a0b5c63461de1 Mon Sep 17 00:00:00 2001 From: Steve Calvert Date: Fri, 4 Mar 2022 16:50:21 -0800 Subject: [PATCH] Fix import cycles --- bin/ember-template-lint.js | 2 +- lib/formatters/load-formatter.js | 3 + lib/formatters/multi.js | 40 +-- lib/helpers/print-results.js | 7 +- test/acceptance/formatters/multi-test.js | 412 +++++++++++++++++++++++ 5 files changed, 442 insertions(+), 22 deletions(-) create mode 100644 test/acceptance/formatters/multi-test.js diff --git a/bin/ember-template-lint.js b/bin/ember-template-lint.js index d51f67f3e..9e0beb465 100755 --- a/bin/ember-template-lint.js +++ b/bin/ember-template-lint.js @@ -249,7 +249,7 @@ async function run() { process.exitCode = 1; } - printResults(results, { options, todoInfo, config: linter.config }); + await printResults(results, { options, todoInfo, config: linter.config }); } run(); diff --git a/lib/formatters/load-formatter.js b/lib/formatters/load-formatter.js index d34fe54d1..4dd408b29 100644 --- a/lib/formatters/load-formatter.js +++ b/lib/formatters/load-formatter.js @@ -2,6 +2,9 @@ import { createRequire } from 'node:module'; import path from 'node:path'; import JsonFormatter from './json.js'; +// The following disable should be safe. This particular rule does not need to identify +// cycles that are broken when using dynamic imports. See https://github.com/import-js/eslint-plugin-import/issues/2265 +// eslint-disable-next-line import/no-cycle import MultiFormatter from './multi.js'; import PrettyFormatter from './pretty.js'; import SarifFormatter from './sarif.js'; diff --git a/lib/formatters/multi.js b/lib/formatters/multi.js index c39397eb7..e1a32ad85 100644 --- a/lib/formatters/multi.js +++ b/lib/formatters/multi.js @@ -1,31 +1,33 @@ -import printResults from '../helpers/print-results.js'; - -export default class PrettyFormatter { +export default class MultiFormatter { constructor(options = {}) { this.options = options; } - format(results, todoInfo) { + async format(results, todoInfo) { let formatConfig = this.options.config.format; - if (!formatConfig && !formatConfig.formatters) { - return ''; - } + if (formatConfig && formatConfig.formatters) { + // The following disable should be safe. This particular rule does not need to identify + // cycles that are broken when using dynamic imports. See https://github.com/import-js/eslint-plugin-import/issues/2265 + // eslint-disable-next-line import/no-cycle + const { default: printResults } = await import('../helpers/print-results.js'); + for (let formatter of formatConfig.formatters) { + let formatterOptions = { + options: Object.assign({}, this.options, { + format: formatter.name, + }), + todoInfo, + config: this.options.config, + }; - for (let formatter of formatConfig.formatters) { - let formatterOptions = { - options: Object.assign({}, this.options, { - format: formatter.name, - }), - todoInfo, - config: this.options.config, - }; + if (formatter.outputFile) { + formatterOptions.options.outputFile = formatter.outputFile; + } - if (formatter.outputFile) { - formatterOptions.options.outputFile = formatter.outputFile; + await printResults(results, formatterOptions); } - - printResults(results, formatterOptions); } + + return ''; } } diff --git a/lib/helpers/print-results.js b/lib/helpers/print-results.js index 99074cbbf..aaefcefae 100644 --- a/lib/helpers/print-results.js +++ b/lib/helpers/print-results.js @@ -1,7 +1,10 @@ +// The following disable should be safe. This particular rule does not need to identify +// cycles that are broken when using dynamic imports. See https://github.com/import-js/eslint-plugin-import/issues/2265 +// eslint-disable-next-line import/no-cycle import { loadFormatter } from '../formatters/load-formatter.js'; import writeOutputFile from './write-output-file.js'; -export default function printResults(results, { options, todoInfo, config }) { +export default async function printResults(results, { options, todoInfo, config }) { let hasErrors = results.errorCount > 0; let hasWarnings = results.warningCount > 0; let hasTodos = options.includeTodo && results.todoCount; @@ -14,7 +17,7 @@ export default function printResults(results, { options, todoInfo, config }) { }); if (typeof formatter.format === 'function') { - let output = formatter.format(results, todoInfo); + let output = await formatter.format(results, todoInfo); if ('outputFile' in options && typeof options.outputFile !== 'undefined') { let outputPath = writeOutputFile(output, formatter.defaultFileExtension || 'txt', options); diff --git a/test/acceptance/formatters/multi-test.js b/test/acceptance/formatters/multi-test.js new file mode 100644 index 000000000..490ade763 --- /dev/null +++ b/test/acceptance/formatters/multi-test.js @@ -0,0 +1,412 @@ +import { Project, getOutputFileContents, run, setupEnvVar } from '../../helpers/index.js'; + +const ROOT = process.cwd(); + +describe('multi formatter', () => { + setupEnvVar('FORCE_COLOR', '0'); + + let project; + beforeEach(function () { + project = Project.defaultSetup(); + project.chdir(); + }); + + afterEach(function () { + process.chdir(ROOT); + project.dispose(); + }); + + it('should format errors', async function () { + project.setConfig({ + rules: { + 'no-bare-strings': true, + }, + format: { + formatters: [ + { + name: 'pretty', + }, + { + name: 'json', + outputFile: 'my-results.json', + }, + ], + }, + }); + project.write({ + app: { + templates: { + 'application.hbs': '

Here too!!

Bare strings are bad...
', + components: { + 'foo.hbs': '{{fooData}}', + }, + }, + }, + }); + + let result = await run(['.', '--format', 'multi']); + + expect(result.exitCode).toEqual(1); + expect(result.stdout.replace(project.baseDir, '')).toMatchInlineSnapshot(` + "app/templates/application.hbs + 1:4 error Non-translated string used no-bare-strings + 1:25 error Non-translated string used no-bare-strings + + ✖ 2 problems (2 errors, 0 warnings) + Report written to /my-results.json + " + `); + expect(getOutputFileContents(result.stdout)).toMatchInlineSnapshot(` + "{ + \\"app/templates/application.hbs\\": [ + { + \\"rule\\": \\"no-bare-strings\\", + \\"severity\\": 2, + \\"filePath\\": \\"app/templates/application.hbs\\", + \\"line\\": 1, + \\"column\\": 4, + \\"endLine\\": 1, + \\"endColumn\\": 14, + \\"source\\": \\"Here too!!\\", + \\"message\\": \\"Non-translated string used\\" + }, + { + \\"rule\\": \\"no-bare-strings\\", + \\"severity\\": 2, + \\"filePath\\": \\"app/templates/application.hbs\\", + \\"line\\": 1, + \\"column\\": 25, + \\"endLine\\": 1, + \\"endColumn\\": 48, + \\"source\\": \\"Bare strings are bad...\\", + \\"message\\": \\"Non-translated string used\\" + } + ] + }" + `); + expect(result.stderr).toBeFalsy(); + }); + + // it('should format errors and warnings', async function () { + // project.setConfig({ + // rules: { + // 'no-bare-strings': true, + // 'no-html-comments': 'warn', + // }, + // }); + // project.write({ + // app: { + // templates: { + // 'application.hbs': + // '

Here too!!

Bare strings are bad...
', + // }, + // }, + // }); + + // let result = await run(['.', '--format', 'json']); + + // expect(result.exitCode).toEqual(1); + // expect(result.stdout).toMatchInlineSnapshot(` + // "{ + // \\"app/templates/application.hbs\\": [ + // { + // \\"rule\\": \\"no-bare-strings\\", + // \\"severity\\": 2, + // \\"filePath\\": \\"app/templates/application.hbs\\", + // \\"line\\": 1, + // \\"column\\": 4, + // \\"endLine\\": 1, + // \\"endColumn\\": 14, + // \\"source\\": \\"Here too!!\\", + // \\"message\\": \\"Non-translated string used\\" + // }, + // { + // \\"rule\\": \\"no-bare-strings\\", + // \\"severity\\": 2, + // \\"filePath\\": \\"app/templates/application.hbs\\", + // \\"line\\": 1, + // \\"column\\": 24, + // \\"endLine\\": 1, + // \\"endColumn\\": 47, + // \\"source\\": \\"Bare strings are bad...\\", + // \\"message\\": \\"Non-translated string used\\" + // }, + // { + // \\"rule\\": \\"no-html-comments\\", + // \\"severity\\": 1, + // \\"filePath\\": \\"app/templates/application.hbs\\", + // \\"line\\": 1, + // \\"column\\": 53, + // \\"endLine\\": 1, + // \\"endColumn\\": 79, + // \\"source\\": \\"\\", + // \\"message\\": \\"HTML comment detected\\", + // \\"fix\\": { + // \\"text\\": \\"{{! bad html comment! }}\\" + // } + // } + // ] + // }" + // `); + // expect(result.stderr).toBeFalsy(); + // }); + + // it('should include information about available fixes', async function () { + // project.setConfig({ + // rules: { + // 'require-button-type': true, + // }, + // }); + + // project.write({ + // app: { + // components: { + // 'click-me-button.hbs': '', + // }, + // }, + // }); + + // let result = await run(['.', '--format', 'json']); + + // expect(result.exitCode).toEqual(1); + // expect(result.stdout).toMatchInlineSnapshot(` + // "{ + // \\"app/components/click-me-button.hbs\\": [ + // { + // \\"rule\\": \\"require-button-type\\", + // \\"severity\\": 2, + // \\"filePath\\": \\"app/components/click-me-button.hbs\\", + // \\"line\\": 1, + // \\"column\\": 0, + // \\"endLine\\": 1, + // \\"endColumn\\": 26, + // \\"source\\": \\"\\", + // \\"message\\": \\"All \`