MetaMask / module-lint

Analyzes one or more repos for divergence from a template repo.
1 stars 3 forks source link

Flatten rules when reporting execution results #42

Closed mcmire closed 5 months ago

mcmire commented 7 months ago

Rules are arranged in a tree structure before being executed, so that dependent rules are executed before their dependencies. Currently, the report which summarizes the results of executing rules is arranged in the same tree structure, with dependencies indented under their dependents. While this could be interesting to some users, it is not necessary. This commit flattens the list when reporting.

For instance, here is the output for yarn run-tool utils before this change:

- 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? ❌
    - `packageManager` is "yarn@3.2.3", when it should be "yarn@3.2.1".
  - Does the `engines.node` field in `package.json` conform? ❌
    - `engines.node` is ">=16.0.0", when it should be "^18.18 || >=20".
  - Do the lint-related `devDependencies` in `package.json` conform? ❌
    - `@metamask/eslint-config-jest` is "^12.0.0", when it should be "^12.1.0".
    - `@metamask/eslint-config-nodejs` is "^12.0.0", when it should be "^12.1.0".
    - `@metamask/eslint-config-typescript` is "^12.0.0", when it should be "^12.1.0".
    - `eslint-plugin-import` is "^2.27.5", when it should be "~2.26.0".
  - Do the jest-related `devDependencies` in `package.json` conform? ❌
    - `jest` is "^29.2.2", when it should be "^28.1.3".
  - Do the test-related `scripts` in `package.json` conform? ❌
    - `scripts.[test]` is 'yarn test:source && yarn test:types', when it should be 'jest && jest-it-up'.
  - Do the typescript-related `devDependencies` in `package.json` conform? ❌
    - `devDependencies.[@types/node]` is '^17.0.23', when it should be '^16'.
  - Do the typescript-related `scripts` in `package.json` conform? ❌
    - `scripts.[build]` is 'tsup && yarn build:types', when it should be 'tsup --clean && yarn build:types'.
  - Does the `exports` field in `package.json` conform? ✅
  - Does the `main` field in `package.json` conform? ✅
  - Does the `module` field in `package.json` conform? ✅
  - Does the `types` field in `package.json` conform? ✅
  - Does the `files` field in `package.json` conform? ✅
  - Does LavaMoat allow scripts for `tsup>esbuild`? ✅
  - Do the typedoc-related `devDependencies` in `package.json` conform? ✅
  - Do the typedoc-related `scripts` in `package.json` conform? ✅
- Is `README.md` present? ✅
  - Does the README conform by recommending the correct Yarn version to install? ✅
  - Does the README conform by recommending node install from nodejs.org? ❌
    - `README.md` should contain "Install the current LTS version of [Node.js](https://nodejs.org)", but does not.
- Are all of the files for Yarn Modern present, and do they conform? ❌
  - `.yarnrc.yml` does not match the same file in the template repo.
  - `.yarn/releases/yarn-3.2.1.cjs` does not exist in this project.
  - `.yarn/plugins/@yarnpkg/plugin-allow-scripts.cjs` does not match the same file in the template repo.
  - `.yarn/plugins/@yarnpkg/plugin-constraints.cjs` does not match the same file in the template repo.
- Does the `src/` directory exist? ✅
- Is `.nvmrc` present, and does it conform? ❌
  - `.nvmrc` does not match the same file in the template repo.
- Is `jest.config.js` present, and does it conform? ❌
  - `jest.config.js` does not match the same file in the template repo.
- Is `tsconfig.json` present, and does it conform? ❌
  - `tsconfig.json` does not match the same file in the template repo.
- Is `tsconfig.build.json` present, and does it conform? ✅
- Is `tsup.config.ts` present, and does it conform? ✅
- Is `typedoc.json` present, and does it conform? ✅

And here is the output after this change:

- 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? ❌
  - `packageManager` is "yarn@3.2.3", when it should be "yarn@3.2.1".
- Does the `engines.node` field in `package.json` conform? ❌
  - `engines.node` is ">=16.0.0", when it should be "^18.18 || >=20".
- Do the lint-related `devDependencies` in `package.json` conform? ❌
  - `@metamask/eslint-config-jest` is "^12.0.0", when it should be "^12.1.0".
  - `@metamask/eslint-config-nodejs` is "^12.0.0", when it should be "^12.1.0".
  - `@metamask/eslint-config-typescript` is "^12.0.0", when it should be "^12.1.0".
  - `eslint-plugin-import` is "^2.27.5", when it should be "~2.26.0".
