From 6a02598eeedf6c482ae0bc64783109387029b9dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Josh=20Goldberg=20=E2=9C=A8?= Date: Thu, 23 Jan 2025 14:11:41 -0500 Subject: [PATCH] chore: use `eslint-fix-utils` for element and property removals (#750) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## PR Checklist - [x] Addresses an existing open issue: fixes #747 - [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 Uses the exported APIs added in https://github.com/JoshuaKGoldberg/eslint-fix-utils/commit/6bf5c3329eb47c43d38128328fd4501277b3c8ad. @michaelfaith I'd love to get your input on this: how does the API feel to you? If you think they should be changed drastically -here and/or in the other repo- I'm definitely up for iterating on them. 🙂 ~~The build failures can be ignored, they're some problem in `eslint-fix-utils` that I haven't dug into yet.~~ ~~Blocked on: https://github.com/JoshuaKGoldberg/eslint-fix-utils/issues/7~~ ✅ `eslint-fix-utils@0.2.0` should work now. 💖 --- package.json | 1 + pnpm-lock.yaml | 15 ++++++++++ src/rules/no-empty-fields.ts | 38 ++++++++++--------------- src/rules/no-redundant-files.ts | 39 ++++---------------------- src/rules/unique-dependencies.ts | 48 +++++++++++++++++--------------- 5 files changed, 62 insertions(+), 79 deletions(-) diff --git a/package.json b/package.json index 152ebdf8..ed00063f 100644 --- a/package.json +++ b/package.json @@ -60,6 +60,7 @@ "@altano/repository-tools": "^0.1.1", "detect-indent": "6.1.0", "detect-newline": "3.1.0", + "eslint-fix-utils": "^0.2.0", "package-json-validator": "^0.8.0", "semver": "^7.5.4", "sort-object-keys": "^1.1.3", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a92a1f2c..5e7ab451 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -17,6 +17,9 @@ importers: detect-newline: specifier: 3.1.0 version: 3.1.0 + eslint-fix-utils: + specifier: ^0.2.0 + version: 0.2.0(@types/estree@1.0.6)(eslint@9.18.0(jiti@2.4.2)) package-json-validator: specifier: ^0.8.0 version: 0.8.0 @@ -1835,6 +1838,13 @@ packages: peerDependencies: eslint: '>= 8.57.1' + eslint-fix-utils@0.2.0: + resolution: {integrity: sha512-2z/I/1VfyL1mkLjk18W7Q3X+gqzeFmc2aNxSY/RmT6vNPyzQfT1MR/yP/+RUbqRuNUNMhTTYeP7AK7UUpcUD4w==} + engines: {node: '>=18.3.0'} + peerDependencies: + '@types/estree': '>=1' + eslint: '>=8' + eslint-json-compat-utils@0.2.1: resolution: {integrity: sha512-YzEodbDyW8DX8bImKhAcCeu/L31Dd/70Bidx2Qex9OFUtgzXLqtfWL4Hr5fM/aCCB8QUZLuJur0S9k6UfgFkfg==} engines: {node: '>=12'} @@ -5348,6 +5358,11 @@ snapshots: - supports-color - typescript + eslint-fix-utils@0.2.0(@types/estree@1.0.6)(eslint@9.18.0(jiti@2.4.2)): + dependencies: + '@types/estree': 1.0.6 + eslint: 9.18.0(jiti@2.4.2) + eslint-json-compat-utils@0.2.1(eslint@9.18.0(jiti@2.4.2))(jsonc-eslint-parser@2.4.0): dependencies: eslint: 9.18.0(jiti@2.4.2) diff --git a/src/rules/no-empty-fields.ts b/src/rules/no-empty-fields.ts index c57c0175..2da92280 100644 --- a/src/rules/no-empty-fields.ts +++ b/src/rules/no-empty-fields.ts @@ -1,5 +1,9 @@ import type { AST as JsonAST } from "jsonc-eslint-parser"; +import { + fixRemoveArrayElement, + fixRemoveObjectProperty, +} from "eslint-fix-utils"; import * as ESTree from "estree"; import { createRule, PackageJsonRuleContext } from "../createRule"; @@ -52,29 +56,17 @@ const report = ( node: node as unknown as ESTree.Node, suggest: [ { - *fix(fixer) { - yield fixer.remove(node as unknown as ESTree.Node); - - const tokenFromCurrentLine = - context.sourceCode.getTokenAfter( - node as unknown as ESTree.Node, - ); - const tokenFromPreviousLine = - context.sourceCode.getTokenBefore( - node as unknown as ESTree.Node, - ); - - if ( - tokenFromPreviousLine?.value === "," && - tokenFromCurrentLine?.value !== "," - ) { - yield fixer.remove(tokenFromPreviousLine); - } - - if (tokenFromCurrentLine?.value === ",") { - yield fixer.remove(tokenFromCurrentLine); - } - }, + fix: + node.type === "JSONProperty" + ? fixRemoveObjectProperty( + context, + node as unknown as ESTree.Property, + ) + : fixRemoveArrayElement( + context, + node as unknown as ESTree.Expression, + node.parent as unknown as ESTree.ArrayExpression, + ), messageId: "remove", }, ], diff --git a/src/rules/no-redundant-files.ts b/src/rules/no-redundant-files.ts index 2e093cc3..fca6d8be 100644 --- a/src/rules/no-redundant-files.ts +++ b/src/rules/no-redundant-files.ts @@ -1,5 +1,6 @@ import type { AST as JsonAST } from "jsonc-eslint-parser"; +import { fixRemoveArrayElement } from "eslint-fix-utils"; import * as ESTree from "estree"; import { createRule } from "../createRule.js"; @@ -58,39 +59,11 @@ export const rule = createRule({ 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, - ); - } - } - }, + fix: fixRemoveArrayElement( + context, + index, + elements as unknown as ESTree.Expression[], + ), messageId: "remove", }, ], diff --git a/src/rules/unique-dependencies.ts b/src/rules/unique-dependencies.ts index c9c45796..d54173f7 100644 --- a/src/rules/unique-dependencies.ts +++ b/src/rules/unique-dependencies.ts @@ -1,5 +1,9 @@ import type { AST as JsonAST } from "jsonc-eslint-parser"; +import { + fixRemoveArrayElement, + fixRemoveObjectProperty, +} from "eslint-fix-utils"; import * as ESTree from "estree"; import { createRule } from "../createRule.js"; @@ -19,7 +23,7 @@ export const rule = createRule({ create(context) { function check( elements: (JsonAST.JSONNode | null)[], - getNodeToRemove: (element: JsonAST.JSONNode) => ESTree.Node, + getNodeToRemove: (element: JsonAST.JSONNode) => JsonAST.JSONNode, ) { const seen = new Set(); @@ -28,32 +32,33 @@ export const rule = createRule({ .filter(isJSONStringLiteral) .reverse()) { if (seen.has(element.value)) { - report(element); + report(element, elements); } else { seen.add(element.value); } } - function report(node: JsonAST.JSONNode) { + function report( + node: JsonAST.JSONNode, + elements: (JsonAST.JSONNode | null)[], + ) { + const removal = getNodeToRemove(node); 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, - )!, - ), - ]; - }, + fix: + removal.type === "JSONProperty" + ? fixRemoveObjectProperty( + context, + removal as unknown as ESTree.Property, + ) + : fixRemoveArrayElement( + context, + removal as unknown as ESTree.Expression, + elements as unknown as ESTree.ArrayExpression, + ), messageId: "remove", }, ], @@ -73,18 +78,15 @@ export const rule = createRule({ switch (node.value.type) { case "JSONArrayExpression": - check( - node.value.elements, - (element) => element as unknown as ESTree.Node, - ); + check(node.value.elements, (element) => element); break; case "JSONObjectExpression": check( node.value.properties.map( (property) => property.key, ), - (property) => - property.parent as unknown as ESTree.Node, + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + (property) => property.parent!, ); break; }