From 235b6020c337131890352c88a136908aa60af561 Mon Sep 17 00:00:00 2001 From: RobertAKARobin Date: Tue, 18 Jun 2024 12:59:10 -0500 Subject: [PATCH] feat: add attrs-newline rule and close #191 --- .cspell.json | 4 +- docs/rules.md | 1 + docs/rules/attrs-newline.md | 90 +++++ .../eslint-plugin/lib/configs/recommended.js | 1 + .../eslint-plugin/lib/rules/attrs-newline.js | 163 +++++++++ packages/eslint-plugin/lib/rules/index.js | 2 + .../tests/rules/attrs-newline.test.js | 322 ++++++++++++++++++ 7 files changed, 582 insertions(+), 1 deletion(-) create mode 100644 docs/rules/attrs-newline.md create mode 100644 packages/eslint-plugin/lib/rules/attrs-newline.js create mode 100644 packages/eslint-plugin/tests/rules/attrs-newline.test.js diff --git a/.cspell.json b/.cspell.json index 7695b0b0..3020ab5d 100644 --- a/.cspell.json +++ b/.cspell.json @@ -32,6 +32,8 @@ "roletype", "nextid", "screenreader", - "mspace" + "mspace", + "multiline", + "sameline" ] } diff --git a/docs/rules.md b/docs/rules.md index 50621534..aac5a14f 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -52,6 +52,7 @@ | Rule | Description | | | -------------------------------------------------------- | ----------------------------------------------------------------- | ---- | +| [attrs-newline](rules/attrs-newline) | Enforce newline between attributes | ⭐🔧 | | [element-newline](rules/element-newline) | Enforce newline between elements. | ⭐🔧 | | [id-naming-convention](rules/id-naming-convention) | Enforce consistent naming id attributes | | | [indent](rules/indent) | Enforce consistent indentation | ⭐🔧 | diff --git a/docs/rules/attrs-newline.md b/docs/rules/attrs-newline.md new file mode 100644 index 00000000..32364bff --- /dev/null +++ b/docs/rules/attrs-newline.md @@ -0,0 +1,90 @@ +# attrs-newline + +This rule enforces a newline between attributes, when more than a certain number of attributes is present. + +## How to use + +```js,.eslintrc.js +module.exports = { + rules: { + "@html-eslint/attrs-newline": "error", + }, +}; +``` + +## Rule Details + +Examples of **incorrect** code for this rule: + + +```html,incorrect +

+ +

+``` + +Examples of **correct** code for this rule: + +```html,correct +

+ +

+``` + +### Options + +This rule has an object option: + +```ts +//... +"@html-eslint/attrs-newline": ["error", { + "closeStyle": "sameline" | "newline", // Default `"newline"` + "ifAttrsMoreThan": number, // Default `2` +}] +``` + +#### ifAttrsMoreThan + +If there are more than this number of attributes, all attributes should be separated by newlines. Either they should _not_ be separated by newlines. + +The default is `2`. + +Examples of **correct** code for `"ifAttrsMoreThan": 2` + + +```html +

+ +

