From 9d1a299fe24e9c9d51bd17bc58a8088607f78b1c Mon Sep 17 00:00:00 2001 From: Ari Perkkio Date: Tue, 9 Feb 2021 21:03:54 +0200 Subject: [PATCH] feat: enable checking all rules --- README.md | 34 ++++++------ lib/config/types.ts | 4 +- lib/config/validator.ts | 17 ++++-- lib/engine/worker-task.ts | 50 ++++++++++------- test/integration/integration.test.ts | 82 ++++++++++++++++++++++++++++ test/unit/config.test.ts | 17 +++++- test/utils.ts | 5 ++ 7 files changed, 165 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index b884922a..a52d78c5 100644 --- a/README.md +++ b/README.md @@ -74,23 +74,23 @@ module.exports = { #### Configuration options -| Name | Description                               | Type | Required | Default | Example | -| :-------------------------- | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :--------------------------------------------------------------------------------- | :----------------- | :------------------------------------- | :------------------------------------------ | -| `repositories` | Repositories to scan in format of `owner/project` | `string[]` | :white_check_mark: | :x: | `['mui-org/material-ui', 'reach/reach-ui']` | -| `extensions` | Extensions of files under scanning | `string[]` | :white_check_mark: | :x: | `['js', 'jsx', 'ts', 'tsx']` | -| `eslintrc` | ESLint configuration | See [Configuring ESLint] | :white_check_mark: | :x: | `{ root: true, extends: ['eslint:all'] }` | -| `pathIgnorePattern` | Regexp pattern string used to exclude paths | `string` | :x: | :x: | `(node_modules\|docs\|\\/\\.git)` | -| `maxFileSizeBytes` | Max file size used to exclude bigger files | `number` | :x: | `2000000` | `1500000` | -| `rulesUnderTesting` | Array of rules used to filter out results. Use `undefined` or empty array when ESLint crashes are the only interest. | `string[]` | :x: | `[]` | `['no-empty', 'react/sort-prop-types']` | -| `resultParser` | Syntax for the result parser | `plaintext\|markdown` | :x: | `markdown` on CLI. `plaintext` on CI | `markdown` | -| `concurrentTasks` | Maximum amount of tasks run concurrently | `number` | :x: | `5` | `3` | -| `CI` | Flag used to set CI mode. `process.env.CI` is used when not set. | `boolean` | :x: | value of `process.env.CI === 'true'` | `true` | -| `logLevel` | Filter log messages based on their priority | `verbose\|info\|warn\|error` | :x: | `verbose` | `warn` | -| `cache` | Flag used to enable caching of cloned repositories. For CIs it's ideal to disable caching due to limited disk space. | `boolean` | :x: | `true` | `true` | -| `timeLimit` | Time limit before scan is interrupted and **exited successfully**. Ideal for avoiding CI timeouts in regression tests. | `number` | :x: | `5.5 * 60 * 60, // 5 hours 30 minutes` | `5 * 60 * 60 // 5 hours` | -| `compare` | Flag used to enable result comparison mode. Compares results of two scans and output the diff. Ideal for identifying new false positives when fixing existing rules. See [Fixing existing rules]. | `boolean` | :x: | `false` | `true` | -| `updateComparisonReference` | Flag used to enable result comparison reference updating. Indicates whether comparison base should be updated after scan has finished. Ideal to be turned off once initial comparison base has been collected. | `boolean` | :x: | `true` | `true` | -| `onComplete` | Callback invoked once scan is completed. Asynchronous functions are supported. Ideal for extending the process with custom features. | `(results, comparisonResults) => void`\|`Promise`. See [onComplete example]. | :x: | :x: | `async (results, comparisonResults) => {}` | +| Name | Description                               | Type | Required | Default | Example | +| :-------------------------- | :------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :--------------------------------------------------------------------------------- | :----------------- | :------------------------------------- | :-------------------------------------------------------------------------------------------------------------------------- | +| `repositories` | Repositories to scan in format of `owner/project` | `string[]` | :white_check_mark: | :x: | `['mui-org/material-ui', 'reach/reach-ui']` | +| `extensions` | Extensions of files under scanning | `string[]` | :white_check_mark: | :x: | `['js', 'jsx', 'ts', 'tsx']` | +| `eslintrc` | ESLint configuration | See [Configuring ESLint] | :white_check_mark: | :x: | `{ root: true, extends: ['eslint:all'] }` | +| `pathIgnorePattern` | Regexp pattern string used to exclude paths | `string` | :x: | :x: | `(node_modules\|docs\|\\/\\.git)` | +| `maxFileSizeBytes` | Max file size used to exclude bigger files | `number` | :x: | `2000000` | `1500000` | +| `rulesUnderTesting` | Array of rules or a filter method used to filter out results. Use `undefined` or empty array when ESLint crashes are the only interest. Filter method is called with `ruleId` and `options`. | `string[] \| (ruleId, { repository }) => boolean` | :x: | `[]` | `['no-empty', 'react/sort-prop-types']` `(ruleId, options) => ruleId === 'no-undef' && options.repository === 'owner/repo'` | +| `resultParser` | Syntax for the result parser | `plaintext\|markdown` | :x: | `markdown` on CLI. `plaintext` on CI | `markdown` | +| `concurrentTasks` | Maximum amount of tasks run concurrently | `number` | :x: | `5` | `3` | +| `CI` | Flag used to set CI mode. `process.env.CI` is used when not set. | `boolean` | :x: | value of `process.env.CI === 'true'` | `true` | +| `logLevel` | Filter log messages based on their priority | `verbose\|info\|warn\|error` | :x: | `verbose` | `warn` | +| `cache` | Flag used to enable caching of cloned repositories. For CIs it's ideal to disable caching due to limited disk space. | `boolean` | :x: | `true` | `true` | +| `timeLimit` | Time limit before scan is interrupted and **exited successfully**. Ideal for avoiding CI timeouts in regression tests. | `number` | :x: | `5.5 * 60 * 60, // 5 hours 30 minutes` | `5 * 60 * 60 // 5 hours` | +| `compare` | Flag used to enable result comparison mode. Compares results of two scans and output the diff. Ideal for identifying new false positives when fixing existing rules. See [Fixing existing rules]. | `boolean` | :x: | `false` | `true` | +| `updateComparisonReference` | Flag used to enable result comparison reference updating. Indicates whether comparison base should be updated after scan has finished. Ideal to be turned off once initial comparison base has been collected. | `boolean` | :x: | `true` | `true` | +| `onComplete` | Callback invoked once scan is completed. Asynchronous functions are supported. Ideal for extending the process with custom features. | `(results, comparisonResults) => void`\|`Promise`. See [onComplete example]. | :x: | :x: | `async (results, comparisonResults) => {}` | [configuring eslint]: https://eslint.org/docs/user-guide/configuring [fixing existing rules]: #fixing-existing-rules diff --git a/lib/config/types.ts b/lib/config/types.ts index 2c3a26fb..f141e380 100644 --- a/lib/config/types.ts +++ b/lib/config/types.ts @@ -18,7 +18,9 @@ export interface Config { extensions: string[]; pathIgnorePattern?: RegExp; maxFileSizeBytes: number; - rulesUnderTesting: string[]; + rulesUnderTesting: + | string[] + | ((ruleId: string, options: { repository: string }) => boolean); resultParser: ResultParser; concurrentTasks: number; eslintrc: Linter.Config; diff --git a/lib/config/validator.ts b/lib/config/validator.ts index 06581f70..26265b34 100644 --- a/lib/config/validator.ts +++ b/lib/config/validator.ts @@ -146,10 +146,19 @@ export default async function validate( // TODO nice-to-have: Validate rules match eslintrc config // https://eslint.org/docs/developer-guide/nodejs-api#lintergetrules - if (rulesUnderTesting && rulesUnderTesting.length) { - errors.push( - validateStringArray('rulesUnderTesting', rulesUnderTesting) - ); + if (rulesUnderTesting) { + if (Array.isArray(rulesUnderTesting)) { + // Empty rulesUnderTesting is valid + if (rulesUnderTesting.length) { + errors.push( + validateStringArray('rulesUnderTesting', rulesUnderTesting) + ); + } + } else if (typeof rulesUnderTesting !== 'function') { + errors.push( + `rulesUnderTesting should be either an array or function.` + ); + } } if (pathIgnorePattern) { diff --git a/lib/engine/worker-task.ts b/lib/engine/worker-task.ts index e0ad77cb..8cc04fcd 100644 --- a/lib/engine/worker-task.ts +++ b/lib/engine/worker-task.ts @@ -68,27 +68,36 @@ async function executionTimeWarningWrapper( * Picks out messages which are under testing and constructs a small snippet of * the erroneous code block */ -function mergeMessagesWithSource( - all: Linter.LintMessage[], - result: ESLint.LintResult -): Linter.LintMessage[] { - const messages = result.messages.filter( - message => - message.ruleId && config.rulesUnderTesting.includes(message.ruleId) - ); - - // Process only rules that are under testing - if (messages.length === 0) { - return all; +function getMessageReducer(repository: string) { + function messageFilter(message: Linter.LintMessage) { + if (!message.ruleId) return false; + + if (typeof config.rulesUnderTesting === 'function') { + return config.rulesUnderTesting(message.ruleId, { repository }); + } + + return config.rulesUnderTesting.includes(message.ruleId); } - return [ - ...all, - ...messages.map(message => ({ - ...message, - source: constructCodeFrame(result.source, message), - })), - ]; + return function reducer( + all: Linter.LintMessage[], + result: ESLint.LintResult + ): Linter.LintMessage[] { + const messages = result.messages.filter(messageFilter); + + // Process only rules that are under testing + if (messages.length === 0) { + return all; + } + + return [ + ...all, + ...messages.map(message => ({ + ...message, + source: constructCodeFrame(result.source, message), + })), + ]; + }; } /** @@ -162,6 +171,7 @@ export default async function workerTask(): Promise { }); const { repository } = workerData as WorkerData; + const messageReducer = getMessageReducer(repository); const files = await getFiles({ repository, @@ -210,7 +220,7 @@ export default async function workerTask(): Promise { } const messages = result - .reduce(mergeMessagesWithSource, []) + .reduce(messageReducer, []) .filter(Boolean) .map(message => ({ ...message, path })); diff --git a/test/integration/integration.test.ts b/test/integration/integration.test.ts index f83e906c..67867118 100644 --- a/test/integration/integration.test.ts +++ b/test/integration/integration.test.ts @@ -978,4 +978,86 @@ describe('integration', () => { [TEST-ON-COMPLETE-END]" `); }); + + test('enables all rules when rulesUnderTesting returns true', async () => { + await runProductionBuild({ + CI: false, + rulesUnderTesting: () => true, + eslintrc: { root: true, extends: ['eslint:all'] }, + }); + const results = getResults(); + const rules = results.match(/Rule: (\S)*/g) || []; + + expect(rules.join('\n')).toMatchInlineSnapshot(` + "Rule: no-undef + Rule: no-var + Rule: no-implicit-globals + Rule: no-undef + Rule: no-empty + Rule: one-var + Rule: vars-on-top + Rule: no-var + Rule: no-implicit-globals + Rule: id-length + Rule: quote-props + Rule: getter-return + Rule: space-before-function-paren + Rule: strict + Rule: space-before-blocks + Rule: capitalized-comments + Rule: no-compare-neg-zero + Rule: no-magic-numbers + Rule: indent + Rule: capitalized-comments + Rule: eol-last" + `); + }); + + test('calls rulesUnderTesting filter with ruleId and repository', async () => { + const { output } = await runProductionBuild({ + CI: false, + rulesUnderTesting: (ruleId, options) => { + const { parentPort } = require('worker_threads'); + + parentPort.postMessage({ + type: 'DEBUG', + payload: `${ruleId} - ${options.repository}`, + }); + return true; + }, + eslintrc: { root: true, extends: ['eslint:all'] }, + }); + + const finalLog = output.pop(); + const withoutTimestamps = finalLog!.replace(/\[DEBUG (\S|:)*\]/g, ''); + + expect(withoutTimestamps).toMatchInlineSnapshot(` + "Full log: + no-undef - AriPerkkio/eslint-remote-tester-integration-test-target + no-var - AriPerkkio/eslint-remote-tester-integration-test-target + no-implicit-globals - AriPerkkio/eslint-remote-tester-integration-test-target + no-undef - AriPerkkio/eslint-remote-tester-integration-test-target + no-empty - AriPerkkio/eslint-remote-tester-integration-test-target + one-var - AriPerkkio/eslint-remote-tester-integration-test-target + vars-on-top - AriPerkkio/eslint-remote-tester-integration-test-target + no-var - AriPerkkio/eslint-remote-tester-integration-test-target + no-implicit-globals - AriPerkkio/eslint-remote-tester-integration-test-target + id-length - AriPerkkio/eslint-remote-tester-integration-test-target + quote-props - AriPerkkio/eslint-remote-tester-integration-test-target + getter-return - AriPerkkio/eslint-remote-tester-integration-test-target + space-before-function-paren - AriPerkkio/eslint-remote-tester-integration-test-target + strict - AriPerkkio/eslint-remote-tester-integration-test-target + space-before-blocks - AriPerkkio/eslint-remote-tester-integration-test-target + capitalized-comments - AriPerkkio/eslint-remote-tester-integration-test-target + no-compare-neg-zero - AriPerkkio/eslint-remote-tester-integration-test-target + no-magic-numbers - AriPerkkio/eslint-remote-tester-integration-test-target + indent - AriPerkkio/eslint-remote-tester-integration-test-target + capitalized-comments - AriPerkkio/eslint-remote-tester-integration-test-target + eol-last - AriPerkkio/eslint-remote-tester-integration-test-target + [ERROR] AriPerkkio/eslint-remote-tester-integration-test-target 21 errors + [DONE] Finished scan of 1 repositories + + " + `); + }); }); diff --git a/test/unit/config.test.ts b/test/unit/config.test.ts index 7e98d790..b390b2e4 100644 --- a/test/unit/config.test.ts +++ b/test/unit/config.test.ts @@ -162,6 +162,12 @@ describe('validateConfig', () => { rulesUnderTesting: undefined, }); }); + test('rulesUnderTesting accepts empty array', async () => { + await validateConfig({ + ...DEFAULT_CONFIGURATION, + rulesUnderTesting: [], + }); + }); test('rulesUnderTesting accepts array with unique strings', async () => { await validateConfig({ @@ -177,6 +183,13 @@ describe('validateConfig', () => { }); }); + test('rulesUnderTesting accepts function', async () => { + await validateConfig({ + ...DEFAULT_CONFIGURATION, + rulesUnderTesting: () => true, + }); + }); + test('pathIgnorePattern is optional', async () => { await validateConfig({ ...DEFAULT_CONFIGURATION, @@ -481,7 +494,7 @@ describe('validateConfig', () => { ); }); - test('rulesUnderTesting requires an array', async () => { + test('rulesUnderTesting requires an array or function', async () => { const rulesUnderTesting: any = { length: 10 }; await validateConfig({ ...DEFAULT_CONFIGURATION, @@ -490,7 +503,7 @@ describe('validateConfig', () => { const [validationError] = getConsoleLogCalls(); expect(validationError).toMatch( - 'rulesUnderTesting should be an array.' + 'rulesUnderTesting should be either an array or function.' ); }); diff --git a/test/utils.ts b/test/utils.ts index cfee7abd..1baca78a 100644 --- a/test/utils.ts +++ b/test/utils.ts @@ -20,6 +20,7 @@ export const REPOSITORY_CACHE = `${CACHE_LOCATION}/${INTEGRATION_REPO_OWNER}/${I const LAST_RENDER_PATTERN = /(Results|Full log)[\s|\S]*/; const COMPARISON_RESULTS_PATTERN = /(Comparison results:[\s|\S]*)Results/; const ON_COMPLETE_PATTERN = /("onComplete": )"([\s|\S]*)"/; +const RULES_UNDER_TESTING_PATTERN = /("rulesUnderTesting": )"([\s|\S]*)",/; const ESCAPED_NEWLINE_PATTERN = /\\n/g; let idCounter = 0; @@ -42,9 +43,13 @@ function createConfiguration( if (options.onComplete) { config.onComplete = options.onComplete.toString() as any; } + if (typeof options.rulesUnderTesting === 'function') { + config.rulesUnderTesting = options.rulesUnderTesting.toString() as any; + } const configText = JSON.stringify(config, null, 4) .replace(ON_COMPLETE_PATTERN, '$1$2') + .replace(RULES_UNDER_TESTING_PATTERN, '$1$2,') .replace(ESCAPED_NEWLINE_PATTERN, '\n'); fs.writeFileSync(name, `module.exports=${configText}`, 'utf8');