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/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..fb094706 --- /dev/null +++ b/src/rules/no-redundant-files.ts @@ -0,0 +1,260 @@ +import type { Rule } from "eslint"; +import type { AST as JsonAST } from "jsonc-eslint-parser"; +import type { JSONStringLiteral } from "jsonc-eslint-parser/lib/parser/ast.js"; + +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 cachedRegex = new Map(); +const getCachedLocalFileRegex = (filename: string) => { + // Strip the leading `./`, if there is one, since we'll be incorporating + // it into the regex. + const baseFilename = filename.replace("./", ""); + let regex = cachedRegex.get(baseFilename); + if (regex) { + return regex; + } else { + regex = new RegExp(`^(./)?${baseFilename}$`, "i"); + cachedRegex.set(baseFilename, regex); + return regex; + } +}; + +/** + * A function to use as a fix, that will remove an element from an array of elements + * and manage the removal of any commas that would be necessary to maintain proper + * json. If this is the last entry, and we don't have a comma (strict json), then + * we don't need to worry about removing this element's comma, but we should remove + * the comma from the element before, since it will the last element after the removal. + * If this is not the last line, and it's not the only line (which wouldn't have a comma) + * then we need to remove the comma from this line, so that it's not left behind after + * the node removal. + * @param elements All file entry elements + * @param currentIndex The index of the entry that we want to remove + * @param fixer The eslint fixer function we'll use to remove the entry + * @param context Current context + * @yields Fixes that the `context.report` fix needs to perform the removals for us + */ +function* removeEntryAndAppropriateCommas( + elements: (JsonAST.JSONExpression | null)[], + currentIndex: number, + fixer: Rule.RuleFixer, + context: Rule.RuleContext, +): Generator { + const element = elements[currentIndex]; + 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 (currentIndex > 0 && tokenFromCurrentLine?.value !== ",") { + const tokenFromPreviousLine = context.sourceCode.getTokenAfter( + elements[currentIndex - 1] as unknown as ESTree.Node, + ); + if (tokenFromPreviousLine?.value === ",") { + yield fixer.remove(tokenFromPreviousLine); + } + } +} + +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: JSONStringLiteral[]; + main?: string; + } = { bin: [], files: [] }; + + 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; + 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)) { + context.report({ + data: { file: element.value }, + messageId: "duplicate", + node: element as unknown as ESTree.Node, + suggest: [ + { + *fix(fixer) { + yield* removeEntryAndAppropriateCommas( + elements, + index, + fixer, + context, + ); + }, + messageId: "remove", + }, + ], + }); + } else { + seen.add(element.value); + entryCache.files.push(element); + } + + // 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)) { + context.report({ + data: { file: element.value }, + messageId: "unnecessaryDefault", + node: element as unknown as ESTree.Node, + suggest: [ + { + *fix(fixer) { + yield* removeEntryAndAppropriateCommas( + elements, + index, + fixer, + context, + ); + }, + messageId: "remove", + }, + ], + }); + } + } + } + } + } + }, + "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()) { + const regex = getCachedLocalFileRegex( + fileEntry.value, + ); + if (regex.test(fileToCheck)) { + context.report({ + data: { file: fileEntry.value }, + messageId: validation.messageId, + node: fileEntry as unknown as ESTree.Node, + suggest: [ + { + *fix(fixer) { + yield* removeEntryAndAppropriateCommas( + files, + index, + fixer, + context, + ); + }, + messageId: "remove", + }, + ], + }); + } + } + } + } + }, + }; + }, + + 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"] }`, + ], +});