+``` + +#### closeStyle + +How the open tag's closing bracket `>` should be spaced: + +- `"newline"`: The closing bracket should be on a newline following the last attribute: + + ```html + + ``` + +- `"sameline"`: The closing bracket should be on the same line following the last attribute + + ```html + + ``` diff --git a/packages/eslint-plugin/lib/configs/recommended.js b/packages/eslint-plugin/lib/configs/recommended.js index 2d5753b6..9ef2ecd3 100644 --- a/packages/eslint-plugin/lib/configs/recommended.js +++ b/packages/eslint-plugin/lib/configs/recommended.js @@ -6,6 +6,7 @@ module.exports = { "@html-eslint/require-title": "error", "@html-eslint/no-multiple-h1": "error", "@html-eslint/no-extra-spacing-attrs": "error", + "@html-eslint/attrs-newline": "error", "@html-eslint/element-newline": "error", "@html-eslint/no-duplicate-id": "error", "@html-eslint/indent": "error", diff --git a/packages/eslint-plugin/lib/rules/attrs-newline.js b/packages/eslint-plugin/lib/rules/attrs-newline.js new file mode 100644 index 00000000..c0259b18 --- /dev/null +++ b/packages/eslint-plugin/lib/rules/attrs-newline.js @@ -0,0 +1,163 @@ +/** + * @typedef { import("../types").RuleFixer } RuleFixer + * @typedef { import("../types").RuleModule } RuleModule + * @typedef { import("../types").TagNode } TagNode + * @typedef {Object} MessageId + * @property {"closeStyleWrong"} CLOSE_STYLE_WRONG + * @property {"newlineMissing"} NEWLINE_MISSING + * @property {"newlineUnexpected"} NEWLINE_UNEXPECTED + */ + +const { RULE_CATEGORY } = require("../constants"); + +/** + * @type {MessageId} + */ + +const MESSAGE_ID = { + CLOSE_STYLE_WRONG: "closeStyleWrong", + NEWLINE_MISSING: "newlineMissing", + NEWLINE_UNEXPECTED: "newlineUnexpected", +}; + +/** + * @type {RuleModule} + */ +module.exports = { + meta: { + type: "code", + + docs: { + description: "Enforce newline between attributes", + category: RULE_CATEGORY.STYLE, + recommended: true, + }, + + fixable: true, + schema: [ + { + type: "object", + properties: { + closeStyle: { + enum: ["newline", "sameline"], + }, + ifAttrsMoreThan: { + type: "integer", + }, + }, + }, + ], + messages: { + [MESSAGE_ID.CLOSE_STYLE_WRONG]: + "Closing bracket was on {{actual}}; expected {{expected}}", + [MESSAGE_ID.NEWLINE_MISSING]: "Newline expected before {{attrName}}", + [MESSAGE_ID.NEWLINE_UNEXPECTED]: + "Newlines not expected between attributes, since this tag has fewer than {{attrMin}} attributes", + }, + }, + + create(context) { + const options = context.options[0] || {}; + const attrMin = isNaN(options.ifAttrsMoreThan) + ? 2 + : options.ifAttrsMoreThan; + const closeStyle = options.closeStyle || "newline"; + + return { + /** + * @param {TagNode} node + */ + Tag(node) { + const shouldBeMultiline = node.attributes.length > attrMin; + + /** + * This doesn't do any indentation, so the result will look silly. Indentation should be covered by the `indent` rule + * @param {RuleFixer} fixer + */ + function fix(fixer) { + const spacer = shouldBeMultiline ? "\n" : " "; + let expected = node.openStart.value; + for (const attr of node.attributes) { + expected += `${spacer}${attr.key.value}`; + if (attr.startWrapper && attr.value && attr.endWrapper) { + expected += `=${attr.startWrapper.value}${attr.value.value}${attr.endWrapper.value}`; + } + } + if (shouldBeMultiline && closeStyle === "newline") { + expected += "\n"; + } else if (node.selfClosing) { + expected += " "; + } + expected += node.openEnd.value; + + return fixer.replaceTextRange( + [node.openStart.range[0], node.openEnd.range[1]], + expected + ); + } + + if (shouldBeMultiline) { + let index = 0; + for (const attr of node.attributes) { + const attrPrevious = node.attributes[index - 1]; + const relativeToNode = attrPrevious || node.openStart; + if (attr.loc.start.line === relativeToNode.loc.end.line) { + return context.report({ + node, + data: { + attrName: attr.key.value, + }, + fix, + messageId: MESSAGE_ID.NEWLINE_MISSING, + }); + } + index += 1; + } + + const attrLast = node.attributes[node.attributes.length - 1]; + const closeStyleActual = + node.openEnd.loc.start.line === attrLast.loc.end.line + ? "sameline" + : "newline"; + if (closeStyle !== closeStyleActual) { + return context.report({ + node, + data: { + actual: closeStyleActual, + expected: closeStyle, + }, + fix, + messageId: MESSAGE_ID.CLOSE_STYLE_WRONG, + }); + } + } else { + let expectedLastLineNum = node.openStart.loc.start.line; + for (const attr of node.attributes) { + if (shouldBeMultiline) { + expectedLastLineNum += 1; + } + if (attr.value) { + const valueLineSpan = + attr.value.loc.end.line - attr.value.loc.start.line; + expectedLastLineNum += valueLineSpan; + } + } + if (shouldBeMultiline && closeStyle === "newline") { + expectedLastLineNum += 1; + } + + if (node.openEnd.loc.end.line !== expectedLastLineNum) { + return context.report({ + node, + data: { + attrMin, + }, + fix, + messageId: MESSAGE_ID.NEWLINE_UNEXPECTED, + }); + } + } + }, + }; + }, +}; diff --git a/packages/eslint-plugin/lib/rules/index.js b/packages/eslint-plugin/lib/rules/index.js index becc3c6c..cc40ac0c 100644 --- a/packages/eslint-plugin/lib/rules/index.js +++ b/packages/eslint-plugin/lib/rules/index.js @@ -6,6 +6,7 @@ const noDuplicateId = require("./no-duplicate-id"); const noInlineStyles = require("./no-inline-styles"); const noMultipleH1 = require("./no-multiple-h1"); const noExtraSpacingAttrs = require("./no-extra-spacing-attrs"); +const attrsNewline = require("./attrs-newline"); const elementNewLine = require("./element-newline"); const noSkipHeadingLevels = require("./no-skip-heading-levels"); const indent = require("./indent"); @@ -45,6 +46,7 @@ module.exports = { "no-inline-styles": noInlineStyles, "no-multiple-h1": noMultipleH1, "no-extra-spacing-attrs": noExtraSpacingAttrs, + "attrs-newline": attrsNewline, "element-newline": elementNewLine, "no-skip-heading-levels": noSkipHeadingLevels, "require-li-container": requireLiContainer, diff --git a/packages/eslint-plugin/tests/rules/attrs-newline.test.js b/packages/eslint-plugin/tests/rules/attrs-newline.test.js new file mode 100644 index 00000000..dc0a0333 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/attrs-newline.test.js @@ -0,0 +1,322 @@ +const createRuleTester = require("../rule-tester"); +const rule = require("../../lib/rules/attrs-newline"); + +const ruleTester = createRuleTester(); + +const closeStyles = ["sameline", "newline"]; + +ruleTester.run("attrs-newline", rule, { + valid: [ + ...closeStyles.map((closeStyle) => ({ + code: ` +

