From 5d25bd3d1bac1a02c4b5c4c97a11607e971d32b3 Mon Sep 17 00:00:00 2001 From: Jakub Freisler Date: Mon, 16 Dec 2024 03:35:52 +0100 Subject: [PATCH] feat: add `prefer-class-fields` rule --- docs/rules/prefer-class-fields.md | 41 ++++++ readme.md | 1 + rules/prefer-class-fields.js | 111 +++++++++++++++ test/prefer-class-fields.mjs | 86 +++++++++++ test/snapshots/prefer-class-fields.mjs.md | 149 ++++++++++++++++++++ test/snapshots/prefer-class-fields.mjs.snap | Bin 0 -> 642 bytes 6 files changed, 388 insertions(+) create mode 100644 docs/rules/prefer-class-fields.md create mode 100644 rules/prefer-class-fields.js create mode 100644 test/prefer-class-fields.mjs create mode 100644 test/snapshots/prefer-class-fields.mjs.md create mode 100644 test/snapshots/prefer-class-fields.mjs.snap diff --git a/docs/rules/prefer-class-fields.md b/docs/rules/prefer-class-fields.md new file mode 100644 index 0000000000..e14478eddd --- /dev/null +++ b/docs/rules/prefer-class-fields.md @@ -0,0 +1,41 @@ +# Prefer class field declarations over assigning static values in constructor using `this` + +💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs-eslintconfigjs). + +🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix). + + + + +This rule will enforce the use of field declarations over `this` assignments in contructors for static values. + +> To avoid leaving empty constructors after autofix use [`no-useless-contructor` rule](https://eslint.org/docs/latest/rules/no-useless-constructor). + +## Fail + +```js +class Foo { + constructor() { + this.foo = 'foo'; + } +} + +class MyError extends Error { + constructor(message: string) { + super(message); + this.name = "MyError"; + } +} +``` + +## Pass + +```js +class Foo { + foo = 'foo'; +} + +class MyError extends Error { + name = "MyError" +} +``` diff --git a/readme.md b/readme.md index b1f5d022c3..9147209d7a 100644 --- a/readme.md +++ b/readme.md @@ -181,6 +181,7 @@ If you don't use the preset, ensure you use the same `env` and `parserOptions` c | [prefer-array-some](docs/rules/prefer-array-some.md) | Prefer `.some(…)` over `.filter(…).length` check and `.{find,findLast,findIndex,findLastIndex}(…)`. | ✅ | 🔧 | 💡 | | [prefer-at](docs/rules/prefer-at.md) | Prefer `.at()` method for index access and `String#charAt()`. | ✅ | 🔧 | 💡 | | [prefer-blob-reading-methods](docs/rules/prefer-blob-reading-methods.md) | Prefer `Blob#arrayBuffer()` over `FileReader#readAsArrayBuffer(…)` and `Blob#text()` over `FileReader#readAsText(…)`. | ✅ | | | +| [prefer-class-fields](docs/rules/prefer-class-fields.md) | Prefer class field declarations over assigning static values in constructor using `this`. | ✅ | 🔧 | | | [prefer-code-point](docs/rules/prefer-code-point.md) | Prefer `String#codePointAt(…)` over `String#charCodeAt(…)` and `String.fromCodePoint(…)` over `String.fromCharCode(…)`. | ✅ | | 💡 | | [prefer-date-now](docs/rules/prefer-date-now.md) | Prefer `Date.now()` to get the number of milliseconds since the Unix Epoch. | ✅ | 🔧 | | | [prefer-default-parameters](docs/rules/prefer-default-parameters.md) | Prefer default parameters over reassignment. | ✅ | 🔧 | 💡 | diff --git a/rules/prefer-class-fields.js b/rules/prefer-class-fields.js new file mode 100644 index 0000000000..bcaae2f9b9 --- /dev/null +++ b/rules/prefer-class-fields.js @@ -0,0 +1,111 @@ +'use strict'; +const getIndentString = require('./utils/get-indent-string.js'); + +const MESSAGE_ID = 'prefer-class-fields/error'; +const messages = { + [MESSAGE_ID]: + 'Prefer class field declaration over `this` assignment in constructor for static values.', +}; + +/** + * @param {import('eslint').Rule.Node} node + * @returns {node is import('estree').ExpressionStatement & {expression: import('estree').AssignmentExpression & {left: import('estree').MemberExpression & {object: import('estree').ThisExpression}}}} + */ +const isThisAssignmentExpression = node => { + if ( + node.type !== 'ExpressionStatement' + || node.expression.type !== 'AssignmentExpression' + ) { + return false; + } + + const lhs = node.expression.left; + + if (!lhs.object || lhs.object.type !== 'ThisExpression') { + return false; + } + + return true; +}; + +/** + * @template Array + * @param {Array} array + * @returns {Array} + */ +const reverseArray = array => [...array].reverse(); + +/** + * @param {import('eslint').Rule.Node} node + * @param {import('eslint').Rule.RuleContext['sourceCode']} sourceCode + * @param {import('eslint').Rule.RuleFixer} fixer + */ +const removeThisFieldAssignment = (node, sourceCode, fixer) => { + const {line} = node.loc.start; + const nodeText = sourceCode.getText(node); + const lineText = sourceCode.lines[line - 1]; + const isOnlyNodeOnLine = lineText.trim() === nodeText; + + return isOnlyNodeOnLine + ? fixer.removeRange([ + sourceCode.getIndexFromLoc({line, column: 0}), + sourceCode.getIndexFromLoc({line: line + 1, column: 0}), + ]) + : fixer.remove(node); +}; + +/** @type {import('eslint').Rule.RuleModule['create']} */ +const create = context => { + const {sourceCode} = context; + + return { + ClassBody(node) { + const constructor = node.body.find(x => x.kind === 'constructor'); + + if (!constructor || constructor.type !== 'MethodDefinition') { + return; + } + + const constructorBody = constructor.value.body.body; + const classBodyStartRange = [node.range[0], node.range[0] + 1]; + const indent = getIndentString(constructor, sourceCode); + + for (const node of reverseArray(constructorBody)) { + if ( + isThisAssignmentExpression(node) + && node.expression.right?.type === 'Literal' + ) { + return { + node, + messageId: MESSAGE_ID, + + /** @param {import('eslint').Rule.RuleFixer} fixer */ + * fix(fixer) { + yield removeThisFieldAssignment(node, sourceCode, fixer); + yield fixer.insertTextAfterRange( + classBodyStartRange, + `\n${indent}${node.expression.left.property.name} = ${node.expression.right.raw};`, + ); + }, + }; + } + } + }, + }; +}; + +/** @type {import('eslint').Rule.RuleModule} */ +module.exports = { + create, + meta: { + type: 'suggestion', + docs: { + description: + 'Prefer class field declarations over assigning static values in constructor using `this`.', + recommended: true, + }, + fixable: 'code', + hasSuggestions: false, + messages, + }, +}; diff --git a/test/prefer-class-fields.mjs b/test/prefer-class-fields.mjs new file mode 100644 index 0000000000..ef827cd1ff --- /dev/null +++ b/test/prefer-class-fields.mjs @@ -0,0 +1,86 @@ +import outdent from 'outdent'; +import {getTester} from './utils/test.mjs'; + +const {test} = getTester(import.meta); + +const MESSAGE_ID = 'prefer-class-fields/error'; + +test.snapshot({ + valid: [ + outdent` + class Foo { + foo = 'foo'; + } + `, + outdent` + class MyError extends Error { + name = "MyError"; + } + `, + ], + invalid: [ + outdent` + class Foo { + constructor() { + this.foo = 'foo'; + } + } + `, + outdent` + class Foo { + constructor() { + this.foo = 'foo'; + this.foo2 = 'foo2'; + } + } + `, + outdent` + class Foo { + constructor(argument) { + this.foo = 'foo'; + this.foo2 = argument + 'test'; + this.foo3 = 'foo3'; + } + } + `, + outdent` + class MyError extends Error { + constructor(message) { + super(message); + this.name = "MyError"; + } + } + `, + ], +}); + +test.typescript({ + valid: [ + outdent` + class Foo { + foo: string = 'foo'; + } + `, + ], + invalid: [ + { + code: outdent` + class MyError extends Error { + constructor(message: string) { + super(message); + this.name = "MyError"; + } + } + `, + errors: [{messageId: MESSAGE_ID}], + output: outdent` + class MyError extends Error { + name = "MyError"; + constructor(message: string) { + super(message); + } + } + `, + }, + ], +}); diff --git a/test/snapshots/prefer-class-fields.mjs.md b/test/snapshots/prefer-class-fields.mjs.md new file mode 100644 index 0000000000..39fde7e9e7 --- /dev/null +++ b/test/snapshots/prefer-class-fields.mjs.md @@ -0,0 +1,149 @@ +# Snapshot report for `test/prefer-class-fields.mjs` + +The actual snapshot is saved in `prefer-class-fields.mjs.snap`. + +Generated by [AVA](https://avajs.dev). + +## invalid(1): class Foo { constructor() { this.foo = 'foo'; } } + +> Input + + `␊ + 1 | class Foo {␊ + 2 | constructor() {␊ + 3 | this.foo = 'foo';␊ + 4 | }␊ + 5 | }␊ + ` + +> Output + + `␊ + 1 | class Foo {␊ + 2 | foo = 'foo';␊ + 3 | constructor() {␊ + 4 | }␊ + 5 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | class Foo {␊ + 2 | constructor() {␊ + > 3 | this.foo = 'foo';␊ + | ^^^^^^^^^^^^^^^^^ Prefer class field declaration over \`this\` assignment in constructor for static values.␊ + 4 | }␊ + 5 | }␊ + ` + +## invalid(2): class Foo { constructor() { this.foo = 'foo'; this.foo2 = 'foo2'; } } + +> Input + + `␊ + 1 | class Foo {␊ + 2 | constructor() {␊ + 3 | this.foo = 'foo';␊ + 4 | this.foo2 = 'foo2';␊ + 5 | }␊ + 6 | }␊ + ` + +> Output + + `␊ + 1 | class Foo {␊ + 2 | foo = 'foo';␊ + 3 | foo2 = 'foo2';␊ + 4 | constructor() {␊ + 5 | }␊ + 6 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | class Foo {␊ + 2 | constructor() {␊ + 3 | this.foo = 'foo';␊ + > 4 | this.foo2 = 'foo2';␊ + | ^^^^^^^^^^^^^^^^^^^ Prefer class field declaration over \`this\` assignment in constructor for static values.␊ + 5 | }␊ + 6 | }␊ + ` + +## invalid(3): class Foo { constructor(argument) { this.foo = 'foo'; this.foo2 = argument + 'test'; this.foo3 = 'foo3'; } } + +> Input + + `␊ + 1 | class Foo {␊ + 2 | constructor(argument) {␊ + 3 | this.foo = 'foo';␊ + 4 | this.foo2 = argument + 'test';␊ + 5 | this.foo3 = 'foo3';␊ + 6 | }␊ + 7 | }␊ + ` + +> Output + + `␊ + 1 | class Foo {␊ + 2 | foo = 'foo';␊ + 3 | foo3 = 'foo3';␊ + 4 | constructor(argument) {␊ + 5 | this.foo2 = argument + 'test';␊ + 6 | }␊ + 7 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | class Foo {␊ + 2 | constructor(argument) {␊ + 3 | this.foo = 'foo';␊ + 4 | this.foo2 = argument + 'test';␊ + > 5 | this.foo3 = 'foo3';␊ + | ^^^^^^^^^^^^^^^^^^^ Prefer class field declaration over \`this\` assignment in constructor for static values.␊ + 6 | }␊ + 7 | }␊ + ` + +## invalid(4): class MyError extends Error { constructor(message) { super(message); this.name = "MyError"; } } + +> Input + + `␊ + 1 | class MyError extends Error {␊ + 2 | constructor(message) {␊ + 3 | super(message);␊ + 4 | this.name = "MyError";␊ + 5 | }␊ + 6 | }␊ + ` + +> Output + + `␊ + 1 | class MyError extends Error {␊ + 2 | name = "MyError";␊ + 3 | constructor(message) {␊ + 4 | super(message);␊ + 5 | }␊ + 6 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | class MyError extends Error {␊ + 2 | constructor(message) {␊ + 3 | super(message);␊ + > 4 | this.name = "MyError";␊ + | ^^^^^^^^^^^^^^^^^^^^^^ Prefer class field declaration over \`this\` assignment in constructor for static values.␊ + 5 | }␊ + 6 | }␊ + ` diff --git a/test/snapshots/prefer-class-fields.mjs.snap b/test/snapshots/prefer-class-fields.mjs.snap new file mode 100644 index 0000000000000000000000000000000000000000..708c7022c00767eb63068ae80f2e7d9b47cfa28e GIT binary patch literal 642 zcmV-|0)72KRzVGR52CNQHRAt60u;rx$^{mM5M$4=QxMYW2r>qD=xNoa1M3K|>0HOCxeZBl-z%vOwo;bEd?ioc7B8^3DHSK6(p6w{q|^ABDM z8d8y+9x@sQ2&f+sK@{UL_>c-5rw1N`NXdF}pT-I-F18UN7m$i5=3_{rgvvczsqC2$ zT%h@|nRD_g=cKhVC;3V%TWZp5-g1Cf+fcFV)vIT$!J>}i6iHMX8gXh zhU~kQ*{@@SjKP&h=qw+h_RR>r_;CFe9Vr?I64$@!^H?m}JsXO3ObNhKEw