diff --git a/docs/rules/prefer-class-fields.md b/docs/rules/prefer-class-fields.md new file mode 100644 index 0000000000..660898df43 --- /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 enforces the use of class field declarations for static values, instead of assigning them in constructors using `this`. + +> To avoid leaving empty constructors after autofixing, use the [`no-useless-constructor` 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..74c332354a --- /dev/null +++ b/rules/prefer-class-fields.js @@ -0,0 +1,182 @@ +'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; +}; + +/** +@param {import('eslint').Rule.Node} node +@param {import('eslint').Rule.RuleContext['sourceCode']} sourceCode +@param {import('eslint').Rule.RuleFixer} fixer +*/ +const removeFieldAssignment = (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); +}; + +/** +@param {string} propertyName +@param {import('estree').ClassBody} classBody +*/ +const findClassFieldNamed = (propertyName, classBody) => { + for (const classBodyChild of classBody.body) { + if ( + classBodyChild.type === 'PropertyDefinition' + && classBodyChild.key.type === 'Identifier' + && classBodyChild.key.name === propertyName + ) { + return classBodyChild; + } + } +}; + +/** +@param {string} propertyName +@param {string} propertyValue +@param {import('estree').ClassBody} classBody +@param {import('estree').MethodDefinition} constructor +@param {import('eslint').Rule.RuleContext['sourceCode']} sourceCode +@param {import('eslint').Rule.RuleFixer} fixer +*/ +const addOrReplaceClassFieldDeclaration = ( + propertyName, + propertyValue, + classBody, + constructor, + sourceCode, + fixer, +) => { + const alreadyExistingDeclaration = findClassFieldNamed( + propertyName, + classBody, + ); + + if (alreadyExistingDeclaration) { + return fixer.replaceText( + alreadyExistingDeclaration, + `${propertyName} = ${propertyValue};`, + ); + } + + const classBodyStartRange = [classBody.range[0], classBody.range[0] + 1]; + const indent = getIndentString(constructor, sourceCode); + return fixer.insertTextAfterRange( + classBodyStartRange, + `\n${indent}${propertyName} = ${propertyValue};`, + ); +}; + +/** +@type {import('eslint').Rule.RuleModule['create']} +*/ +const create = context => { + const {sourceCode} = context; + + return { + ClassBody(classBody) { + const constructor = classBody.body.find(x => x.kind === 'constructor'); + + if (!constructor || constructor.type !== 'MethodDefinition') { + return; + } + + const constructorBody = constructor.value.body?.body; + + if (!constructorBody) { + return; + } + + const firstInvalidProperty = constructorBody.findIndex( + node => node.type !== 'ExpressionStatement', + ); + const validConstructorProperties + = firstInvalidProperty === -1 + ? constructorBody + : constructorBody.slice(0, firstInvalidProperty); + + for ( + let index = validConstructorProperties.length - 1; + index >= 0; + index-- + ) { + const node = validConstructorProperties[index]; + if ( + isThisAssignmentExpression(node) + && node.expression.right?.type === 'Literal' + && node.expression.operator === '=' + && node.expression.left.property.type === 'Identifier' + && !node.expression.left.computed + ) { + return { + node, + messageId: MESSAGE_ID, + + /** + @param {import('eslint').Rule.RuleFixer} fixer + */ + * fix(fixer) { + yield removeFieldAssignment(node, sourceCode, fixer); + yield addOrReplaceClassFieldDeclaration( + node.expression.left.property.name, + node.expression.right.raw, + classBody, + constructor, + sourceCode, + fixer, + ); + }, + }; + } + } + }, + }; +}; + +/** @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..dfe06842ba --- /dev/null +++ b/test/prefer-class-fields.mjs @@ -0,0 +1,136 @@ +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"; + } + `, + outdent` + class Foo { + constructor() { + this.foo += 'foo'; + } + } + `, + outdent` + class Foo { + constructor() { + this.foo ??= 'foo'; + } + } + `, + outdent` + class Foo { + constructor() { + this[foo] = 'foo'; + } + } + `, + outdent` + class Foo { + something = 'a'; + constructor() { + this[this.something] = 'foo'; + } + } + `, + outdent` + class Foo { + constructor() { + if (something) { return; } + this.elo = 'foo'; + } + } + `, + ], + 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"; + } + } + `, + outdent` + class Foo { + foo = 'test'; + constructor() { + this.foo = 'foo'; + } + } + `, + ], +}); + +test.typescript({ + valid: [ + outdent` + class Foo { + foo: string = 'foo'; + } + `, + outdent` + declare class Foo { + constructor(foo?: string); + } + `, + ], + 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..c21d297e0e --- /dev/null +++ b/test/snapshots/prefer-class-fields.mjs.md @@ -0,0 +1,184 @@ +# 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 | }␊ + ` + +## invalid(5): class Foo { foo = 'test'; constructor() { this.foo = 'foo'; } } + +> Input + + `␊ + 1 | class Foo {␊ + 2 | foo = 'test';␊ + 3 | constructor() {␊ + 4 | this.foo = 'foo';␊ + 5 | }␊ + 6 | }␊ + ` + +> Output + + `␊ + 1 | class Foo {␊ + 2 | foo = 'foo'␊ + 3 | constructor() {␊ + 4 | }␊ + 5 | }␊ + ` + +> Error 1/1 + + `␊ + 1 | class Foo {␊ + 2 | foo = 'test';␊ + 3 | constructor() {␊ + > 4 | this.foo = 'foo';␊ + | ^^^^^^^^^^^^^^^^^ 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 0000000000..445d2abffe Binary files /dev/null and b/test/snapshots/prefer-class-fields.mjs.snap differ