`, + options: [ + { + closeStyle, + ifAttrsMoreThan: 0, + }, + ], + })), + ...closeStyles.map((closeStyle) => ({ + code: ` +

+ +

`, + options: [ + { + closeStyle, + ifAttrsMoreThan: 0, + }, + ], + })), + { + code: ` +

+ +

`, + options: [ + { + closeStyle: "sameline", + ifAttrsMoreThan: 0, + }, + ], + }, + { + code: ` +

+ +

`, + options: [ + { + closeStyle: "newline", + ifAttrsMoreThan: 0, + }, + ], + }, + ...closeStyles.map((closeStyle) => ({ + code: ` +

+ +

`, + options: [ + { + closeStyle, + ifAttrsMoreThan: 1, + }, + ], + })), + ...closeStyles.map((closeStyle) => ({ + code: ` +

+ +

`, + options: [ + { + closeStyle, + ifAttrsMoreThan: 1, + }, + ], + })), + { + code: ` +

+

`, + options: [ + { + closeStyle: "sameline", + ifAttrsMoreThan: 1, + }, + ], + }, + { + code: ` +

+ +

`, + options: [ + { + closeStyle: "sameline", + ifAttrsMoreThan: 1, + }, + ], + }, + { + code: ` +

+ +

`, + options: [ + { + closeStyle: "newline", + ifAttrsMoreThan: 1, + }, + ], + }, + { + code: ` +

+ +

`, + options: [ + { + closeStyle: "sameline", + ifAttrsMoreThan: 1, + }, + ], + }, + { + code: ` +

+ +

`, + options: [ + { + closeStyle: "newline", + ifAttrsMoreThan: 1, + }, + ], + }, + { + code: ` +

+ +

`, + options: [ + { + closeStyle: "sameline", + ifAttrsMoreThan: 1, + }, + ], + }, + { + code: ` +

+ +

`, + options: [ + { + closeStyle: "newline", + ifAttrsMoreThan: 1, + }, + ], + }, + ], + + invalid: [ + { + code: ` +

+ +

`, + options: [ + { + closeStyle: "sameline", + ifAttrsMoreThan: 0, + }, + ], + output: ` +

+ +

`, + errors: [{ messageId: "newlineMissing" }], + }, + { + code: ` +

+ +

`, + options: [ + { + closeStyle: "newline", + ifAttrsMoreThan: 0, + }, + ], + output: ` +

+ +

`, + errors: [{ messageId: "closeStyleWrong" }], + }, + { + code: ` +

+ +

`, + options: [ + { + closeStyle: "newline", + ifAttrsMoreThan: 1, + }, + ], + output: ` +

+ +

`, + errors: [{ messageId: "newlineMissing" }], + }, + ...closeStyles.map((closeStyle) => ({ + code: ` +

+ +

`, + options: [ + { + closeStyle, + ifAttrsMoreThan: 1, + }, + ], + output: ` +

+ +

`, + errors: [{ messageId: "newlineUnexpected" }], + })), + ...closeStyles.map((closeStyle) => ({ + code: ` +

+ +

`, + options: [ + { + closeStyle, + ifAttrsMoreThan: 2, + }, + ], + output: ` +

+ +

`, + errors: [{ messageId: "newlineUnexpected" }], + })), + ], +});