From a9417d1685808dd8e4c27eff9e3b9c7399be6e1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Sat, 20 Jan 2024 15:36:05 -0500 Subject: [PATCH] feat: added unique-dependencies rule (#126) ## PR Checklist - [x] Addresses an existing open issue: fixes #50 - [x] That issue was marked as [`status: accepting prs`](https://github.com/JoshuaKGoldberg/eslint-plugin-package-json/issues?q=is%3Aopen+is%3Aissue+label%3A%22status%3A+accepting+prs%22) - [x] Steps in [CONTRIBUTING.md](https://github.com/JoshuaKGoldberg/eslint-plugin-package-json/blob/main/.github/CONTRIBUTING.md) were taken ## Overview I ended up naming it `unique-dependencies` because I'm not a fan of `no-` prefixes for lint rules (https://github.com/typescript-eslint/typescript-eslint/discussions/6022). --- README.md | 1 + docs/rules/unique-dependencies.md | 7 + src/index.ts | 2 + src/rules/unique-dependencies.ts | 109 ++++++++ src/tests/rules/unique-dependencies.test.ts | 267 ++++++++++++++++++++ src/utils/predicates.ts | 13 + 6 files changed, 399 insertions(+) create mode 100644 docs/rules/unique-dependencies.md create mode 100644 src/rules/unique-dependencies.ts create mode 100644 src/tests/rules/unique-dependencies.test.ts create mode 100644 src/utils/predicates.ts diff --git a/README.md b/README.md index 2bf60d6d..d92bdb44 100644 --- a/README.md +++ b/README.md @@ -82,6 +82,7 @@ The default settings don't conflict, and Prettier plugins can quickly fix up ord | [order-properties](docs/rules/order-properties.md) | Package properties must be declared in standard order | ✅ | 🔧 | | | [prefer-repository-shorthand](docs/rules/prefer-repository-shorthand.md) | Enforce shorthand declaration for GitHub repository. | ✅ | 🔧 | | | [sort-collections](docs/rules/sort-collections.md) | Dependencies, scripts, and configuration values must be declared in alphabetical order. | ✅ | 🔧 | | +| [unique-dependencies](docs/rules/unique-dependencies.md) | Enforce that if repository directory is specified, it matches the path to the package.json file | ✅ | | 💡 | | [valid-local-dependency](docs/rules/valid-local-dependency.md) | Checks existence of local dependencies in the package.json | ✅ | | | | [valid-package-def](docs/rules/valid-package-def.md) | Enforce that package.json has all properties required by the npm spec | ✅ | | | | [valid-repository-directory](docs/rules/valid-repository-directory.md) | Enforce that if repository directory is specified, it matches the path to the package.json file | ✅ | | 💡 | diff --git a/docs/rules/unique-dependencies.md b/docs/rules/unique-dependencies.md new file mode 100644 index 00000000..a776a53d --- /dev/null +++ b/docs/rules/unique-dependencies.md @@ -0,0 +1,7 @@ +# Enforce that if repository directory is specified, it matches the path to the package.json file (`package-json/unique-dependencies`) + +💼 This rule is enabled in the ✅ `recommended` config. + +💡 This rule is manually fixable by [editor suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions). + + diff --git a/src/index.ts b/src/index.ts index d25082ff..48a8a230 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,7 @@ import orderProperties from "./rules/order-properties.js"; import preferRepositoryShorthand from "./rules/prefer-repository-shorthand.js"; import sortCollections from "./rules/sort-collections.js"; +import uniqueDependencies from "./rules/unique-dependencies.js"; import validLocalDependency from "./rules/valid-local-dependency.js"; import validPackageDef from "./rules/valid-package-def.js"; import validRepositoryDirectory from "./rules/valid-repository-directory.js"; @@ -9,6 +10,7 @@ export const rules = { "order-properties": orderProperties, "prefer-repository-shorthand": preferRepositoryShorthand, "sort-collections": sortCollections, + "unique-dependencies": uniqueDependencies, "valid-local-dependency": validLocalDependency, "valid-package-def": validPackageDef, "valid-repository-directory": validRepositoryDirectory, diff --git a/src/rules/unique-dependencies.ts b/src/rules/unique-dependencies.ts new file mode 100644 index 00000000..51445f23 --- /dev/null +++ b/src/rules/unique-dependencies.ts @@ -0,0 +1,109 @@ +import type { AST as JsonAST } from "jsonc-eslint-parser"; + +import * as ESTree from "estree"; + +import { createRule } from "../createRule.js"; +import { isJSONStringLiteral, isNotNullish } from "../utils/predicates.js"; + +const dependencyPropertyNames = new Set([ + "bundleDependencies", + "bundledDependencies", + "dependencies", + "devDependencies", + "optionalDependencies", + "peerDependencies", + "overrides", +]); + +export default createRule({ + create(context) { + function check( + elements: (JsonAST.JSONNode | null)[], + getNodeToRemove: (element: JsonAST.JSONNode) => ESTree.Node, + ) { + const seen = new Set(); + + for (const element of elements + .filter(isNotNullish) + .filter(isJSONStringLiteral) + .reverse()) { + if (seen.has(element.value)) { + report(element); + } else { + seen.add(element.value); + } + } + + function report(node: JsonAST.JSONNode) { + context.report({ + messageId: "overridden", + node: node as unknown as ESTree.Node, + suggest: [ + { + fix(fixer) { + const removal = getNodeToRemove(node); + return [ + fixer.remove(removal), + fixer.remove( + // A listing that's overridden can't be last, + // so we're guaranteed there's a comma after. + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + context.sourceCode.getTokenAfter( + removal, + )!, + ), + ]; + }, + messageId: "remove", + }, + ], + }); + } + } + + return { + "Program > JSONExpressionStatement > JSONObjectExpression > JSONProperty[key.type=JSONLiteral]"( + node: JsonAST.JSONProperty & { + key: JsonAST.JSONStringLiteral; + }, + ) { + if (!dependencyPropertyNames.has(node.key.value)) { + return; + } + + switch (node.value.type) { + case "JSONArrayExpression": + check( + node.value.elements, + (element) => element as unknown as ESTree.Node, + ); + break; + case "JSONObjectExpression": + check( + node.value.properties.map( + (property) => property.key, + ), + (property) => + property.parent as unknown as ESTree.Node, + ); + break; + } + }, + }; + }, + + meta: { + docs: { + category: "Best Practices", + description: + "Enforce that if repository directory is specified, it matches the path to the package.json file", + recommended: true, + }, + hasSuggestions: true, + messages: { + overridden: + "Package name is overridden by a duplicate listing later on.", + remove: "Remove this redundant dependency listing.", + }, + }, +}); diff --git a/src/tests/rules/unique-dependencies.test.ts b/src/tests/rules/unique-dependencies.test.ts new file mode 100644 index 00000000..3df0e269 --- /dev/null +++ b/src/tests/rules/unique-dependencies.test.ts @@ -0,0 +1,267 @@ +import rule from "../../rules/unique-dependencies.js"; +import { ruleTester } from "./ruleTester.js"; + +ruleTester.run("unique-dependencies", rule, { + invalid: [ + { + code: `{ + "bundleDependencies": ["abc", "abc"] + }`, + errors: [ + { + column: 26, + endColumn: 31, + line: 2, + messageId: "overridden", + suggestions: [ + { + messageId: "remove", + output: `{ + "bundleDependencies": [ "abc"] + }`, + }, + ], + }, + ], + filename: "package.json", + }, + { + code: `{ + "bundledDependencies": ["abc", "abc"] + }`, + errors: [ + { + column: 27, + endColumn: 32, + line: 2, + messageId: "overridden", + suggestions: [ + { + messageId: "remove", + output: `{ + "bundledDependencies": [ "abc"] + }`, + }, + ], + }, + ], + filename: "package.json", + }, + // ... + { + code: `{ + "dependencies": { + "abc": "1.2.3", + "abc": "1.2.3" + } + }`, + errors: [ + { + column: 4, + endColumn: 9, + line: 3, + messageId: "overridden", + suggestions: [ + { + messageId: "remove", + output: `{ + "dependencies": { + + "abc": "1.2.3" + } + }`, + }, + ], + }, + ], + filename: "package.json", + }, + // ... + { + code: `{ + "devDependencies": { + "abc": "1.2.3", + "abc": "1.2.3" + } + }`, + errors: [ + { + column: 4, + endColumn: 9, + line: 3, + messageId: "overridden", + suggestions: [ + { + messageId: "remove", + output: `{ + "devDependencies": { + + "abc": "1.2.3" + } + }`, + }, + ], + }, + ], + filename: "package.json", + }, + { + code: `{ + "optionalDependencies": { + "abc": "1.2.3", + "abc": "1.2.3" + } + }`, + errors: [ + { + column: 4, + endColumn: 9, + line: 3, + messageId: "overridden", + suggestions: [ + { + messageId: "remove", + output: `{ + "optionalDependencies": { + + "abc": "1.2.3" + } + }`, + }, + ], + }, + ], + filename: "package.json", + }, + { + code: `{ + "peerDependencies": { + "abc": "1.2.3", + "abc": "1.2.3" + } + }`, + errors: [ + { + column: 4, + endColumn: 9, + line: 3, + messageId: "overridden", + suggestions: [ + { + messageId: "remove", + output: `{ + "peerDependencies": { + + "abc": "1.2.3" + } + }`, + }, + ], + }, + ], + filename: "package.json", + }, + // ... + // ... + { + code: `{ + "overrides": ["abc", "abc"] + }`, + errors: [ + { + column: 17, + endColumn: 22, + line: 2, + messageId: "overridden", + suggestions: [ + { + messageId: "remove", + output: `{ + "overrides": [ "abc"] + }`, + }, + ], + }, + ], + filename: "package.json", + }, + ], + + valid: [ + `{}`, + `{ + "dependencies": {} +}`, + `{ + "dependencies": { + "abc": "1.2.3" + } +}`, + `{ + "dependencies": { + "abc": "1.2.3", + "def": "1.2.3" + } +}`, + `{ + "devDependencies": {} +}`, + `{ + "devDependencies": { + "abc": "1.2.3" + } +}`, + `{ + "devDependencies": { + "abc": "1.2.3", + "def": "1.2.3" + } +}`, + `{ + "peerDependencies": {} +}`, + `{ + "peerDependencies": { + "abc": "1.2.3" + } +}`, + `{ + "peerDependencies": { + "abc": "1.2.3", + "def": "1.2.3" + } +}`, + `{ + "optionalDependencies": {} +}`, + `{ + "optionalDependencies": { + "abc": "1.2.3" + } +}`, + `{ + "optionalDependencies": { + "abc": "1.2.3", + "def": "1.2.3" + } +}`, + `{ + "overrides": [] +}`, + `{ + "overrides": ["abc"] +}`, + `{ + "overrides": ["abc", "def"] +}`, + `{ + "unrelated": ["abc", "abc"] +}`, + `{ + "unrelated": { + "abc": "1.2.3", + "abc": "1.2.3" + } +}`, + ], +}); diff --git a/src/utils/predicates.ts b/src/utils/predicates.ts new file mode 100644 index 00000000..c9b52b01 --- /dev/null +++ b/src/utils/predicates.ts @@ -0,0 +1,13 @@ +import type { AST as JsonAST } from "jsonc-eslint-parser"; + +export function isNotNullish>( + value: T | null | undefined, +): value is T { + return value !== null && value !== undefined; +} + +export function isJSONStringLiteral( + node: JsonAST.JSONNode, +): node is JsonAST.JSONStringLiteral { + return node.type === "JSONLiteral" && typeof node.value === "string"; +}