From 2e9e0ed99ec9ff90af0e025fc80e41e2dcc50121 Mon Sep 17 00:00:00 2001 From: Kevin Yang <61671317+kevin-y-ang@users.noreply.github.com> Date: Tue, 21 Nov 2023 16:49:48 -0800 Subject: [PATCH] fix: incorporate feedback --- common/config/rush/pnpm-lock.yaml | 13 ++------ common/config/rush/repo-state.json | 2 +- eslint/eslint-bulk/README.md | 25 ++++++++------- eslint/eslint-bulk/src/start.ts | 4 +-- eslint/eslint-patch/README.md | 22 ++++++------- eslint/eslint-patch/package.json | 2 +- .../eslint-bulk-suppressions/ast-guards.ts | 5 +-- .../src/eslint-bulk-suppressions/cli/prune.ts | 25 +++------------ .../src/eslint-bulk-suppressions/cli/start.ts | 31 ++++++++++++++++--- .../eslint-bulk-suppressions/cli/suppress.ts | 2 +- .../cli/utils/print-help.ts | 30 +++++++++++------- .../generate-patched-file.ts | 4 +-- .../eslint-bulk-suppressions/path-utils.ts | 7 +---- eslint/eslint-patch/tsconfig.json | 2 -- 14 files changed, 86 insertions(+), 88 deletions(-) diff --git a/common/config/rush/pnpm-lock.yaml b/common/config/rush/pnpm-lock.yaml index 6bb73580f3e..30dcc902a1d 100644 --- a/common/config/rush/pnpm-lock.yaml +++ b/common/config/rush/pnpm-lock.yaml @@ -2268,8 +2268,8 @@ importers: specifier: 18.17.15 version: 18.17.15 '@typescript-eslint/types': - specifier: ~6.5.0 - version: 6.5.0(typescript@5.0.4) + specifier: ~5.59.2 + version: 5.59.11(typescript@5.0.4) typescript: specifier: ~5.0.4 version: 5.0.4 @@ -11817,15 +11817,6 @@ packages: dependencies: typescript: 5.0.4 - /@typescript-eslint/types@6.5.0(typescript@5.0.4): - resolution: {integrity: sha512-eqLLOEF5/lU8jW3Bw+8auf4lZSbbljHR2saKnYqON12G/WsJrGeeDHWuQePoEf9ro22+JkbPfWQwKEC5WwLQ3w==} - engines: {node: ^16.0.0 || >=18.0.0} - peerDependencies: - typescript: '*' - dependencies: - typescript: 5.0.4 - dev: true - /@typescript-eslint/types@6.7.3(typescript@5.0.4): resolution: {integrity: sha512-4g+de6roB2NFcfkZb439tigpAMnvEIg3rIjWQ+EM7IBaYt/CdJt6em9BJ4h4UpdgaBWdmx2iWsafHTrqmgIPNw==} engines: {node: ^16.0.0 || >=18.0.0} diff --git a/common/config/rush/repo-state.json b/common/config/rush/repo-state.json index 7f4e8bcb490..35a6be621b1 100644 --- a/common/config/rush/repo-state.json +++ b/common/config/rush/repo-state.json @@ -1,5 +1,5 @@ // DO NOT MODIFY THIS FILE MANUALLY BUT DO COMMIT IT. It is generated and used by Rush. { - "pnpmShrinkwrapHash": "c21a1b9c1c70b123e0f49d27d8f56a39094a067a", + "pnpmShrinkwrapHash": "15c2cb269e436071e883ba0b7b76515ea824c01a", "preferredVersionsHash": "1926a5b12ac8f4ab41e76503a0d1d0dccc9c0e06" } diff --git a/eslint/eslint-bulk/README.md b/eslint/eslint-bulk/README.md index 1cadde48b0e..cb4a0a12250 100755 --- a/eslint/eslint-bulk/README.md +++ b/eslint/eslint-bulk/README.md @@ -1,29 +1,32 @@ # @rushstack/eslint-bulk -This is a companion package for @rushstack/eslint-patch. +This is a companion package for @rushstack/eslint-patch that should be installed globally as follows: +```bash +npm i -g @rushstack/eslint-bulk +``` -The **eslint-bulk** package is a set of command line tools to use with the ESLint bulk suppressions -patch. The commands are optional as they are just a thin wrapper over eslint shipped with the correct -environment variables to interface with the patch. +The **eslint-bulk** package is a set of command line tools to use with the ESLint bulk suppressions patch. +eslint-bulk commands must be run in the same current working directory containing your package's pertaining +.eslintrc.js or .eslintrc.cjs file. ## eslint-bulk suppress Use this command to automatically generate bulk suppressions for the given files and given rules. -Supply the files as the main argument. The "files" argument is a glob pattern that follows the same -rules as the "eslint" command. +Supply the paths as the main argument. The paths argument is a glob pattern that follows the same +rules as the "files" argument in the "eslint" command. ```bash - eslint-bulk suppress --rule rule1 --rule rule2 path/to/file1 path/to/file2 path/to/directory +eslint-bulk suppress --rule NAME1 [--rule NAME2...] PATH1 [PATH2...] +eslint-bulk suppress --all PATH1 [PATH2...] ``` ## eslint-bulk prune -Use this command to automatically delete unused suppression entries for the given files in the -corresponding .eslint-bulk-suppressions.json file(s). Supply the files as the main argument. The -"files" argument is a glob pattern that follows the same rules as the "eslint" command. +Use this command to automatically delete all unused suppression entries in all .eslint-bulk-suppressions.json +files under the current working directory. ```bash - eslint-bulk prune path/to/file1 path/to/file2 path/to/directory +eslint-bulk prune ``` # Links diff --git a/eslint/eslint-bulk/src/start.ts b/eslint/eslint-bulk/src/start.ts index bb87fbb2f1d..e9ee6ff2538 100644 --- a/eslint/eslint-bulk/src/start.ts +++ b/eslint/eslint-bulk/src/start.ts @@ -30,11 +30,11 @@ function findPatchPath(): string { process.exit(1); } - const env: { [key: string]: string } = { ...process.env, _RUSHSTACK_ESLINT_BULK_DETECT: 'true' }; + const env: NodeJS.ProcessEnv = { ...process.env, _RUSHSTACK_ESLINT_BULK_DETECT: 'true' }; let stdout: Buffer; try { - stdout = execSync(`echo "" | eslint --stdin --config ${eslintrcPath}`, { env, stdio: 'pipe' }); + stdout = execSync(`eslint --stdin --config ${eslintrcPath}`, { env, input: '', stdio: 'pipe' }); } catch (e) { console.error('@rushstack/eslint-bulk: Error finding patch path: ' + e.message); process.exit(1); diff --git a/eslint/eslint-patch/README.md b/eslint/eslint-patch/README.md index 6d7746e2381..231a5d5e509 100644 --- a/eslint/eslint-patch/README.md +++ b/eslint/eslint-patch/README.md @@ -128,28 +128,28 @@ We also highly recommend globally installing the companion CLI tool to your loca npm i -g @rushstack/eslint-bulk ``` -The **eslint-bulk** package is a set of command line tools to use with the ESLint bulk suppressions -patch. The commands are optional as they are just a thin wrapper over eslint shipped with the correct -environment variables to interface with the patch. +The **eslint-bulk** package is a set of command line tools to use with the ESLint bulk suppressions patch. +eslint-bulk commands must be run in the same current working directory containing your package's pertaining +.eslintrc.js or .eslintrc.cjs file. ## eslint-bulk suppress -Use this command to automatically either generate a new .eslint-bulk-suppressions.json file next to -the corresponding .eslintrc.js file for the given files and given rules. Supply the files as the main -argument. The "files" argument is a glob pattern that follows the same rules as the "eslint" command. +Use this command to automatically generate bulk suppressions for the given files and given rules. +Supply the paths as the main argument. The paths argument is a glob pattern that follows the same +rules as the "files" argument in the "eslint" command. ```bash -eslint-bulk suppress path/to/file1 path/to/file2 path/to/directory --rule rule1 --rule rule2 +eslint-bulk suppress --rule NAME1 [--rule NAME2...] PATH1 [PATH2...] +eslint-bulk suppress --all PATH1 [PATH2...] ``` ## eslint-bulk prune -Use this command to automatically delete unused suppression entries for the given files in the -corresponding .eslint-bulk-suppressions.json file(s). Supply the files as the main argument. The -"files" argument is a glob pattern thatfollows the same rules as the "eslint" command. +Use this command to automatically delete all unused suppression entries in all .eslint-bulk-suppressions.json +files under the current working directory. ```bash -eslint-bulk prune path/to/file1 path/to/file2 path/to/directory +eslint-bulk prune ``` # Links diff --git a/eslint/eslint-patch/package.json b/eslint/eslint-patch/package.json index 731edef5553..2afeaa89d59 100644 --- a/eslint/eslint-patch/package.json +++ b/eslint/eslint-patch/package.json @@ -29,6 +29,6 @@ "@rushstack/heft-node-rig": "2.3.2", "@types/node": "18.17.15", "typescript": "~5.0.4", - "@typescript-eslint/types": "~6.5.0" + "@typescript-eslint/types": "~5.59.2" } } diff --git a/eslint/eslint-patch/src/eslint-bulk-suppressions/ast-guards.ts b/eslint/eslint-patch/src/eslint-bulk-suppressions/ast-guards.ts index 94c23e0bd91..46e15a78e8e 100644 --- a/eslint/eslint-patch/src/eslint-bulk-suppressions/ast-guards.ts +++ b/eslint/eslint-patch/src/eslint-bulk-suppressions/ast-guards.ts @@ -3,9 +3,6 @@ import type { TSESTree } from '@typescript-eslint/types'; -/** https://twitter.com/branmcconnell/status/1623077891423731712 */ -export type Compile = T extends object ? { [K in keyof T]: Compile } : T; - export function isArrayExpression(node: TSESTree.Node): node is TSESTree.ArrayExpression { return node.type === 'ArrayExpression'; } @@ -193,7 +190,7 @@ export function isNormalObjectProperty(node: TSESTree.Node): node is NormalObjec return isProperty(node) && (isIdentifier(node.key) || isPrivateIdentifier(node.key)); } -export interface NormalVariableDeclarator extends TSESTree.LetOrConstOrVarDeclarator { +export interface NormalVariableDeclarator extends TSESTree.VariableDeclarator { id: TSESTree.Identifier; init: TSESTree.Expression; } diff --git a/eslint/eslint-patch/src/eslint-bulk-suppressions/cli/prune.ts b/eslint/eslint-patch/src/eslint-bulk-suppressions/cli/prune.ts index 5b2a076bbc5..0326af598f8 100755 --- a/eslint/eslint-patch/src/eslint-bulk-suppressions/cli/prune.ts +++ b/eslint/eslint-patch/src/eslint-bulk-suppressions/cli/prune.ts @@ -13,29 +13,16 @@ export function prune() { process.exit(0); } - const parsedArgs = args.reduce<{ - files: string[]; - }>( - (acc, arg, index, arr) => { - acc.files.push(arg); - return acc; - }, - { files: [] } - ); - - if (parsedArgs.files.length === 0) { - throw new Error( - '@rushstack/eslint-bulk: Files argument is required. Use glob patterns to specify files or use `.` to suppress all files for the specified rules.' - ); + if (args.length > 0) { + throw new Error(`@rushstack/eslint-bulk: Unknown arguments: ${args.join(' ')}`); } const eslintCLI = getEslintCli(process.cwd()); - const env = Object.assign({}, process.env); - env.ESLINT_BULK_PRUNE = 'true'; + const env: NodeJS.ProcessEnv = { ...process.env, ESLINT_BULK_PRUNE: 'true' }; exec( - `${eslintCLI} ${parsedArgs.files.join(' ')} --format=json`, + `${eslintCLI} . --format=json`, { env }, (error: ExecException | null, stdout: string, stderr: string) => { // if errorCount != 0, ESLint will process.exit(1) giving the false impression @@ -52,9 +39,7 @@ export function prune() { } console.log( - `@rushstack/eslint-bulk: Successfully pruned unused suppressions in .eslint-bulk-suppressions.json at ${process.cwd()} for ${ - parsedArgs.files - }` + `@rushstack/eslint-bulk: Successfully pruned unused suppressions in all .eslint-bulk-suppressions.json files under directory ${process.cwd()}` ); } ); diff --git a/eslint/eslint-patch/src/eslint-bulk-suppressions/cli/start.ts b/eslint/eslint-patch/src/eslint-bulk-suppressions/cli/start.ts index 30e16022466..b5a8c21fbd2 100644 --- a/eslint/eslint-patch/src/eslint-bulk-suppressions/cli/start.ts +++ b/eslint/eslint-patch/src/eslint-bulk-suppressions/cli/start.ts @@ -11,17 +11,40 @@ if (process.argv.includes('-h') || process.argv.includes('-H') || process.argv.i process.exit(0); } +if (process.argv.length < 3) { + printHelp(); + process.exit(1); +} + if (!isCorrectCwd(process.cwd())) { - throw new Error( + console.error( '@rushstack/eslint-bulk: Please call this command from the directory that contains .eslintrc.js or .eslintrc.cjs' ); + process.exit(1); } const subcommand = process.argv[2]; if (subcommand === 'suppress') { - suppress(); + try { + suppress(); + } catch (e) { + if (e instanceof Error) { + console.error(e.message); + process.exit(1); + } + throw e; + } } else if (subcommand === 'prune') { - prune(); + try { + prune(); + } catch (e) { + if (e instanceof Error) { + console.error(e.message); + process.exit(1); + } + throw e; + } } else { - throw new Error('@rushstack/eslint-bulk: Unknown subcommand: ' + subcommand); + console.error('@rushstack/eslint-bulk: Unknown subcommand: ' + subcommand); + process.exit(1); } diff --git a/eslint/eslint-patch/src/eslint-bulk-suppressions/cli/suppress.ts b/eslint/eslint-patch/src/eslint-bulk-suppressions/cli/suppress.ts index 24e90b10482..0058c095b82 100755 --- a/eslint/eslint-patch/src/eslint-bulk-suppressions/cli/suppress.ts +++ b/eslint/eslint-patch/src/eslint-bulk-suppressions/cli/suppress.ts @@ -62,7 +62,7 @@ export function suppress() { ); } - const env = Object.assign({}, process.env); + const env: NodeJS.ProcessEnv = { ...process.env }; if (parsedArgs.all) { env.ESLINT_BULK_SUPPRESS = '*'; } else if (parsedArgs.rules.length > 0) { diff --git a/eslint/eslint-patch/src/eslint-bulk-suppressions/cli/utils/print-help.ts b/eslint/eslint-patch/src/eslint-bulk-suppressions/cli/utils/print-help.ts index 23b1ba4ecd6..ceb8489618f 100644 --- a/eslint/eslint-patch/src/eslint-bulk-suppressions/cli/utils/print-help.ts +++ b/eslint/eslint-patch/src/eslint-bulk-suppressions/cli/utils/print-help.ts @@ -4,13 +4,13 @@ import { wrapWordsToLines } from './wrap-words-to-lines'; export function printPruneHelp() { - const help = `Usage: eslint-bulk prune [options] + const help = `eslint-bulk prune -This command is a thin wrapper around ESLint that communicates with @rushstack/eslint-patch to delete all unused suppression entries in the local .eslint-bulk-suppressions.json file that correspond to the target files given as arguments. +Usage: -Argument: - - Glob patterns for paths to suppress, same as eslint files argument. Should be relative to the project root.`; +eslint-bulk prune + +This command is a thin wrapper around ESLint that communicates with @rushstack/eslint-patch to delete all unused suppression entries in all .eslint-bulk-suppressions.json files under the current working directory.`; const wrapped = wrapWordsToLines(help); for (const line of wrapped) { @@ -23,11 +23,11 @@ export function printHelp() { Usage: -eslint-bulk suppress --rule NAME1 [--rule NAME2...] PATH1 [PATH2...] +eslint-bulk suppress --rule RULENAME1 [--rule RULENAME2...] PATH1 [PATH2...] eslint-bulk suppress --all PATH1 [PATH2...] eslint-bulk suppress --help -eslint-bulk prune PATH1 [PATH2...] +eslint-bulk prune eslint-bulk prune --help eslint-bulk --help @@ -36,10 +36,12 @@ This command line tool is a thin wrapper around ESLint that communicates with @r Commands: eslint-bulk suppress [options] + Use this command to generate a new .eslint-bulk-suppressions.json file or add suppression entries to the existing file. Specify the files and rules you want to suppress. Please run "eslint-bulk suppress --help" to learn more. - eslint-bulk prune - Please run "eslint-bulk prune --help" to learn how to use this command. + eslint-bulk prune + Use this command to delete all unused suppression entries in all .eslint-bulk-suppressions.json files under the current working directory. + Please run "eslint-bulk prune --help" to learn more. `; const wrapped = wrapWordsToLines(help); @@ -49,7 +51,13 @@ Commands: } export function printSuppressHelp() { - const help = `Usage: eslint-bulk suppress [options] + const help = `eslint-bulk suppress [options] + +Usage: + +eslint-bulk suppress --rule RULENAME1 [--rule RULENAME2...] PATH1 [PATH2...] +eslint-bulk suppress --all PATH1 [PATH2...] +eslint-bulk suppress --help This command is a thin wrapper around ESLint that communicates with @rushstack/eslint-patch to either generate a new .eslint-bulk-suppressions.json file or add suppression entries to the existing file. Specify the files and rules you want to suppress. @@ -62,7 +70,7 @@ Options: Display this help message. -R, --rule - The full name of the ESLint rule you want to bulk-suppress. Specify multiple rules with --rule NAME1 --rule NAME2. + The full name of the ESLint rule you want to bulk-suppress. Specify multiple rules with --rule NAME1 --rule RULENAME2. -A, --all Bulk-suppress all rules in the specified file patterns.`; diff --git a/eslint/eslint-patch/src/eslint-bulk-suppressions/generate-patched-file.ts b/eslint/eslint-patch/src/eslint-bulk-suppressions/generate-patched-file.ts index 321850eb961..f9232240b25 100644 --- a/eslint/eslint-patch/src/eslint-bulk-suppressions/generate-patched-file.ts +++ b/eslint/eslint-patch/src/eslint-bulk-suppressions/generate-patched-file.ts @@ -95,7 +95,6 @@ export function generatePatchedFileIfDoesntExist(inputFilePath: string, outputFi } /** - * * @param {number} indexToScanTo * @returns {string} */ @@ -186,8 +185,6 @@ const requireFromPathToLinterJS = bulkSuppressionsPatch.requireFromPathToLinterJ // --- END MONKEY PATCH --- `; - // TODO: MORE PATCHING GOES HERE - // Match this: // nodeQueue.forEach((traversalInfo) => { // currentNode = traversalInfo.node; @@ -205,6 +202,7 @@ const requireFromPathToLinterJS = bulkSuppressionsPatch.requireFromPathToLinterJ // }); // // return lintingProblems; + // // Convert to this: // nodeQueue.forEach((traversalInfo) => { // currentNode = traversalInfo.node; diff --git a/eslint/eslint-patch/src/eslint-bulk-suppressions/path-utils.ts b/eslint/eslint-patch/src/eslint-bulk-suppressions/path-utils.ts index 1ca26050547..3001a351fe5 100644 --- a/eslint/eslint-patch/src/eslint-bulk-suppressions/path-utils.ts +++ b/eslint/eslint-patch/src/eslint-bulk-suppressions/path-utils.ts @@ -36,12 +36,7 @@ export function getPathToLinterJS(): string { } export function getPathToGeneratedPatch(patchPath: string, nameOfGeneratedPatchFile: string): string { - if (!fs.existsSync(path.join(patchPath, 'temp'))) { - fs.mkdirSync(path.join(patchPath, 'temp')); - } - if (!fs.existsSync(path.join(patchPath, 'temp', 'patches'))) { - fs.mkdirSync(path.join(patchPath, 'temp', 'patches')); - } + fs.mkdirSync(path.join(patchPath, 'temp', 'patches'), { recursive: true }); const pathToGeneratedPatch = path.join(patchPath, 'temp', 'patches', nameOfGeneratedPatchFile); return pathToGeneratedPatch; diff --git a/eslint/eslint-patch/tsconfig.json b/eslint/eslint-patch/tsconfig.json index 2f99a822345..c67fe4658e6 100644 --- a/eslint/eslint-patch/tsconfig.json +++ b/eslint/eslint-patch/tsconfig.json @@ -1,8 +1,6 @@ { "extends": "./node_modules/@rushstack/heft-node-rig/profiles/default/tsconfig-base.json", "compilerOptions": { - "module": "NodeNext", - "moduleResolution": "NodeNext", "types": ["node"] } }