From 0fa421b26f3d126e77cadcfa2ebb31f09e980fc3 Mon Sep 17 00:00:00 2001 From: Kanthesha Devaramane Date: Thu, 18 Jan 2024 11:52:26 +0000 Subject: [PATCH 1/4] added lint related dependencies conform and related unit tests --- src/main.test.ts | 14 +- src/rules/index.ts | 2 + ...ackage-engines-node-field-conforms.test.ts | 24 +++- ...package-lint-dependencies-conforms.test.ts | 134 ++++++++++++++++++ .../package-lint-dependencies-conforms.ts | 103 ++++++++++++++ .../package-manager-field-conforms.test.ts | 24 +++- .../require-valid-package-manifest.test.ts | 8 +- src/rules/types.ts | 2 + 8 files changed, 297 insertions(+), 14 deletions(-) create mode 100644 src/rules/package-lint-dependencies-conforms.test.ts create mode 100644 src/rules/package-lint-dependencies-conforms.ts diff --git a/src/main.test.ts b/src/main.test.ts index de124e3..644aac3 100644 --- a/src/main.test.ts +++ b/src/main.test.ts @@ -73,6 +73,7 @@ describe('main', () => { JSON.stringify({ packageManager: 'yarn', engines: { node: 'test' }, + devDependencies: { eslint: '1.1.0' }, }), ); await writeFile( @@ -106,6 +107,7 @@ repo-1 - Does the package have a well-formed manifest (\`package.json\`)? ✅ - Does the \`packageManager\` field in \`package.json\` conform? ✅ - Does the \`engines.node\` field in \`package.json\` conform? ✅ + - Does the lint related \`devDependencies\` in \`package.json\` conform? ✅ - Is \`README.md\` present? ✅ - Does the README conform by recommending the correct Yarn version to install? ✅ - Does the README conform by recommending node install from nodejs.org? ✅ @@ -114,7 +116,7 @@ repo-1 - Does the \`src/\` directory exist? ✅ - Is \`.nvmrc\` present, and does it conform? ✅ -Results: 11 passed, 0 failed, 11 total +Results: 12 passed, 0 failed, 12 total Elapsed time: 0 ms @@ -125,6 +127,7 @@ repo-2 - Does the package have a well-formed manifest (\`package.json\`)? ✅ - Does the \`packageManager\` field in \`package.json\` conform? ✅ - Does the \`engines.node\` field in \`package.json\` conform? ✅ + - Does the lint related \`devDependencies\` in \`package.json\` conform? ✅ - Is \`README.md\` present? ✅ - Does the README conform by recommending the correct Yarn version to install? ✅ - Does the README conform by recommending node install from nodejs.org? ✅ @@ -133,7 +136,7 @@ repo-2 - Does the \`src/\` directory exist? ✅ - Is \`.nvmrc\` present, and does it conform? ✅ -Results: 11 passed, 0 failed, 11 total +Results: 12 passed, 0 failed, 12 total Elapsed time: 0 ms `, @@ -337,6 +340,7 @@ Elapsed time: 0 ms JSON.stringify({ packageManager: 'yarn', engines: { node: 'test' }, + devDependencies: { eslint: '1.1.0' }, }), ); await writeFile( @@ -370,6 +374,7 @@ repo-1 - Does the package have a well-formed manifest (\`package.json\`)? ✅ - Does the \`packageManager\` field in \`package.json\` conform? ✅ - Does the \`engines.node\` field in \`package.json\` conform? ✅ + - Does the lint related \`devDependencies\` in \`package.json\` conform? ✅ - Is \`README.md\` present? ✅ - Does the README conform by recommending the correct Yarn version to install? ✅ - Does the README conform by recommending node install from nodejs.org? ✅ @@ -378,7 +383,7 @@ repo-1 - Does the \`src/\` directory exist? ✅ - Is \`.nvmrc\` present, and does it conform? ✅ -Results: 11 passed, 0 failed, 11 total +Results: 12 passed, 0 failed, 12 total Elapsed time: 0 ms @@ -389,6 +394,7 @@ repo-2 - Does the package have a well-formed manifest (\`package.json\`)? ✅ - Does the \`packageManager\` field in \`package.json\` conform? ✅ - Does the \`engines.node\` field in \`package.json\` conform? ✅ + - Does the lint related \`devDependencies\` in \`package.json\` conform? ✅ - Is \`README.md\` present? ✅ - Does the README conform by recommending the correct Yarn version to install? ✅ - Does the README conform by recommending node install from nodejs.org? ✅ @@ -397,7 +403,7 @@ repo-2 - Does the \`src/\` directory exist? ✅ - Is \`.nvmrc\` present, and does it conform? ✅ -Results: 11 passed, 0 failed, 11 total +Results: 12 passed, 0 failed, 12 total Elapsed time: 0 ms `, diff --git a/src/rules/index.ts b/src/rules/index.ts index 309d27b..c77703e 100644 --- a/src/rules/index.ts +++ b/src/rules/index.ts @@ -1,6 +1,7 @@ import allYarnModernFilesConform from './all-yarn-modern-files-conform'; import classicYarnConfigFileAbsent from './classic-yarn-config-file-absent'; import packageEnginesNodeFieldConforms from './package-engines-node-field-conforms'; +import packageLintDependenciesConforms from './package-lint-dependencies-conforms'; import packageManagerFieldConforms from './package-manager-field-conforms'; import readmeListsCorrectYarnVersion from './readme-lists-correct-yarn-version'; import readmeListsNodejsWebsite from './readme-recommends-node-install'; @@ -20,4 +21,5 @@ export const rules = [ requireNvmrc, packageEnginesNodeFieldConforms, readmeListsNodejsWebsite, + packageLintDependenciesConforms, ] as const; diff --git a/src/rules/package-engines-node-field-conforms.test.ts b/src/rules/package-engines-node-field-conforms.test.ts index cd59da3..412c2bf 100644 --- a/src/rules/package-engines-node-field-conforms.test.ts +++ b/src/rules/package-engines-node-field-conforms.test.ts @@ -14,7 +14,11 @@ describe('Rule: package-engines-node-field-conforms', () => { }); await writeFile( path.join(template.directoryPath, 'package.json'), - JSON.stringify({ packageManager: 'a', engines: { node: 'test' } }), + JSON.stringify({ + packageManager: 'a', + engines: { node: 'test' }, + devDependencies: { eslint: '1.0.0' }, + }), ); const project = buildMetaMaskRepository({ shortname: 'project', @@ -22,7 +26,11 @@ describe('Rule: package-engines-node-field-conforms', () => { }); await writeFile( path.join(project.directoryPath, 'package.json'), - JSON.stringify({ packageManager: 'a', engines: { node: 'test' } }), + JSON.stringify({ + packageManager: 'a', + engines: { node: 'test' }, + devDependencies: { eslint: '1.0.0' }, + }), ); const result = await packageEnginesNodeFieldConforms.execute({ @@ -46,7 +54,11 @@ describe('Rule: package-engines-node-field-conforms', () => { }); await writeFile( path.join(template.directoryPath, 'package.json'), - JSON.stringify({ packageManager: 'a', engines: { node: 'test1' } }), + JSON.stringify({ + packageManager: 'a', + engines: { node: 'test1' }, + devDependencies: { eslint: '1.0.0' }, + }), ); const project = buildMetaMaskRepository({ shortname: 'project', @@ -54,7 +66,11 @@ describe('Rule: package-engines-node-field-conforms', () => { }); await writeFile( path.join(project.directoryPath, 'package.json'), - JSON.stringify({ packageManager: 'a', engines: { node: 'test2' } }), + JSON.stringify({ + packageManager: 'a', + engines: { node: 'test2' }, + devDependencies: { eslint: '1.0.0' }, + }), ); const result = await packageEnginesNodeFieldConforms.execute({ diff --git a/src/rules/package-lint-dependencies-conforms.test.ts b/src/rules/package-lint-dependencies-conforms.test.ts new file mode 100644 index 0000000..6d8d787 --- /dev/null +++ b/src/rules/package-lint-dependencies-conforms.test.ts @@ -0,0 +1,134 @@ +import { writeFile } from '@metamask/utils/node'; +import path from 'path'; + +import packageLintDependenciesConforms from './package-lint-dependencies-conforms'; +import { buildMetaMaskRepository, withinSandbox } from '../../tests/helpers'; +import { fail, pass } from '../rule-helpers'; + +describe('Rule: package-lint-dependencies-conforms', () => { + it("passes if the lint related dependencies in the project's package.json matches the one in the template's package.json", async () => { + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join(template.directoryPath, 'package.json'), + JSON.stringify({ + packageManager: 'a', + engines: { node: 'test' }, + devDependencies: { eslint: '1.0.0' }, + }), + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, 'package.json'), + JSON.stringify({ + packageManager: 'a', + engines: { node: 'test' }, + devDependencies: { eslint: '1.0.0' }, + }), + ); + + const result = await packageLintDependenciesConforms.execute({ + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: true, + }); + }); + }); + + it("fails if the version of lint related dependencies in the project's package.json does not match the one in the template's package.json", async () => { + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join(template.directoryPath, 'package.json'), + JSON.stringify({ + packageManager: 'a', + engines: { node: 'test' }, + devDependencies: { eslint: '1.1.0' }, + }), + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, 'package.json'), + JSON.stringify({ + packageManager: 'a', + engines: { node: 'test' }, + devDependencies: { eslint: '1.0.0' }, + }), + ); + + const result = await packageLintDependenciesConforms.execute({ + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { message: 'eslint version is "1.0.0", when it should be "1.1.0".' }, + ], + }); + }); + }); + + it("fails if the lint related dependency exist in the template's package.json, but not in the project's package.json", async () => { + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join(template.directoryPath, 'package.json'), + JSON.stringify({ + packageManager: 'a', + engines: { node: 'test' }, + devDependencies: { eslint: '1.1.0' }, + }), + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, 'package.json'), + JSON.stringify({ + packageManager: 'a', + engines: { node: 'test' }, + devDependencies: { testlint: '1.0.0' }, + }), + ); + + const result = await packageLintDependenciesConforms.execute({ + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { message: '`package.json` should contain "eslint", but does not.' }, + ], + }); + }); + }); +}); diff --git a/src/rules/package-lint-dependencies-conforms.ts b/src/rules/package-lint-dependencies-conforms.ts new file mode 100644 index 0000000..4d7d408 --- /dev/null +++ b/src/rules/package-lint-dependencies-conforms.ts @@ -0,0 +1,103 @@ +import { buildRule } from './build-rule'; +import { PackageManifestSchema, RuleName } from './types'; +import type { RuleExecutionFailure } from '../execute-rules'; + +export default buildRule({ + name: RuleName.PackageLintDependenciesConforms, + description: + 'Does the lint related `devDependencies` in `package.json` conform?', + dependencies: [RuleName.RequireValidPackageManifest], + execute: async ({ project, template, pass, fail }) => { + const entryPath = 'package.json'; + + const templateManifest = await template.fs.readJsonFileAs( + entryPath, + PackageManifestSchema, + ); + + const projectManifest = await project.fs.readJsonFileAs( + entryPath, + PackageManifestSchema, + ); + + const templateDependenciesMap = new Map( + Object.entries(templateManifest.devDependencies), + ); + const projectDependenciesMap = new Map( + Object.entries(projectManifest.devDependencies), + ); + const failMessages: RuleExecutionFailure[] = await lintPackageConforms( + templateDependenciesMap, + projectDependenciesMap, + ); + + return failMessages.length === 0 ? pass() : fail(failMessages); + }, +}); + +/** + * Validates whether target project has all the required lint packages with versions matching with template project. + * + * @param templateDependenciesMap - The map of lint package name and version from template. + * @param projectDependenciesMap - The map of lint package name and version from project. + */ +async function lintPackageConforms( + templateDependenciesMap: Map, + projectDependenciesMap: Map, +): Promise { + const templateLintPackageNames = await getTemplateLintPackageNames( + templateDependenciesMap, + ); + const failMessages: RuleExecutionFailure[] = []; + for (const lintPackage of new Set(templateLintPackageNames)) { + const projectPackageVersion = projectDependenciesMap.get(lintPackage); + if (!projectPackageVersion) { + failMessages.push({ + message: `\`package.json\` should contain "${lintPackage}", but does not.`, + }); + + continue; + } + + const templatePackageVersion = templateDependenciesMap.get(lintPackage); + if (projectPackageVersion !== templatePackageVersion) { + failMessages.push({ + message: `${lintPackage} version is "${projectPackageVersion}", when it should be "${ + templatePackageVersion as string + }".`, + }); + } + } + + return failMessages; +} + +/** + * Extracts list of lint package names from template's package.json. + * + * @param templateDependenciesMap - The map of lint package name and version. + */ +async function getTemplateLintPackageNames( + templateDependenciesMap: Map, +): Promise { + const requiredPackagePatterns: string[] = [ + '@metamask/eslint-config*', + '@typescript-eslint*', + 'eslint*', + 'prettier*', + ]; + + const templatePackangeNames: string[] = Array.from( + templateDependenciesMap.keys(), + ); + const templateLintPackageNames: string[] = []; + requiredPackagePatterns.forEach((requiredPackage) => { + templateLintPackageNames.push( + ...templatePackangeNames.filter((packageName) => + packageName.match(requiredPackage), + ), + ); + }); + + return templateLintPackageNames; +} diff --git a/src/rules/package-manager-field-conforms.test.ts b/src/rules/package-manager-field-conforms.test.ts index dbeca8d..7c85505 100644 --- a/src/rules/package-manager-field-conforms.test.ts +++ b/src/rules/package-manager-field-conforms.test.ts @@ -14,7 +14,11 @@ describe('Rule: package-manager-field-conforms', () => { }); await writeFile( path.join(template.directoryPath, 'package.json'), - JSON.stringify({ packageManager: 'a', engines: { node: 'test' } }), + JSON.stringify({ + packageManager: 'a', + engines: { node: 'test' }, + devDependencies: { eslint: '1.0.0' }, + }), ); const project = buildMetaMaskRepository({ shortname: 'project', @@ -22,7 +26,11 @@ describe('Rule: package-manager-field-conforms', () => { }); await writeFile( path.join(project.directoryPath, 'package.json'), - JSON.stringify({ packageManager: 'a', engines: { node: 'test' } }), + JSON.stringify({ + packageManager: 'a', + engines: { node: 'test' }, + devDependencies: { eslint: '1.0.0' }, + }), ); const result = await packageManagerFieldConforms.execute({ @@ -46,7 +54,11 @@ describe('Rule: package-manager-field-conforms', () => { }); await writeFile( path.join(template.directoryPath, 'package.json'), - JSON.stringify({ packageManager: 'a', engines: { node: 'test' } }), + JSON.stringify({ + packageManager: 'a', + engines: { node: 'test' }, + devDependencies: { eslint: '1.0.0' }, + }), ); const project = buildMetaMaskRepository({ shortname: 'project', @@ -54,7 +66,11 @@ describe('Rule: package-manager-field-conforms', () => { }); await writeFile( path.join(project.directoryPath, 'package.json'), - JSON.stringify({ packageManager: 'b', engines: { node: 'test' } }), + JSON.stringify({ + packageManager: 'b', + engines: { node: 'test' }, + devDependencies: { eslint: '1.0.0' }, + }), ); const result = await packageManagerFieldConforms.execute({ diff --git a/src/rules/require-valid-package-manifest.test.ts b/src/rules/require-valid-package-manifest.test.ts index 92dacab..44d5cf8 100644 --- a/src/rules/require-valid-package-manifest.test.ts +++ b/src/rules/require-valid-package-manifest.test.ts @@ -14,7 +14,11 @@ describe('Rule: require-package-manifest', () => { }); await writeFile( path.join(project.directoryPath, 'package.json'), - JSON.stringify({ packageManager: 'foo', engines: { node: 'test' } }), + JSON.stringify({ + packageManager: 'foo', + engines: { node: 'test' }, + devDependencies: { eslint: '1.0.0' }, + }), ); const result = await requireValidPackageManifest.execute({ @@ -78,7 +82,7 @@ describe('Rule: require-package-manifest', () => { failures: [ { message: - 'Invalid `package.json`: Missing `packageManager`; Missing `engines`.', + 'Invalid `package.json`: Missing `packageManager`; Missing `engines`; Missing `devDependencies`.', }, ], }); diff --git a/src/rules/types.ts b/src/rules/types.ts index d16df93..39299dc 100644 --- a/src/rules/types.ts +++ b/src/rules/types.ts @@ -14,6 +14,7 @@ export enum RuleName { RequireNvmrc = 'require-nvmrc', PackageEnginesNodeFieldConforms = 'package-engines-node-field-conforms', ReadmeRecommendsNodeInstall = 'readme-recommends-node-install', + PackageLintDependenciesConforms = 'package-lint-dependencies-conforms', } export const PackageManifestSchema = type({ @@ -21,4 +22,5 @@ export const PackageManifestSchema = type({ engines: type({ node: string(), }), + devDependencies: type({}), }); From 1d934ef12699a9c4cd5573d26ce95224a48f3bce Mon Sep 17 00:00:00 2001 From: Kanthesha Devaramane Date: Fri, 19 Jan 2024 11:41:41 +0000 Subject: [PATCH 2/4] review suggestions and fixes --- src/main.test.ts | 8 +- src/rules/index.ts | 4 +- ...package-lint-dependencies-conform.test.ts} | 37 +++++-- .../package-lint-dependencies-conform.ts | 88 +++++++++++++++ .../package-lint-dependencies-conforms.ts | 103 ------------------ src/rules/types.ts | 6 +- 6 files changed, 126 insertions(+), 120 deletions(-) rename src/rules/{package-lint-dependencies-conforms.test.ts => package-lint-dependencies-conform.test.ts} (74%) create mode 100644 src/rules/package-lint-dependencies-conform.ts delete mode 100644 src/rules/package-lint-dependencies-conforms.ts diff --git a/src/main.test.ts b/src/main.test.ts index 644aac3..2afc7b1 100644 --- a/src/main.test.ts +++ b/src/main.test.ts @@ -107,7 +107,7 @@ repo-1 - Does the package have a well-formed manifest (\`package.json\`)? ✅ - Does the \`packageManager\` field in \`package.json\` conform? ✅ - Does the \`engines.node\` field in \`package.json\` conform? ✅ - - Does the lint related \`devDependencies\` in \`package.json\` conform? ✅ + - Do the lint-related \`devDependencies\` in \`package.json\` conform? ✅ - Is \`README.md\` present? ✅ - Does the README conform by recommending the correct Yarn version to install? ✅ - Does the README conform by recommending node install from nodejs.org? ✅ @@ -127,7 +127,7 @@ repo-2 - Does the package have a well-formed manifest (\`package.json\`)? ✅ - Does the \`packageManager\` field in \`package.json\` conform? ✅ - Does the \`engines.node\` field in \`package.json\` conform? ✅ - - Does the lint related \`devDependencies\` in \`package.json\` conform? ✅ + - Do the lint-related \`devDependencies\` in \`package.json\` conform? ✅ - Is \`README.md\` present? ✅ - Does the README conform by recommending the correct Yarn version to install? ✅ - Does the README conform by recommending node install from nodejs.org? ✅ @@ -374,7 +374,7 @@ repo-1 - Does the package have a well-formed manifest (\`package.json\`)? ✅ - Does the \`packageManager\` field in \`package.json\` conform? ✅ - Does the \`engines.node\` field in \`package.json\` conform? ✅ - - Does the lint related \`devDependencies\` in \`package.json\` conform? ✅ + - Do the lint-related \`devDependencies\` in \`package.json\` conform? ✅ - Is \`README.md\` present? ✅ - Does the README conform by recommending the correct Yarn version to install? ✅ - Does the README conform by recommending node install from nodejs.org? ✅ @@ -394,7 +394,7 @@ repo-2 - Does the package have a well-formed manifest (\`package.json\`)? ✅ - Does the \`packageManager\` field in \`package.json\` conform? ✅ - Does the \`engines.node\` field in \`package.json\` conform? ✅ - - Does the lint related \`devDependencies\` in \`package.json\` conform? ✅ + - Do the lint-related \`devDependencies\` in \`package.json\` conform? ✅ - Is \`README.md\` present? ✅ - Does the README conform by recommending the correct Yarn version to install? ✅ - Does the README conform by recommending node install from nodejs.org? ✅ diff --git a/src/rules/index.ts b/src/rules/index.ts index c77703e..53fbdc3 100644 --- a/src/rules/index.ts +++ b/src/rules/index.ts @@ -1,7 +1,7 @@ import allYarnModernFilesConform from './all-yarn-modern-files-conform'; import classicYarnConfigFileAbsent from './classic-yarn-config-file-absent'; import packageEnginesNodeFieldConforms from './package-engines-node-field-conforms'; -import packageLintDependenciesConforms from './package-lint-dependencies-conforms'; +import packageLintDependenciesConform from './package-lint-dependencies-conform'; import packageManagerFieldConforms from './package-manager-field-conforms'; import readmeListsCorrectYarnVersion from './readme-lists-correct-yarn-version'; import readmeListsNodejsWebsite from './readme-recommends-node-install'; @@ -21,5 +21,5 @@ export const rules = [ requireNvmrc, packageEnginesNodeFieldConforms, readmeListsNodejsWebsite, - packageLintDependenciesConforms, + packageLintDependenciesConform, ] as const; diff --git a/src/rules/package-lint-dependencies-conforms.test.ts b/src/rules/package-lint-dependencies-conform.test.ts similarity index 74% rename from src/rules/package-lint-dependencies-conforms.test.ts rename to src/rules/package-lint-dependencies-conform.test.ts index 6d8d787..a16ddf6 100644 --- a/src/rules/package-lint-dependencies-conforms.test.ts +++ b/src/rules/package-lint-dependencies-conform.test.ts @@ -1,11 +1,11 @@ import { writeFile } from '@metamask/utils/node'; import path from 'path'; -import packageLintDependenciesConforms from './package-lint-dependencies-conforms'; +import packageLintDependenciesConform from './package-lint-dependencies-conform'; import { buildMetaMaskRepository, withinSandbox } from '../../tests/helpers'; import { fail, pass } from '../rule-helpers'; -describe('Rule: package-lint-dependencies-conforms', () => { +describe('Rule: package-lint-dependencies-conform', () => { it("passes if the lint related dependencies in the project's package.json matches the one in the template's package.json", async () => { await withinSandbox(async (sandbox) => { const template = buildMetaMaskRepository({ @@ -17,7 +17,16 @@ describe('Rule: package-lint-dependencies-conforms', () => { JSON.stringify({ packageManager: 'a', engines: { node: 'test' }, - devDependencies: { eslint: '1.0.0' }, + devDependencies: { + '@metamask/eslint-config-foo': '1.0.0', + '@typescript-eslint/foo': '1.0.0', + eslint: '1.0.0', + 'eslint-plugin-foo': '1.0.0', + 'eslint-config-foo': '1.0.0', + prettier: '1.0.0', + 'prettier-plugin-foo': '1.0.0', + 'prettier-config-foo': '1.0.0', + }, }), ); const project = buildMetaMaskRepository({ @@ -29,11 +38,20 @@ describe('Rule: package-lint-dependencies-conforms', () => { JSON.stringify({ packageManager: 'a', engines: { node: 'test' }, - devDependencies: { eslint: '1.0.0' }, + devDependencies: { + '@metamask/eslint-config-foo': '1.0.0', + '@typescript-eslint/foo': '1.0.0', + eslint: '1.0.0', + 'eslint-plugin-foo': '1.0.0', + 'eslint-config-foo': '1.0.0', + prettier: '1.0.0', + 'prettier-plugin-foo': '1.0.0', + 'prettier-config-foo': '1.0.0', + }, }), ); - const result = await packageLintDependenciesConforms.execute({ + const result = await packageLintDependenciesConform.execute({ template, project, pass, @@ -73,7 +91,7 @@ describe('Rule: package-lint-dependencies-conforms', () => { }), ); - const result = await packageLintDependenciesConforms.execute({ + const result = await packageLintDependenciesConform.execute({ template, project, pass, @@ -116,7 +134,7 @@ describe('Rule: package-lint-dependencies-conforms', () => { }), ); - const result = await packageLintDependenciesConforms.execute({ + const result = await packageLintDependenciesConform.execute({ template, project, pass, @@ -126,7 +144,10 @@ describe('Rule: package-lint-dependencies-conforms', () => { expect(result).toStrictEqual({ passed: false, failures: [ - { message: '`package.json` should contain "eslint", but does not.' }, + { + message: + '`package.json` should list `"eslint": "1.1.0"` in `devDependencies`, but does not.', + }, ], }); }); diff --git a/src/rules/package-lint-dependencies-conform.ts b/src/rules/package-lint-dependencies-conform.ts new file mode 100644 index 0000000..e0ceca6 --- /dev/null +++ b/src/rules/package-lint-dependencies-conform.ts @@ -0,0 +1,88 @@ +import { buildRule } from './build-rule'; +import { PackageManifestSchema, RuleName } from './types'; +import type { RuleExecutionFailure } from '../execute-rules'; + +export default buildRule({ + name: RuleName.PackageLintDependenciesConform, + description: + 'Do the lint-related `devDependencies` in `package.json` conform?', + dependencies: [RuleName.RequireValidPackageManifest], + execute: async ({ project, template, pass, fail }) => { + const entryPath = 'package.json'; + + const templateManifest = await template.fs.readJsonFileAs( + entryPath, + PackageManifestSchema, + ); + + const projectManifest = await project.fs.readJsonFileAs( + entryPath, + PackageManifestSchema, + ); + + const failures: RuleExecutionFailure[] = await lintPackageConform( + templateManifest.devDependencies, + projectManifest.devDependencies, + ); + + return failures.length === 0 ? pass() : fail(failures); + }, +}); + +/** + * Validates whether target project has all the required lint packages with versions matching with template project. + * + * @param templateDependencies - The record of lint package name and version from template. + * @param projectDependencies - The record of lint package name and version from project. + */ +async function lintPackageConform( + templateDependencies: Record, + projectDependencies: Record, +): Promise { + const templateLintPackageNames = await getTemplateLintPackageNames( + templateDependencies, + ); + const failures: RuleExecutionFailure[] = []; + for (const lintPackage of templateLintPackageNames) { + const projectPackageVersion = projectDependencies[lintPackage]; + const templatePackageVersion = templateDependencies[lintPackage] as string; + if (!projectPackageVersion) { + failures.push({ + message: `\`package.json\` should list \`"${lintPackage}": "${templatePackageVersion}"\` in \`devDependencies\`, but does not.`, + }); + + continue; + } + + if (projectPackageVersion !== templatePackageVersion) { + failures.push({ + message: `${lintPackage} version is "${projectPackageVersion}", when it should be "${templatePackageVersion}".`, + }); + } + } + + return failures; +} + +/** + * Extracts list of lint package names from template's package.json. + * + * @param templateDependencies - The record of lint package name and version. + */ +async function getTemplateLintPackageNames( + templateDependencies: Record, +): Promise { + const requiredPackagePatterns: RegExp[] = [ + /^@metamask\/eslint-config-[^/]+$/u, + /^@typescript-eslint\/[^/]+$/u, + /^eslint(-[^/]+)?$/u, + /^prettier(-[^/]+)?$/u, + ]; + + const templatePackageNames = Object.keys(templateDependencies); + return templatePackageNames.filter((packageName) => { + return requiredPackagePatterns.some((pattern) => { + return packageName.match(pattern); + }); + }); +} diff --git a/src/rules/package-lint-dependencies-conforms.ts b/src/rules/package-lint-dependencies-conforms.ts deleted file mode 100644 index 4d7d408..0000000 --- a/src/rules/package-lint-dependencies-conforms.ts +++ /dev/null @@ -1,103 +0,0 @@ -import { buildRule } from './build-rule'; -import { PackageManifestSchema, RuleName } from './types'; -import type { RuleExecutionFailure } from '../execute-rules'; - -export default buildRule({ - name: RuleName.PackageLintDependenciesConforms, - description: - 'Does the lint related `devDependencies` in `package.json` conform?', - dependencies: [RuleName.RequireValidPackageManifest], - execute: async ({ project, template, pass, fail }) => { - const entryPath = 'package.json'; - - const templateManifest = await template.fs.readJsonFileAs( - entryPath, - PackageManifestSchema, - ); - - const projectManifest = await project.fs.readJsonFileAs( - entryPath, - PackageManifestSchema, - ); - - const templateDependenciesMap = new Map( - Object.entries(templateManifest.devDependencies), - ); - const projectDependenciesMap = new Map( - Object.entries(projectManifest.devDependencies), - ); - const failMessages: RuleExecutionFailure[] = await lintPackageConforms( - templateDependenciesMap, - projectDependenciesMap, - ); - - return failMessages.length === 0 ? pass() : fail(failMessages); - }, -}); - -/** - * Validates whether target project has all the required lint packages with versions matching with template project. - * - * @param templateDependenciesMap - The map of lint package name and version from template. - * @param projectDependenciesMap - The map of lint package name and version from project. - */ -async function lintPackageConforms( - templateDependenciesMap: Map, - projectDependenciesMap: Map, -): Promise { - const templateLintPackageNames = await getTemplateLintPackageNames( - templateDependenciesMap, - ); - const failMessages: RuleExecutionFailure[] = []; - for (const lintPackage of new Set(templateLintPackageNames)) { - const projectPackageVersion = projectDependenciesMap.get(lintPackage); - if (!projectPackageVersion) { - failMessages.push({ - message: `\`package.json\` should contain "${lintPackage}", but does not.`, - }); - - continue; - } - - const templatePackageVersion = templateDependenciesMap.get(lintPackage); - if (projectPackageVersion !== templatePackageVersion) { - failMessages.push({ - message: `${lintPackage} version is "${projectPackageVersion}", when it should be "${ - templatePackageVersion as string - }".`, - }); - } - } - - return failMessages; -} - -/** - * Extracts list of lint package names from template's package.json. - * - * @param templateDependenciesMap - The map of lint package name and version. - */ -async function getTemplateLintPackageNames( - templateDependenciesMap: Map, -): Promise { - const requiredPackagePatterns: string[] = [ - '@metamask/eslint-config*', - '@typescript-eslint*', - 'eslint*', - 'prettier*', - ]; - - const templatePackangeNames: string[] = Array.from( - templateDependenciesMap.keys(), - ); - const templateLintPackageNames: string[] = []; - requiredPackagePatterns.forEach((requiredPackage) => { - templateLintPackageNames.push( - ...templatePackangeNames.filter((packageName) => - packageName.match(requiredPackage), - ), - ); - }); - - return templateLintPackageNames; -} diff --git a/src/rules/types.ts b/src/rules/types.ts index 39299dc..4ffa5c7 100644 --- a/src/rules/types.ts +++ b/src/rules/types.ts @@ -1,4 +1,4 @@ -import { type, string } from 'superstruct'; +import { type, string, record } from 'superstruct'; /** * All of the known rules. @@ -14,7 +14,7 @@ export enum RuleName { RequireNvmrc = 'require-nvmrc', PackageEnginesNodeFieldConforms = 'package-engines-node-field-conforms', ReadmeRecommendsNodeInstall = 'readme-recommends-node-install', - PackageLintDependenciesConforms = 'package-lint-dependencies-conforms', + PackageLintDependenciesConform = 'package-lint-dependencies-conform', } export const PackageManifestSchema = type({ @@ -22,5 +22,5 @@ export const PackageManifestSchema = type({ engines: type({ node: string(), }), - devDependencies: type({}), + devDependencies: record(string(), string()), }); From 555ec7ab586f2f470ece675c47b51050d49b169e Mon Sep 17 00:00:00 2001 From: Kanthesha Devaramane Date: Mon, 22 Jan 2024 11:39:01 +0000 Subject: [PATCH 3/4] remove unnecessary async --- src/rules/package-lint-dependencies-conform.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/rules/package-lint-dependencies-conform.ts b/src/rules/package-lint-dependencies-conform.ts index e0ceca6..11ede03 100644 --- a/src/rules/package-lint-dependencies-conform.ts +++ b/src/rules/package-lint-dependencies-conform.ts @@ -39,9 +39,8 @@ async function lintPackageConform( templateDependencies: Record, projectDependencies: Record, ): Promise { - const templateLintPackageNames = await getTemplateLintPackageNames( - templateDependencies, - ); + const templateLintPackageNames = + getTemplateLintPackageNames(templateDependencies); const failures: RuleExecutionFailure[] = []; for (const lintPackage of templateLintPackageNames) { const projectPackageVersion = projectDependencies[lintPackage]; @@ -65,13 +64,14 @@ async function lintPackageConform( } /** - * Extracts list of lint package names from template's package.json. + * Extracts array of lint package names from template's package.json. * * @param templateDependencies - The record of lint package name and version. + * @returns The array of lint package names. */ -async function getTemplateLintPackageNames( +function getTemplateLintPackageNames( templateDependencies: Record, -): Promise { +): string[] { const requiredPackagePatterns: RegExp[] = [ /^@metamask\/eslint-config-[^/]+$/u, /^@typescript-eslint\/[^/]+$/u, From 8a9c71022e9ddab5324d6f0a9a869e142c185829 Mon Sep 17 00:00:00 2001 From: Kanthesha Devaramane Date: Mon, 22 Jan 2024 20:54:20 +0000 Subject: [PATCH 4/4] getting lint packages refactored --- .../package-lint-dependencies-conform.test.ts | 53 ++++++++++++++++++- .../package-lint-dependencies-conform.ts | 40 +++++++------- 2 files changed, 74 insertions(+), 19 deletions(-) diff --git a/src/rules/package-lint-dependencies-conform.test.ts b/src/rules/package-lint-dependencies-conform.test.ts index a16ddf6..14394e1 100644 --- a/src/rules/package-lint-dependencies-conform.test.ts +++ b/src/rules/package-lint-dependencies-conform.test.ts @@ -101,7 +101,7 @@ describe('Rule: package-lint-dependencies-conform', () => { expect(result).toStrictEqual({ passed: false, failures: [ - { message: 'eslint version is "1.0.0", when it should be "1.1.0".' }, + { message: '`eslint` is "1.0.0", when it should be "1.1.0".' }, ], }); }); @@ -152,4 +152,55 @@ describe('Rule: package-lint-dependencies-conform', () => { }); }); }); + + it("passes if the there're no lint related dependencies in the template's package.json", async () => { + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join(template.directoryPath, 'package.json'), + JSON.stringify({ + packageManager: 'a', + engines: { node: 'test' }, + devDependencies: { + '@metamask/test-config-foo': '1.0.0', + }, + }), + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, 'package.json'), + JSON.stringify({ + packageManager: 'a', + engines: { node: 'test' }, + devDependencies: { + '@metamask/eslint-config-foo': '1.0.0', + '@typescript-eslint/foo': '1.0.0', + eslint: '1.0.0', + 'eslint-plugin-foo': '1.0.0', + 'eslint-config-foo': '1.0.0', + prettier: '1.0.0', + 'prettier-plugin-foo': '1.0.0', + 'prettier-config-foo': '1.0.0', + }, + }), + ); + + const result = await packageLintDependenciesConform.execute({ + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: true, + }); + }); + }); }); diff --git a/src/rules/package-lint-dependencies-conform.ts b/src/rules/package-lint-dependencies-conform.ts index 11ede03..e5cc649 100644 --- a/src/rules/package-lint-dependencies-conform.ts +++ b/src/rules/package-lint-dependencies-conform.ts @@ -39,15 +39,15 @@ async function lintPackageConform( templateDependencies: Record, projectDependencies: Record, ): Promise { - const templateLintPackageNames = - getTemplateLintPackageNames(templateDependencies); + const templateLintPackages = getTemplateLintPackages(templateDependencies); const failures: RuleExecutionFailure[] = []; - for (const lintPackage of templateLintPackageNames) { - const projectPackageVersion = projectDependencies[lintPackage]; - const templatePackageVersion = templateDependencies[lintPackage] as string; + for (const [templatePackageName, templatePackageVersion] of Object.entries( + templateLintPackages, + )) { + const projectPackageVersion = projectDependencies[templatePackageName]; if (!projectPackageVersion) { failures.push({ - message: `\`package.json\` should list \`"${lintPackage}": "${templatePackageVersion}"\` in \`devDependencies\`, but does not.`, + message: `\`package.json\` should list \`"${templatePackageName}": "${templatePackageVersion}"\` in \`devDependencies\`, but does not.`, }); continue; @@ -55,7 +55,7 @@ async function lintPackageConform( if (projectPackageVersion !== templatePackageVersion) { failures.push({ - message: `${lintPackage} version is "${projectPackageVersion}", when it should be "${templatePackageVersion}".`, + message: `\`${templatePackageName}\` is "${projectPackageVersion}", when it should be "${templatePackageVersion}".`, }); } } @@ -64,25 +64,29 @@ async function lintPackageConform( } /** - * Extracts array of lint package names from template's package.json. + * Extracts the records of lint package name and version from template's package.json. * * @param templateDependencies - The record of lint package name and version. - * @returns The array of lint package names. + * @returns The records of lint package name and version. */ -function getTemplateLintPackageNames( +function getTemplateLintPackages( templateDependencies: Record, -): string[] { +): Record { const requiredPackagePatterns: RegExp[] = [ /^@metamask\/eslint-config-[^/]+$/u, /^@typescript-eslint\/[^/]+$/u, /^eslint(-[^/]+)?$/u, /^prettier(-[^/]+)?$/u, ]; - - const templatePackageNames = Object.keys(templateDependencies); - return templatePackageNames.filter((packageName) => { - return requiredPackagePatterns.some((pattern) => { - return packageName.match(pattern); - }); - }); + return Object.entries(templateDependencies).reduce>( + (packages, [packageName, packageVersion]) => { + if ( + requiredPackagePatterns.some((pattern) => packageName.match(pattern)) + ) { + return { ...packages, [packageName]: packageVersion }; + } + return packages; + }, + {}, + ); }