From e57765b3de7c4703f1c697d54758e5789494281a Mon Sep 17 00:00:00 2001 From: chouchouji <70570907+chouchouji@users.noreply.github.com> Date: Thu, 23 Jan 2025 22:19:20 +0800 Subject: [PATCH] feat: add no-empty-fields rule (#741) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## PR Checklist - [x] Addresses an existing open issue: fixes #683 - [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 --- README.md | 1 + docs/rules/no-empty-fields.md | 37 +++ src/plugin.ts | 2 + src/rules/no-empty-fields.ts | 122 ++++++++++ src/tests/rules/no-empty-fields.test.ts | 307 ++++++++++++++++++++++++ 5 files changed, 469 insertions(+) create mode 100644 docs/rules/no-empty-fields.md create mode 100644 src/rules/no-empty-fields.ts create mode 100644 src/tests/rules/no-empty-fields.test.ts diff --git a/README.md b/README.md index f5d43d0a..757457e7 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-empty-fields](docs/rules/no-empty-fields.md) | Reports on unnecessary empty arrays and objects. | ✅ | | 💡 | | | [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. | ✅ | 🔧 | | | diff --git a/docs/rules/no-empty-fields.md b/docs/rules/no-empty-fields.md new file mode 100644 index 00000000..a8b184f6 --- /dev/null +++ b/docs/rules/no-empty-fields.md @@ -0,0 +1,37 @@ +# no-empty-fields + +💼 This rule is enabled in the ✅ `recommended` config. + +💡 This rule is manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions). + + + +This rule flags all empty arrays and objects in a `package.json`, +as such empty expressions do nothing, and are often the result of a mistake. +It will report both named properties that are empty, as well as nested arrays and objects +that are empty. + +Example of **incorrect** code for this rule: + +```json +{ + "main": "lib/index.js", + "scripts": {}, + "files": [], + "simple-git-hooks": { + "pre-commit": "pnpm exec nano-staged --allow-empty", + "preserveUnused": [] + } +} +``` + +Example of **correct** code for this rule: + +```json +{ + "main": "lib/index.js", + "simple-git-hooks": { + "pre-commit": "pnpm exec nano-staged --allow-empty" + } +} +``` diff --git a/src/plugin.ts b/src/plugin.ts index d8b87691..a09d3077 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 noEmptyFields } from "./rules/no-empty-fields.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"; @@ -21,6 +22,7 @@ const { name, version } = require("../package.json") as { }; const rules: Record = { + "no-empty-fields": noEmptyFields, "no-redundant-files": noRedundantFiles, "order-properties": orderProperties, "repository-shorthand": preferRepositoryShorthand, diff --git a/src/rules/no-empty-fields.ts b/src/rules/no-empty-fields.ts new file mode 100644 index 00000000..c57c0175 --- /dev/null +++ b/src/rules/no-empty-fields.ts @@ -0,0 +1,122 @@ +import type { AST as JsonAST } from "jsonc-eslint-parser"; + +import * as ESTree from "estree"; + +import { createRule, PackageJsonRuleContext } from "../createRule"; + +const getDataAndMessageId = ( + node: + | JsonAST.JSONArrayExpression + | JsonAST.JSONObjectExpression + | JsonAST.JSONProperty, +): { + data: Record; + messageId: "emptyExpression" | "emptyFields"; +} => { + switch (node.type) { + case "JSONArrayExpression": + return { + data: { + expressionType: "array", + }, + messageId: "emptyExpression", + }; + case "JSONObjectExpression": + return { + data: { + expressionType: "object", + }, + messageId: "emptyExpression", + }; + case "JSONProperty": + return { + data: { + field: (node.key as JsonAST.JSONStringLiteral).value, + }, + messageId: "emptyFields", + }; + } +}; + +const report = ( + context: PackageJsonRuleContext, + node: + | JsonAST.JSONArrayExpression + | JsonAST.JSONObjectExpression + | JsonAST.JSONProperty, +) => { + const { data, messageId } = getDataAndMessageId(node); + context.report({ + data, + messageId, + 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); + } + }, + messageId: "remove", + }, + ], + }); +}; + +const getNode = ( + node: JsonAST.JSONArrayExpression | JsonAST.JSONObjectExpression, +) => { + return node.parent.type === "JSONProperty" ? node.parent : node; +}; + +export const rule = createRule({ + create(context) { + return { + JSONArrayExpression(node: JsonAST.JSONArrayExpression) { + if (!node.elements.length) { + report(context, getNode(node)); + } + }, + JSONObjectExpression(node: JsonAST.JSONObjectExpression) { + if (!node.properties.length) { + report(context, getNode(node)); + } + }, + }; + }, + meta: { + docs: { + category: "Best Practices", + description: "Reports on unnecessary empty arrays and objects.", + recommended: true, + }, + hasSuggestions: true, + messages: { + emptyExpression: + "This {{expressionType}} does nothing and can be removed.", + emptyFields: + "The field '{{field}}' does nothing and can be removed.", + remove: "Remove this empty field.", + }, + schema: [], + type: "suggestion", + }, +}); diff --git a/src/tests/rules/no-empty-fields.test.ts b/src/tests/rules/no-empty-fields.test.ts new file mode 100644 index 00000000..301942bc --- /dev/null +++ b/src/tests/rules/no-empty-fields.test.ts @@ -0,0 +1,307 @@ +import { rule } from "../../rules/no-empty-fields.js"; +import { ruleTester } from "./ruleTester.js"; + +ruleTester.run("no-empty-fields", rule, { + invalid: [ + { + code: `{ +\t\t"name": "test", +\t\t"files": [], +\t\t"dependencies": { +\t\t\t\t"@altano/repository-tools": "^0.1.1" +\t\t} +} +`, + errors: [ + { + messageId: "emptyFields", + suggestions: [ + { + messageId: "remove", + output: `{ +\t\t"name": "test", +\t\t +\t\t"dependencies": { +\t\t\t\t"@altano/repository-tools": "^0.1.1" +\t\t} +} +`, + }, + ], + }, + ], + }, + { + code: `{ +\t\t"name": "test", +\t\t"config": [ [], ["test"] ] +} +`, + errors: [ + { + messageId: "emptyExpression", + suggestions: [ + { + messageId: "remove", + output: `{ +\t\t"name": "test", +\t\t"config": [ ["test"] ] +} +`, + }, + ], + }, + ], + }, + { + code: `{ +\t\t"name": "test", +\t\t"config": [ {}, ["test"], [] ] +} +`, + errors: [ + { + messageId: "emptyExpression", + suggestions: [ + { + messageId: "remove", + output: `{ +\t\t"name": "test", +\t\t"config": [ ["test"], [] ] +} +`, + }, + ], + }, + { + messageId: "emptyExpression", + suggestions: [ + { + messageId: "remove", + output: `{ +\t\t"name": "test", +\t\t"config": [ {}, ["test"] ] +} +`, + }, + ], + }, + ], + }, + { + code: `{ +\t\t"name": "test", +\t\t"dependencies": {} +} +`, + errors: [ + { + messageId: "emptyFields", + suggestions: [ + { + messageId: "remove", + output: `{ +\t\t"name": "test" +\t\t +} +`, + }, + ], + }, + ], + }, + { + code: `{ +\t\t"name": "test", +\t\t"files": [ +\t\t\t\t"index.js" +\t\t], +\t\t"peerDependencies": {} +} +`, + errors: [ + { + messageId: "emptyFields", + suggestions: [ + { + messageId: "remove", + output: `{ +\t\t"name": "test", +\t\t"files": [ +\t\t\t\t"index.js" +\t\t] +\t\t +} +`, + }, + ], + }, + ], + }, + { + code: `{ +\t\t"name": "test", +\t\t"scripts": {} +} +`, + errors: [ + { + messageId: "emptyFields", + suggestions: [ + { + messageId: "remove", + output: `{ +\t\t"name": "test" +\t\t +} +`, + }, + ], + }, + ], + }, + { + code: `{ +\t\t"name": "test", +\t\t"devDependencies": {} +} +`, + errors: [ + { + messageId: "emptyFields", + suggestions: [ + { + messageId: "remove", + output: `{ +\t\t"name": "test" +\t\t +} +`, + }, + ], + }, + ], + }, + { + code: `{ +\t\t"name": "test", +\t\t"peerDependencyMeta": { +\t\t\t\t"@altano/repository-tools": { +\t\t\t\t\t\t"optional": true +\t\t\t\t}, +\t\t\t\t"nin": {} +\t\t} +} +`, + errors: [ + { + messageId: "emptyFields", + suggestions: [ + { + messageId: "remove", + output: `{ +\t\t"name": "test", +\t\t"peerDependencyMeta": { +\t\t\t\t"@altano/repository-tools": { +\t\t\t\t\t\t"optional": true +\t\t\t\t} +\t\t\t\t +\t\t} +} +`, + }, + ], + }, + ], + }, + { + code: `{ +\t\t"name": "test", +\t\t"peerDependencyMeta": { +\t\t\t\t"@altano/repository-tools": { +\t\t\t\t\t\t"optional": true +\t\t\t\t}, +\t\t\t\t"nin": [] +\t\t} +} +`, + errors: [ + { + messageId: "emptyFields", + suggestions: [ + { + messageId: "remove", + output: `{ +\t\t"name": "test", +\t\t"peerDependencyMeta": { +\t\t\t\t"@altano/repository-tools": { +\t\t\t\t\t\t"optional": true +\t\t\t\t} +\t\t\t\t +\t\t} +} +`, + }, + ], + }, + ], + }, + { + code: `{ +\t\t"name": "test", +\t\t"files": [], +\t\t"dependencies": { +\t\t\t\t"@altano/repository-tools": "^0.1.1", +\t\t\t\t"test": [] +\t\t} +} +`, + errors: [ + { + messageId: "emptyFields", + suggestions: [ + { + messageId: "remove", + output: `{ +\t\t"name": "test", +\t\t +\t\t"dependencies": { +\t\t\t\t"@altano/repository-tools": "^0.1.1", +\t\t\t\t"test": [] +\t\t} +} +`, + }, + ], + }, + { + messageId: "emptyFields", + suggestions: [ + { + messageId: "remove", + output: `{ +\t\t"name": "test", +\t\t"files": [], +\t\t"dependencies": { +\t\t\t\t"@altano/repository-tools": "^0.1.1" +\t\t\t\t +\t\t} +} +`, + }, + ], + }, + ], + }, + ], + valid: [ + `{ "name": "test", "files": ["index.js"] }`, + `{ "name": "test", "peerDependencies": { "eslint": ">=8.0.0" } }`, + `{ "name": "test", "dependencies": { "eslint": ">=8.0.0" } }`, + `{ "name": "test", "devDependencies": { "eslint": ">=8.0.0" } }`, + `{ "name": "test", "scripts": { "lint": "eslint --fix ." } }`, + `{ "name": "test", "peerDependencyMeta": { "@altano/repository-tools": { "optional": true } } }`, + `{ "name": "test", "peerDependencyMeta": { "@altano/repository-tools": { "optional": true, "test": [{"test": ["1"]}] } } }`, + `{ "name": "test", "peerDependencyMeta": { "@altano/repository-tools": { "optional": true, "test": ["field1"] } } }`, + ], +});