diff --git a/README.md b/README.md index 502ec324..a429186d 100644 --- a/README.md +++ b/README.md @@ -122,6 +122,7 @@ The default settings don't conflict, and Prettier plugins can quickly fix up ord | Name                       | Description | 💼 | 🔧 | 💡 | ❌ | | :--------------------------------------------------------------------- | :------------------------------------------------------------------------------------------------ | :- | :- | :- | :- | +| [no-redundant-files](docs/rules/no-redundant-files.md) | Prevents adding unnecessary / redundant files. | | | 💡 | | | [order-properties](docs/rules/order-properties.md) | Package properties must be declared in standard order | ✅ | 🔧 | | | | [repository-shorthand](docs/rules/repository-shorthand.md) | Enforce either object or shorthand declaration for repository. | ✅ | 🔧 | | | | [sort-collections](docs/rules/sort-collections.md) | Dependencies, scripts, and configuration values must be declared in alphabetical order. | ✅ | 🔧 | | | diff --git a/docs/rules/no-redundant-files.md b/docs/rules/no-redundant-files.md new file mode 100644 index 00000000..20973999 --- /dev/null +++ b/docs/rules/no-redundant-files.md @@ -0,0 +1,40 @@ +# no-redundant-files + +💡 This rule is manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions). + + + +This rule checks that the `files` property of a `package.json` doesn't contain +any redundant or unnecessary file entries. By default, [npm will automatically](https://docs.npmjs.com/cli/v11/configuring-npm/package-json#files) +include certain files, based on a number of circumstances. + +It will always include the following files, if present: + +- `package.json` +- `README` + +- `LICENSE` / `LICENCE` + +Additionally, it will include any files that are declared in the `main` and `bin` +fields of the `package.json`. + +This rule will check that the `files` don't contain any of the above. It will +also check for duplicate entries. + +Example of **incorrect** code for this rule: + +```json +{ + "files": ["README.md", "CHANGELOG.md", "lib/index.js"], + "main": "lib/index.js" +} +``` + +Example of **correct** code for this rule: + +```json +{ + "files": ["CHANGELOG.md"], + "main": "lib/index.js" +} +``` diff --git a/eslint.config.js b/eslint.config.js index cadf4ff9..664a6ec6 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -65,6 +65,16 @@ module.exports = tseslint.config( "no-useless-rename": "error", "object-shorthand": "error", "operator-assignment": "error", + "perfectionist/sort-objects": [ + "error", + { + customGroups: { + programExit: "Program:exit", + }, + groups: ["unknown", "programExit"], + type: "alphabetical", + }, + ], }, settings: { perfectionist: { partitionByComment: true, type: "natural" }, diff --git a/package.json b/package.json index 0090f0fb..f837c136 100644 --- a/package.json +++ b/package.json @@ -58,6 +58,7 @@ }, "dependencies": { "@altano/repository-tools": "^0.1.1", + "cached-factory": "^0.1.0", "detect-indent": "6.1.0", "detect-newline": "3.1.0", "package-json-validator": "^0.7.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 329e743a..183d0940 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -11,6 +11,9 @@ importers: '@altano/repository-tools': specifier: ^0.1.1 version: 0.1.1 + cached-factory: + specifier: ^0.1.0 + version: 0.1.0 detect-indent: specifier: 6.1.0 version: 6.1.0 @@ -1362,6 +1365,10 @@ packages: resolution: {integrity: sha512-b6Ilus+c3RrdDk+JhLKUAQfzzgLEPy6wcXqS7f/xe1EETvsDP6GORG7SFuOs6cID5YkqchW/LXZbX5bc8j7ZcQ==} engines: {node: '>=8'} + cached-factory@0.1.0: + resolution: {integrity: sha512-IGOSWu+NuED5UzCRmBeqQPZ8z7SkgrD/nN67W2iY1Qv83CVhevyMexkGclJ86saXisIqxoOnbeiTWvsCHRqJBw==} + engines: {node: '>=18'} + callsites@3.1.0: resolution: {integrity: sha512-P8BjAsXvZS+VIDUI11hHCQEv74YT67YUi5JJFNWIqL235sBmjX4+qx9Muvls5ivyNENctx46xQLQ3aTuE7ssaQ==} engines: {node: '>=6'} @@ -4836,6 +4843,8 @@ snapshots: cac@6.7.14: {} + cached-factory@0.1.0: {} + callsites@3.1.0: {} camelcase@8.0.0: {} diff --git a/src/plugin.ts b/src/plugin.ts index 0a44fda6..d8b87691 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -2,6 +2,7 @@ import { createRequire } from "node:module"; import type { PackageJsonRuleModule } from "./createRule.js"; +import { rule as noRedundantFiles } from "./rules/no-redundant-files.js"; import { rule as orderProperties } from "./rules/order-properties.js"; import { rule as preferRepositoryShorthand } from "./rules/repository-shorthand.js"; import { rule as sortCollections } from "./rules/sort-collections.js"; @@ -20,6 +21,7 @@ const { name, version } = require("../package.json") as { }; const rules: Record = { + "no-redundant-files": noRedundantFiles, "order-properties": orderProperties, "repository-shorthand": preferRepositoryShorthand, "sort-collections": sortCollections, diff --git a/src/rules/no-redundant-files.ts b/src/rules/no-redundant-files.ts new file mode 100644 index 00000000..c01dcabf --- /dev/null +++ b/src/rules/no-redundant-files.ts @@ -0,0 +1,225 @@ +import type { AST as JsonAST } from "jsonc-eslint-parser"; + +import { CachedFactory } from "cached-factory"; +import * as ESTree from "estree"; + +import { createRule } from "../createRule.js"; +import { isJSONStringLiteral, isNotNullish } from "../utils/predicates.js"; + +const defaultFiles = [ + /* cspell:disable-next-line */ + "LICENCE", + /* cspell:disable-next-line */ + "LICENCE.md", + "LICENSE", + "LICENSE.md", + "package.json", + "README.md", +] as const; + +const regexCache = new CachedFactory( + (filename: string) => new RegExp(`^(./)?${filename}$`, "i"), +); +const getCachedLocalFileRegex = (filename: string) => { + // Strip the leading `./`, if there is one, since we'll be incorporating + // it into the regex. + const baseFilename = filename.replace("./", ""); + return regexCache.get(baseFilename); +}; + +export const rule = createRule({ + create(context) { + // We need to cache these as we find them, since we need to know some of + // the other values to ensure that files doesn't contain duplicates. + const entryCache: { + bin: string[]; + files: (JsonAST.JSONExpression | null)[]; + main?: string; + } = { bin: [], files: [] }; + + /** + * Report rule violations + */ + const report = ( + elements: (JsonAST.JSONExpression | null)[], + index: number, + messageId: string, + ) => { + const element = elements[index]; + + if (isNotNullish(element) && isJSONStringLiteral(element)) { + context.report({ + data: { file: element.value }, + messageId, + node: element as unknown as ESTree.Node, + suggest: [ + { + *fix(fixer) { + yield fixer.remove( + element as unknown as ESTree.Node, + ); + + // If this is not the last entry, then we need to remove the comma from this line. + const tokenFromCurrentLine = + context.sourceCode.getTokenAfter( + element as unknown as ESTree.Node, + ); + if (tokenFromCurrentLine?.value === ",") { + yield fixer.remove(tokenFromCurrentLine); + } + + // If this is the last line and it's not the only entry, then the line above this one + // will become the last line, and should not have a trailing comma. + if ( + index > 0 && + tokenFromCurrentLine?.value !== "," + ) { + const tokenFromPreviousLine = + context.sourceCode.getTokenAfter( + elements[ + index - 1 + ] as unknown as ESTree.Node, + ); + if (tokenFromPreviousLine?.value === ",") { + yield fixer.remove( + tokenFromPreviousLine, + ); + } + } + }, + messageId: "remove", + }, + ], + }); + } + }; + + return { + "Program > JSONExpressionStatement > JSONObjectExpression > JSONProperty[key.value=bin]"( + node: JsonAST.JSONProperty, + ) { + const binValue = node.value; + + // "bin" can be either a simple string or a map of commands to files. + // If it's anything else, then this is malformed and we can't really + // do anything with it. + if (isJSONStringLiteral(binValue)) { + entryCache.bin.push(binValue.value); + } else if (binValue.type === "JSONObjectExpression") { + for (const prop of binValue.properties) { + if (isJSONStringLiteral(prop.value)) { + entryCache.bin.push(prop.value.value); + } + } + } + }, + "Program > JSONExpressionStatement > JSONObjectExpression > JSONProperty[key.value=files]"( + node: JsonAST.JSONProperty, + ) { + // "files" should only ever be an array of strings. + if (node.value.type === "JSONArrayExpression") { + // We want to add it to the files cache, but also check for + // duplicates as we go. + const seen = new Set(); + const elements = node.value.elements; + entryCache.files = elements; + for (const [index, element] of elements.entries()) { + // We only care about JSONStringLiteral values + // That _should_ be all that's here, be in order to process + // the fix correctly we'll act on the full array of elements + if ( + isNotNullish(element) && + isJSONStringLiteral(element) + ) { + if (seen.has(element.value)) { + report(elements, index, "duplicate"); + } else { + seen.add(element.value); + } + + // We can also go ahead and check if this matches one + // of the static default files + const regex = getCachedLocalFileRegex( + element.value, + ); + for (const defaultFile of defaultFiles) { + if (regex.test(defaultFile)) { + report( + elements, + index, + "unnecessaryDefault", + ); + } + } + } + } + } + }, + "Program > JSONExpressionStatement > JSONObjectExpression > JSONProperty[key.value=main]"( + node: JsonAST.JSONProperty, + ) { + // "main" should only ever be a string. + if (isJSONStringLiteral(node.value)) { + entryCache.main = node.value.value; + } + }, + "Program:exit"() { + // Now that we have all of the entries, we can check for unnecessary files. + const files = entryCache.files; + + // Bail out early if there are no files. + if (files.length === 0) { + return; + } + + const validations = [ + // First check if the "main" entry is included in "files". + { + files: entryCache.main ? [entryCache.main] : [], + messageId: "unnecessaryMain", + }, + // Next check if any "bin" entries are included in "files". + { + files: entryCache.bin, + messageId: "unnecessaryBin", + }, + ]; + for (const validation of validations) { + for (const fileToCheck of validation.files) { + for (const [index, fileEntry] of files.entries()) { + if ( + isNotNullish(fileEntry) && + isJSONStringLiteral(fileEntry) + ) { + const regex = getCachedLocalFileRegex( + fileEntry.value, + ); + if (regex.test(fileToCheck)) { + report(files, index, validation.messageId); + } + } + } + } + } + }, + }; + }, + + meta: { + docs: { + category: "Best Practices", + description: "Prevents adding unnecessary / redundant files.", + recommended: false, + }, + hasSuggestions: true, + messages: { + duplicate: 'Files has more than one entry for "{{file}}".', + remove: "Remove this redundant entry.", + unnecessaryBin: `Explicitly declaring "{{file}}" in "files" is unnecessary; it's included in "bin".`, + unnecessaryDefault: `Explicitly declaring "{{file}}" in "files" is unnecessary; it's included by default.`, + unnecessaryMain: `Explicitly declaring "{{file}}" in "files" is unnecessary; it's the "main" entry.`, + }, + schema: [], + type: "suggestion", + }, +}); diff --git a/src/tests/rules/no-redundant-files.test.ts b/src/tests/rules/no-redundant-files.test.ts new file mode 100644 index 00000000..e9eeeea9 --- /dev/null +++ b/src/tests/rules/no-redundant-files.test.ts @@ -0,0 +1,329 @@ +import { rule } from "../../rules/no-redundant-files.js"; +import { ruleTester } from "./ruleTester.js"; + +ruleTester.run("no-redundant-files", rule, { + invalid: [ + { + code: `{ +\t"files": [ +\t\t"README.md", +\t\t"./package.json" + ] +} +`, + errors: [ + { + data: { file: "README.md" }, + line: 3, + messageId: "unnecessaryDefault", + suggestions: [ + { + messageId: "remove", + output: `{ +\t"files": [ +\t\t +\t\t"./package.json" + ] +} +`, + }, + ], + }, + { + data: { file: "./package.json" }, + line: 4, + messageId: "unnecessaryDefault", + suggestions: [ + { + messageId: "remove", + output: `{ +\t"files": [ +\t\t"README.md" +\t\t + ] +} +`, + }, + ], + }, + ], + }, + { + code: `{ +\t"files": [ +\t\t"CHANGELOG.md", +\t\t"dist", +\t\t"CHANGELOG.md" + ] +} +`, + errors: [ + { + data: { file: "CHANGELOG.md" }, + line: 5, + messageId: "duplicate", + suggestions: [ + { + messageId: "remove", + output: `{ +\t"files": [ +\t\t"CHANGELOG.md", +\t\t"dist" +\t\t + ] +} +`, + }, + ], + }, + ], + }, + { + code: `{ + "main": "./index.js", +\t"files": [ +\t\t"CHANGELOG.md", +\t\t"./index.js" + ] +} +`, + errors: [ + { + data: { file: "./index.js" }, + line: 5, + messageId: "unnecessaryMain", + suggestions: [ + { + messageId: "remove", + output: `{ + "main": "./index.js", +\t"files": [ +\t\t"CHANGELOG.md" +\t\t + ] +} +`, + }, + ], + }, + ], + }, + { + code: `{ + "main": "./index.js", +\t"files": [ +\t\t"CHANGELOG.md", +\t\t"index.js" + ] +} +`, + errors: [ + { + data: { file: "index.js" }, + line: 5, + messageId: "unnecessaryMain", + suggestions: [ + { + messageId: "remove", + output: `{ + "main": "./index.js", +\t"files": [ +\t\t"CHANGELOG.md" +\t\t + ] +} +`, + }, + ], + }, + ], + }, + { + code: `{ + "bin": "./dist/cli.js", +\t"files": [ +\t\t"CHANGELOG.md", +\t\t"./dist/cli.js" + ] +} +`, + errors: [ + { + data: { file: "./dist/cli.js" }, + line: 5, + messageId: "unnecessaryBin", + suggestions: [ + { + messageId: "remove", + output: `{ + "bin": "./dist/cli.js", +\t"files": [ +\t\t"CHANGELOG.md" +\t\t + ] +} +`, + }, + ], + }, + ], + }, + { + code: `{ + "bin": "./dist/cli.js", +\t"files": [ +\t\t"CHANGELOG.md", +\t\t"dist/cli.js" + ] +} +`, + errors: [ + { + data: { file: "dist/cli.js" }, + line: 5, + messageId: "unnecessaryBin", + suggestions: [ + { + messageId: "remove", + output: `{ + "bin": "./dist/cli.js", +\t"files": [ +\t\t"CHANGELOG.md" +\t\t + ] +} +`, + }, + ], + }, + ], + }, + { + code: `{ + "bin": { + "cli": "./dist/cli.js", + }, +\t"files": [ +\t\t"CHANGELOG.md", +\t\t"./dist/cli.js" + ] +} +`, + errors: [ + { + data: { file: "./dist/cli.js" }, + line: 7, + messageId: "unnecessaryBin", + suggestions: [ + { + messageId: "remove", + output: `{ + "bin": { + "cli": "./dist/cli.js", + }, +\t"files": [ +\t\t"CHANGELOG.md" +\t\t + ] +} +`, + }, + ], + }, + ], + }, + { + code: `{ + "bin": { + "cli": "./dist/cli.js", + }, +\t"files": [ +\t\tnull, +\t\t"CHANGELOG.md", +\t\t"./dist/cli.js" + ] +} +`, + errors: [ + { + data: { file: "./dist/cli.js" }, + line: 8, + messageId: "unnecessaryBin", + suggestions: [ + { + messageId: "remove", + output: `{ + "bin": { + "cli": "./dist/cli.js", + }, +\t"files": [ +\t\tnull, +\t\t"CHANGELOG.md" +\t\t + ] +} +`, + }, + ], + }, + ], + }, + { + code: `{ +\t"files": [ +\t\t"README.md", +\t\tnull, +\t\t"./package.json" + ] +} +`, + errors: [ + { + data: { file: "README.md" }, + line: 3, + messageId: "unnecessaryDefault", + suggestions: [ + { + messageId: "remove", + output: `{ +\t"files": [ +\t\t +\t\tnull, +\t\t"./package.json" + ] +} +`, + }, + ], + }, + { + data: { file: "./package.json" }, + line: 5, + messageId: "unnecessaryDefault", + suggestions: [ + { + messageId: "remove", + output: `{ +\t"files": [ +\t\t"README.md", +\t\tnull +\t\t + ] +} +`, + }, + ], + }, + ], + }, + ], + valid: [ + "{}", + `{ "main": "./index.js" }`, + `{ "bin": "./bin/cli.js" }`, + `{ "bin": { + "cli": "./bin/cli.js" +}}`, + `{ "main": "./dist/index.js", "files": ["CHANGELOG.md", "dist"] }`, + ], +});