- Do the jest-related `devDependencies` in `package.json` conform? ❌
  - `jest` is "^29.2.2", when it should be "^28.1.3".
- Do the test-related `scripts` in `package.json` conform? ❌
  - `scripts.[test]` is 'yarn test:source && yarn test:types', when it should be 'jest && jest-it-up'.
- Do the typescript-related `devDependencies` in `package.json` conform? ❌
  - `devDependencies.[@types/node]` is '^17.0.23', when it should be '^16'.
- Do the typescript-related `scripts` in `package.json` conform? ❌
  - `scripts.[build]` is 'tsup && yarn build:types', when it should be 'tsup --clean && yarn build:types'.
- Does the `exports` field in `package.json` conform? ✅
- Does the `main` field in `package.json` conform? ✅
- Does the `module` field in `package.json` conform? ✅
- Does the `types` field in `package.json` conform? ✅
- Does the `files` field in `package.json` conform? ✅
- Does LavaMoat allow scripts for `tsup>esbuild`? ✅
- Do the typedoc-related `devDependencies` in `package.json` conform? ✅
- Do the typedoc-related `scripts` in `package.json` conform? ✅
- Is `README.md` present? ✅
- Does the README conform by recommending the correct Yarn version to install? ✅
- Does the README conform by recommending node install from nodejs.org? ❌
  - `README.md` should contain "Install the current LTS version of [Node.js](https://nodejs.org)", but does not.
- Are all of the files for Yarn Modern present, and do they conform? ❌
  - `.yarnrc.yml` does not match the same file in the template repo.
  - `.yarn/releases/yarn-3.2.1.cjs` does not exist in this project.
  - `.yarn/plugins/@yarnpkg/plugin-allow-scripts.cjs` does not match the same file in the template repo.
  - `.yarn/plugins/@yarnpkg/plugin-constraints.cjs` does not match the same file in the template repo.
- Does the `src/` directory exist? ✅
- Is `.nvmrc` present, and does it conform? ❌
  - `.nvmrc` does not match the same file in the template repo.
- Is `jest.config.js` present, and does it conform? ❌
  - `jest.config.js` does not match the same file in the template repo.
- Is `tsconfig.json` present, and does it conform? ❌
  - `tsconfig.json` does not match the same file in the template repo.
- Is `tsconfig.build.json` present, and does it conform? ✅
- Is `tsup.config.ts` present, and does it conform? ✅
- Is `typedoc.json` present, and does it conform? ✅
mcmire commented 6 months ago

@kanthesha Good question. I haven't explained this yet.

I agree that using a structure in the output would make it easier to read, but we will want to do it a different way, especially as we move into the next phase where we introduce a UI for the dashboard.

Currently, we use the dependencies property of each rule to group rules together in the output. So, all rules that rely on the package.json rule, for instance, get indented underneath that rule:

- Does the package have a well-formed manifest (`package.json`)? ✅
  - Does the `packageManager` field in `package.json` conform? ❌
    - `packageManager` is "yarn@3.2.3", when it should be "yarn@3.2.1".
  - Does the `engines.node` field in `package.json` conform? ❌
    - `engines.node` is ">=16.0.0", when it should be "^18.18 || >=20".
  ...

However, dependencies weren't ever intended to be used for grouping; they just exist to control the logical order in which rules are run based on what's being checked. Besides this, we want to use broader categories to group rules: TypeScript, Yarn, ESLint, changelog, etc. In other words we want something like:

- Is the project generally structured correctly? ✅
  - Does the package have a well-formed manifest (`package.json`)? ✅
  - Does the `src/` directory exist? ✅
- Does the project use the correct version of Yarn and is it configured correctly? ❌
  - Is the classic Yarn config file (`.yarnrc`) absent? ✅
  - Does the `packageManager` field in `package.json` conform? ❌
    - `packageManager` is "yarn@3.2.3", when it should be "yarn@3.2.1".
  - Are all of the files for Yarn Modern present, and do they conform? ❌
    - `.yarnrc.yml` does not match the same file in the template repo.
    - `.yarn/releases/yarn-3.2.1.cjs` does not exist in this project.
    - `.yarn/plugins/@yarnpkg/plugin-allow-scripts.cjs` does not match the same file in the template repo.
  - `.yarn/plugins/@yarnpkg/plugin-constraints.cjs` does not match the same file in the template repo.
