From a2f2d7d3e9157b867a12e9a26958ea5dca0178cf Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Mon, 18 Dec 2023 23:16:51 -0700 Subject: [PATCH 01/18] Add Yarn-related rules For a given project, we want to ensure that it is using the same Yarn version as the module template, that Yarn is configured exactly the same way as in the module template, and that all of the Yarn-related files are present, including the Yarn "binary" in `.yarn/releases` and the plugins in `.yarn/plugins`. We also want to ensure that the version we recommend users install in the README matches the same version as the module template. To accomplish this, this commit adds six new rules: - `all-yarn-modern-files-conform` - This checks that `.yarnrc.yml` is present and matches the module template, and any files in the template's `.yarn/plugins` and `.yarn/releases` directories are present in the project and match. - `classic-yarn-config-file-absent` - This checks that `.yarnrc`, which is the configuration file for Yarn Classic, is not present. - `package-manager-conforms` - This checks that the `packageManager` field in the project's `package.json` is the same as that of the module template's. - `readme-lists-correct-yarn-version` - This checks that the Yarn version in the project's README is the same as that of the module template's. - `require-readme` - This checks that the project has a README. - `require-valid-package-manifest` - This checks the project has a `package.json` and that the JSON-parsed content of this file matches a known shape. --- package.json | 2 + src/execute-rules.ts | 42 +- src/main.test.ts | 270 ++++++++--- src/misc-utils.test.ts | 446 ++++++++++++++++++ src/misc-utils.ts | 170 ++++++- src/report-project-lint-result.test.ts | 84 +++- src/repository-filesystem.test.ts | 183 ++++++- src/repository-filesystem.ts | 65 ++- .../all-yarn-modern-files-conform.test.ts | 323 +++++++++++++ src/rules/all-yarn-modern-files-conform.ts | 23 + .../classic-yarn-config-file-absent.test.ts | 62 +++ src/rules/classic-yarn-config-file-absent.ts | 22 + src/rules/helpers.test.ts | 173 +++++++ src/rules/helpers.ts | 165 ++++++- src/rules/index.ts | 16 +- .../package-manager-field-conforms.test.ts | 79 ++++ src/rules/package-manager-field-conforms.ts | 28 ++ .../readme-lists-correct-yarn-version.test.ts | 114 +++++ .../readme-lists-correct-yarn-version.ts | 32 ++ src/rules/require-readme.test.ts | 61 +++ src/rules/require-readme.ts | 11 + src/rules/require-source-directory.ts | 20 +- .../require-valid-package-manifest.test.ts | 116 +++++ src/rules/require-valid-package-manifest.ts | 38 ++ src/rules/types.ts | 12 + yarn.lock | 2 + 26 files changed, 2448 insertions(+), 111 deletions(-) create mode 100644 src/rules/all-yarn-modern-files-conform.test.ts create mode 100644 src/rules/all-yarn-modern-files-conform.ts create mode 100644 src/rules/classic-yarn-config-file-absent.test.ts create mode 100644 src/rules/classic-yarn-config-file-absent.ts create mode 100644 src/rules/helpers.test.ts create mode 100644 src/rules/package-manager-field-conforms.test.ts create mode 100644 src/rules/package-manager-field-conforms.ts create mode 100644 src/rules/readme-lists-correct-yarn-version.test.ts create mode 100644 src/rules/readme-lists-correct-yarn-version.ts create mode 100644 src/rules/require-readme.test.ts create mode 100644 src/rules/require-readme.ts create mode 100644 src/rules/require-valid-package-manifest.test.ts create mode 100644 src/rules/require-valid-package-manifest.ts diff --git a/package.json b/package.json index ef71683..8dc3f10 100644 --- a/package.json +++ b/package.json @@ -54,6 +54,8 @@ "chalk": "^4.1.2", "dependency-graph": "^0.11.0", "execa": "^5.1.1", + "pony-cause": "^2.1.10", + "superstruct": "^1.0.3", "yargs": "^17.7.2" }, "devDependencies": { diff --git a/src/execute-rules.ts b/src/execute-rules.ts index 9d02597..2bc3bfa 100644 --- a/src/execute-rules.ts +++ b/src/execute-rules.ts @@ -65,6 +65,31 @@ export type RootRuleExecutionResultNode = { children: RuleExecutionResultNode[]; }; +/** + * The arguments passed to every rule's `execute` method. + */ +export type RuleExecutionArguments = { + /** + * A reference to a template repository that serves as a baseline for the + * project. + */ + template: MetaMaskRepository; + /** + * A reference to the project repository. + */ + project: MetaMaskRepository; + /** + * A supporting function that causes the rule to pass. + */ + pass: () => SuccessfulPartialRuleExecutionResult; + /** + * A supporting function that causes the rule to fail. + */ + fail: ( + failures: FailedPartialRuleExecutionResult['failures'], + ) => FailedPartialRuleExecutionResult; +}; + /** * A lint rule that can be executed against a project. */ @@ -85,14 +110,7 @@ export type Rule = { /** * The "body" of the rule. */ - execute(args: { - project: MetaMaskRepository; - template: MetaMaskRepository; - pass: () => SuccessfulPartialRuleExecutionResult; - fail: ( - failures: FailedPartialRuleExecutionResult['failures'], - ) => FailedPartialRuleExecutionResult; - }): Promise; + execute(args: RuleExecutionArguments): Promise; }; /** @@ -218,8 +236,8 @@ async function executeRule({ } /** - * A helper intended to be used in a rule which ends its execution by marking it - * as passing. + * A helper for a rule which is intended to end its execution by marking it as + * passing. * * @returns Part of a successful rule execution result (the rest will be filled * in automatically). @@ -231,8 +249,8 @@ export function pass(): SuccessfulPartialRuleExecutionResult { } /** - * A helper intended to be used in a rule which ends its execution by marking it - * as failing. + * A helper for a rule which is intended to end its execution by marking it as + * failing. * * @param failures - The list of associated failures. * @returns Part of a failed rule execution result (the rest will be filled diff --git a/src/main.test.ts b/src/main.test.ts index 3aee31f..7e44d1b 100644 --- a/src/main.test.ts +++ b/src/main.test.ts @@ -1,10 +1,14 @@ -import { ensureDirectoryStructureExists } from '@metamask/utils/node'; +import { + ensureDirectoryStructureExists, + writeFile, +} from '@metamask/utils/node'; import execa from 'execa'; import path from 'path'; import { MockWritable } from 'stdio-mock'; import stripAnsi from 'strip-ansi'; import { main } from './main'; +import { FakeOutputLogger } from '../tests/fake-output-logger'; import type { PrimaryExecaFunction } from '../tests/helpers'; import { fakeDateOnly, withinSandbox } from '../tests/helpers'; import { setupToolWithMockRepositories } from '../tests/setup-tool-with-mock-repositories'; @@ -23,7 +27,7 @@ describe('main', () => { }); describe('given a list of project references', () => { - it('lists the rules executed against the default repositories which pass', async () => { + it('lists the passing rules executed against the given repositories', async () => { await withinSandbox(async ({ directoryPath: sandboxDirectoryPath }) => { const projectNames = ['repo-1', 'repo-2']; const { cachedRepositoriesDirectoryPath, repositories } = @@ -38,46 +42,85 @@ describe('main', () => { })), ], }); - const projects = repositories.filter( - (repository) => repository.name !== 'metamask-module-template', - ); - for (const project of projects) { + for (const repository of repositories) { await ensureDirectoryStructureExists( - path.join(project.directoryPath, 'src'), + path.join(repository.directoryPath, 'src'), + ); + await writeFile( + path.join(repository.directoryPath, '.yarnrc.yml'), + '', + ); + await writeFile( + path.join( + repository.directoryPath, + '.yarn', + 'plugins', + 'some-plugin', + ), + 'content for some-plugin', + ); + await writeFile( + path.join( + repository.directoryPath, + '.yarn', + 'releases', + 'some-release', + ), + 'content for some-release', + ); + await writeFile( + path.join(repository.directoryPath, 'package.json'), + JSON.stringify({ packageManager: 'yarn' }), + ); + await writeFile( + path.join(repository.directoryPath, 'README.md'), + 'Install [Yarn whatever](...)', ); } - const stdout = new MockWritable(); - const stderr = new MockWritable(); + const outputLogger = new FakeOutputLogger(); await main({ argv: ['node', 'module-lint', ...projectNames], - stdout, - stderr, + stdout: outputLogger.stdout, + stderr: outputLogger.stderr, config: { cachedRepositoriesDirectoryPath, defaultProjectNames: [], }, }); - const output = stdout.data().map(stripAnsi).join(''); - - expect(output).toBe( + expect(outputLogger.getStderr()).toBe(''); + expect(outputLogger.getStdout()).toBe( ` repo-1 ------ +- Is the classic Yarn config file (\`.yarnrc\`) absent? ✅ +- Does the package have a well-formed manifest (\`package.json\`)? ✅ + - Does the \`packageManager\` field in \`package.json\` conform? ✅ +- Is \`README.md\` present? ✅ + - Does the README conform by recommending the correct Yarn version to install? ✅ +- Are all of the files for Yarn Modern present, and do they conform? ✅ + - Does the README conform by recommending the correct Yarn version to install? ✅ - Does the \`src/\` directory exist? ✅ -Results: 1 passed, 0 failed, 1 total +Results: 8 passed, 0 failed, 8 total Elapsed time: 0 ms repo-2 ------ +- Is the classic Yarn config file (\`.yarnrc\`) absent? ✅ +- Does the package have a well-formed manifest (\`package.json\`)? ✅ + - Does the \`packageManager\` field in \`package.json\` conform? ✅ +- Is \`README.md\` present? ✅ + - Does the README conform by recommending the correct Yarn version to install? ✅ +- Are all of the files for Yarn Modern present, and do they conform? ✅ + - Does the README conform by recommending the correct Yarn version to install? ✅ - Does the \`src/\` directory exist? ✅ -Results: 1 passed, 0 failed, 1 total +Results: 8 passed, 0 failed, 8 total Elapsed time: 0 ms `, @@ -85,7 +128,7 @@ Elapsed time: 0 ms }); }); - it('lists the rules executed against the default repositories which fail', async () => { + it('lists the failing rules executed against the given repositories', async () => { await withinSandbox(async ({ directoryPath: sandboxDirectoryPath }) => { const projectNames = ['repo-1', 'repo-2']; const { cachedRepositoriesDirectoryPath } = @@ -100,40 +143,56 @@ Elapsed time: 0 ms })), ], }); - const stdout = new MockWritable(); - const stderr = new MockWritable(); + const outputLogger = new FakeOutputLogger(); await main({ argv: ['node', 'module-lint', ...projectNames], - stdout, - stderr, + stdout: outputLogger.stdout, + stderr: outputLogger.stderr, config: { cachedRepositoriesDirectoryPath, defaultProjectNames: [], }, }); - const output = stdout.data().map(stripAnsi).join(''); - - expect(output).toBe( + expect(outputLogger.getStderr()).toBe(''); + expect(outputLogger.getStdout()).toBe( ` repo-1 ------ +- Is the classic Yarn config file (\`.yarnrc\`) absent? ✅ +- Does the package have a well-formed manifest (\`package.json\`)? ❌ + - \`package.json\` does not exist in this project. +- Is \`README.md\` present? ❌ + - \`README.md\` does not exist in this project. +- Are all of the files for Yarn Modern present, and do they conform? ❌ + - \`.yarnrc.yml\` does not exist in this project. + - \`.yarn/releases/\` does not exist in this project. + - \`.yarn/plugins/\` does not exist in this project. - Does the \`src/\` directory exist? ❌ - \`src/\` does not exist in this project. -Results: 0 passed, 1 failed, 1 total +Results: 1 passed, 4 failed, 5 total Elapsed time: 0 ms repo-2 ------ +- Is the classic Yarn config file (\`.yarnrc\`) absent? ✅ +- Does the package have a well-formed manifest (\`package.json\`)? ❌ + - \`package.json\` does not exist in this project. +- Is \`README.md\` present? ❌ + - \`README.md\` does not exist in this project. +- Are all of the files for Yarn Modern present, and do they conform? ❌ + - \`.yarnrc.yml\` does not exist in this project. + - \`.yarn/releases/\` does not exist in this project. + - \`.yarn/plugins/\` does not exist in this project. - Does the \`src/\` directory exist? ❌ - \`src/\` does not exist in this project. -Results: 0 passed, 1 failed, 1 total +Results: 1 passed, 4 failed, 5 total Elapsed time: 0 ms `, @@ -163,44 +222,51 @@ Elapsed time: 0 ms }, ], }); - const stdout = new MockWritable(); - const stderr = new MockWritable(); + const outputLogger = new FakeOutputLogger(); await main({ argv: ['node', 'module-lint', ...projectNames], - stdout, - stderr, + stdout: outputLogger.stdout, + stderr: outputLogger.stderr, config: { cachedRepositoriesDirectoryPath, defaultProjectNames: [], }, }); - expect(stdout.data().map(stripAnsi).join('')).toBe( + expect(outputLogger.getStderr()).toContain( + `Could not resolve 'repo-1' as it is neither a reference to a directory nor the name of a known MetaMask repository.`, + ); + expect(outputLogger.getStdout()).toBe( ` Cloning repository MetaMask/repo-2, please wait... repo-2 ------ +- Is the classic Yarn config file (\`.yarnrc\`) absent? ✅ +- Does the package have a well-formed manifest (\`package.json\`)? ❌ + - \`package.json\` does not exist in this project. +- Is \`README.md\` present? ❌ + - \`README.md\` does not exist in this project. +- Are all of the files for Yarn Modern present, and do they conform? ❌ + - \`.yarnrc.yml\` does not exist in this project. + - \`.yarn/releases/\` does not exist in this project. + - \`.yarn/plugins/\` does not exist in this project. - Does the \`src/\` directory exist? ❌ - \`src/\` does not exist in this project. -Results: 0 passed, 1 failed, 1 total +Results: 1 passed, 4 failed, 5 total Elapsed time: 0 ms `.trimStart(), ); - - expect(stderr.data().map(stripAnsi).join('')).toContain( - `Could not resolve 'repo-1' as it is neither a reference to a directory nor the name of a known MetaMask repository.`, - ); }); }); }); describe('given no project references', () => { - it('lists the rules executed against the default repositories which pass', async () => { + it('lists the passing rules executed against the default repositories', async () => { await withinSandbox(async ({ directoryPath: sandboxDirectoryPath }) => { const projectNames = ['repo-1', 'repo-2']; const { cachedRepositoriesDirectoryPath, repositories } = @@ -215,46 +281,85 @@ Elapsed time: 0 ms })), ], }); - const projects = repositories.filter( - (repository) => repository.name !== 'metamask-module-template', - ); - for (const project of projects) { + for (const repository of repositories) { await ensureDirectoryStructureExists( - path.join(project.directoryPath, 'src'), + path.join(repository.directoryPath, 'src'), + ); + await writeFile( + path.join(repository.directoryPath, '.yarnrc.yml'), + '', + ); + await writeFile( + path.join( + repository.directoryPath, + '.yarn', + 'plugins', + 'some-plugin', + ), + 'content for some-plugin', + ); + await writeFile( + path.join( + repository.directoryPath, + '.yarn', + 'releases', + 'some-release', + ), + 'content for some-release', + ); + await writeFile( + path.join(repository.directoryPath, 'package.json'), + JSON.stringify({ packageManager: 'yarn' }), + ); + await writeFile( + path.join(repository.directoryPath, 'README.md'), + 'Install [Yarn whatever](...)', ); } - const stdout = new MockWritable(); - const stderr = new MockWritable(); + const outputLogger = new FakeOutputLogger(); await main({ argv: ['node', 'module-lint'], - stdout, - stderr, + stdout: outputLogger.stdout, + stderr: outputLogger.stderr, config: { cachedRepositoriesDirectoryPath, defaultProjectNames: projectNames, }, }); - const output = stdout.data().map(stripAnsi).join(''); - - expect(output).toBe( + expect(outputLogger.getStderr()).toBe(''); + expect(outputLogger.getStdout()).toBe( ` repo-1 ------ +- Is the classic Yarn config file (\`.yarnrc\`) absent? ✅ +- Does the package have a well-formed manifest (\`package.json\`)? ✅ + - Does the \`packageManager\` field in \`package.json\` conform? ✅ +- Is \`README.md\` present? ✅ + - Does the README conform by recommending the correct Yarn version to install? ✅ +- Are all of the files for Yarn Modern present, and do they conform? ✅ + - Does the README conform by recommending the correct Yarn version to install? ✅ - Does the \`src/\` directory exist? ✅ -Results: 1 passed, 0 failed, 1 total +Results: 8 passed, 0 failed, 8 total Elapsed time: 0 ms repo-2 ------ +- Is the classic Yarn config file (\`.yarnrc\`) absent? ✅ +- Does the package have a well-formed manifest (\`package.json\`)? ✅ + - Does the \`packageManager\` field in \`package.json\` conform? ✅ +- Is \`README.md\` present? ✅ + - Does the README conform by recommending the correct Yarn version to install? ✅ +- Are all of the files for Yarn Modern present, and do they conform? ✅ + - Does the README conform by recommending the correct Yarn version to install? ✅ - Does the \`src/\` directory exist? ✅ -Results: 1 passed, 0 failed, 1 total +Results: 8 passed, 0 failed, 8 total Elapsed time: 0 ms `, @@ -262,7 +367,7 @@ Elapsed time: 0 ms }); }); - it('lists the rules executed against the default repositories which fail', async () => { + it('lists the failing rules executed against the default repositories', async () => { await withinSandbox(async ({ directoryPath: sandboxDirectoryPath }) => { const projectNames = ['repo-1', 'repo-2']; const { cachedRepositoriesDirectoryPath } = @@ -277,40 +382,56 @@ Elapsed time: 0 ms })), ], }); - const stdout = new MockWritable(); - const stderr = new MockWritable(); + const outputLogger = new FakeOutputLogger(); await main({ argv: ['node', 'module-lint'], - stdout, - stderr, + stdout: outputLogger.stdout, + stderr: outputLogger.stderr, config: { cachedRepositoriesDirectoryPath, defaultProjectNames: projectNames, }, }); - const output = stdout.data().map(stripAnsi).join(''); - - expect(output).toBe( + expect(outputLogger.getStderr()).toBe(''); + expect(outputLogger.getStdout()).toBe( ` repo-1 ------ +- Is the classic Yarn config file (\`.yarnrc\`) absent? ✅ +- Does the package have a well-formed manifest (\`package.json\`)? ❌ + - \`package.json\` does not exist in this project. +- Is \`README.md\` present? ❌ + - \`README.md\` does not exist in this project. +- Are all of the files for Yarn Modern present, and do they conform? ❌ + - \`.yarnrc.yml\` does not exist in this project. + - \`.yarn/releases/\` does not exist in this project. + - \`.yarn/plugins/\` does not exist in this project. - Does the \`src/\` directory exist? ❌ - \`src/\` does not exist in this project. -Results: 0 passed, 1 failed, 1 total +Results: 1 passed, 4 failed, 5 total Elapsed time: 0 ms repo-2 ------ +- Is the classic Yarn config file (\`.yarnrc\`) absent? ✅ +- Does the package have a well-formed manifest (\`package.json\`)? ❌ + - \`package.json\` does not exist in this project. +- Is \`README.md\` present? ❌ + - \`README.md\` does not exist in this project. +- Are all of the files for Yarn Modern present, and do they conform? ❌ + - \`.yarnrc.yml\` does not exist in this project. + - \`.yarn/releases/\` does not exist in this project. + - \`.yarn/plugins/\` does not exist in this project. - Does the \`src/\` directory exist? ❌ - \`src/\` does not exist in this project. -Results: 0 passed, 1 failed, 1 total +Results: 1 passed, 4 failed, 5 total Elapsed time: 0 ms `, @@ -340,37 +461,44 @@ Elapsed time: 0 ms }, ], }); - const stdout = new MockWritable(); - const stderr = new MockWritable(); + const outputLogger = new FakeOutputLogger(); await main({ argv: ['node', 'module-lint'], - stdout, - stderr, + stdout: outputLogger.stdout, + stderr: outputLogger.stderr, config: { cachedRepositoriesDirectoryPath, defaultProjectNames: projectNames, }, }); - expect(stdout.data().map(stripAnsi).join('')).toBe( + expect(outputLogger.getStderr()).toContain( + `Could not resolve 'repo-1' as it is neither a reference to a directory nor the name of a known MetaMask repository.`, + ); + expect(outputLogger.getStdout()).toBe( `Cloning repository MetaMask/repo-2, please wait... repo-2 ------ +- Is the classic Yarn config file (\`.yarnrc\`) absent? ✅ +- Does the package have a well-formed manifest (\`package.json\`)? ❌ + - \`package.json\` does not exist in this project. +- Is \`README.md\` present? ❌ + - \`README.md\` does not exist in this project. +- Are all of the files for Yarn Modern present, and do they conform? ❌ + - \`.yarnrc.yml\` does not exist in this project. + - \`.yarn/releases/\` does not exist in this project. + - \`.yarn/plugins/\` does not exist in this project. - Does the \`src/\` directory exist? ❌ - \`src/\` does not exist in this project. -Results: 0 passed, 1 failed, 1 total +Results: 1 passed, 4 failed, 5 total Elapsed time: 0 ms `, ); - - expect(stderr.data().map(stripAnsi).join('')).toContain( - `Could not resolve 'repo-1' as it is neither a reference to a directory nor the name of a known MetaMask repository.`, - ); }); }); }); diff --git a/src/misc-utils.test.ts b/src/misc-utils.test.ts index 722d448..d61c504 100644 --- a/src/misc-utils.test.ts +++ b/src/misc-utils.test.ts @@ -1,16 +1,29 @@ import { writeFile } from '@metamask/utils/node'; import fs from 'fs'; import path from 'path'; +import * as superstruct from 'superstruct'; import { + assertJsonMatchesStruct, getEntryStats, indent, isPromiseFulfilledResult, isPromiseRejectedResult, + readDirectoryRecursively, repeat, + wrapError, } from './misc-utils'; import { withinSandbox } from '../tests/helpers'; +// Clone the superstruct module so we can spy on its exports +jest.mock('superstruct', () => { + return { + ...jest.requireActual('superstruct'), + // eslint-disable-next-line @typescript-eslint/naming-convention + __esModule: true, + }; +}); + describe('getEntryStats', () => { describe('given a file', () => { it('returns the stats for the file', async () => { @@ -69,6 +82,439 @@ describe('getEntryStats', () => { }); }); +describe('assertJsonMatchesStruct', () => { + it('does not throw if the given object conforms to the given Superstruct schema', () => { + const Repo = superstruct.object({ + name: superstruct.string(), + numberOfStars: superstruct.integer(), + }); + + expect(() => { + assertJsonMatchesStruct({ name: 'utils', numberOfStars: 294 }, Repo); + }).not.toThrow(); + }); + + it('throws a descriptive error if the given object does not conform to the given Superstruct schema', () => { + const Repo = superstruct.object({ + name: superstruct.string(), + numberOfStars: superstruct.integer(), + }); + + expect(() => { + assertJsonMatchesStruct({ numberOfStars: 'whatever' }, Repo); + }).toThrow( + new Error( + 'Missing `name`; Invalid `numberOfStars` (Expected an integer, but received: "whatever").', + ), + ); + }); + + it('re-throws an unknown error that the Superstruct assert function throws', () => { + const Repo = superstruct.object({ + name: superstruct.string(), + numberOfStars: superstruct.integer(), + }); + const error = new Error('oops'); + jest.spyOn(superstruct, 'assert').mockImplementation(() => { + throw error; + }); + + expect(() => { + assertJsonMatchesStruct({ numberOfStars: 'whatever' }, Repo); + }).toThrow(error); + }); +}); + +describe('readDirectoryRecursively', () => { + it('reads the directory and all of its child directories, returning a flat list of files and their paths', async () => { + await withinSandbox(async (sandbox) => { + const innerDirectoryPath = path.join(sandbox.directoryPath, 'x'); + await writeFile(path.join(innerDirectoryPath, 'a'), ''); + await writeFile(path.join(innerDirectoryPath, 'b', 'c'), ''); + await writeFile(path.join(innerDirectoryPath, 'b', 'd', 'e'), ''); + + const entries = await readDirectoryRecursively(innerDirectoryPath); + + expect(entries).toStrictEqual([ + expect.objectContaining({ + fullPath: path.join(innerDirectoryPath, 'a'), + relativePath: 'a', + }), + expect.objectContaining({ + fullPath: path.join(innerDirectoryPath, 'b/c'), + relativePath: 'b/c', + }), + expect.objectContaining({ + fullPath: path.join(innerDirectoryPath, 'b/d/e'), + relativePath: 'b/d/e', + }), + ]); + }); + }); + + it('maps the paths for each file relative to the rootDirectoryPath', async () => { + await withinSandbox(async (sandbox) => { + const { directoryPath: rootDirectoryPath } = sandbox; + const innerDirectoryPath = path.join(sandbox.directoryPath, 'x'); + await writeFile(path.join(innerDirectoryPath, 'a'), ''); + await writeFile(path.join(innerDirectoryPath, 'b', 'c'), ''); + await writeFile(path.join(innerDirectoryPath, 'b', 'd', 'e'), ''); + + const entries = await readDirectoryRecursively( + innerDirectoryPath, + rootDirectoryPath, + ); + + expect(entries).toStrictEqual([ + expect.objectContaining({ + fullPath: path.join(innerDirectoryPath, 'a'), + relativePath: 'x/a', + }), + expect.objectContaining({ + fullPath: path.join(innerDirectoryPath, 'b/c'), + relativePath: 'x/b/c', + }), + expect.objectContaining({ + fullPath: path.join(innerDirectoryPath, 'b/d/e'), + relativePath: 'x/b/d/e', + }), + ]); + }); + }); + + it('re-throws a wrapped version of any other error that occurs, assigning it the same code and giving it a stack', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + // Make sandbox root directory non-readable. + await fs.promises.chmod(sandbox.directoryPath, 0o000); + + try { + await expect( + readDirectoryRecursively(sandbox.directoryPath), + ).rejects.toThrow( + expect.objectContaining({ + message: `Could not read directory '${sandbox.directoryPath}'`, + code: 'EACCES', + stack: expect.any(String), + cause: expect.objectContaining({ + message: `EACCES: permission denied, scandir '${sandbox.directoryPath}'`, + code: 'EACCES', + }), + }), + ); + } finally { + // Make sandbox root directory executable again. + // Ideally, this should be handled by @metamask/utils. + await fs.promises.chmod(sandbox.directoryPath, 0o700); + } + }); + }); +}); + +// NOTE: A lot of these errors were copied from `@metamask/utils` +// TODO: Copy these back to `@metamask/utils` +describe('wrapError', () => { + describe('if the original error is an Error instance', () => { + describe('if the Error was not generated by fs.promises', () => { + describe('if the global Error constructor takes a "cause" argument', () => { + let OriginalError: ErrorConstructor; + + beforeEach(() => { + OriginalError = global.Error; + + class MockError extends Error { + constructor(message: string, options: { cause?: Error } = {}) { + super(message); + this.cause = options.cause; + } + + cause: Error | undefined; + } + + // This is necessary as the `length` of our Error won't be 2 otherwise + Object.defineProperty(MockError, 'length', { value: 2 }); + + // NOTE: When we upgrade Jest, change this to use: + // jest.replaceProperty(global, 'Error', MockError); + global.Error = MockError as unknown as ErrorConstructor; + }); + + afterEach(() => { + global.Error = OriginalError; + }); + + it('returns a new Error with the given message', () => { + const originalError = new Error('oops'); + + const newError = wrapError(originalError, 'Some message'); + + expect(newError.message).toBe('Some message'); + }); + + it('links to the original error via "cause"', () => { + const originalError = new Error('oops'); + + const newError = wrapError(originalError, 'Some message'); + + // @ts-expect-error Error causes are not supported by our current `tsc` + // target (ES2020 — we need ES2022 to make this work). + expect(newError.cause).toBe(originalError); + }); + + it('copies over any "code" property that exists on the given Error', () => { + const originalError = new Error('oops'); + // @ts-expect-error The Error interface doesn't have a "code" property + originalError.code = 'CODE'; + + const newError = wrapError(originalError, 'Some message'); + + expect(newError.code).toBe('CODE'); + }); + + it('allows the "code" to be overridden', () => { + const originalError = new Error('oops'); + // @ts-expect-error The Error interface doesn't have a "code" property + originalError.code = 'CODE'; + + const newError = wrapError( + originalError, + 'Some message', + 'CUSTOM_CODE', + ); + + expect(newError.code).toBe('CUSTOM_CODE'); + }); + }); + + describe('if the global Error constructor does not take a "cause" argument', () => { + it('returns a new Error with the given message', () => { + const originalError = new Error('oops'); + + const newError = wrapError(originalError, 'Some message'); + + expect(newError.message).toBe('Some message'); + }); + + it('links to the original error via "cause"', () => { + const originalError = new Error('oops'); + + const newError = wrapError(originalError, 'Some message'); + + // @ts-expect-error Error causes are not supported by our current `tsc` + // target (ES2020 — we need ES2022 to make this work). + expect(newError.cause).toBe(originalError); + }); + + it('copies over any "code" property that exists on the given Error', () => { + const originalError = new Error('oops'); + // @ts-expect-error The Error interface doesn't have a "code" property + originalError.code = 'CODE'; + + const newError = wrapError(originalError, 'Some message'); + + expect(newError.code).toBe('CODE'); + }); + + it('allows the code to be overridden', () => { + const originalError = new Error('oops'); + // @ts-expect-error The Error interface doesn't have a "code" property + originalError.code = 'CODE'; + + const newError = wrapError( + originalError, + 'Some message', + 'CUSTOM_CODE', + ); + + expect(newError.code).toBe('CUSTOM_CODE'); + }); + }); + }); + + describe('if the Error was generated by fs.promises', () => { + it('returns a new Error with the given message', async () => { + let originalError; + try { + await fs.promises.readFile('/tmp/nonexistent', 'utf8'); + } catch (error: any) { + originalError = error; + } + + const newError = wrapError(originalError, 'Some message'); + + expect(newError.message).toBe('Some message'); + }); + + it("links to the original error via 'cause'", async () => { + let originalError; + try { + await fs.promises.readFile('/tmp/nonexistent', 'utf8'); + } catch (error: any) { + originalError = error; + } + + const newError = wrapError(originalError, 'Some message'); + + // @ts-expect-error Error causes are not supported by our current `tsc` + // target (ES2020 — we need ES2022 to make this work). + expect(newError.cause).toBe(originalError); + }); + + it('copies over any "code" property that exists on the given Error', async () => { + let originalError; + try { + await fs.promises.readFile('/tmp/nonexistent', 'utf8'); + } catch (error: any) { + originalError = error; + } + + const newError = wrapError(originalError, 'Some message'); + + expect(newError.code).toBe('ENOENT'); + }); + + it('allows the code to be overridden', () => { + const originalError = new Error('oops'); + // @ts-expect-error The Error interface doesn't have a "code" property + originalError.code = 'CODE'; + + const newError = wrapError( + originalError, + 'Some message', + 'CUSTOM_CODE', + ); + + expect(newError.code).toBe('CUSTOM_CODE'); + }); + }); + }); + + describe('if the original error is an object but not an Error instance', () => { + describe('if the message is a non-empty string', () => { + it('combines a string version of the original error and message together in a new Error', () => { + const originalError = { some: 'error' }; + + const newError = wrapError(originalError, 'Some message'); + + expect(newError.message).toBe('[object Object]: Some message'); + }); + + it('does not set a cause on the new Error', async () => { + const originalError = { some: 'error' }; + + const newError = wrapError(originalError, 'Some message'); + + // @ts-expect-error Error causes are not supported by our current `tsc` + // target (ES2020 — we need ES2022 to make this work). + expect(newError.cause).toBeUndefined(); + }); + + it('does not set a code on the new Error by default', async () => { + const originalError = { some: 'error' }; + + const newError = wrapError(originalError, 'Some message'); + + expect(newError.code).toBeUndefined(); + }); + + it('allows the code to be set', () => { + const originalError = { some: 'error' }; + + const newError = wrapError( + originalError, + 'Some message', + 'CUSTOM_CODE', + ); + + expect(newError.code).toBe('CUSTOM_CODE'); + }); + }); + + describe('if the message is an empty string', () => { + it('places a string version of the original error in a new Error object without an additional message', () => { + const originalError = { some: 'error' }; + + const newError = wrapError(originalError, ''); + + expect(newError.message).toBe('[object Object]'); + }); + + it('does not set a cause on the new Error', async () => { + const originalError = { some: 'error' }; + + const newError = wrapError(originalError, ''); + + // @ts-expect-error Error causes are not supported by our current `tsc` + // target (ES2020 — we need ES2022 to make this work). + expect(newError.cause).toBeUndefined(); + }); + + it('does not set a code on the new Error', async () => { + const originalError = { some: 'error' }; + + const newError = wrapError(originalError, ''); + + expect(newError.code).toBeUndefined(); + }); + + it('allows the code to be set', () => { + const originalError = { some: 'error' }; + + const newError = wrapError(originalError, '', 'CUSTOM_CODE'); + + expect(newError.code).toBe('CUSTOM_CODE'); + }); + }); + }); + + describe('if the original error is a string', () => { + describe('if the message is a non-empty string', () => { + it('combines the original error and message together in a new Error', () => { + const newError = wrapError('Some original message', 'Some message'); + + expect(newError.message).toBe('Some original message: Some message'); + }); + + it('does not set a cause on the new Error', () => { + const newError = wrapError('Some original message', 'Some message'); + + // @ts-expect-error Error causes are not supported by our current `tsc` + // target (ES2020 — we need ES2022 to make this work). + expect(newError.cause).toBeUndefined(); + }); + + it('does not set a code on the new Error', () => { + const newError = wrapError('Some original message', 'Some message'); + + expect(newError.code).toBeUndefined(); + }); + }); + + describe('if the message is an empty string', () => { + it('places the original error in a new Error object without an additional message', () => { + const newError = wrapError('Some original message', ''); + + expect(newError.message).toBe('Some original message'); + }); + + it('does not set a cause on the new Error', () => { + const newError = wrapError('Some original message', ''); + + // @ts-expect-error Error causes are not supported by our current `tsc` + // target (ES2020 — we need ES2022 to make this work). + expect(newError.cause).toBeUndefined(); + }); + + it('does not set a code on the new Error', () => { + const newError = wrapError('Some original message', ''); + + expect(newError.code).toBeUndefined(); + }); + }); + }); +}); + describe('repeat', () => { it('returns a string of the given character that is of the given length', () => { expect(repeat('-', 10)).toBe('----------'); diff --git a/src/misc-utils.ts b/src/misc-utils.ts index ee09bee..f03f260 100644 --- a/src/misc-utils.ts +++ b/src/misc-utils.ts @@ -1,6 +1,26 @@ -import { isErrorWithCode, wrapError } from '@metamask/utils/node'; -import type fs from 'fs'; -import * as fsPromises from 'fs/promises'; +import { isErrorWithCode, isObject } from '@metamask/utils/node'; +import type { Json } from '@metamask/utils/node'; +import fs from 'fs'; +import path from 'path'; +import { ErrorWithCause } from 'pony-cause'; +import { StructError, assert } from 'superstruct'; +import type { Struct } from 'superstruct'; +import type { ObjectSchema } from 'superstruct/dist/utils'; + +/** + * Represents a directory entry. Like fs.Dirent, but includes a `path` property + * which is the full path to the entry. + */ +export type DirectoryEntry = fs.Dirent & { + fullPath: string; + relativePath: string; +}; + +/** + * The error code used when a JSON file does not conform to a Superstruct + * schema. + */ +const INVALID_JSON_FILE_ERROR_CODE = 'ERR_INVALID_JSON_FILE'; /** * Retrieves information about the file or directory using `fs.stat`. @@ -14,7 +34,7 @@ export async function getEntryStats( entryPath: string, ): Promise { try { - return await fsPromises.stat(entryPath); + return await fs.promises.stat(entryPath); } catch (error) { if (isErrorWithCode(error) && error.code === 'ENOENT') { return null; @@ -26,6 +46,148 @@ export async function getEntryStats( } } +/** + * Asserts that the given object matches the shape outlined by the given + * Superstruct struct. + * + * @param object - The object to check. + * @param struct - The Superstruct object struct that you want to match against + * the object. + */ +export function assertJsonMatchesStruct< + Value extends Json, + Schema extends ObjectSchema, +>(object: Json, struct: Struct): asserts object is Value { + try { + assert(object, struct); + } catch (error) { + if (error instanceof StructError) { + const failureMessages = error + .failures() + .map((failure) => { + return failure.message.endsWith('but received: undefined') + ? `Missing \`${String(failure.key)}\`` + : `Invalid \`${String(failure.key)}\` (${failure.message})`; + }) + .join('; '); + + throw wrapError( + error, + `${failureMessages}.`, + INVALID_JSON_FILE_ERROR_CODE, + ); + } + throw error; + } +} + +/** + * Read the directory at the given path, and any directories underneath it, to + * arrive at all of the non-directories underneath that directory recursively. + * + * TODO: Move to @metamask/utils. + * + * @param directoryPath - The path to the directory. + * @param rootDirectoryPath - The path to another directory that will be used as + * the base for calculating relative paths for all entries. Defaults to + * `directoryPath`. + * @returns Objects representing all files underneath the directory. + * @throws An error with a stack trace if reading fails in any way. + */ +export async function readDirectoryRecursively( + directoryPath: string, + rootDirectoryPath = directoryPath, +): Promise { + try { + const dirents = await fs.promises.readdir(directoryPath, { + withFileTypes: true, + }); + const groupsOfChildEntries = await Promise.all( + dirents.map(async (dirent) => { + const fullPath = path.join(directoryPath, dirent.name); + + if (dirent.isDirectory()) { + return await readDirectoryRecursively(fullPath, rootDirectoryPath); + } + + const entry: DirectoryEntry = Object.assign({}, dirent, { + fullPath, + relativePath: path.relative(rootDirectoryPath, fullPath), + }); + return [entry]; + }), + ); + return groupsOfChildEntries.flat(); + } catch (error) { + throw wrapError(error, `Could not read directory '${directoryPath}'`); + } +} + +/** + * Builds a new error object, linking it to the original error via the `cause` + * property if it is an Error. + * + * This is different from the `@metamask/utils` version because it allows for + * customizing the error code. + * + * @param originalError - The error to be wrapped (something throwable). + * @param message - The desired message of the new error. + * @param code - A code to add to the error. + * @returns A new error object. + */ +export function wrapError( + originalError: Throwable, + message: string, + code?: string, +): Error & { code?: string } { + let error: Error & { code?: string }; + + if (isError(originalError)) { + if (Error.length === 2) { + // @ts-expect-error Error causes are not supported by our current `tsc` + // target (ES2020 — we need ES2022 to make this work). + error = new Error(message, { cause: originalError }); + } else { + error = new ErrorWithCause(message, { cause: originalError }); + } + + if (code !== undefined) { + error.code = code; + } else if (isErrorWithCode(originalError)) { + error.code = originalError.code; + } + } else { + error = new Error( + message.length > 0 + ? `${String(originalError)}: ${message}` + : String(originalError), + ); + + if (code !== undefined) { + error.code = code; + } + } + + return error; +} + +/** + * Type guard for determining whether the given value is an instance of Error. + * For errors generated via `fs.promises`, `error instanceof Error` won't work, + * so we have to come up with another way of testing. + * + * TODO: Expose this method in `@metamask/utils`. + * + * @param error - The object to check. + * @returns A boolean. + */ +function isError(error: unknown): error is Error { + return ( + error instanceof Error || + (isObject(error) && error.constructor.name === 'Error') + ); +} + /** * Builds a string by repeating the same character some number of times. * diff --git a/src/report-project-lint-result.test.ts b/src/report-project-lint-result.test.ts index d03323e..adb9da9 100644 --- a/src/report-project-lint-result.test.ts +++ b/src/report-project-lint-result.test.ts @@ -3,7 +3,7 @@ import { reportProjectLintResult } from './report-project-lint-result'; import { FakeOutputLogger } from '../tests/fake-output-logger'; describe('reportProjectLintResult', () => { - it('outputs the rules executed against a project, in the same hierarchy as they exist, and whether they passed or failed', () => { + it('outputs the rules executed against a project, in the same hierarchy as they were run, and whether they passed or failed, along with a summary', () => { const projectLintResult: ProjectLintResult = { projectName: 'some-project', elapsedTimeIncludingLinting: 30, @@ -68,6 +68,88 @@ some-project Results: 2 passed, 1 failed, 3 total Elapsed time: 30 ms +`.trimStart(), + ); + }); + + it('prints "0 passed" if no rules passed', () => { + const projectLintResult: ProjectLintResult = { + projectName: 'some-project', + elapsedTimeIncludingLinting: 30, + elapsedTimeExcludingLinting: 0, + ruleExecutionResultTree: { + children: [ + { + result: { + ruleName: 'some-rule', + ruleDescription: 'Description for rule', + passed: false, + failures: [{ message: 'Some failure' }], + }, + elapsedTimeExcludingChildren: 0, + elapsedTimeIncludingChildren: 0, + children: [], + }, + ], + }, + }; + const outputLogger = new FakeOutputLogger(); + + reportProjectLintResult({ + projectLintResult, + outputLogger, + }); + + expect(outputLogger.getStdout()).toBe( + ` +some-project +------------ + +- Description for rule ❌ + - Some failure + +Results: 0 passed, 1 failed, 1 total +Elapsed time: 30 ms +`.trimStart(), + ); + }); + + it('prints "0 failed" if no rules failed', () => { + const projectLintResult: ProjectLintResult = { + projectName: 'some-project', + elapsedTimeIncludingLinting: 30, + elapsedTimeExcludingLinting: 0, + ruleExecutionResultTree: { + children: [ + { + result: { + ruleName: 'some-rule', + ruleDescription: 'Description for rule', + passed: true, + }, + elapsedTimeExcludingChildren: 0, + elapsedTimeIncludingChildren: 0, + children: [], + }, + ], + }, + }; + const outputLogger = new FakeOutputLogger(); + + reportProjectLintResult({ + projectLintResult, + outputLogger, + }); + + expect(outputLogger.getStdout()).toBe( + ` +some-project +------------ + +- Description for rule ✅ + +Results: 1 passed, 0 failed, 1 total +Elapsed time: 30 ms `.trimStart(), ); }); diff --git a/src/repository-filesystem.test.ts b/src/repository-filesystem.test.ts index 2f490ae..6dffe7e 100644 --- a/src/repository-filesystem.test.ts +++ b/src/repository-filesystem.test.ts @@ -6,6 +6,7 @@ import { import fs from 'fs'; import { mock } from 'jest-mock-extended'; import path from 'path'; +import { integer, object, string } from 'superstruct'; import { RepositoryFilesystem } from './repository-filesystem'; import { withinSandbox } from '../tests/helpers'; @@ -85,6 +86,186 @@ describe('RepositoryFilesystem', () => { }); }); + describe('readJsonFile', () => { + describe('if the file has not already been read', () => { + it('reads the file from the repository directory', async () => { + jest.spyOn(utilsMock, 'readJsonFile').mockResolvedValue('{}'); + const repositoryFilesystem = new RepositoryFilesystem( + '/some/directory', + ); + + await repositoryFilesystem.readJsonFile('some.file'); + + expect(utilsMock.readJsonFile).toHaveBeenCalledWith( + '/some/directory/some.file', + ); + }); + + it('returns the contents of the file as a JSON value', async () => { + await withinSandbox(async ({ directoryPath: sandboxDirectoryPath }) => { + await writeFile( + path.join(sandboxDirectoryPath, 'some.file'), + JSON.stringify({ foo: 'bar' }), + ); + const repositoryFilesystem = new RepositoryFilesystem( + sandboxDirectoryPath, + ); + + const content = await repositoryFilesystem.readJsonFile('some.file'); + + expect(content).toStrictEqual({ foo: 'bar' }); + }); + }); + }); + + describe('if the file has already been read', () => { + it('does not read the file from the repository directory again', async () => { + jest.spyOn(utilsMock, 'readJsonFile').mockResolvedValue('{}'); + const repositoryFilesystem = new RepositoryFilesystem( + '/some/directory', + ); + await repositoryFilesystem.readJsonFile('/some/file'); + + await repositoryFilesystem.readJsonFile('/some/file'); + + expect(utilsMock.readJsonFile).toHaveBeenCalledTimes(1); + }); + + it('returns the content of the file, with extra whitespace trimmed', async () => { + await withinSandbox(async ({ directoryPath: sandboxDirectoryPath }) => { + await writeFile( + path.join(sandboxDirectoryPath, 'some.file'), + JSON.stringify({ foo: 'bar' }), + ); + const repositoryFilesystem = new RepositoryFilesystem( + sandboxDirectoryPath, + ); + await repositoryFilesystem.readJsonFile('some.file'); + + const content = await repositoryFilesystem.readJsonFile('some.file'); + + expect(content).toStrictEqual({ foo: 'bar' }); + }); + }); + }); + }); + + describe('readJsonFileAs', () => { + describe('if the file has not already been read', () => { + it('reads the file from the repository directory', async () => { + jest.spyOn(utilsMock, 'readJsonFile').mockResolvedValue('{}'); + const repositoryFilesystem = new RepositoryFilesystem( + '/some/directory', + ); + + await repositoryFilesystem.readJsonFile('somefile.json'); + + expect(utilsMock.readJsonFile).toHaveBeenCalledWith( + '/some/directory/somefile.json', + ); + }); + + it('returns the content of the file as a JSON value if it conforms to the given Superstruct schema', async () => { + await withinSandbox(async ({ directoryPath: sandboxDirectoryPath }) => { + await writeFile( + path.join(sandboxDirectoryPath, 'somefile.json'), + JSON.stringify({ name: 'utils', numberOfStars: 294 }), + ); + const Repo = object({ + name: string(), + numberOfStars: integer(), + }); + const repositoryFilesystem = new RepositoryFilesystem( + sandboxDirectoryPath, + ); + + const person = await repositoryFilesystem.readJsonFileAs( + 'somefile.json', + Repo, + ); + + expect(person).toStrictEqual({ name: 'utils', numberOfStars: 294 }); + }); + }); + + it('throws a descriptive error if the content of the file does not conform to the given Superstruct schema', async () => { + await withinSandbox(async ({ directoryPath: sandboxDirectoryPath }) => { + await writeFile( + path.join(sandboxDirectoryPath, 'somefile.json'), + JSON.stringify({ numberOfStars: 'whatever' }), + ); + const Repo = object({ + name: string(), + numberOfStars: integer(), + }); + const repositoryFilesystem = new RepositoryFilesystem( + sandboxDirectoryPath, + ); + + await expect( + repositoryFilesystem.readJsonFileAs('somefile.json', Repo), + ).rejects.toThrow( + new Error( + 'Missing `name`; Invalid `numberOfStars` (Expected an integer, but received: "whatever").', + ), + ); + }); + }); + }); + + describe('if the file has already been read', () => { + it('does not read the file from the repository directory again', async () => { + jest + .spyOn(utilsMock, 'readJsonFile') + .mockResolvedValue({ name: 'utils', numberOfStars: 294 }); + const Repo = object({ + name: string(), + numberOfStars: integer(), + }); + const repositoryFilesystem = new RepositoryFilesystem( + '/some/directory', + ); + + await repositoryFilesystem.readJsonFileAs('/some/file', Repo); + await repositoryFilesystem.readJsonFileAs('/some/file', Repo); + + expect(utilsMock.readJsonFile).toHaveBeenCalledTimes(1); + }); + }); + }); + + describe('readDirectoryRecursively', () => { + it('reads the directory and all of its child directories, returning a flat list of files and their paths', async () => { + await withinSandbox(async ({ directoryPath: sandboxDirectoryPath }) => { + await writeFile(path.join(sandboxDirectoryPath, 'a'), ''); + await writeFile(path.join(sandboxDirectoryPath, 'b', 'c'), ''); + await writeFile(path.join(sandboxDirectoryPath, 'b', 'd', 'e'), ''); + + const repositoryFilesystem = new RepositoryFilesystem( + sandboxDirectoryPath, + ); + const entries = await repositoryFilesystem.readDirectoryRecursively( + '.', + ); + + expect(entries).toStrictEqual([ + expect.objectContaining({ + fullPath: path.join(sandboxDirectoryPath, 'a'), + relativePath: 'a', + }), + expect.objectContaining({ + fullPath: path.join(sandboxDirectoryPath, 'b/c'), + relativePath: 'b/c', + }), + expect.objectContaining({ + fullPath: path.join(sandboxDirectoryPath, 'b/d/e'), + relativePath: 'b/d/e', + }), + ]); + }); + }); + }); + describe('getEntryStats', () => { describe('given a file', () => { describe('if stats have not been requested for the file already', () => { @@ -164,7 +345,7 @@ describe('RepositoryFilesystem', () => { '/some/directory', ); - await repositoryFilesystem.getEntryStats('/another/directory'); + await repositoryFilesystem.getEntryStats('another/directory'); expect(fs.promises.stat).toHaveBeenCalledWith( '/some/directory/another/directory', diff --git a/src/repository-filesystem.ts b/src/repository-filesystem.ts index b46d390..c585abc 100644 --- a/src/repository-filesystem.ts +++ b/src/repository-filesystem.ts @@ -1,8 +1,16 @@ -import { readFile } from '@metamask/utils/node'; +import type { Json } from '@metamask/utils/node'; +import { readFile, readJsonFile } from '@metamask/utils/node'; import type fs from 'fs'; import path from 'path'; +import type { Struct } from 'superstruct'; +import type { ObjectSchema } from 'superstruct/dist/utils'; -import { getEntryStats } from './misc-utils'; +import type { DirectoryEntry } from './misc-utils'; +import { + assertJsonMatchesStruct, + getEntryStats, + readDirectoryRecursively, +} from './misc-utils'; /** * Used to access files within either a project (the repository being linted) or @@ -15,6 +23,8 @@ export class RepositoryFilesystem { #fileContents: Record; + #jsonFileContents: Record; + #entryStats: Record; /** @@ -25,6 +35,7 @@ export class RepositoryFilesystem { constructor(directoryPath: string) { this.#directoryPath = directoryPath; this.#fileContents = {}; + this.#jsonFileContents = {}; this.#entryStats = {}; } @@ -42,6 +53,54 @@ export class RepositoryFilesystem { return content; } + /** + * Reads a JSON file within the repository. + * + * @param filePath - The path to the file relative to the repository root. + * @returns The contents of the file as a JSON object. + */ + async readJsonFile(filePath: string): Promise { + const cachedContent = this.#jsonFileContents[filePath]; + const fullPath = this.#getFullPath(filePath); + const content = cachedContent ?? (await readJsonFile(fullPath)); + this.#jsonFileContents[filePath] = content; + return content; + } + + /** + * Reads a JSON file within the repository, ensuring that it matches the + * given Superstruct struct. + * + * @param filePath - The path to the file relative to the repository root. + * @param struct - The Superstruct object struct that you want to match + * against the content of the file. + * @returns The contents of the file as a JSON object. + */ + async readJsonFileAs( + filePath: string, + struct: Struct, + ): Promise { + const content = await this.readJsonFile(filePath); + assertJsonMatchesStruct(content, struct); + return content; + } + + /** + * Reads a directory recursively within the repository. + * + * @param directoryPath - The path to the directory relative to the repository + * root. + * @returns Objects representing the entries in the directory. + */ + async readDirectoryRecursively( + directoryPath: string, + ): Promise { + return await readDirectoryRecursively( + this.#getFullPath(directoryPath), + this.#directoryPath, + ); + } + /** * Retrieves stats for the given file or directory. * @@ -64,6 +123,6 @@ export class RepositoryFilesystem { * @returns The full path. */ #getFullPath(entryPath: string): string { - return path.join(this.#directoryPath, entryPath); + return path.resolve(this.#directoryPath, entryPath); } } diff --git a/src/rules/all-yarn-modern-files-conform.test.ts b/src/rules/all-yarn-modern-files-conform.test.ts new file mode 100644 index 0000000..3b78011 --- /dev/null +++ b/src/rules/all-yarn-modern-files-conform.test.ts @@ -0,0 +1,323 @@ +import { writeFile } from '@metamask/utils/node'; +import path from 'path'; + +import allYarnModernFilesConform from './all-yarn-modern-files-conform'; +import { buildMetaMaskRepository, withinSandbox } from '../../tests/helpers'; +import { fail, pass } from '../execute-rules'; + +describe('Rule: all-yarn-modern-files-conform', () => { + it("passes if the template's .yarnrc.yml file, .yarn/releases directory, and .yarn/plugins directory match the project's", async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join(template.directoryPath, '.yarnrc.yml'), + 'content of yarnrc', + ); + await writeFile( + path.join(template.directoryPath, '.yarn', 'releases', 'some-release'), + 'content of some-release', + ); + await writeFile( + path.join(template.directoryPath, '.yarn', 'plugins', 'some-plugin'), + 'content of some-plugin', + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, '.yarnrc.yml'), + 'content of yarnrc', + ); + await writeFile( + path.join(project.directoryPath, '.yarn', 'releases', 'some-release'), + 'content of some-release', + ); + await writeFile( + path.join(project.directoryPath, '.yarn', 'plugins', 'some-plugin'), + 'content of some-plugin', + ); + + const result = await allYarnModernFilesConform.execute({ + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: true, + }); + }); + }); + + it('fails if the project does not have a .yarnrc.yml', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join(template.directoryPath, '.yarnrc.yml'), + 'content of yarnrc', + ); + await writeFile( + path.join(template.directoryPath, '.yarn', 'releases', 'some-release'), + 'content of some-release', + ); + await writeFile( + path.join(template.directoryPath, '.yarn', 'plugins', 'some-plugin'), + 'content of some-plugin', + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, '.yarn', 'releases', 'some-release'), + 'content of some-release', + ); + await writeFile( + path.join(project.directoryPath, '.yarn', 'plugins', 'some-plugin'), + 'content of some-plugin', + ); + + const result = await allYarnModernFilesConform.execute({ + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { + message: '`.yarnrc.yml` does not exist in this project.', + }, + ], + }); + }); + }); + + it('fails if the project does not have a .yarn/releases directory', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join(template.directoryPath, '.yarnrc.yml'), + 'content of yarnrc', + ); + await writeFile( + path.join(template.directoryPath, '.yarn', 'releases', 'some-release'), + 'content of some-release', + ); + await writeFile( + path.join(template.directoryPath, '.yarn', 'plugins', 'some-plugin'), + 'content of some-plugin', + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, '.yarnrc.yml'), + 'content of yarnrc', + ); + await writeFile( + path.join(project.directoryPath, '.yarn', 'plugins', 'some-plugin'), + 'content of some-plugin', + ); + + const result = await allYarnModernFilesConform.execute({ + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { + message: '`.yarn/releases/` does not exist in this project.', + }, + ], + }); + }); + }); + + it("fails if a file in the template's .yarn/releases directory does not match the same file in the project", async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join(template.directoryPath, '.yarnrc.yml'), + 'content of yarnrc', + ); + await writeFile( + path.join(template.directoryPath, '.yarn', 'releases', 'some-release'), + 'content of some-release', + ); + await writeFile( + path.join(template.directoryPath, '.yarn', 'plugins', 'some-plugin'), + 'content of some-plugin', + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, '.yarnrc.yml'), + 'content of yarnrc', + ); + await writeFile( + path.join(project.directoryPath, '.yarn', 'releases', 'some-release'), + 'content of some-release xxxxxx', + ); + await writeFile( + path.join(project.directoryPath, '.yarn', 'plugins', 'some-plugin'), + 'content of some-plugin', + ); + + const result = await allYarnModernFilesConform.execute({ + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { + message: + '`.yarn/releases/some-release` does not match the same file in the template repo.', + }, + ], + }); + }); + }); + + it('fails if the project does not have a .yarn/plugins directory', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join(template.directoryPath, '.yarnrc.yml'), + 'content of yarnrc', + ); + await writeFile( + path.join(template.directoryPath, '.yarn', 'releases', 'some-release'), + 'content of some-release', + ); + await writeFile( + path.join(template.directoryPath, '.yarn', 'plugins', 'some-plugin'), + 'content of some-plugin', + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, '.yarnrc.yml'), + 'content of yarnrc', + ); + await writeFile( + path.join(project.directoryPath, '.yarn', 'releases', 'some-release'), + 'content of some-release', + ); + + const result = await allYarnModernFilesConform.execute({ + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { + message: '`.yarn/plugins/` does not exist in this project.', + }, + ], + }); + }); + }); + + it("fails if a file in the template's .yarn/plugins directory does not match the same file in the project", async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join(template.directoryPath, '.yarnrc.yml'), + 'content of yarnrc', + ); + await writeFile( + path.join(template.directoryPath, '.yarn', 'releases', 'some-release'), + 'content of some-release', + ); + await writeFile( + path.join(template.directoryPath, '.yarn', 'plugins', 'some-plugin'), + 'content of some-plugin', + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, '.yarnrc.yml'), + 'content of yarnrc', + ); + await writeFile( + path.join(project.directoryPath, '.yarn', 'releases', 'some-release'), + 'content of some-release', + ); + await writeFile( + path.join(project.directoryPath, '.yarn', 'plugins', 'some-plugin'), + 'content of some-plugin xxxxxx', + ); + + const result = await allYarnModernFilesConform.execute({ + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { + message: + '`.yarn/plugins/some-plugin` does not match the same file in the template repo.', + }, + ], + }); + }); + }); +}); diff --git a/src/rules/all-yarn-modern-files-conform.ts b/src/rules/all-yarn-modern-files-conform.ts new file mode 100644 index 0000000..f975615 --- /dev/null +++ b/src/rules/all-yarn-modern-files-conform.ts @@ -0,0 +1,23 @@ +import { + buildRule, + combineRuleExecutionResults, + directoryConforms, + fileConforms, +} from './helpers'; +import { RuleName } from './types'; + +export default buildRule({ + name: RuleName.AllYarnModernFilesConform, + description: + 'Are all of the files for Yarn Modern present, and do they conform?', + dependencies: [], + execute: async (ruleExecutionArguments) => { + const results = await Promise.all([ + fileConforms('.yarnrc.yml', ruleExecutionArguments), + directoryConforms('.yarn/releases', ruleExecutionArguments), + directoryConforms('.yarn/plugins', ruleExecutionArguments), + ]); + + return combineRuleExecutionResults(results, ruleExecutionArguments); + }, +}); diff --git a/src/rules/classic-yarn-config-file-absent.test.ts b/src/rules/classic-yarn-config-file-absent.test.ts new file mode 100644 index 0000000..a86252a --- /dev/null +++ b/src/rules/classic-yarn-config-file-absent.test.ts @@ -0,0 +1,62 @@ +import { writeFile } from '@metamask/utils/node'; +import path from 'path'; + +import classicYarnConfigFileAbsent from './classic-yarn-config-file-absent'; +import { buildMetaMaskRepository, withinSandbox } from '../../tests/helpers'; +import { fail, pass } from '../execute-rules'; + +describe('Rule: classic-yarn-config-file-absent', () => { + it('passes if .yarnrc is not present in the project', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + + const result = await classicYarnConfigFileAbsent.execute({ + template: buildMetaMaskRepository(), + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: true, + }); + }); + }); + + it('fails if .yarnrc is present in the project', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, '.yarnrc'), + 'content of yarnrc', + ); + + const result = await classicYarnConfigFileAbsent.execute({ + template: buildMetaMaskRepository(), + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { + message: + 'The config file for Yarn Classic, `.yarnrc`, is present. Please upgrade this project to Yarn Modern.', + }, + ], + }); + }); + }); +}); diff --git a/src/rules/classic-yarn-config-file-absent.ts b/src/rules/classic-yarn-config-file-absent.ts new file mode 100644 index 0000000..7757661 --- /dev/null +++ b/src/rules/classic-yarn-config-file-absent.ts @@ -0,0 +1,22 @@ +import { buildRule } from './helpers'; +import { RuleName } from './types'; +import { fail, pass } from '../execute-rules'; + +export default buildRule({ + name: RuleName.ClassicYarnConfigFileAbsent, + description: 'Is the classic Yarn config file (`.yarnrc`) absent?', + dependencies: [], + execute: async ({ project }) => { + const entryPath = '.yarnrc'; + + const stats = await project.fs.getEntryStats(entryPath); + if (stats) { + return fail([ + { + message: `The config file for Yarn Classic, \`${entryPath}\`, is present. Please upgrade this project to Yarn Modern.`, + }, + ]); + } + return pass(); + }, +}); diff --git a/src/rules/helpers.test.ts b/src/rules/helpers.test.ts new file mode 100644 index 0000000..d19c072 --- /dev/null +++ b/src/rules/helpers.test.ts @@ -0,0 +1,173 @@ +import { + ensureDirectoryStructureExists, + writeFile, +} from '@metamask/utils/node'; +import path from 'path'; + +import { directoryExists, fileExists } from './helpers'; +import { buildMetaMaskRepository, withinSandbox } from '../../tests/helpers'; +import { fail, pass } from '../execute-rules'; + +describe('fileExists', () => { + it('passes if the given path refers to an existing file', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + const filePath = path.join(project.directoryPath, 'some.file'); + await writeFile(filePath, ''); + + const result = await fileExists(filePath, { + template: buildMetaMaskRepository(), + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: true, + }); + }); + }); + + it('fails if the given path does not refer to an existing file', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + const filePath = path.join(project.directoryPath, 'some.file'); + + const result = await fileExists(filePath, { + template: buildMetaMaskRepository(), + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { + message: `\`${filePath}\` does not exist in this project.`, + }, + ], + }); + }); + }); + + it('fails if the given path does refers to an entry, but it is not a file', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + const filePath = path.join(project.directoryPath, 'some.file'); + await ensureDirectoryStructureExists(filePath); + + const result = await fileExists(filePath, { + template: buildMetaMaskRepository(), + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { message: `\`${filePath}\` is not a file when it should be.` }, + ], + }); + }); + }); +}); + +describe('directoryExists', () => { + it('passes if the given path refers to an existing directory', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + const directoryPath = path.join(project.directoryPath, 'some-directory'); + await ensureDirectoryStructureExists(directoryPath); + + const result = await directoryExists(directoryPath, { + template: buildMetaMaskRepository(), + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: true, + }); + }); + }); + + it('fails if the given path does not refer to an existing directory', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + const directoryPath = path.join(project.directoryPath, 'some-directory'); + + const result = await directoryExists(directoryPath, { + template: buildMetaMaskRepository(), + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { + message: `\`${directoryPath}/\` does not exist in this project.`, + }, + ], + }); + }); + }); + + it('fails if the given path does refers to an entry, but it is not a directory', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + const directoryPath = path.join(project.directoryPath, 'some-directory'); + await writeFile(directoryPath, ''); + + const result = await directoryExists(directoryPath, { + template: buildMetaMaskRepository(), + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { + message: `\`${directoryPath}/\` is not a directory when it should be.`, + }, + ], + }); + }); + }); +}); diff --git a/src/rules/helpers.ts b/src/rules/helpers.ts index 39fe05a..23b88b4 100644 --- a/src/rules/helpers.ts +++ b/src/rules/helpers.ts @@ -4,6 +4,7 @@ import type { FailedPartialRuleExecutionResult, PartialRuleExecutionResult, Rule, + RuleExecutionArguments, SuccessfulPartialRuleExecutionResult, } from '../execute-rules'; @@ -38,8 +39,8 @@ export function buildRule({ description: string; dependencies: Exclude[]; execute(args: { - project: MetaMaskRepository; template: MetaMaskRepository; + project: MetaMaskRepository; pass: () => SuccessfulPartialRuleExecutionResult; fail: ( failures: FailedPartialRuleExecutionResult['failures'], @@ -53,3 +54,165 @@ export function buildRule({ execute, }; } + +/** + * A rule utility which determines whether a file exists in the project. + * + * @param filePath - The path to test. + * @param ruleExecutionArguments - Rule execution arguments. + * @returns Either a successful or failed result. + */ +export async function fileExists( + filePath: string, + ruleExecutionArguments: RuleExecutionArguments, +): Promise { + const { project, pass, fail } = ruleExecutionArguments; + const stats = await project.fs.getEntryStats(filePath); + + if (!stats) { + return fail([ + { + message: `\`${filePath}\` does not exist in this project.`, + }, + ]); + } + + if (!stats.isFile()) { + return fail([ + { + message: `\`${filePath}\` is not a file when it should be.`, + }, + ]); + } + + return pass(); +} + +/** + * A rule utility which determines whether a directory exists in the project. + * + * @param directoryPath - The path to test. + * @param ruleExecutionArguments - Rule execution arguments. + * @returns Either a successful or failed result. + */ +export async function directoryExists( + directoryPath: string, + ruleExecutionArguments: RuleExecutionArguments, +): Promise { + const { project, pass, fail } = ruleExecutionArguments; + const stats = await project.fs.getEntryStats(directoryPath); + + if (!stats) { + return fail([ + { + message: `\`${directoryPath}/\` does not exist in this project.`, + }, + ]); + } + + if (!stats.isDirectory()) { + return fail([ + { + message: `\`${directoryPath}/\` is not a directory when it should be.`, + }, + ]); + } + + return pass(); +} + +/** + * A rule utility which determines not only whether a file that's assumed to + * exist in the template exists in the project as well, but also whether it + * matches the same file in the template content-wise. + * + * @param filePath - The path to a file in both the template and project. + * @param ruleExecutionArguments - Rule execution arguments. + * @returns Either a successful or failed result. + */ +export async function fileConforms( + filePath: string, + ruleExecutionArguments: RuleExecutionArguments, +): Promise { + const { template, project, pass, fail } = ruleExecutionArguments; + const fileExistsResult = await fileExists(filePath, ruleExecutionArguments); + if (!fileExistsResult.passed) { + return fileExistsResult; + } + + const fileContentInTemplate = await template.fs.readFile(filePath); + const fileContentInProject = await project.fs.readFile(filePath); + + if (fileContentInProject !== fileContentInTemplate) { + return fail([ + { + message: [ + `\`${filePath}\` does not match the same file in the template repo.`, + ].join('\n'), + }, + ]); + } + return pass(); +} + +/** + * A rule utility which determines not only whether a directory that's assumed to + * exist in the template exists in the project as well, but also whether all + * files in that directory in the template are present in the project and match + * content-wise. + * + * @param directoryPath - The path to a directory in both the template and + * project. + * @param ruleExecutionArguments - Rule execution arguments. + * @returns Either a successful or failed result. + */ +export async function directoryConforms( + directoryPath: string, + ruleExecutionArguments: RuleExecutionArguments, +): Promise { + const { template } = ruleExecutionArguments; + const directoryExistsResult = await directoryExists( + directoryPath, + ruleExecutionArguments, + ); + if (!directoryExistsResult.passed) { + return directoryExistsResult; + } + + const files = await template.fs.readDirectoryRecursively(directoryPath); + const fileConformsResults = await Promise.all( + files.map(async (file) => { + return await fileConforms(file.relativePath, ruleExecutionArguments); + }), + ); + return combineRuleExecutionResults( + fileConformsResults, + ruleExecutionArguments, + ); +} + +/** + * Encapsulates multiple rule execution results into one. If all of the results + * are passing, then the combined result will be passing; otherwise, the + * combined result will be failing, and messages from failing results will be + * consolidated into a single array. + * + * @param results - The rule execution results. + * @param ruleExecutionArguments - Rule execution arguments. + * @returns The combined rule execution result. + */ +export function combineRuleExecutionResults( + results: PartialRuleExecutionResult[], + ruleExecutionArguments: RuleExecutionArguments, +): PartialRuleExecutionResult { + const { pass, fail } = ruleExecutionArguments; + const failures: FailedPartialRuleExecutionResult['failures'] = []; + + for (const result of results) { + if (!result.passed) { + failures.push(...result.failures); + } + } + + return failures.length > 0 ? fail(failures) : pass(); +} diff --git a/src/rules/index.ts b/src/rules/index.ts index 77c1f4e..bc4f9de 100644 --- a/src/rules/index.ts +++ b/src/rules/index.ts @@ -1,3 +1,17 @@ +import allYarnModernFilesConform from './all-yarn-modern-files-conform'; +import classicYarnConfigFileAbsent from './classic-yarn-config-file-absent'; +import packageManagerFieldConforms from './package-manager-field-conforms'; +import readmeListsCorrectYarnVersion from './readme-lists-correct-yarn-version'; +import requireReadme from './require-readme'; import requireSourceDirectory from './require-source-directory'; +import requireValidPackageManifest from './require-valid-package-manifest'; -export const rules = [requireSourceDirectory] as const; +export const rules = [ + allYarnModernFilesConform, + classicYarnConfigFileAbsent, + packageManagerFieldConforms, + readmeListsCorrectYarnVersion, + requireReadme, + requireSourceDirectory, + requireValidPackageManifest, +] as const; diff --git a/src/rules/package-manager-field-conforms.test.ts b/src/rules/package-manager-field-conforms.test.ts new file mode 100644 index 0000000..4d69d3a --- /dev/null +++ b/src/rules/package-manager-field-conforms.test.ts @@ -0,0 +1,79 @@ +import { writeFile } from '@metamask/utils/node'; +import path from 'path'; + +import packageManagerFieldConforms from './package-manager-field-conforms'; +import { buildMetaMaskRepository, withinSandbox } from '../../tests/helpers'; +import { fail, pass } from '../execute-rules'; + +describe('Rule: package-manager-field-conforms', () => { + it('passes if the "packageManager" field in the project\'s package.json matches the one in the template\'s package.json', async () => { + expect.assertions(1); + + 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' }), + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, 'package.json'), + JSON.stringify({ packageManager: 'a' }), + ); + + const result = await packageManagerFieldConforms.execute({ + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: true, + }); + }); + }); + + it('fails if the "packageManager" field in the project\'s package.json does not match the one in the template\'s package.json', async () => { + expect.assertions(1); + + 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' }), + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, 'package.json'), + JSON.stringify({ packageManager: 'b' }), + ); + + const result = await packageManagerFieldConforms.execute({ + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { message: '`packageManager` is "b", when it should be "a".' }, + ], + }); + }); + }); +}); diff --git a/src/rules/package-manager-field-conforms.ts b/src/rules/package-manager-field-conforms.ts new file mode 100644 index 0000000..17e3c01 --- /dev/null +++ b/src/rules/package-manager-field-conforms.ts @@ -0,0 +1,28 @@ +import { buildRule } from './helpers'; +import { PackageManifestSchema, RuleName } from './types'; + +export default buildRule({ + name: RuleName.PackageManagerFieldConforms, + description: 'Does the `packageManager` field 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, + ); + + if (projectManifest.packageManager !== templateManifest.packageManager) { + return fail([ + { + message: `\`packageManager\` is "${projectManifest.packageManager}", when it should be "${templateManifest.packageManager}".`, + }, + ]); + } + return pass(); + }, +}); diff --git a/src/rules/readme-lists-correct-yarn-version.test.ts b/src/rules/readme-lists-correct-yarn-version.test.ts new file mode 100644 index 0000000..873df4e --- /dev/null +++ b/src/rules/readme-lists-correct-yarn-version.test.ts @@ -0,0 +1,114 @@ +import { writeFile } from '@metamask/utils/node'; +import path from 'path'; + +import readmeListsCorrectYarnVersion from './readme-lists-correct-yarn-version'; +import { buildMetaMaskRepository, withinSandbox } from '../../tests/helpers'; +import { fail, pass } from '../execute-rules'; + +describe('Rule: readme-lists-correct-yarn-version', () => { + it("passes if the Yarn version listed in the project's README matches the same one in the template", async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join(template.directoryPath, 'README.md'), + 'Install [Yarn Modern](https://...)', + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, 'README.md'), + 'Install [Yarn Modern](https://...)', + ); + + const result = await readmeListsCorrectYarnVersion.execute({ + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: true, + }); + }); + }); + + it("fails if the Yarn version listed in the project's README does not match the same one in the template", async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join(template.directoryPath, 'README.md'), + 'Install [Yarn Modern](https://...)', + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, 'README.md'), + 'Install [Yarn v1](https://...)', + ); + + const result = await readmeListsCorrectYarnVersion.execute({ + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { + message: + '`README.md` should contain "Install [Yarn Modern](https://...)", but does not.', + }, + ], + }); + }); + }); + + it('throws if the template does not have the Yarn version listed in its README for some reason', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join(template.directoryPath, 'README.md'), + 'clearly something else', + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, 'README.md'), + 'does not matter', + ); + + await expect( + readmeListsCorrectYarnVersion.execute({ + template, + project, + pass, + fail, + }), + ).rejects.toThrow("Could not find Yarn version in template's README"); + }); + }); +}); diff --git a/src/rules/readme-lists-correct-yarn-version.ts b/src/rules/readme-lists-correct-yarn-version.ts new file mode 100644 index 0000000..c98118f --- /dev/null +++ b/src/rules/readme-lists-correct-yarn-version.ts @@ -0,0 +1,32 @@ +import { buildRule } from './helpers'; +import { RuleName } from './types'; + +export default buildRule({ + name: RuleName.ReadmeListsCorrectYarnVersion, + description: + 'Does the README conform by recommending the correct Yarn version to install?', + dependencies: [RuleName.RequireReadme, RuleName.AllYarnModernFilesConform], + execute: async ({ template, project, pass, fail }) => { + const entryPath = 'README.md'; + + const fileContentInTemplate = await template.fs.readFile(entryPath); + const fileContentInProject = await project.fs.readFile(entryPath); + + const match = fileContentInTemplate.match( + /Install \[Yarn [^[\]]+\]\([^()]+\)/u, + ); + if (!match?.[0]) { + throw new Error( + "Could not find Yarn version in template's README. This is not the fault of the project, but is rather a bug in a rule.", + ); + } + if (!fileContentInProject.includes(match[0])) { + return fail([ + { + message: `\`README.md\` should contain "${match[0]}", but does not.`, + }, + ]); + } + return pass(); + }, +}); diff --git a/src/rules/require-readme.test.ts b/src/rules/require-readme.test.ts new file mode 100644 index 0000000..3d88a59 --- /dev/null +++ b/src/rules/require-readme.test.ts @@ -0,0 +1,61 @@ +import { writeFile } from '@metamask/utils/node'; +import path from 'path'; + +import requireReadme from './require-readme'; +import { buildMetaMaskRepository, withinSandbox } from '../../tests/helpers'; +import { fail, pass } from '../execute-rules'; + +describe('Rule: require-readme', () => { + it('passes if the project has a README.md', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, 'README.md'), + 'content for README', + ); + + const result = await requireReadme.execute({ + template: buildMetaMaskRepository(), + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: true, + }); + }); + }); + + it('passes if the project does not have a README.md', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + + const result = await requireReadme.execute({ + template: buildMetaMaskRepository(), + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { + message: '`README.md` does not exist in this project.', + }, + ], + }); + }); + }); +}); diff --git a/src/rules/require-readme.ts b/src/rules/require-readme.ts new file mode 100644 index 0000000..da2f22c --- /dev/null +++ b/src/rules/require-readme.ts @@ -0,0 +1,11 @@ +import { buildRule, fileExists } from './helpers'; +import { RuleName } from './types'; + +export default buildRule({ + name: RuleName.RequireReadme, + description: 'Is `README.md` present?', + dependencies: [], + execute: async (ruleExecutionArguments) => { + return await fileExists('README.md', ruleExecutionArguments); + }, +}); diff --git a/src/rules/require-source-directory.ts b/src/rules/require-source-directory.ts index 281814f..a526c63 100644 --- a/src/rules/require-source-directory.ts +++ b/src/rules/require-source-directory.ts @@ -1,25 +1,11 @@ -import { buildRule } from './helpers'; +import { buildRule, directoryExists } from './helpers'; import { RuleName } from './types'; export default buildRule({ name: RuleName.RequireSourceDirectory, description: 'Does the `src/` directory exist?', dependencies: [], - execute: async ({ project, pass, fail }) => { - const entryPath = 'src'; - - const stats = await project.fs.getEntryStats(entryPath); - - if (!stats) { - return fail([ - { message: `\`${entryPath}/\` does not exist in this project.` }, - ]); - } else if (!stats.isDirectory()) { - return fail([ - { message: `\`${entryPath}/\` is not a directory when it should be.` }, - ]); - } - - return pass(); + execute: async (ruleExecutionArguments) => { + return await directoryExists('src', ruleExecutionArguments); }, }); diff --git a/src/rules/require-valid-package-manifest.test.ts b/src/rules/require-valid-package-manifest.test.ts new file mode 100644 index 0000000..c8dd7bc --- /dev/null +++ b/src/rules/require-valid-package-manifest.test.ts @@ -0,0 +1,116 @@ +import { writeFile } from '@metamask/utils/node'; +import path from 'path'; + +import requireValidPackageManifest from './require-valid-package-manifest'; +import { buildMetaMaskRepository, withinSandbox } from '../../tests/helpers'; +import { fail, pass } from '../execute-rules'; + +describe('Rule: require-package-manifest', () => { + it('passes if the project has a well-formed package.json', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, 'package.json'), + JSON.stringify({ packageManager: 'foo' }), + ); + + const result = await requireValidPackageManifest.execute({ + template: buildMetaMaskRepository(), + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: true, + }); + }); + }); + + it('fails if the project does not have a package.json', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + + const result = await requireValidPackageManifest.execute({ + template: buildMetaMaskRepository(), + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { + message: '`package.json` does not exist in this project.', + }, + ], + }); + }); + }); + + it('fails if the project has a malformed package.json', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, 'package.json'), + JSON.stringify({ foo: 'bar' }), + ); + + const result = await requireValidPackageManifest.execute({ + template: buildMetaMaskRepository(), + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { message: 'Invalid `package.json`: Missing `packageManager`.' }, + ], + }); + }); + }); + + it('re-throws a unknown error that readJsonFileAs produces', async () => { + expect.assertions(1); + + await withinSandbox(async (sandbox) => { + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, 'package.json'), + JSON.stringify({ foo: 'bar' }), + ); + const error = new Error('oops'); + jest.spyOn(project.fs, 'readJsonFileAs').mockRejectedValue(error); + + await expect( + requireValidPackageManifest.execute({ + template: buildMetaMaskRepository(), + project, + pass, + fail, + }), + ).rejects.toThrow(error); + }); + }); +}); diff --git a/src/rules/require-valid-package-manifest.ts b/src/rules/require-valid-package-manifest.ts new file mode 100644 index 0000000..c22146c --- /dev/null +++ b/src/rules/require-valid-package-manifest.ts @@ -0,0 +1,38 @@ +import { isErrorWithCode, isErrorWithMessage } from '@metamask/utils/node'; + +import { buildRule, fileExists } from './helpers'; +import { PackageManifestSchema, RuleName } from './types'; + +export default buildRule({ + name: RuleName.RequireValidPackageManifest, + description: 'Does the package have a well-formed manifest (`package.json`)?', + dependencies: [], + execute: async (ruleExecutionArguments) => { + const { project, pass, fail } = ruleExecutionArguments; + const entryPath = 'package.json'; + + const fileExistsResult = await fileExists( + entryPath, + ruleExecutionArguments, + ); + if (!fileExistsResult.passed) { + return fileExistsResult; + } + + try { + await project.fs.readJsonFileAs(entryPath, PackageManifestSchema); + return pass(); + } catch (error) { + if ( + isErrorWithCode(error) && + isErrorWithMessage(error) && + error.code === 'ERR_INVALID_JSON_FILE' + ) { + return fail([ + { message: `Invalid \`${entryPath}\`: ${error.message}` }, + ]); + } + throw error; + } + }, +}); diff --git a/src/rules/types.ts b/src/rules/types.ts index ebe527c..fe1fd41 100644 --- a/src/rules/types.ts +++ b/src/rules/types.ts @@ -1,6 +1,18 @@ +import { type, string } from 'superstruct'; + /** * All of the known rules. */ export enum RuleName { + AllYarnModernFilesConform = 'all-yarn-modern-files-conform', + ClassicYarnConfigFileAbsent = 'classic-yarn-config-file-absent', + PackageManagerFieldConforms = 'package-manager-field-conforms', + ReadmeListsCorrectYarnVersion = 'readme-lists-correct-yarn-version', + RequireReadme = 'require-readme', RequireSourceDirectory = 'require-source-directory', + RequireValidPackageManifest = 'require-valid-package-manifest', } + +export const PackageManifestSchema = type({ + packageManager: string(), +}); diff --git a/yarn.lock b/yarn.lock index 63c109a..7887425 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1018,11 +1018,13 @@ __metadata: jest: ^28.1.3 jest-it-up: ^2.0.2 jest-mock-extended: ^3.0.5 + pony-cause: ^2.1.10 prettier: ^2.7.1 prettier-plugin-packagejson: ^2.3.0 rimraf: ^3.0.2 stdio-mock: ^1.2.0 strip-ansi: ^6.0.0 + superstruct: ^1.0.3 ts-jest: ^28.0.7 ts-node: ^10.7.0 typedoc: ^0.23.15 From 52e9ce70d073c8f95cfe8933af068afcc3e89ccf Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 4 Jan 2024 13:08:42 -0700 Subject: [PATCH 02/18] Simplify customized wrapError --- package.json | 1 - src/misc-utils.ts | 51 +++++++---------------------------------------- yarn.lock | 1 - 3 files changed, 7 insertions(+), 46 deletions(-) diff --git a/package.json b/package.json index 8dc3f10..3031b0f 100644 --- a/package.json +++ b/package.json @@ -54,7 +54,6 @@ "chalk": "^4.1.2", "dependency-graph": "^0.11.0", "execa": "^5.1.1", - "pony-cause": "^2.1.10", "superstruct": "^1.0.3", "yargs": "^17.7.2" }, diff --git a/src/misc-utils.ts b/src/misc-utils.ts index f03f260..c34c427 100644 --- a/src/misc-utils.ts +++ b/src/misc-utils.ts @@ -1,8 +1,10 @@ -import { isErrorWithCode, isObject } from '@metamask/utils/node'; +import { + isErrorWithCode, + wrapError as originalWrapError, +} from '@metamask/utils/node'; import type { Json } from '@metamask/utils/node'; import fs from 'fs'; import path from 'path'; -import { ErrorWithCause } from 'pony-cause'; import { StructError, assert } from 'superstruct'; import type { Struct } from 'superstruct'; import type { ObjectSchema } from 'superstruct/dist/utils'; @@ -140,54 +142,15 @@ export function wrapError( message: string, code?: string, ): Error & { code?: string } { - let error: Error & { code?: string }; + const error = originalWrapError(originalError, message); - if (isError(originalError)) { - if (Error.length === 2) { - // @ts-expect-error Error causes are not supported by our current `tsc` - // target (ES2020 — we need ES2022 to make this work). - error = new Error(message, { cause: originalError }); - } else { - error = new ErrorWithCause(message, { cause: originalError }); - } - - if (code !== undefined) { - error.code = code; - } else if (isErrorWithCode(originalError)) { - error.code = originalError.code; - } - } else { - error = new Error( - message.length > 0 - ? `${String(originalError)}: ${message}` - : String(originalError), - ); - - if (code !== undefined) { - error.code = code; - } + if (code !== undefined) { + error.code = code; } return error; } -/** - * Type guard for determining whether the given value is an instance of Error. - * For errors generated via `fs.promises`, `error instanceof Error` won't work, - * so we have to come up with another way of testing. - * - * TODO: Expose this method in `@metamask/utils`. - * - * @param error - The object to check. - * @returns A boolean. - */ -function isError(error: unknown): error is Error { - return ( - error instanceof Error || - (isObject(error) && error.constructor.name === 'Error') - ); -} - /** * Builds a string by repeating the same character some number of times. * diff --git a/yarn.lock b/yarn.lock index 7887425..3fd32f0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1018,7 +1018,6 @@ __metadata: jest: ^28.1.3 jest-it-up: ^2.0.2 jest-mock-extended: ^3.0.5 - pony-cause: ^2.1.10 prettier: ^2.7.1 prettier-plugin-packagejson: ^2.3.0 rimraf: ^3.0.2 From 685621e04ee299e7055b2d5f24288a22c71036d9 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 4 Jan 2024 13:25:09 -0700 Subject: [PATCH 03/18] Fix docs for DirectoryEntry; rename fullPath to absolutePath --- src/misc-utils.test.ts | 12 ++++++------ src/misc-utils.ts | 18 +++++++++++------- src/repository-filesystem.test.ts | 6 +++--- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/misc-utils.test.ts b/src/misc-utils.test.ts index d61c504..360057f 100644 --- a/src/misc-utils.test.ts +++ b/src/misc-utils.test.ts @@ -137,15 +137,15 @@ describe('readDirectoryRecursively', () => { expect(entries).toStrictEqual([ expect.objectContaining({ - fullPath: path.join(innerDirectoryPath, 'a'), + absolutePath: path.join(innerDirectoryPath, 'a'), relativePath: 'a', }), expect.objectContaining({ - fullPath: path.join(innerDirectoryPath, 'b/c'), + absolutePath: path.join(innerDirectoryPath, 'b/c'), relativePath: 'b/c', }), expect.objectContaining({ - fullPath: path.join(innerDirectoryPath, 'b/d/e'), + absolutePath: path.join(innerDirectoryPath, 'b/d/e'), relativePath: 'b/d/e', }), ]); @@ -167,15 +167,15 @@ describe('readDirectoryRecursively', () => { expect(entries).toStrictEqual([ expect.objectContaining({ - fullPath: path.join(innerDirectoryPath, 'a'), + absolutePath: path.join(innerDirectoryPath, 'a'), relativePath: 'x/a', }), expect.objectContaining({ - fullPath: path.join(innerDirectoryPath, 'b/c'), + absolutePath: path.join(innerDirectoryPath, 'b/c'), relativePath: 'x/b/c', }), expect.objectContaining({ - fullPath: path.join(innerDirectoryPath, 'b/d/e'), + absolutePath: path.join(innerDirectoryPath, 'b/d/e'), relativePath: 'x/b/d/e', }), ]); diff --git a/src/misc-utils.ts b/src/misc-utils.ts index c34c427..50ebf5a 100644 --- a/src/misc-utils.ts +++ b/src/misc-utils.ts @@ -10,11 +10,12 @@ import type { Struct } from 'superstruct'; import type { ObjectSchema } from 'superstruct/dist/utils'; /** - * Represents a directory entry. Like fs.Dirent, but includes a `path` property - * which is the full path to the entry. + * Represents a directory entry. Like fs.Dirent, but includes two additional + * properties, `absolutePath` and `relativePath`, where `relativePath` is the + * path relative to its parent directory. */ export type DirectoryEntry = fs.Dirent & { - fullPath: string; + absolutePath: string; relativePath: string; }; @@ -106,15 +107,18 @@ export async function readDirectoryRecursively( }); const groupsOfChildEntries = await Promise.all( dirents.map(async (dirent) => { - const fullPath = path.join(directoryPath, dirent.name); + const absolutePath = path.join(directoryPath, dirent.name); if (dirent.isDirectory()) { - return await readDirectoryRecursively(fullPath, rootDirectoryPath); + return await readDirectoryRecursively( + absolutePath, + rootDirectoryPath, + ); } const entry: DirectoryEntry = Object.assign({}, dirent, { - fullPath, - relativePath: path.relative(rootDirectoryPath, fullPath), + absolutePath, + relativePath: path.relative(rootDirectoryPath, absolutePath), }); return [entry]; }), diff --git a/src/repository-filesystem.test.ts b/src/repository-filesystem.test.ts index 6dffe7e..6ed8e13 100644 --- a/src/repository-filesystem.test.ts +++ b/src/repository-filesystem.test.ts @@ -250,15 +250,15 @@ describe('RepositoryFilesystem', () => { expect(entries).toStrictEqual([ expect.objectContaining({ - fullPath: path.join(sandboxDirectoryPath, 'a'), + absolutePath: path.join(sandboxDirectoryPath, 'a'), relativePath: 'a', }), expect.objectContaining({ - fullPath: path.join(sandboxDirectoryPath, 'b/c'), + absolutePath: path.join(sandboxDirectoryPath, 'b/c'), relativePath: 'b/c', }), expect.objectContaining({ - fullPath: path.join(sandboxDirectoryPath, 'b/d/e'), + absolutePath: path.join(sandboxDirectoryPath, 'b/d/e'), relativePath: 'b/d/e', }), ]); From 0bd735ac5492dc08fa694fcc46b8984c29fab281 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 4 Jan 2024 13:48:15 -0700 Subject: [PATCH 04/18] Consolidate rule helpers, and keep outside rules directory for simplicity --- src/execute-rules.test.ts | 17 +-- src/execute-rules.ts | 28 +---- .../helpers.test.ts => rule-helpers.test.ts} | 20 +++- src/{rules/helpers.ts => rule-helpers.ts} | 106 +++++++----------- .../all-yarn-modern-files-conform.test.ts | 2 +- src/rules/all-yarn-modern-files-conform.ts | 8 +- src/rules/build-rule.ts | 55 +++++++++ .../classic-yarn-config-file-absent.test.ts | 2 +- src/rules/classic-yarn-config-file-absent.ts | 5 +- .../package-manager-field-conforms.test.ts | 2 +- src/rules/package-manager-field-conforms.ts | 2 +- .../readme-lists-correct-yarn-version.test.ts | 2 +- .../readme-lists-correct-yarn-version.ts | 2 +- src/rules/require-readme.test.ts | 2 +- src/rules/require-readme.ts | 3 +- src/rules/require-source-directory.test.ts | 2 +- src/rules/require-source-directory.ts | 3 +- .../require-valid-package-manifest.test.ts | 2 +- src/rules/require-valid-package-manifest.ts | 3 +- 19 files changed, 134 insertions(+), 132 deletions(-) rename src/{rules/helpers.test.ts => rule-helpers.test.ts} (89%) rename src/{rules/helpers.ts => rule-helpers.ts} (56%) create mode 100644 src/rules/build-rule.ts diff --git a/src/execute-rules.test.ts b/src/execute-rules.test.ts index 683f950..1d35480 100644 --- a/src/execute-rules.test.ts +++ b/src/execute-rules.test.ts @@ -2,7 +2,7 @@ import { mockDeep } from 'jest-mock-extended'; import type { MetaMaskRepository } from './establish-metamask-repository'; import type { Rule } from './execute-rules'; -import { executeRules, pass, fail } from './execute-rules'; +import { executeRules } from './execute-rules'; import { fakeDateOnly } from '../tests/helpers'; describe('executeRules', () => { @@ -171,18 +171,3 @@ describe('executeRules', () => { expect(ruleExecutionResultTree).toStrictEqual({ children: [] }); }); }); - -describe('pass', () => { - it('returns a result that represents a passing rule', () => { - expect(pass()).toStrictEqual({ passed: true }); - }); -}); - -describe('fail', () => { - it('returns a result that represents a failing rule, with the given failures', () => { - expect(fail([{ message: 'oops' }])).toStrictEqual({ - passed: false, - failures: [{ message: 'oops' }], - }); - }); -}); diff --git a/src/execute-rules.ts b/src/execute-rules.ts index 2bc3bfa..513c777 100644 --- a/src/execute-rules.ts +++ b/src/execute-rules.ts @@ -4,6 +4,7 @@ import type { RuleNode } from './build-rule-tree'; import { buildRuleTree } from './build-rule-tree'; import type { MetaMaskRepository } from './establish-metamask-repository'; import { createModuleLogger, projectLogger } from './logging-utils'; +import { fail, pass } from './rule-helpers'; const log = createModuleLogger(projectLogger, 'establish-metamask-repository'); @@ -234,30 +235,3 @@ async function executeRule({ children, }; } - -/** - * A helper for a rule which is intended to end its execution by marking it as - * passing. - * - * @returns Part of a successful rule execution result (the rest will be filled - * in automatically). - */ -export function pass(): SuccessfulPartialRuleExecutionResult { - return { - passed: true, - }; -} - -/** - * A helper for a rule which is intended to end its execution by marking it as - * failing. - * - * @param failures - The list of associated failures. - * @returns Part of a failed rule execution result (the rest will be filled - * in automatically). - */ -export function fail( - failures: FailedPartialRuleExecutionResult['failures'], -): FailedPartialRuleExecutionResult { - return { passed: false, failures }; -} diff --git a/src/rules/helpers.test.ts b/src/rule-helpers.test.ts similarity index 89% rename from src/rules/helpers.test.ts rename to src/rule-helpers.test.ts index d19c072..fbaf39f 100644 --- a/src/rules/helpers.test.ts +++ b/src/rule-helpers.test.ts @@ -4,9 +4,23 @@ import { } from '@metamask/utils/node'; import path from 'path'; -import { directoryExists, fileExists } from './helpers'; -import { buildMetaMaskRepository, withinSandbox } from '../../tests/helpers'; -import { fail, pass } from '../execute-rules'; +import { directoryExists, fail, fileExists, pass } from './rule-helpers'; +import { buildMetaMaskRepository, withinSandbox } from '../tests/helpers'; + +describe('pass', () => { + it('returns a result that represents a passing rule', () => { + expect(pass()).toStrictEqual({ passed: true }); + }); +}); + +describe('fail', () => { + it('returns a result that represents a failing rule, with the given failures', () => { + expect(fail([{ message: 'oops' }])).toStrictEqual({ + passed: false, + failures: [{ message: 'oops' }], + }); + }); +}); describe('fileExists', () => { it('passes if the given path refers to an existing file', async () => { diff --git a/src/rules/helpers.ts b/src/rule-helpers.ts similarity index 56% rename from src/rules/helpers.ts rename to src/rule-helpers.ts index 23b88b4..869063c 100644 --- a/src/rules/helpers.ts +++ b/src/rule-helpers.ts @@ -1,62 +1,39 @@ -import type { RuleName } from './types'; -import type { MetaMaskRepository } from '../establish-metamask-repository'; import type { + SuccessfulPartialRuleExecutionResult, FailedPartialRuleExecutionResult, PartialRuleExecutionResult, - Rule, RuleExecutionArguments, - SuccessfulPartialRuleExecutionResult, -} from '../execute-rules'; +} from './execute-rules'; /** - * Rule objects are fairly abstract: the name of a rule and the dependencies of - * a rule (which are themselves names) can be anything; and unfortunately, we - * cannot really enforce names, or else it would mean we'd have to have a - * `RuleName` type everywhere. + * A helper for a rule which is intended to end its execution by marking it as + * passing. * - * This function exists to bridge that gap at the point where the rule is - * actually defined by validating the name and dependencies against a known set - * of rules. - * - * @param args - The arguments to this function. - * @param args.name - The name of a rule. This function assumes that all rule - * names are predefined in an enum and that this is one of the values in that - * enum. - * @param args.description - The description of the rule. This will show up when - * listing rules as a part of the lint report for a project. - * @param args.dependencies - The names of rules that must be executed first - * before executing this one. - * @param args.execute - The "body" of the rule. - * @returns The (validated) rule. + * @returns Part of a successful rule execution result (the rest will be filled + * in automatically). */ -export function buildRule({ - name, - description, - dependencies, - execute, -}: { - name: Name; - description: string; - dependencies: Exclude[]; - execute(args: { - template: MetaMaskRepository; - project: MetaMaskRepository; - pass: () => SuccessfulPartialRuleExecutionResult; - fail: ( - failures: FailedPartialRuleExecutionResult['failures'], - ) => FailedPartialRuleExecutionResult; - }): Promise; -}): Rule { +export function pass(): SuccessfulPartialRuleExecutionResult { return { - name, - description, - dependencies, - execute, + passed: true, }; } /** - * A rule utility which determines whether a file exists in the project. + * A helper for a rule which is intended to end its execution by marking it as + * failing. + * + * @param failures - The list of associated failures. + * @returns Part of a failed rule execution result (the rest will be filled + * in automatically). + */ +export function fail( + failures: FailedPartialRuleExecutionResult['failures'], +): FailedPartialRuleExecutionResult { + return { passed: false, failures }; +} + +/** + * A helper for a rule which determines whether a file exists in the project. * * @param filePath - The path to test. * @param ruleExecutionArguments - Rule execution arguments. @@ -66,7 +43,7 @@ export async function fileExists( filePath: string, ruleExecutionArguments: RuleExecutionArguments, ): Promise { - const { project, pass, fail } = ruleExecutionArguments; + const { project } = ruleExecutionArguments; const stats = await project.fs.getEntryStats(filePath); if (!stats) { @@ -89,7 +66,8 @@ export async function fileExists( } /** - * A rule utility which determines whether a directory exists in the project. + * A helper for a rule which determines whether a directory exists in the + * project. * * @param directoryPath - The path to test. * @param ruleExecutionArguments - Rule execution arguments. @@ -99,7 +77,7 @@ export async function directoryExists( directoryPath: string, ruleExecutionArguments: RuleExecutionArguments, ): Promise { - const { project, pass, fail } = ruleExecutionArguments; + const { project } = ruleExecutionArguments; const stats = await project.fs.getEntryStats(directoryPath); if (!stats) { @@ -122,8 +100,8 @@ export async function directoryExists( } /** - * A rule utility which determines not only whether a file that's assumed to - * exist in the template exists in the project as well, but also whether it + * A helper for a rule which determines not only whether a file that's assumed + * to exist in the template exists in the project as well, but also whether it * matches the same file in the template content-wise. * * @param filePath - The path to a file in both the template and project. @@ -134,7 +112,7 @@ export async function fileConforms( filePath: string, ruleExecutionArguments: RuleExecutionArguments, ): Promise { - const { template, project, pass, fail } = ruleExecutionArguments; + const { template, project } = ruleExecutionArguments; const fileExistsResult = await fileExists(filePath, ruleExecutionArguments); if (!fileExistsResult.passed) { return fileExistsResult; @@ -156,10 +134,10 @@ export async function fileConforms( } /** - * A rule utility which determines not only whether a directory that's assumed to - * exist in the template exists in the project as well, but also whether all - * files in that directory in the template are present in the project and match - * content-wise. + * A helper for a rule which determines not only whether a directory that's + * assumed to exist in the template exists in the project as well, but also + * whether all files in that directory in the template are present in the + * project and match content-wise. * * @param directoryPath - The path to a directory in both the template and * project. @@ -185,27 +163,21 @@ export async function directoryConforms( return await fileConforms(file.relativePath, ruleExecutionArguments); }), ); - return combineRuleExecutionResults( - fileConformsResults, - ruleExecutionArguments, - ); + return combineRuleExecutionResults(fileConformsResults); } /** - * Encapsulates multiple rule execution results into one. If all of the results - * are passing, then the combined result will be passing; otherwise, the - * combined result will be failing, and messages from failing results will be - * consolidated into a single array. + * A utility which encapsulates multiple rule execution results into one. If all + * of the results are passing, then the combined result will be passing; + * otherwise, the combined result will be failing, and messages from failing + * results will be consolidated into a single array. * * @param results - The rule execution results. - * @param ruleExecutionArguments - Rule execution arguments. * @returns The combined rule execution result. */ export function combineRuleExecutionResults( results: PartialRuleExecutionResult[], - ruleExecutionArguments: RuleExecutionArguments, ): PartialRuleExecutionResult { - const { pass, fail } = ruleExecutionArguments; const failures: FailedPartialRuleExecutionResult['failures'] = []; for (const result of results) { diff --git a/src/rules/all-yarn-modern-files-conform.test.ts b/src/rules/all-yarn-modern-files-conform.test.ts index 3b78011..c27e735 100644 --- a/src/rules/all-yarn-modern-files-conform.test.ts +++ b/src/rules/all-yarn-modern-files-conform.test.ts @@ -3,7 +3,7 @@ import path from 'path'; import allYarnModernFilesConform from './all-yarn-modern-files-conform'; import { buildMetaMaskRepository, withinSandbox } from '../../tests/helpers'; -import { fail, pass } from '../execute-rules'; +import { fail, pass } from '../rule-helpers'; describe('Rule: all-yarn-modern-files-conform', () => { it("passes if the template's .yarnrc.yml file, .yarn/releases directory, and .yarn/plugins directory match the project's", async () => { diff --git a/src/rules/all-yarn-modern-files-conform.ts b/src/rules/all-yarn-modern-files-conform.ts index f975615..1138081 100644 --- a/src/rules/all-yarn-modern-files-conform.ts +++ b/src/rules/all-yarn-modern-files-conform.ts @@ -1,10 +1,10 @@ +import { buildRule } from './build-rule'; +import { RuleName } from './types'; import { - buildRule, combineRuleExecutionResults, directoryConforms, fileConforms, -} from './helpers'; -import { RuleName } from './types'; +} from '../rule-helpers'; export default buildRule({ name: RuleName.AllYarnModernFilesConform, @@ -18,6 +18,6 @@ export default buildRule({ directoryConforms('.yarn/plugins', ruleExecutionArguments), ]); - return combineRuleExecutionResults(results, ruleExecutionArguments); + return combineRuleExecutionResults(results); }, }); diff --git a/src/rules/build-rule.ts b/src/rules/build-rule.ts new file mode 100644 index 0000000..5175175 --- /dev/null +++ b/src/rules/build-rule.ts @@ -0,0 +1,55 @@ +import type { RuleName } from './types'; +import type { MetaMaskRepository } from '../establish-metamask-repository'; +import type { + FailedPartialRuleExecutionResult, + PartialRuleExecutionResult, + Rule, + SuccessfulPartialRuleExecutionResult, +} from '../execute-rules'; + +/** + * Rule objects are fairly abstract: the name of a rule and the dependencies of + * a rule (which are themselves names) can be anything; and unfortunately, we + * cannot really enforce names, or else it would mean we'd have to have a + * `RuleName` type everywhere. + * + * This function exists to bridge that gap at the point where the rule is + * actually defined by validating the name and dependencies against a known set + * of rules. + * + * @param args - The arguments to this function. + * @param args.name - The name of a rule. This function assumes that all rule + * names are predefined in an enum and that this is one of the values in that + * enum. + * @param args.description - The description of the rule. This will show up when + * listing rules as a part of the lint report for a project. + * @param args.dependencies - The names of rules that must be executed first + * before executing this one. + * @param args.execute - The "body" of the rule. + * @returns The (validated) rule. + */ +export function buildRule({ + name, + description, + dependencies, + execute, +}: { + name: Name; + description: string; + dependencies: Exclude[]; + execute(args: { + template: MetaMaskRepository; + project: MetaMaskRepository; + pass: () => SuccessfulPartialRuleExecutionResult; + fail: ( + failures: FailedPartialRuleExecutionResult['failures'], + ) => FailedPartialRuleExecutionResult; + }): Promise; +}): Rule { + return { + name, + description, + dependencies, + execute, + }; +} diff --git a/src/rules/classic-yarn-config-file-absent.test.ts b/src/rules/classic-yarn-config-file-absent.test.ts index a86252a..c69b511 100644 --- a/src/rules/classic-yarn-config-file-absent.test.ts +++ b/src/rules/classic-yarn-config-file-absent.test.ts @@ -3,7 +3,7 @@ import path from 'path'; import classicYarnConfigFileAbsent from './classic-yarn-config-file-absent'; import { buildMetaMaskRepository, withinSandbox } from '../../tests/helpers'; -import { fail, pass } from '../execute-rules'; +import { fail, pass } from '../rule-helpers'; describe('Rule: classic-yarn-config-file-absent', () => { it('passes if .yarnrc is not present in the project', async () => { diff --git a/src/rules/classic-yarn-config-file-absent.ts b/src/rules/classic-yarn-config-file-absent.ts index 7757661..8dcfbad 100644 --- a/src/rules/classic-yarn-config-file-absent.ts +++ b/src/rules/classic-yarn-config-file-absent.ts @@ -1,12 +1,11 @@ -import { buildRule } from './helpers'; +import { buildRule } from './build-rule'; import { RuleName } from './types'; -import { fail, pass } from '../execute-rules'; export default buildRule({ name: RuleName.ClassicYarnConfigFileAbsent, description: 'Is the classic Yarn config file (`.yarnrc`) absent?', dependencies: [], - execute: async ({ project }) => { + execute: async ({ project, pass, fail }) => { const entryPath = '.yarnrc'; const stats = await project.fs.getEntryStats(entryPath); diff --git a/src/rules/package-manager-field-conforms.test.ts b/src/rules/package-manager-field-conforms.test.ts index 4d69d3a..8d73a38 100644 --- a/src/rules/package-manager-field-conforms.test.ts +++ b/src/rules/package-manager-field-conforms.test.ts @@ -3,7 +3,7 @@ import path from 'path'; import packageManagerFieldConforms from './package-manager-field-conforms'; import { buildMetaMaskRepository, withinSandbox } from '../../tests/helpers'; -import { fail, pass } from '../execute-rules'; +import { fail, pass } from '../rule-helpers'; describe('Rule: package-manager-field-conforms', () => { it('passes if the "packageManager" field in the project\'s package.json matches the one in the template\'s package.json', async () => { diff --git a/src/rules/package-manager-field-conforms.ts b/src/rules/package-manager-field-conforms.ts index 17e3c01..e307e30 100644 --- a/src/rules/package-manager-field-conforms.ts +++ b/src/rules/package-manager-field-conforms.ts @@ -1,4 +1,4 @@ -import { buildRule } from './helpers'; +import { buildRule } from './build-rule'; import { PackageManifestSchema, RuleName } from './types'; export default buildRule({ diff --git a/src/rules/readme-lists-correct-yarn-version.test.ts b/src/rules/readme-lists-correct-yarn-version.test.ts index 873df4e..a756815 100644 --- a/src/rules/readme-lists-correct-yarn-version.test.ts +++ b/src/rules/readme-lists-correct-yarn-version.test.ts @@ -3,7 +3,7 @@ import path from 'path'; import readmeListsCorrectYarnVersion from './readme-lists-correct-yarn-version'; import { buildMetaMaskRepository, withinSandbox } from '../../tests/helpers'; -import { fail, pass } from '../execute-rules'; +import { fail, pass } from '../rule-helpers'; describe('Rule: readme-lists-correct-yarn-version', () => { it("passes if the Yarn version listed in the project's README matches the same one in the template", async () => { diff --git a/src/rules/readme-lists-correct-yarn-version.ts b/src/rules/readme-lists-correct-yarn-version.ts index c98118f..d2e11f3 100644 --- a/src/rules/readme-lists-correct-yarn-version.ts +++ b/src/rules/readme-lists-correct-yarn-version.ts @@ -1,4 +1,4 @@ -import { buildRule } from './helpers'; +import { buildRule } from './build-rule'; import { RuleName } from './types'; export default buildRule({ diff --git a/src/rules/require-readme.test.ts b/src/rules/require-readme.test.ts index 3d88a59..79ef38c 100644 --- a/src/rules/require-readme.test.ts +++ b/src/rules/require-readme.test.ts @@ -3,7 +3,7 @@ import path from 'path'; import requireReadme from './require-readme'; import { buildMetaMaskRepository, withinSandbox } from '../../tests/helpers'; -import { fail, pass } from '../execute-rules'; +import { fail, pass } from '../rule-helpers'; describe('Rule: require-readme', () => { it('passes if the project has a README.md', async () => { diff --git a/src/rules/require-readme.ts b/src/rules/require-readme.ts index da2f22c..7f201fb 100644 --- a/src/rules/require-readme.ts +++ b/src/rules/require-readme.ts @@ -1,5 +1,6 @@ -import { buildRule, fileExists } from './helpers'; +import { buildRule } from './build-rule'; import { RuleName } from './types'; +import { fileExists } from '../rule-helpers'; export default buildRule({ name: RuleName.RequireReadme, diff --git a/src/rules/require-source-directory.test.ts b/src/rules/require-source-directory.test.ts index 8fa9ebd..ba65654 100644 --- a/src/rules/require-source-directory.test.ts +++ b/src/rules/require-source-directory.test.ts @@ -6,7 +6,7 @@ import path from 'path'; import requireSourceDirectory from './require-source-directory'; import { buildMetaMaskRepository, withinSandbox } from '../../tests/helpers'; -import { fail, pass } from '../execute-rules'; +import { fail, pass } from '../rule-helpers'; describe('Rule: require-source-directory', () => { it('passes if the project has a src/ directory', async () => { diff --git a/src/rules/require-source-directory.ts b/src/rules/require-source-directory.ts index a526c63..6b8c640 100644 --- a/src/rules/require-source-directory.ts +++ b/src/rules/require-source-directory.ts @@ -1,5 +1,6 @@ -import { buildRule, directoryExists } from './helpers'; +import { buildRule } from './build-rule'; import { RuleName } from './types'; +import { directoryExists } from '../rule-helpers'; export default buildRule({ name: RuleName.RequireSourceDirectory, diff --git a/src/rules/require-valid-package-manifest.test.ts b/src/rules/require-valid-package-manifest.test.ts index c8dd7bc..16a9de2 100644 --- a/src/rules/require-valid-package-manifest.test.ts +++ b/src/rules/require-valid-package-manifest.test.ts @@ -3,7 +3,7 @@ import path from 'path'; import requireValidPackageManifest from './require-valid-package-manifest'; import { buildMetaMaskRepository, withinSandbox } from '../../tests/helpers'; -import { fail, pass } from '../execute-rules'; +import { fail, pass } from '../rule-helpers'; describe('Rule: require-package-manifest', () => { it('passes if the project has a well-formed package.json', async () => { diff --git a/src/rules/require-valid-package-manifest.ts b/src/rules/require-valid-package-manifest.ts index c22146c..7630f18 100644 --- a/src/rules/require-valid-package-manifest.ts +++ b/src/rules/require-valid-package-manifest.ts @@ -1,7 +1,8 @@ import { isErrorWithCode, isErrorWithMessage } from '@metamask/utils/node'; -import { buildRule, fileExists } from './helpers'; +import { buildRule } from './build-rule'; import { PackageManifestSchema, RuleName } from './types'; +import { fileExists } from '../rule-helpers'; export default buildRule({ name: RuleName.RequireValidPackageManifest, From 401adc3a411e2765111fbc4ba7dd07f0dd291b48 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 4 Jan 2024 13:56:01 -0700 Subject: [PATCH 05/18] Remove expect.assertions() calls --- src/rule-helpers.test.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/rule-helpers.test.ts b/src/rule-helpers.test.ts index fbaf39f..5fb661d 100644 --- a/src/rule-helpers.test.ts +++ b/src/rule-helpers.test.ts @@ -24,8 +24,6 @@ describe('fail', () => { describe('fileExists', () => { it('passes if the given path refers to an existing file', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', @@ -48,8 +46,6 @@ describe('fileExists', () => { }); it('fails if the given path does not refer to an existing file', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', @@ -76,8 +72,6 @@ describe('fileExists', () => { }); it('fails if the given path does refers to an entry, but it is not a file', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', @@ -105,8 +99,6 @@ describe('fileExists', () => { describe('directoryExists', () => { it('passes if the given path refers to an existing directory', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', @@ -129,8 +121,6 @@ describe('directoryExists', () => { }); it('fails if the given path does not refer to an existing directory', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', @@ -157,8 +147,6 @@ describe('directoryExists', () => { }); it('fails if the given path does refers to an entry, but it is not a directory', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', From c4eb8cd27f3e344c13c11af8bb486aca4959432f Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 4 Jan 2024 14:01:22 -0700 Subject: [PATCH 06/18] Move combineRuleExecutionResults up with the other utilities --- src/rule-helpers.ts | 50 ++++++++++++++++++++++----------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/rule-helpers.ts b/src/rule-helpers.ts index 869063c..06694fb 100644 --- a/src/rule-helpers.ts +++ b/src/rule-helpers.ts @@ -6,7 +6,7 @@ import type { } from './execute-rules'; /** - * A helper for a rule which is intended to end its execution by marking it as + * A utility for a rule which is intended to end its execution by marking it as * passing. * * @returns Part of a successful rule execution result (the rest will be filled @@ -19,7 +19,7 @@ export function pass(): SuccessfulPartialRuleExecutionResult { } /** - * A helper for a rule which is intended to end its execution by marking it as + * A utility for a rule which is intended to end its execution by marking it as * failing. * * @param failures - The list of associated failures. @@ -32,6 +32,29 @@ export function fail( return { passed: false, failures }; } +/** + * A utility which encapsulates multiple rule execution results into one. If all + * of the results are passing, then the combined result will be passing; + * otherwise, the combined result will be failing, and messages from failing + * results will be consolidated into a single array. + * + * @param results - The rule execution results. + * @returns The combined rule execution result. + */ +export function combineRuleExecutionResults( + results: PartialRuleExecutionResult[], +): PartialRuleExecutionResult { + const failures: FailedPartialRuleExecutionResult['failures'] = []; + + for (const result of results) { + if (!result.passed) { + failures.push(...result.failures); + } + } + + return failures.length > 0 ? fail(failures) : pass(); +} + /** * A helper for a rule which determines whether a file exists in the project. * @@ -165,26 +188,3 @@ export async function directoryConforms( ); return combineRuleExecutionResults(fileConformsResults); } - -/** - * A utility which encapsulates multiple rule execution results into one. If all - * of the results are passing, then the combined result will be passing; - * otherwise, the combined result will be failing, and messages from failing - * results will be consolidated into a single array. - * - * @param results - The rule execution results. - * @returns The combined rule execution result. - */ -export function combineRuleExecutionResults( - results: PartialRuleExecutionResult[], -): PartialRuleExecutionResult { - const failures: FailedPartialRuleExecutionResult['failures'] = []; - - for (const result of results) { - if (!result.passed) { - failures.push(...result.failures); - } - } - - return failures.length > 0 ? fail(failures) : pass(); -} From 71c0cc86804b1914ed559909907ec199395812fc Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 4 Jan 2024 14:23:26 -0700 Subject: [PATCH 07/18] Simplify regex Co-authored-by: Mark Stacey --- src/rules/readme-lists-correct-yarn-version.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/readme-lists-correct-yarn-version.ts b/src/rules/readme-lists-correct-yarn-version.ts index d2e11f3..3ceb421 100644 --- a/src/rules/readme-lists-correct-yarn-version.ts +++ b/src/rules/readme-lists-correct-yarn-version.ts @@ -13,7 +13,7 @@ export default buildRule({ const fileContentInProject = await project.fs.readFile(entryPath); const match = fileContentInTemplate.match( - /Install \[Yarn [^[\]]+\]\([^()]+\)/u, + /Install \[Yarn .+?\]\(.+?\)/u, ); if (!match?.[0]) { throw new Error( From 65c22a3092a27d425ad43777bea829bec5e84fdd Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 4 Jan 2024 14:34:53 -0700 Subject: [PATCH 08/18] Name functional tests better, and replace 'minimal' test with all-failing test --- src/main.test.ts | 44 ++++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/src/main.test.ts b/src/main.test.ts index 7e44d1b..9a9c987 100644 --- a/src/main.test.ts +++ b/src/main.test.ts @@ -27,7 +27,7 @@ describe('main', () => { }); describe('given a list of project references', () => { - it('lists the passing rules executed against the given repositories', async () => { + it('produces a fully passing report if all rules executed against the given projects pass', async () => { await withinSandbox(async ({ directoryPath: sandboxDirectoryPath }) => { const projectNames = ['repo-1', 'repo-2']; const { cachedRepositoriesDirectoryPath, repositories } = @@ -128,10 +128,10 @@ Elapsed time: 0 ms }); }); - it('lists the failing rules executed against the given repositories', async () => { + it('produces a fully failing report if all rules executed against the given projects fail, listing reasons for failure', async () => { await withinSandbox(async ({ directoryPath: sandboxDirectoryPath }) => { const projectNames = ['repo-1', 'repo-2']; - const { cachedRepositoriesDirectoryPath } = + const { cachedRepositoriesDirectoryPath, repositories } = await setupToolWithMockRepositories({ execaMock, sandboxDirectoryPath, @@ -143,6 +143,10 @@ Elapsed time: 0 ms })), ], }); + // Skip first repo since it's the module template + for (const repository of repositories.slice(1)) { + await writeFile(path.join(repository.directoryPath, '.yarnrc'), ''); + } const outputLogger = new FakeOutputLogger(); await main({ @@ -161,7 +165,8 @@ Elapsed time: 0 ms repo-1 ------ -- Is the classic Yarn config file (\`.yarnrc\`) absent? ✅ +- Is the classic Yarn config file (\`.yarnrc\`) absent? ❌ + - The config file for Yarn Classic, \`.yarnrc\`, is present. Please upgrade this project to Yarn Modern. - Does the package have a well-formed manifest (\`package.json\`)? ❌ - \`package.json\` does not exist in this project. - Is \`README.md\` present? ❌ @@ -173,14 +178,15 @@ repo-1 - Does the \`src/\` directory exist? ❌ - \`src/\` does not exist in this project. -Results: 1 passed, 4 failed, 5 total +Results: 0 passed, 5 failed, 5 total Elapsed time: 0 ms repo-2 ------ -- Is the classic Yarn config file (\`.yarnrc\`) absent? ✅ +- Is the classic Yarn config file (\`.yarnrc\`) absent? ❌ + - The config file for Yarn Classic, \`.yarnrc\`, is present. Please upgrade this project to Yarn Modern. - Does the package have a well-formed manifest (\`package.json\`)? ❌ - \`package.json\` does not exist in this project. - Is \`README.md\` present? ❌ @@ -192,7 +198,7 @@ repo-2 - Does the \`src/\` directory exist? ❌ - \`src/\` does not exist in this project. -Results: 1 passed, 4 failed, 5 total +Results: 0 passed, 5 failed, 5 total Elapsed time: 0 ms `, @@ -200,7 +206,7 @@ Elapsed time: 0 ms }); }); - it('does not exit immediately if a project fails to lint for any reason, but shows the reason and continues', async () => { + it('does not exit immediately if a project errors during linting, but shows the error and continues', async () => { await withinSandbox(async ({ directoryPath: sandboxDirectoryPath }) => { const projectNames = ['repo-1', 'repo-2']; const { cachedRepositoriesDirectoryPath } = @@ -266,7 +272,7 @@ Elapsed time: 0 ms }); describe('given no project references', () => { - it('lists the passing rules executed against the default repositories', async () => { + it('produces a fully passing report if all rules executed against the default projects pass', async () => { await withinSandbox(async ({ directoryPath: sandboxDirectoryPath }) => { const projectNames = ['repo-1', 'repo-2']; const { cachedRepositoriesDirectoryPath, repositories } = @@ -367,10 +373,10 @@ Elapsed time: 0 ms }); }); - it('lists the failing rules executed against the default repositories', async () => { + it('produces a fully failing report if all rules executed against the default projects fail, listing reasons for failure', async () => { await withinSandbox(async ({ directoryPath: sandboxDirectoryPath }) => { const projectNames = ['repo-1', 'repo-2']; - const { cachedRepositoriesDirectoryPath } = + const { cachedRepositoriesDirectoryPath, repositories } = await setupToolWithMockRepositories({ execaMock, sandboxDirectoryPath, @@ -382,6 +388,10 @@ Elapsed time: 0 ms })), ], }); + // Skip first repo since it's the module template + for (const repository of repositories.slice(1)) { + await writeFile(path.join(repository.directoryPath, '.yarnrc'), ''); + } const outputLogger = new FakeOutputLogger(); await main({ @@ -400,7 +410,8 @@ Elapsed time: 0 ms repo-1 ------ -- Is the classic Yarn config file (\`.yarnrc\`) absent? ✅ +- Is the classic Yarn config file (\`.yarnrc\`) absent? ❌ + - The config file for Yarn Classic, \`.yarnrc\`, is present. Please upgrade this project to Yarn Modern. - Does the package have a well-formed manifest (\`package.json\`)? ❌ - \`package.json\` does not exist in this project. - Is \`README.md\` present? ❌ @@ -412,14 +423,15 @@ repo-1 - Does the \`src/\` directory exist? ❌ - \`src/\` does not exist in this project. -Results: 1 passed, 4 failed, 5 total +Results: 0 passed, 5 failed, 5 total Elapsed time: 0 ms repo-2 ------ -- Is the classic Yarn config file (\`.yarnrc\`) absent? ✅ +- Is the classic Yarn config file (\`.yarnrc\`) absent? ❌ + - The config file for Yarn Classic, \`.yarnrc\`, is present. Please upgrade this project to Yarn Modern. - Does the package have a well-formed manifest (\`package.json\`)? ❌ - \`package.json\` does not exist in this project. - Is \`README.md\` present? ❌ @@ -431,7 +443,7 @@ repo-2 - Does the \`src/\` directory exist? ❌ - \`src/\` does not exist in this project. -Results: 1 passed, 4 failed, 5 total +Results: 0 passed, 5 failed, 5 total Elapsed time: 0 ms `, @@ -439,7 +451,7 @@ Elapsed time: 0 ms }); }); - it('does not exit immediately if a project fails to lint for any reason, but shows the reason and continues', async () => { + it('does not exit immediately if a project errors during linting, but shows the error and continues', async () => { await withinSandbox(async ({ directoryPath: sandboxDirectoryPath }) => { const projectNames = ['repo-1', 'repo-2']; const { cachedRepositoriesDirectoryPath } = From 5a65ee046e250592d3a115356b8022fd893e8c82 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 4 Jan 2024 14:35:49 -0700 Subject: [PATCH 09/18] Fix lint violation --- src/rules/readme-lists-correct-yarn-version.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/rules/readme-lists-correct-yarn-version.ts b/src/rules/readme-lists-correct-yarn-version.ts index 3ceb421..030999e 100644 --- a/src/rules/readme-lists-correct-yarn-version.ts +++ b/src/rules/readme-lists-correct-yarn-version.ts @@ -12,9 +12,7 @@ export default buildRule({ const fileContentInTemplate = await template.fs.readFile(entryPath); const fileContentInProject = await project.fs.readFile(entryPath); - const match = fileContentInTemplate.match( - /Install \[Yarn .+?\]\(.+?\)/u, - ); + const match = fileContentInTemplate.match(/Install \[Yarn .+?\]\(.+?\)/u); if (!match?.[0]) { throw new Error( "Could not find Yarn version in template's README. This is not the fault of the project, but is rather a bug in a rule.", From 404f75f32d49e07bd9c71c0c9cf259d5159a954f Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 4 Jan 2024 14:44:26 -0700 Subject: [PATCH 10/18] directoryConforms -> directoryAndContentsConform --- src/rule-helpers.ts | 2 +- src/rules/all-yarn-modern-files-conform.ts | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/rule-helpers.ts b/src/rule-helpers.ts index 06694fb..6fc5ba7 100644 --- a/src/rule-helpers.ts +++ b/src/rule-helpers.ts @@ -167,7 +167,7 @@ export async function fileConforms( * @param ruleExecutionArguments - Rule execution arguments. * @returns Either a successful or failed result. */ -export async function directoryConforms( +export async function directoryAndContentsConform( directoryPath: string, ruleExecutionArguments: RuleExecutionArguments, ): Promise { diff --git a/src/rules/all-yarn-modern-files-conform.ts b/src/rules/all-yarn-modern-files-conform.ts index 1138081..c198554 100644 --- a/src/rules/all-yarn-modern-files-conform.ts +++ b/src/rules/all-yarn-modern-files-conform.ts @@ -2,7 +2,7 @@ import { buildRule } from './build-rule'; import { RuleName } from './types'; import { combineRuleExecutionResults, - directoryConforms, + directoryAndContentsConform, fileConforms, } from '../rule-helpers'; @@ -14,8 +14,8 @@ export default buildRule({ execute: async (ruleExecutionArguments) => { const results = await Promise.all([ fileConforms('.yarnrc.yml', ruleExecutionArguments), - directoryConforms('.yarn/releases', ruleExecutionArguments), - directoryConforms('.yarn/plugins', ruleExecutionArguments), + directoryAndContentsConform('.yarn/releases', ruleExecutionArguments), + directoryAndContentsConform('.yarn/plugins', ruleExecutionArguments), ]); return combineRuleExecutionResults(results); From ae3da2c2a6fe457ad299c11a6a4ce3c7093a1c66 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 4 Jan 2024 15:00:26 -0700 Subject: [PATCH 11/18] Don't explicitly throw in readme-lists-correct-yarn-version --- .../readme-lists-correct-yarn-version.test.ts | 20 ++++++++++--------- .../readme-lists-correct-yarn-version.ts | 4 +++- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/rules/readme-lists-correct-yarn-version.test.ts b/src/rules/readme-lists-correct-yarn-version.test.ts index a756815..4a81c8a 100644 --- a/src/rules/readme-lists-correct-yarn-version.test.ts +++ b/src/rules/readme-lists-correct-yarn-version.test.ts @@ -80,7 +80,7 @@ describe('Rule: readme-lists-correct-yarn-version', () => { }); }); - it('throws if the template does not have the Yarn version listed in its README for some reason', async () => { + it('passes if the template does not have the Yarn version listed in its README for some reason', async () => { expect.assertions(1); await withinSandbox(async (sandbox) => { @@ -101,14 +101,16 @@ describe('Rule: readme-lists-correct-yarn-version', () => { 'does not matter', ); - await expect( - readmeListsCorrectYarnVersion.execute({ - template, - project, - pass, - fail, - }), - ).rejects.toThrow("Could not find Yarn version in template's README"); + const result = await readmeListsCorrectYarnVersion.execute({ + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: true, + }); }); }); }); diff --git a/src/rules/readme-lists-correct-yarn-version.ts b/src/rules/readme-lists-correct-yarn-version.ts index 030999e..95c632e 100644 --- a/src/rules/readme-lists-correct-yarn-version.ts +++ b/src/rules/readme-lists-correct-yarn-version.ts @@ -14,10 +14,12 @@ export default buildRule({ const match = fileContentInTemplate.match(/Install \[Yarn .+?\]\(.+?\)/u); if (!match?.[0]) { - throw new Error( + console.warn( "Could not find Yarn version in template's README. This is not the fault of the project, but is rather a bug in a rule.", ); + return pass(); } + if (!fileContentInProject.includes(match[0])) { return fail([ { From e1fe87870bfb5ab651755f12b87ab5cc86e82eab Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 4 Jan 2024 15:03:38 -0700 Subject: [PATCH 12/18] Don't throw in require-valid-package-manifest either --- src/rules/require-valid-package-manifest.test.ts | 7 ++----- src/rules/require-valid-package-manifest.ts | 5 ++--- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/rules/require-valid-package-manifest.test.ts b/src/rules/require-valid-package-manifest.test.ts index 16a9de2..65e271f 100644 --- a/src/rules/require-valid-package-manifest.test.ts +++ b/src/rules/require-valid-package-manifest.test.ts @@ -59,7 +59,7 @@ describe('Rule: require-package-manifest', () => { }); }); - it('fails if the project has a malformed package.json', async () => { + it('passes if the project has a malformed package.json', async () => { expect.assertions(1); await withinSandbox(async (sandbox) => { @@ -80,10 +80,7 @@ describe('Rule: require-package-manifest', () => { }); expect(result).toStrictEqual({ - passed: false, - failures: [ - { message: 'Invalid `package.json`: Missing `packageManager`.' }, - ], + passed: true, }); }); }); diff --git a/src/rules/require-valid-package-manifest.ts b/src/rules/require-valid-package-manifest.ts index 7630f18..2be929b 100644 --- a/src/rules/require-valid-package-manifest.ts +++ b/src/rules/require-valid-package-manifest.ts @@ -29,9 +29,8 @@ export default buildRule({ isErrorWithMessage(error) && error.code === 'ERR_INVALID_JSON_FILE' ) { - return fail([ - { message: `Invalid \`${entryPath}\`: ${error.message}` }, - ]); + console.warn(`Invalid \`${entryPath}\`: ${error.message}`); + return pass(); } throw error; } From 3eaaee9ff0d18393ed98061576f8cddea8badd08 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 4 Jan 2024 15:04:27 -0700 Subject: [PATCH 13/18] Fix lint violation --- src/rules/require-valid-package-manifest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/require-valid-package-manifest.ts b/src/rules/require-valid-package-manifest.ts index 2be929b..1d055a6 100644 --- a/src/rules/require-valid-package-manifest.ts +++ b/src/rules/require-valid-package-manifest.ts @@ -9,7 +9,7 @@ export default buildRule({ description: 'Does the package have a well-formed manifest (`package.json`)?', dependencies: [], execute: async (ruleExecutionArguments) => { - const { project, pass, fail } = ruleExecutionArguments; + const { project, pass } = ruleExecutionArguments; const entryPath = 'package.json'; const fileExistsResult = await fileExists( From 6c0ccaec354678219096712a2cb5f2d888615a2c Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 4 Jan 2024 15:07:03 -0700 Subject: [PATCH 14/18] Remove more expect.assertions()'s --- src/misc-utils.test.ts | 2 -- src/rules/all-yarn-modern-files-conform.test.ts | 12 ------------ src/rules/classic-yarn-config-file-absent.test.ts | 4 ---- src/rules/package-manager-field-conforms.test.ts | 4 ---- src/rules/readme-lists-correct-yarn-version.test.ts | 6 ------ src/rules/require-readme.test.ts | 4 ---- src/rules/require-source-directory.test.ts | 6 ------ src/rules/require-valid-package-manifest.test.ts | 8 -------- 8 files changed, 46 deletions(-) diff --git a/src/misc-utils.test.ts b/src/misc-utils.test.ts index 360057f..411ce06 100644 --- a/src/misc-utils.test.ts +++ b/src/misc-utils.test.ts @@ -183,8 +183,6 @@ describe('readDirectoryRecursively', () => { }); it('re-throws a wrapped version of any other error that occurs, assigning it the same code and giving it a stack', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { // Make sandbox root directory non-readable. await fs.promises.chmod(sandbox.directoryPath, 0o000); diff --git a/src/rules/all-yarn-modern-files-conform.test.ts b/src/rules/all-yarn-modern-files-conform.test.ts index c27e735..6e6cf01 100644 --- a/src/rules/all-yarn-modern-files-conform.test.ts +++ b/src/rules/all-yarn-modern-files-conform.test.ts @@ -7,8 +7,6 @@ import { fail, pass } from '../rule-helpers'; describe('Rule: all-yarn-modern-files-conform', () => { it("passes if the template's .yarnrc.yml file, .yarn/releases directory, and .yarn/plugins directory match the project's", async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const template = buildMetaMaskRepository({ shortname: 'template', @@ -57,8 +55,6 @@ describe('Rule: all-yarn-modern-files-conform', () => { }); it('fails if the project does not have a .yarnrc.yml', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const template = buildMetaMaskRepository({ shortname: 'template', @@ -108,8 +104,6 @@ describe('Rule: all-yarn-modern-files-conform', () => { }); it('fails if the project does not have a .yarn/releases directory', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const template = buildMetaMaskRepository({ shortname: 'template', @@ -159,8 +153,6 @@ describe('Rule: all-yarn-modern-files-conform', () => { }); it("fails if a file in the template's .yarn/releases directory does not match the same file in the project", async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const template = buildMetaMaskRepository({ shortname: 'template', @@ -215,8 +207,6 @@ describe('Rule: all-yarn-modern-files-conform', () => { }); it('fails if the project does not have a .yarn/plugins directory', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const template = buildMetaMaskRepository({ shortname: 'template', @@ -266,8 +256,6 @@ describe('Rule: all-yarn-modern-files-conform', () => { }); it("fails if a file in the template's .yarn/plugins directory does not match the same file in the project", async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const template = buildMetaMaskRepository({ shortname: 'template', diff --git a/src/rules/classic-yarn-config-file-absent.test.ts b/src/rules/classic-yarn-config-file-absent.test.ts index c69b511..ede3afa 100644 --- a/src/rules/classic-yarn-config-file-absent.test.ts +++ b/src/rules/classic-yarn-config-file-absent.test.ts @@ -7,8 +7,6 @@ import { fail, pass } from '../rule-helpers'; describe('Rule: classic-yarn-config-file-absent', () => { it('passes if .yarnrc is not present in the project', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', @@ -29,8 +27,6 @@ describe('Rule: classic-yarn-config-file-absent', () => { }); it('fails if .yarnrc is present in the project', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', diff --git a/src/rules/package-manager-field-conforms.test.ts b/src/rules/package-manager-field-conforms.test.ts index 8d73a38..2d8d728 100644 --- a/src/rules/package-manager-field-conforms.test.ts +++ b/src/rules/package-manager-field-conforms.test.ts @@ -7,8 +7,6 @@ import { fail, pass } from '../rule-helpers'; describe('Rule: package-manager-field-conforms', () => { it('passes if the "packageManager" field in the project\'s package.json matches the one in the template\'s package.json', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const template = buildMetaMaskRepository({ shortname: 'template', @@ -41,8 +39,6 @@ describe('Rule: package-manager-field-conforms', () => { }); it('fails if the "packageManager" field in the project\'s package.json does not match the one in the template\'s package.json', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const template = buildMetaMaskRepository({ shortname: 'template', diff --git a/src/rules/readme-lists-correct-yarn-version.test.ts b/src/rules/readme-lists-correct-yarn-version.test.ts index 4a81c8a..d6d93ab 100644 --- a/src/rules/readme-lists-correct-yarn-version.test.ts +++ b/src/rules/readme-lists-correct-yarn-version.test.ts @@ -7,8 +7,6 @@ import { fail, pass } from '../rule-helpers'; describe('Rule: readme-lists-correct-yarn-version', () => { it("passes if the Yarn version listed in the project's README matches the same one in the template", async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const template = buildMetaMaskRepository({ shortname: 'template', @@ -41,8 +39,6 @@ describe('Rule: readme-lists-correct-yarn-version', () => { }); it("fails if the Yarn version listed in the project's README does not match the same one in the template", async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const template = buildMetaMaskRepository({ shortname: 'template', @@ -81,8 +77,6 @@ describe('Rule: readme-lists-correct-yarn-version', () => { }); it('passes if the template does not have the Yarn version listed in its README for some reason', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const template = buildMetaMaskRepository({ shortname: 'template', diff --git a/src/rules/require-readme.test.ts b/src/rules/require-readme.test.ts index 79ef38c..3647e86 100644 --- a/src/rules/require-readme.test.ts +++ b/src/rules/require-readme.test.ts @@ -7,8 +7,6 @@ import { fail, pass } from '../rule-helpers'; describe('Rule: require-readme', () => { it('passes if the project has a README.md', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', @@ -33,8 +31,6 @@ describe('Rule: require-readme', () => { }); it('passes if the project does not have a README.md', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', diff --git a/src/rules/require-source-directory.test.ts b/src/rules/require-source-directory.test.ts index ba65654..1e81774 100644 --- a/src/rules/require-source-directory.test.ts +++ b/src/rules/require-source-directory.test.ts @@ -10,8 +10,6 @@ import { fail, pass } from '../rule-helpers'; describe('Rule: require-source-directory', () => { it('passes if the project has a src/ directory', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', @@ -35,8 +33,6 @@ describe('Rule: require-source-directory', () => { }); it('passes if the project does not have a src/ directory', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', @@ -62,8 +58,6 @@ describe('Rule: require-source-directory', () => { }); it('passes if the project has a "src" path, but it is a file', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', diff --git a/src/rules/require-valid-package-manifest.test.ts b/src/rules/require-valid-package-manifest.test.ts index 65e271f..05fa901 100644 --- a/src/rules/require-valid-package-manifest.test.ts +++ b/src/rules/require-valid-package-manifest.test.ts @@ -7,8 +7,6 @@ import { fail, pass } from '../rule-helpers'; describe('Rule: require-package-manifest', () => { it('passes if the project has a well-formed package.json', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', @@ -33,8 +31,6 @@ describe('Rule: require-package-manifest', () => { }); it('fails if the project does not have a package.json', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', @@ -60,8 +56,6 @@ describe('Rule: require-package-manifest', () => { }); it('passes if the project has a malformed package.json', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', @@ -86,8 +80,6 @@ describe('Rule: require-package-manifest', () => { }); it('re-throws a unknown error that readJsonFileAs produces', async () => { - expect.assertions(1); - await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', From 1b404dbb0a1e0c4ce64ce4f811d91955ccf4d5da Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 4 Jan 2024 15:22:36 -0700 Subject: [PATCH 15/18] Revert "Don't explicitly throw in readme-lists-correct-yarn-version" This reverts commit ae3da2c2a6fe457ad299c11a6a4ce3c7093a1c66. --- .../readme-lists-correct-yarn-version.test.ts | 20 +++++++++---------- .../readme-lists-correct-yarn-version.ts | 4 +--- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/rules/readme-lists-correct-yarn-version.test.ts b/src/rules/readme-lists-correct-yarn-version.test.ts index d6d93ab..63c2a44 100644 --- a/src/rules/readme-lists-correct-yarn-version.test.ts +++ b/src/rules/readme-lists-correct-yarn-version.test.ts @@ -76,7 +76,7 @@ describe('Rule: readme-lists-correct-yarn-version', () => { }); }); - it('passes if the template does not have the Yarn version listed in its README for some reason', async () => { + it('throws if the template does not have the Yarn version listed in its README for some reason', async () => { await withinSandbox(async (sandbox) => { const template = buildMetaMaskRepository({ shortname: 'template', @@ -95,16 +95,14 @@ describe('Rule: readme-lists-correct-yarn-version', () => { 'does not matter', ); - const result = await readmeListsCorrectYarnVersion.execute({ - template, - project, - pass, - fail, - }); - - expect(result).toStrictEqual({ - passed: true, - }); + await expect( + readmeListsCorrectYarnVersion.execute({ + template, + project, + pass, + fail, + }), + ).rejects.toThrow("Could not find Yarn version in template's README"); }); }); }); diff --git a/src/rules/readme-lists-correct-yarn-version.ts b/src/rules/readme-lists-correct-yarn-version.ts index 95c632e..030999e 100644 --- a/src/rules/readme-lists-correct-yarn-version.ts +++ b/src/rules/readme-lists-correct-yarn-version.ts @@ -14,12 +14,10 @@ export default buildRule({ const match = fileContentInTemplate.match(/Install \[Yarn .+?\]\(.+?\)/u); if (!match?.[0]) { - console.warn( + throw new Error( "Could not find Yarn version in template's README. This is not the fault of the project, but is rather a bug in a rule.", ); - return pass(); } - if (!fileContentInProject.includes(match[0])) { return fail([ { From d2afd043fa867da11048ef5d14c1c8358b3db623 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 4 Jan 2024 15:23:05 -0700 Subject: [PATCH 16/18] Revert "Don't throw in require-valid-package-manifest either" This reverts commit e1fe87870bfb5ab651755f12b87ab5cc86e82eab. --- src/rules/require-valid-package-manifest.test.ts | 7 +++++-- src/rules/require-valid-package-manifest.ts | 7 ++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/rules/require-valid-package-manifest.test.ts b/src/rules/require-valid-package-manifest.test.ts index 05fa901..1156250 100644 --- a/src/rules/require-valid-package-manifest.test.ts +++ b/src/rules/require-valid-package-manifest.test.ts @@ -55,7 +55,7 @@ describe('Rule: require-package-manifest', () => { }); }); - it('passes if the project has a malformed package.json', async () => { + it('fails if the project has a malformed package.json', async () => { await withinSandbox(async (sandbox) => { const project = buildMetaMaskRepository({ shortname: 'project', @@ -74,7 +74,10 @@ describe('Rule: require-package-manifest', () => { }); expect(result).toStrictEqual({ - passed: true, + passed: false, + failures: [ + { message: 'Invalid `package.json`: Missing `packageManager`.' }, + ], }); }); }); diff --git a/src/rules/require-valid-package-manifest.ts b/src/rules/require-valid-package-manifest.ts index 1d055a6..7630f18 100644 --- a/src/rules/require-valid-package-manifest.ts +++ b/src/rules/require-valid-package-manifest.ts @@ -9,7 +9,7 @@ export default buildRule({ description: 'Does the package have a well-formed manifest (`package.json`)?', dependencies: [], execute: async (ruleExecutionArguments) => { - const { project, pass } = ruleExecutionArguments; + const { project, pass, fail } = ruleExecutionArguments; const entryPath = 'package.json'; const fileExistsResult = await fileExists( @@ -29,8 +29,9 @@ export default buildRule({ isErrorWithMessage(error) && error.code === 'ERR_INVALID_JSON_FILE' ) { - console.warn(`Invalid \`${entryPath}\`: ${error.message}`); - return pass(); + return fail([ + { message: `Invalid \`${entryPath}\`: ${error.message}` }, + ]); } throw error; } From b683cd0656ce5966a3e022ecfd9b282ae28cda58 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 4 Jan 2024 15:54:25 -0700 Subject: [PATCH 17/18] Add tests for combineRuleExecutionResults, fileConforms, and directoryAndContentsConform --- src/rule-helpers.test.ts | 392 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 391 insertions(+), 1 deletion(-) diff --git a/src/rule-helpers.test.ts b/src/rule-helpers.test.ts index 5fb661d..1ac4241 100644 --- a/src/rule-helpers.test.ts +++ b/src/rule-helpers.test.ts @@ -4,7 +4,15 @@ import { } from '@metamask/utils/node'; import path from 'path'; -import { directoryExists, fail, fileExists, pass } from './rule-helpers'; +import { + combineRuleExecutionResults, + directoryAndContentsConform, + directoryExists, + fail, + fileConforms, + fileExists, + pass, +} from './rule-helpers'; import { buildMetaMaskRepository, withinSandbox } from '../tests/helpers'; describe('pass', () => { @@ -22,6 +30,49 @@ describe('fail', () => { }); }); +describe('combineRuleExecutionResults', () => { + it('returns a single passing result if all of the given results are passing', () => { + const result = combineRuleExecutionResults([ + { + passed: true, + }, + { + passed: true, + }, + ]); + + expect(result).toStrictEqual({ passed: true }); + }); + + it('returns a single failed result, consolidating all failures, if any of the given results are failing', () => { + const result = combineRuleExecutionResults([ + { + passed: true, + }, + { + passed: false, + failures: [{ message: 'message 1' }], + }, + { + passed: true, + }, + { + passed: false, + failures: [{ message: 'message 2' }, { message: 'message 3' }], + }, + ]); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { message: 'message 1' }, + { message: 'message 2' }, + { message: 'message 3' }, + ], + }); + }); +}); + describe('fileExists', () => { it('passes if the given path refers to an existing file', async () => { await withinSandbox(async (sandbox) => { @@ -173,3 +224,342 @@ describe('directoryExists', () => { }); }); }); + +describe('fileConforms', () => { + it('passes if the project and template have the same referenced file with the same content', async () => { + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join(template.directoryPath, 'some.file'), + 'some content', + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, 'some.file'), + 'some content', + ); + + const result = await fileConforms('some.file', { + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ passed: true }); + }); + }); + + it('fails if the project has the same referenced file as the template, but its content does not match', async () => { + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join(template.directoryPath, 'some.file'), + 'some content', + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join(project.directoryPath, 'some.file'), + 'different content', + ); + + const result = await fileConforms('some.file', { + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { + message: + '`some.file` does not match the same file in the template repo.', + }, + ], + }); + }); + }); + + it('fails if the project does not have the same referenced file as the template', async () => { + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join(template.directoryPath, 'some.file'), + 'different content', + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + + const result = await fileConforms('some.file', { + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { + message: '`some.file` does not exist in this project.', + }, + ], + }); + }); + }); +}); + +describe('directoryAndContentsConform', () => { + it('passes if the project and template have the same referenced directory with the same file structure, and all files have the same content', async () => { + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join( + template.directoryPath, + 'some-directory', + 'child-directory', + 'some.file', + ), + 'some content', + ); + await writeFile( + path.join( + template.directoryPath, + 'some-directory', + 'child-directory', + 'grandchild-directory', + 'some.file', + ), + 'some content', + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join( + project.directoryPath, + 'some-directory', + 'child-directory', + 'some.file', + ), + 'some content', + ); + await writeFile( + path.join( + project.directoryPath, + 'some-directory', + 'child-directory', + 'grandchild-directory', + 'some.file', + ), + 'some content', + ); + + const result = await directoryAndContentsConform('some-directory', { + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ passed: true }); + }); + }); + + it('fails if the project and template have the same referenced directory with the same file structure, but some files do not have the same content', async () => { + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join( + template.directoryPath, + 'some-directory', + 'child-directory', + 'some.file', + ), + 'some content', + ); + await writeFile( + path.join( + template.directoryPath, + 'some-directory', + 'child-directory', + 'grandchild-directory', + 'some.file', + ), + 'some content', + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join( + project.directoryPath, + 'some-directory', + 'child-directory', + 'some.file', + ), + 'some content', + ); + await writeFile( + path.join( + project.directoryPath, + 'some-directory', + 'child-directory', + 'grandchild-directory', + 'some.file', + ), + 'different content', + ); + + const result = await directoryAndContentsConform('some-directory', { + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { + message: + '`some-directory/child-directory/grandchild-directory/some.file` does not match the same file in the template repo.', + }, + ], + }); + }); + }); + + it('fails if the project and template have the same referenced directory, but it does not have the same file structure', async () => { + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join( + template.directoryPath, + 'some-directory', + 'child-directory', + 'some.file', + ), + 'some content', + ); + await writeFile( + path.join( + template.directoryPath, + 'some-directory', + 'child-directory', + 'grandchild-directory', + 'some.file', + ), + 'some content', + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + await writeFile( + path.join( + project.directoryPath, + 'some-directory', + 'child-directory', + 'some.file', + ), + 'some content', + ); + + const result = await directoryAndContentsConform('some-directory', { + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { + message: + '`some-directory/child-directory/grandchild-directory/some.file` does not exist in this project.', + }, + ], + }); + }); + }); + + it('fails if the project does not have the same referenced directory as the template', async () => { + await withinSandbox(async (sandbox) => { + const template = buildMetaMaskRepository({ + shortname: 'template', + directoryPath: path.join(sandbox.directoryPath, 'template'), + }); + await writeFile( + path.join( + template.directoryPath, + 'some-directory', + 'child-directory', + 'some.file', + ), + 'some content', + ); + await writeFile( + path.join( + template.directoryPath, + 'some-directory', + 'child-directory', + 'grandchild-directory', + 'some.file', + ), + 'some content', + ); + const project = buildMetaMaskRepository({ + shortname: 'project', + directoryPath: path.join(sandbox.directoryPath, 'project'), + }); + + const result = await directoryAndContentsConform('some-directory', { + template, + project, + pass, + fail, + }); + + expect(result).toStrictEqual({ + passed: false, + failures: [ + { + message: '`some-directory/` does not exist in this project.', + }, + ], + }); + }); + }); +}); From b46092268085264f6b9fde8fe8e7527b089fc76d Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 4 Jan 2024 16:07:43 -0700 Subject: [PATCH 18/18] Add empty test case for combineRuleExecutionResults --- src/rule-helpers.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/rule-helpers.test.ts b/src/rule-helpers.test.ts index 1ac4241..03b007f 100644 --- a/src/rule-helpers.test.ts +++ b/src/rule-helpers.test.ts @@ -71,6 +71,12 @@ describe('combineRuleExecutionResults', () => { ], }); }); + + it('returns a passing result if given nothing to combine', () => { + const result = combineRuleExecutionResults([]); + + expect(result).toStrictEqual({ passed: true }); + }); }); describe('fileExists', () => {