diff --git a/configs/recommended.js b/configs/recommended.js index 7419e65ba4..47ff8200be 100644 --- a/configs/recommended.js +++ b/configs/recommended.js @@ -96,6 +96,7 @@ module.exports = { 'unicorn/require-number-to-fixed-digits-argument': 'error', 'unicorn/require-post-message-target-origin': 'error', 'unicorn/string-content': 'off', + 'unicorn/template-indent': 'warn', 'unicorn/throw-new-error': 'error', ...require('./conflicting-rules.js').rules, }, diff --git a/docs/rules/template-indent.md b/docs/rules/template-indent.md new file mode 100644 index 0000000000..4eaab6df00 --- /dev/null +++ b/docs/rules/template-indent.md @@ -0,0 +1,134 @@ +# Fix whitespace-insensitive template indentation + +[Tagged templates](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals#tagged_templates) often look ugly/jarring because their indentation doesn't match the code they're found in. In many cases, whitespace is insignificant, or a library like [strip-indent](https://www.npmjs.com/package/strip-indent) is used to remove the margin. See [proposal-string-dedent](https://github.com/tc39/proposal-string-dedent) (stage 1 at the time of writing) for a proposal on fixing this in JavaScript. + +This rule will automatically fix the indentation of multiline string templates, to keep them in alignment with the code they are found in. A configurable whitelist is used to ensure no whitespace-sensitive strings are edited. + +## Fail + +```js +function foo() { + const sqlQuery = sql` +select * +from students +where first_name = ${x} +and last_name = ${y} + `; + + const gqlQuery = gql` + query user(id: 5) { + firstName + lastName + } + `; + + const html = /* HTML */ ` +
+ hello +
+ `; +} +``` + +## Pass + +The above will auto-fix to: + +```js +function foo() { + const sqlQuery = sql` + select * + from students + where first_name = ${x} + and last_name = ${y} + `; + + const gqlQuery = gql` + query user(id: 5) { + firstName + lastName + } + `; + + const html = /* HTML */ ` +
+ hello +
+ `; +} +``` + +Under the hood, [strip-indent](https://npmjs.com/package/strip-indent) is used to determine how the template "should" look. Then a common indent is added to each line based on the margin of the line the template started at. This rule will *not* alter the relative whitespace between significant lines, it will only shift the content right or left so that it aligns sensibly with the surrounding code. + +## Options + +The rule accepts lists of `tags`, `functions`, `selectors` and `comments` to match template literals. `tags` are tagged template literal identifiers, functions are names of utility functions like `stripIndent`, selectors can be any [ESLint selector](https://eslint.org/docs/developer-guide/selectors), and comments are `/* block-commented */` strings. + +Default configuration: + +```js +{ + 'unicorn/template-indent': [ + 'warn', + { + tags: [ + 'outdent', + 'dedent', + 'gql', + 'sql', + 'html', + 'styled' + ], + functions: [ + 'dedent', + 'stripIndent' + ], + selectors: [], + comments: [ + 'HTML', + 'indent' + ] + } + ] +} +``` + +You can use a selector for custom use-cases, like indenting *all* template literals, even those without template tags or function callers: + +```js +{ + 'unicorn/template-indent': [ + 'warn', + { + tags: [], + functions: [], + selectors: [ + 'TemplateLiteral' + ] + } + ] +} +``` + +Indentation will be done with tabs or spaces depending on the line of code that the template literal starts at. You can override this by supplying an `indent`, which should be either a number (of spaces) or a string consisting only of whitespace characters: + +```js +{ + 'unicorn/template-indent': [ + 'warn', { + indent: 8, + } + ] +} +``` + +```js +{ + 'unicorn/template-indent': [ + 'warn', + { + indent: '\t\t' + } + ] +} +``` diff --git a/package.json b/package.json index 2618857604..68dc99f4cf 100644 --- a/package.json +++ b/package.json @@ -43,13 +43,16 @@ "clean-regexp": "^1.0.0", "eslint-template-visitor": "^2.3.2", "eslint-utils": "^3.0.0", + "esquery": "^1.4.0", + "indent-string": "4", "is-builtin-module": "^3.1.0", "lodash": "^4.17.21", "pluralize": "^8.0.0", "read-pkg-up": "^7.0.1", "regexp-tree": "^0.1.23", "safe-regex": "^2.1.1", - "semver": "^7.3.5" + "semver": "^7.3.5", + "strip-indent": "^3.0.0" }, "devDependencies": { "@babel/code-frame": "^7.14.5", diff --git a/readme.md b/readme.md index 500c7bbb52..9069dea413 100644 --- a/readme.md +++ b/readme.md @@ -125,6 +125,7 @@ Configure it in `package.json`. "unicorn/require-number-to-fixed-digits-argument": "error", "unicorn/require-post-message-target-origin": "error", "unicorn/string-content": "off", + "unicorn/template-indent": "warn", "unicorn/throw-new-error": "error" }, "overrides": [ @@ -245,6 +246,7 @@ Each rule has emojis denoting: | [require-number-to-fixed-digits-argument](docs/rules/require-number-to-fixed-digits-argument.md) | Enforce using the digits argument with `Number#toFixed()`. | ✅ | 🔧 | | | [require-post-message-target-origin](docs/rules/require-post-message-target-origin.md) | Enforce using the `targetOrigin` argument with `window.postMessage()`. | ✅ | | 💡 | | [string-content](docs/rules/string-content.md) | Enforce better string content. | | 🔧 | 💡 | +| [template-indent](docs/rules/template-indent.md) | Fix whitespace-insensitive template indentation. | | 🔧 | | | [throw-new-error](docs/rules/throw-new-error.md) | Require `new` when throwing an error. | ✅ | 🔧 | | diff --git a/rules/template-indent.js b/rules/template-indent.js new file mode 100644 index 0000000000..726d263d19 --- /dev/null +++ b/rules/template-indent.js @@ -0,0 +1,164 @@ +'use strict'; +const stripIndent = require('strip-indent'); +const indentString = require('indent-string'); +const esquery = require('esquery'); +const {replaceTemplateElement} = require('./fix/index.js'); + +const MESSAGE_ID_IMPROPERLY_INDENTED_TEMPLATE = 'template-indent'; +const messages = { + [MESSAGE_ID_IMPROPERLY_INDENTED_TEMPLATE]: 'Templates should be properly indented.', +}; + +/** @param {import('eslint').Rule.RuleContext} context */ +const create = context => { + const sourceCode = context.getSourceCode(); + const options = { + tags: ['outdent', 'dedent', 'gql', 'sql', 'html', 'styled'], + functions: ['dedent', 'stripIndent'], + selectors: [], + comments: ['HTML', 'indent'], + ...context.options[0], + }; + + options.comments = options.comments.map(comment => comment.toLowerCase()); + + const selectors = [ + ...options.tags.map(tag => `TaggedTemplateExpression[tag.name="${tag}"] > .quasi`), + ...options.functions.map(fn => `CallExpression[callee.name="${fn}"] > .arguments`), + ...options.selectors, + ]; + + /** @param {import('@babel/core').types.TemplateLiteral} node */ + const indentTemplateLiteralNode = node => { + const delimiter = '__PLACEHOLDER__' + Math.random(); + const joined = node.quasis + .map(quasi => { + const untrimmedText = sourceCode.getText(quasi); + return untrimmedText.slice(1, quasi.tail ? -1 : -2); + }) + .join(delimiter); + + const eolMatch = joined.match(/\r?\n/); + if (!eolMatch) { + return; + } + + const eol = eolMatch[0]; + + const startLine = sourceCode.lines[node.loc.start.line - 1]; + const marginMatch = startLine.match(/^(\s*)\S/); + const parentMargin = marginMatch ? marginMatch[1] : ''; + + let indent; + if (typeof options.indent === 'string') { + indent = options.indent; + } else if (typeof options.indent === 'number') { + indent = ' '.repeat(options.indent); + } else { + const tabs = parentMargin.startsWith('\t'); + indent = tabs ? '\t' : ' '; + } + + const dedented = stripIndent(joined); + const fixed + = eol + + indentString(dedented.trim(), 1, {indent: parentMargin + indent}) + + eol + + parentMargin; + + if (fixed === joined) { + return; + } + + context.report({ + node, + messageId: MESSAGE_ID_IMPROPERLY_INDENTED_TEMPLATE, + fix: fixer => fixed + .split(delimiter) + .map((replacement, index) => replaceTemplateElement(fixer, node.quasis[index], replacement)), + }); + }; + + return { + /** @param {import('@babel/core').types.TemplateLiteral} node */ + TemplateLiteral: node => { + if (options.comments.length > 0) { + const previousToken = sourceCode.getTokenBefore(node, {includeComments: true}); + if (previousToken && previousToken.type === 'Block' && options.comments.includes(previousToken.value.trim().toLowerCase())) { + indentTemplateLiteralNode(node); + return; + } + } + + const ancestry = context.getAncestors().reverse(); + const shouldIndent = selectors.some(selector => esquery.matches(node, esquery.parse(selector), ancestry)); + + if (shouldIndent) { + indentTemplateLiteralNode(node); + } + }, + }; +}; + +/** @type {import('json-schema').JSONSchema7[]} */ +const schema = [ + { + type: 'object', + properties: { + indent: { + oneOf: [ + { + type: 'string', + pattern: /^\s+$/.source, + }, + { + type: 'integer', + minimum: 1, + }, + ], + }, + tags: { + type: 'array', + uniqueItems: true, + items: { + type: 'string', + }, + }, + functions: { + type: 'array', + uniqueItems: true, + items: { + type: 'string', + }, + }, + selectors: { + type: 'array', + uniqueItems: true, + items: { + type: 'string', + }, + }, + comments: { + type: 'array', + uniqueItems: true, + items: { + type: 'string', + }, + }, + }, + additionalProperties: false, + }, +]; + +module.exports = { + create, + meta: { + type: 'suggestion', + docs: { + description: 'Fix whitespace-insensitive template indentation.', + }, + fixable: 'code', + schema, + messages, + }, +}; diff --git a/test/consistent-function-scoping.mjs b/test/consistent-function-scoping.mjs index a8b159dbdc..cc089e2fb7 100644 --- a/test/consistent-function-scoping.mjs +++ b/test/consistent-function-scoping.mjs @@ -218,12 +218,12 @@ test({ `, // Functions that could be extracted are conservatively ignored due to JSX masking references outdent` - function Foo() { - function Bar () { - return
- } - return
{ Bar() }
+ function Foo() { + function Bar () { + return
} + return
{ Bar() }
+ } `, outdent` function foo() { diff --git a/test/prefer-keyboard-event-key.mjs b/test/prefer-keyboard-event-key.mjs index 5b81573b32..440cac3947 100644 --- a/test/prefer-keyboard-event-key.mjs +++ b/test/prefer-keyboard-event-key.mjs @@ -509,37 +509,37 @@ test({ }, { code: outdent` - const e = {} - foo.addEventListener('click', (e, r, fg) => { - function a() { - if (true) { - { + const e = {} + foo.addEventListener('click', (e, r, fg) => { + function a() { + if (true) { { - const { charCode } = e; - console.log(e.keyCode, charCode); + { + const { charCode } = e; + console.log(e.keyCode, charCode); + } } } } - } - }); + }); `, errors: [error('charCode'), error('keyCode')], }, { code: outdent` - const e = {} - foo.addEventListener('click', (e, r, fg) => { - function a() { - if (true) { - { + const e = {} + foo.addEventListener('click', (e, r, fg) => { + function a() { + if (true) { { - const { charCode } = e; - console.log(e.keyCode, charCode); + { + const { charCode } = e; + console.log(e.keyCode, charCode); + } } } } - } - }); + }); `, errors: [error('charCode'), error('keyCode')], }, diff --git a/test/prefer-modern-dom-apis.mjs b/test/prefer-modern-dom-apis.mjs index 07369c627b..79612d616a 100644 --- a/test/prefer-modern-dom-apis.mjs +++ b/test/prefer-modern-dom-apis.mjs @@ -72,10 +72,10 @@ test({ }, { code: outdent` - parentNode.replaceChild( - newChildNode, - oldChildNode - ); + parentNode.replaceChild( + newChildNode, + oldChildNode + ); `, errors: [ { @@ -87,10 +87,10 @@ test({ }, { code: outdent` - parentNode.replaceChild( // inline comments - newChildNode, // inline comments - oldChildNode // inline comments - ); + parentNode.replaceChild( // inline comments + newChildNode, // inline comments + oldChildNode // inline comments + ); `, errors: [ { @@ -280,10 +280,10 @@ test({ }, { code: outdent` - referenceNode.insertAdjacentElement( - "afterend", - newNode - ); + referenceNode.insertAdjacentElement( + "afterend", + newNode + ); `, errors: [ { @@ -295,10 +295,10 @@ test({ }, { code: outdent` - referenceNode.insertAdjacentElement( // inline comments - "afterend", // inline comments - newNode // inline comments - ); // inline comments + referenceNode.insertAdjacentElement( // inline comments + "afterend", // inline comments + newNode // inline comments + ); // inline comments `, errors: [ { diff --git a/test/template-indent.mjs b/test/template-indent.mjs new file mode 100644 index 0000000000..cd6073482f --- /dev/null +++ b/test/template-indent.mjs @@ -0,0 +1,589 @@ +import stripIndent from 'strip-indent'; +import {getTester} from './utils/test.mjs'; + +/** + * The interesting things to test for this rule are whitespace and multiline templates. Both of those are _very_ hard to see in a + * normal text editor, so replace spaces with •, and tabs with →→. + * + * @param {string} text + */ +const fixInput = text => stripIndent(text) + .replace(/•/g, ' ') + .replace(/→→/g, '\t'); + +const {test} = getTester(import.meta); + +const errors = [ + { + messageId: 'template-indent', + }, +]; + +test({ + /** @type {import('eslint').RuleTester.InvalidTestCase[]} */ + invalid: [ + { + code: fixInput(` + foo = dedent\` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\` + `), + errors, + output: fixInput(` + foo = dedent\` + ••one + ••two + ••••three + \` + `), + }, + { + code: ['dedent`', 'one', 'two', '`'].join('\r\n'), + errors, + output: ['dedent`', ' one', ' two', '`'].join('\r\n'), + }, + { + options: [{ + tags: ['customIndentableTag'], + }], + code: fixInput(` + foo = customIndentableTag\` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\` + foo = differentTagThatMightBeWhitespaceSensitive\` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\` + foo = \` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\` + `), + errors, + output: fixInput(` + foo = customIndentableTag\` + ••one + ••two + ••••three + \` + foo = differentTagThatMightBeWhitespaceSensitive\` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\` + foo = \` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\` + `), + }, + { + options: [{ + tags: ['customIndentableTag'], + selectors: [':not(TaggedTemplateExpression) > TemplateLiteral'], + }], + code: fixInput(` + foo = customIndentableTag\` + ••••••••one1 + ••••••••two1 + ••••••••••three1 + ••••••••\` + foo = differentTagThatMightBeWhitespaceSensitive\` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\` + foo = \` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\` + `), + errors: [...errors, ...errors], + output: fixInput(` + foo = customIndentableTag\` + ••one1 + ••two1 + ••••three1 + \` + foo = differentTagThatMightBeWhitespaceSensitive\` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\` + foo = \` + ••one + ••two + ••••three + \` + `), + }, + { + code: fixInput(` + function foo() { + ••return dedent\` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\` + } + `), + errors, + output: fixInput(` + function foo() { + ••return dedent\` + ••••one + ••••two + ••••••three + ••\` + } + `), + }, + { + code: fixInput(` + // a + // bb + // ccc + // dddd + function foo() { + ••return dedent\` + ••••••••one + ••••••••two + ••••••••••three \${3} four + ••••••••••••five + ••••••••••••••\${{f: 5}} + ••••••••••••six + ••••••••\` + } + `), + errors, + output: fixInput(` + // a + // bb + // ccc + // dddd + function foo() { + ••return dedent\` + ••••one + ••••two + ••••••three \${3} four + ••••••••five + ••••••••••\${{f: 5}} + ••••••••six + ••\` + } + `), + }, + { + code: fixInput(` + foo = gql\` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\` + foo = sql\` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\` + foo = dedent\` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\` + foo = outdent\` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\` + foo = somethingElse\` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\` + `), + errors: [...errors, ...errors, ...errors, ...errors], + output: fixInput(` + foo = gql\` + ••one + ••two + ••••three + \` + foo = sql\` + ••one + ••two + ••••three + \` + foo = dedent\` + ••one + ••two + ••••three + \` + foo = outdent\` + ••one + ••two + ••••three + \` + foo = somethingElse\` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\` + `), + }, + { + code: fixInput(` + foo = stripIndent(\` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\`) + `), + errors, + output: fixInput(` + foo = stripIndent(\` + ••one + ••two + ••••three + \`) + `), + }, + { + code: fixInput(` + html = /* HTML */ \` + ••••••••
+ ••••••••••hello + ••••••••
+ ••••••••\` + `), + errors, + output: fixInput(` + html = /* HTML */ \` + ••
+ ••••hello + ••
+ \` + `), + }, + { + code: fixInput(` + html = /* html */ \` + ••••••••
+ ••••••••••hello + ••••••••
+ ••••••••\` + `), + errors, + output: fixInput(` + html = /* html */ \` + ••
+ ••••hello + ••
+ \` + `), + }, + { + code: fixInput(` + html = /* indent */ \` + ••••••••
+ ••••••••••hello + ••••••••
+ ••••••••\` + `), + errors, + output: fixInput(` + html = /* indent */ \` + ••
+ ••••hello + ••
+ \` + `), + }, + { + options: [{ + comments: ['please indent me!'], + }], + code: fixInput(` + html = /* please indent me! */ \` + ••••••••
+ ••••••••••hello + ••••••••
+ ••••••••\` + `), + errors, + output: fixInput(` + html = /* please indent me! */ \` + ••
+ ••••hello + ••
+ \` + `), + }, + { + options: [{ + indent: 10, + }], + code: fixInput(` + foo = dedent\` + ••one + ••two + ••••three + \` + `), + errors, + output: fixInput(` + foo = dedent\` + ••••••••••one + ••••••••••two + ••••••••••••three + \` + `), + }, + { + options: [{ + indent: '\t\t\t\t', + }], + code: fixInput(` + foo = dedent\` + ••one + ••two + ••••three + \` + `), + errors, + output: fixInput(` + foo = dedent\` + →→→→→→→→one + →→→→→→→→two + →→→→→→→→••three + \` + `), + }, + { + options: [{ + selectors: ['* TemplateLiteral', '* > TemplateLiteral'], + }], + code: fixInput(` + foo = \` + one + two + ••three + \` + `), + // Make sure we only report one error, even when multiple selectors match + errors, + output: fixInput(` + foo = \` + ••one + ••two + ••••three + \` + `), + }, + { + options: [{ + selectors: ['* > TemplateLiteral'], + comments: ['indentme'], + }], + code: fixInput(` + foo = /* INDENTME */ \` + one + two + ••three + \` + `), + // Make sure we only report one error, even when multiple selectors match + errors, + output: fixInput(` + foo = /* INDENTME */ \` + ••one + ••two + ••••three + \` + `), + }, + { + options: [{ + functions: ['customDedentFunction'], + }], + code: fixInput(` + foo = customDedentFunction(\` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\`) + foo = customDedentFunction('some-other-arg', \` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\`) + `), + errors: [...errors, ...errors], + output: fixInput(` + foo = customDedentFunction(\` + ••one + ••two + ••••three + \`) + foo = customDedentFunction('some-other-arg', \` + ••one + ••two + ••••three + \`) + `), + }, + { + code: fixInput(` + outdent\` + before + before\${ + expression + }after + after + \` + `), + errors, + output: fixInput(` + outdent\` + ••before + ••before\${ + expression + }after + ••after + \` + `), + }, + { + code: fixInput(` + outdent\` + ••before + ••before\${ + →→→→→→outdent\` + inner + →→→→→→\` + }after + ••after + \` + `), + errors, + output: fixInput(` + outdent\` + ••before + ••before\${ + →→→→→→outdent\` + →→→→→→→→inner + →→→→→→\` + }after + ••after + \` + `), + }, + ], + /** @type {import('eslint').RuleTester.ValidTestCase[]} */ + valid: [ + 'foo = dedent`one two three`', + fixInput(` + function f() { + →→foo = dedent\` + →→→→one + →→→→two + →→→→→→three + →→→→four + →→\` + } + `), + fixInput(` + function f() { + →→foo = dedent\` + →→→→one + + →→→→two + →→→→→→three + →→→→four + →→\` + } + `), + fixInput(` + function f() { + ••foo = dedent\` + ••••one + ••••two + ••••••three + ••••four + ••\` + } + `), + { + options: [{ + tags: ['somethingOtherThanDedent'], + functions: ['somethingOtherThanStripIndent'], + }], + code: fixInput(` + foo = stripIndent(\` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\`) + foo = dedent\` + ••••••••one + ••••••••two + ••••••••••three + ••••••••\` + `), + }, + 'stripIndent(foo)', + { + options: [{ + selectors: ['TemplateElement'], + }], + // Bad selector; no template literal match + code: fixInput(` + foo = \` + ••••••one + ••••••two + ••••••••three + \` + `), + }, + '``', + { + options: [{ + comments: [], + }], + code: fixInput(` + foo = /* indent */ \` + ••••••one + ••••••two + ••••••••three + \` + `), + }, + fixInput(` + outdent\` + ••before + ••before\${ + expression + }after + ••after + \` + `), + fixInput(` + outdent\` + ••before + ••before\${ + ••••••normalTemplate\` + inner + ••••••\` + }after + ••after + \` + `), + ], +});