- Does the project use the correct version of Node? ❌
  - Does the `engines.node` field in `package.json` conform? ❌
    - `engines.node` is ">=16.0.0", when it should be "^18.18 || >=20".
  - Is `.nvmrc` present, and does it conform? ❌
  - `.nvmrc` does not match the same file in the template repo.
- Does the project use ESLint and is it configured correctly? ❌
  - Do the lint-related `devDependencies` in `package.json` conform? ❌
    - `@metamask/eslint-config-jest` is "^12.0.0", when it should be "^12.1.0".
    - `@metamask/eslint-config-nodejs` is "^12.0.0", when it should be "^12.1.0".
    - `@metamask/eslint-config-typescript` is "^12.0.0", when it should be "^12.1.0".
    - `eslint-plugin-import` is "^2.27.5", when it should be "~2.26.0".
- Does the project use Jest and is it configured correctly? ❌
  - Do the jest-related `devDependencies` in `package.json` conform? ❌
    - `jest` is "^29.2.2", when it should be "^28.1.3".
  - Do the test-related `scripts` in `package.json` conform? ❌
    - `scripts.[test]` is 'yarn test:source && yarn test:types', when it should be 'jest && jest-it-up'.
  - Is `jest.config.js` present, and does it conform? ❌
    - `jest.config.js` does not match the same file in the template repo.
- Does the project use TypeScript and is it configured correctly? ❌
  - Do the typescript-related `devDependencies` in `package.json` conform? ❌
    - `devDependencies.[@types/node]` is '^17.0.23', when it should be '^16'.
  - Do the typescript-related `scripts` in `package.json` conform? ❌
    - `scripts.[build]` is 'tsup && yarn build:types', when it should be 'tsup --clean && yarn build:types'.
  - Does the `exports` field in `package.json` conform? ✅
  - Does the `main` field in `package.json` conform? ✅
  - Does the `module` field in `package.json` conform? ✅
  - Does the `types` field in `package.json` conform? ✅
  - Does the `files` field in `package.json` conform? ✅
  - Does LavaMoat allow scripts for `tsup>esbuild`? ✅
  - Is `tsconfig.json` present, and does it conform? ❌
    - `tsconfig.json` does not match the same file in the template repo.
  - Is `tsconfig.build.json` present, and does it conform? ✅
  - Is `tsup.config.ts` present, and does it conform? ✅
- Does the project use TypeDoc and is it configured correctly? ✅
  - Do the typedoc-related `devDependencies` in `package.json` conform? ✅
  - Do the typedoc-related `scripts` in `package.json` conform? ✅
  - Is `typedoc.json` present, and does it conform? ✅
- Does the project have a README and is it configured correctly?
  - Is `README.md` present? ✅
  - Does the README conform by recommending the correct Yarn version to install? ✅
  - Does the README conform by recommending node install from nodejs.org? ❌
    - `README.md` should contain "Install the current LTS version of [Node.js](https://nodejs.org)", but does not.

If we were to use dependencies to create these groups — say, we wanted to create the TypeScript group — then it would mean we'd have to create a "dummy" rule that linked to all of the TypeScript rules as dependencies:

export default buildRule({
  name: RuleName.TypeScriptConforms,
  description:
    'Does the project use TypeScript and is it configured correctly?',
  dependencies: [
    RuleName.PackageTypescriptDependenciesConform,
    RuleName.PackageTypescriptScriptsConform,
    RuleName.PackageMainFieldConforms,
    // ...
  ],
  execute: async () => {
    // This rule wouldn't do anything.
    // If every dependency passes, then this rule passes;
    // if any dependency fails, then this rule fails.
  },
});

That would work, but it would be a bit weird. We will want to come up with a different way of grouping.

I suppose we could wait until then to change how the output is structured, but I didn't want anyone to get confused in the meantime about how dependencies ought to be used.

Hopefully that makes sense! Let me know if you have any more questions.

kanthesha commented 6 months ago

Sure. This definitely makes sense. I was thinking the same that the rules requires a functional grouping and hierarchy as you described, grouping based on the type of checks.

One more point in the similar lines, right now we are putting all the files of checks into rules directory, which needs to be organised or grouped based on functional. That means, typescript checks in one folder and yarn checks in one folder and so on .. otherwise it'll be too many files in one directory and takes time to get into intended file.

mcmire commented 6 months ago

@kanthesha Ah, sure, that would be a good idea to reorganize the files slightly when we do that.

mcmire commented 5 months ago

Branch updated and merge conflicts fixed.

mcmire commented 5 months ago

Fixed merge conflicts again.