From 5d25bd3d1bac1a02c4b5c4c97a11607e971d32b3 Mon Sep 17 00:00:00 2001 From: Jakub Freisler Date: Mon, 16 Dec 2024 03:35:52 +0100 Subject: [PATCH 01/15] 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 Date: Mon, 16 Dec 2024 03:52:15 +0100 Subject: [PATCH 02/15] fix: handle edge case of constructor without body fixup "contructor" typos --- docs/rules/prefer-class-fields.md | 4 ++-- rules/prefer-class-fields.js | 7 ++++++- test/prefer-class-fields.mjs | 5 +++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/docs/rules/prefer-class-fields.md b/docs/rules/prefer-class-fields.md index e14478eddd..2149b70616 100644 --- a/docs/rules/prefer-class-fields.md +++ b/docs/rules/prefer-class-fields.md @@ -7,9 +7,9 @@ -This rule will enforce the use of field declarations over `this` assignments in contructors for static values. +This rule will enforce the use of field declarations over `this` assignments in constructors for static values. -> To avoid leaving empty constructors after autofix use [`no-useless-contructor` rule](https://eslint.org/docs/latest/rules/no-useless-constructor). +> To avoid leaving empty constructors after autofix use [`no-useless-constructor` rule](https://eslint.org/docs/latest/rules/no-useless-constructor). ## Fail diff --git a/rules/prefer-class-fields.js b/rules/prefer-class-fields.js index bcaae2f9b9..fd4edefabe 100644 --- a/rules/prefer-class-fields.js +++ b/rules/prefer-class-fields.js @@ -66,7 +66,12 @@ const create = context => { return; } - const constructorBody = constructor.value.body.body; + const constructorBody = constructor.value.body?.body; + + if (!constructorBody) { + return; + } + const classBodyStartRange = [node.range[0], node.range[0] + 1]; const indent = getIndentString(constructor, sourceCode); diff --git a/test/prefer-class-fields.mjs b/test/prefer-class-fields.mjs index ef827cd1ff..5600a8b0b1 100644 --- a/test/prefer-class-fields.mjs +++ b/test/prefer-class-fields.mjs @@ -61,6 +61,11 @@ test.typescript({ foo: string = 'foo'; } `, + outdent` + declare class Foo { + constructor(foo?: string); + } + `, ], invalid: [ { From cd95420111cf8587c3f495cf86a74a1bceb51d85 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Wed, 18 Dec 2024 14:42:33 +0100 Subject: [PATCH 03/15] Update prefer-class-fields.md --- docs/rules/prefer-class-fields.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/rules/prefer-class-fields.md b/docs/rules/prefer-class-fields.md index 2149b70616..f8afafb525 100644 --- a/docs/rules/prefer-class-fields.md +++ b/docs/rules/prefer-class-fields.md @@ -7,9 +7,9 @@ -This rule will enforce the use of field declarations over `this` assignments in constructors for static values. +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 autofix use [`no-useless-constructor` rule](https://eslint.org/docs/latest/rules/no-useless-constructor). +> To avoid leaving empty constructors after autofixing, use the [`no-useless-constructor` rule](https://eslint.org/docs/latest/rules/no-useless-constructor). ## Fail From e5e486dba0c9cdb166ff4c5dd252ac2ed62fe6c2 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Wed, 18 Dec 2024 14:44:46 +0100 Subject: [PATCH 04/15] Update prefer-class-fields.js --- rules/prefer-class-fields.js | 40 ++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/rules/prefer-class-fields.js b/rules/prefer-class-fields.js index fd4edefabe..a9c886a863 100644 --- a/rules/prefer-class-fields.js +++ b/rules/prefer-class-fields.js @@ -3,14 +3,13 @@ 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.', + [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}}}} - */ +@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' @@ -29,17 +28,17 @@ const isThisAssignmentExpression = node => { }; /** - * @template Array - * @param {Array} array - * @returns {Array} - */ +@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 - */ +@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); @@ -54,7 +53,9 @@ const removeThisFieldAssignment = (node, sourceCode, fixer) => { : fixer.remove(node); }; -/** @type {import('eslint').Rule.RuleModule['create']} */ +/** +@type {import('eslint').Rule.RuleModule['create']} +*/ const create = context => { const {sourceCode} = context; @@ -84,7 +85,9 @@ const create = context => { node, messageId: MESSAGE_ID, - /** @param {import('eslint').Rule.RuleFixer} fixer */ + /** + @param {import('eslint').Rule.RuleFixer} fixer + */ * fix(fixer) { yield removeThisFieldAssignment(node, sourceCode, fixer); yield fixer.insertTextAfterRange( @@ -99,14 +102,15 @@ const create = context => { }; }; -/** @type {import('eslint').Rule.RuleModule} */ +/** +@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`.', + description: 'Prefer class field declarations over assigning static values in constructor using `this`.', recommended: true, }, fixable: 'code', From 14025db7a686931f441a38ecf34968d4e1a7c069 Mon Sep 17 00:00:00 2001 From: Jakub Freisler Date: Fri, 20 Dec 2024 01:36:04 +0100 Subject: [PATCH 05/15] perf: iterate over original array instead of copying it over and reversing --- rules/prefer-class-fields.js | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/rules/prefer-class-fields.js b/rules/prefer-class-fields.js index a9c886a863..a66cd2f176 100644 --- a/rules/prefer-class-fields.js +++ b/rules/prefer-class-fields.js @@ -27,12 +27,6 @@ const isThisAssignmentExpression = node => { return true; }; -/** -@template Array -@param {Array} array -@returns {Array} -*/ -const reverseArray = array => [...array].reverse(); /** @param {import('eslint').Rule.Node} node @@ -60,8 +54,8 @@ const create = context => { const {sourceCode} = context; return { - ClassBody(node) { - const constructor = node.body.find(x => x.kind === 'constructor'); + ClassBody(classBody) { + const constructor = classBody.body.find(x => x.kind === 'constructor'); if (!constructor || constructor.type !== 'MethodDefinition') { return; @@ -73,10 +67,11 @@ const create = context => { return; } - const classBodyStartRange = [node.range[0], node.range[0] + 1]; + const classBodyStartRange = [classBody.range[0], classBody.range[0] + 1]; const indent = getIndentString(constructor, sourceCode); - for (const node of reverseArray(constructorBody)) { + for (let i = constructorBody.length - 1; i >= 0; i--) { + const node = constructorBody[i]; if ( isThisAssignmentExpression(node) && node.expression.right?.type === 'Literal' From 21e3cf1a172d1834bb67bbeabe39215cd0d3830e Mon Sep 17 00:00:00 2001 From: Jakub Freisler Date: Fri, 20 Dec 2024 01:40:31 +0100 Subject: [PATCH 06/15] chore: handle only simple assignments --- rules/prefer-class-fields.js | 1 + test/prefer-class-fields.mjs | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/rules/prefer-class-fields.js b/rules/prefer-class-fields.js index a66cd2f176..6e317699e5 100644 --- a/rules/prefer-class-fields.js +++ b/rules/prefer-class-fields.js @@ -75,6 +75,7 @@ const create = context => { if ( isThisAssignmentExpression(node) && node.expression.right?.type === 'Literal' + && node.expression.operator === '=' ) { return { node, diff --git a/test/prefer-class-fields.mjs b/test/prefer-class-fields.mjs index 5600a8b0b1..8dfcb4dc01 100644 --- a/test/prefer-class-fields.mjs +++ b/test/prefer-class-fields.mjs @@ -17,6 +17,20 @@ test.snapshot({ name = "MyError"; } `, + outdent` + class Foo { + constructor() { + this.foo += 'foo'; + } + } + `, + outdent` + class Foo { + constructor() { + this.foo ??= 'foo'; + } + } + `, ], invalid: [ outdent` From 110881dc9457d3f5433401997db92e1ca6cec4d8 Mon Sep 17 00:00:00 2001 From: Jakub Freisler Date: Fri, 20 Dec 2024 01:52:36 +0100 Subject: [PATCH 07/15] lint: fix --- rules/prefer-class-fields.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/rules/prefer-class-fields.js b/rules/prefer-class-fields.js index 6e317699e5..16fdeda9e2 100644 --- a/rules/prefer-class-fields.js +++ b/rules/prefer-class-fields.js @@ -27,7 +27,6 @@ const isThisAssignmentExpression = node => { return true; }; - /** @param {import('eslint').Rule.Node} node @param {import('eslint').Rule.RuleContext['sourceCode']} sourceCode @@ -98,9 +97,7 @@ const create = context => { }; }; -/** -@type {import('eslint').Rule.RuleModule} -*/ +/** @type {import('eslint').Rule.RuleModule} */ module.exports = { create, meta: { From 378f58bf45ea56b930cba9b48f7fbac71e4a2131 Mon Sep 17 00:00:00 2001 From: Jakub Freisler Date: Fri, 20 Dec 2024 01:54:00 +0100 Subject: [PATCH 08/15] chore: spaces to tabs --- docs/rules/prefer-class-fields.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/rules/prefer-class-fields.md b/docs/rules/prefer-class-fields.md index f8afafb525..660898df43 100644 --- a/docs/rules/prefer-class-fields.md +++ b/docs/rules/prefer-class-fields.md @@ -15,16 +15,16 @@ This rule enforces the use of class field declarations for static values, instea ```js class Foo { - constructor() { - this.foo = 'foo'; - } + constructor() { + this.foo = 'foo'; + } } class MyError extends Error { - constructor(message: string) { - super(message); - this.name = "MyError"; - } + constructor(message: string) { + super(message); + this.name = "MyError"; + } } ``` @@ -32,10 +32,10 @@ class MyError extends Error { ```js class Foo { - foo = 'foo'; + foo = 'foo'; } class MyError extends Error { - name = "MyError" + name = "MyError" } ``` From 633829c7daf29bd071a2ed371bbee6de5f28cd9a Mon Sep 17 00:00:00 2001 From: Jakub Freisler Date: Fri, 20 Dec 2024 01:56:26 +0100 Subject: [PATCH 09/15] chore: add dynamic field names to valid test cases --- test/prefer-class-fields.mjs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/prefer-class-fields.mjs b/test/prefer-class-fields.mjs index 8dfcb4dc01..61c5996e45 100644 --- a/test/prefer-class-fields.mjs +++ b/test/prefer-class-fields.mjs @@ -31,6 +31,14 @@ test.snapshot({ } } `, + outdent` + class Foo { + something = 'a'; + constructor() { + this[this.something] = 'foo'; + } + } + `, ], invalid: [ outdent` From 0e6afb3a27ad2de8ccb5dc8cc7dcb2e89261863c Mon Sep 17 00:00:00 2001 From: Jakub Freisler Date: Fri, 20 Dec 2024 02:05:18 +0100 Subject: [PATCH 10/15] feat: support strings and template strings --- rules/prefer-class-fields.js | 66 ++++++++++++++------ test/prefer-class-fields.mjs | 14 +++++ test/snapshots/prefer-class-fields.mjs.md | 66 ++++++++++++++++++++ test/snapshots/prefer-class-fields.mjs.snap | Bin 642 -> 726 bytes 4 files changed, 127 insertions(+), 19 deletions(-) diff --git a/rules/prefer-class-fields.js b/rules/prefer-class-fields.js index 16fdeda9e2..f93dcad002 100644 --- a/rules/prefer-class-fields.js +++ b/rules/prefer-class-fields.js @@ -27,12 +27,33 @@ const isThisAssignmentExpression = node => { return true; }; +/** +@param {import('estree').Expression | import('estree').PrivateIdentifier} node +*/ +const getPropertyName = node => { + if (node.type === 'Identifier') { + return node.name; + } + + if (node.type === 'Literal') { + return `[${node.raw}]`; + } + + if ( + node.type === 'TemplateLiteral' + && node.expressions.length === 0 + && node.quasis.length === 1 + ) { + return `[\`${node.quasis[0].value.raw}\`]`; + } +}; + /** @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 removeFieldAssignment = (node, sourceCode, fixer) => { const {line} = node.loc.start; const nodeText = sourceCode.getText(node); const lineText = sourceCode.lines[line - 1]; @@ -72,26 +93,33 @@ const create = context => { for (let i = constructorBody.length - 1; i >= 0; i--) { const node = constructorBody[i]; if ( - isThisAssignmentExpression(node) - && node.expression.right?.type === 'Literal' - && node.expression.operator === '=' + !isThisAssignmentExpression(node) + || node.expression.right?.type !== 'Literal' + || node.expression.operator !== '=' ) { - 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};`, - ); - }, - }; + continue; } + + const propertyName = getPropertyName(node.expression.left.property); + if (!propertyName) { + continue; + } + + return { + node, + messageId: MESSAGE_ID, + + /** + @param {import('eslint').Rule.RuleFixer} fixer + */ + * fix(fixer) { + yield removeFieldAssignment(node, sourceCode, fixer); + yield fixer.insertTextAfterRange( + classBodyStartRange, + `\n${indent}${propertyName} = ${node.expression.right.raw};`, + ); + }, + }; } }, }; diff --git a/test/prefer-class-fields.mjs b/test/prefer-class-fields.mjs index 61c5996e45..52beae2b7f 100644 --- a/test/prefer-class-fields.mjs +++ b/test/prefer-class-fields.mjs @@ -73,6 +73,20 @@ test.snapshot({ } } `, + outdent` + class Foo { + constructor() { + this['foo'] = 'foo'; + } + } + `, + outdent` + class Foo { + constructor() { + this[\`foo\`] = 'foo'; + } + } + `, ], }); diff --git a/test/snapshots/prefer-class-fields.mjs.md b/test/snapshots/prefer-class-fields.mjs.md index 39fde7e9e7..d3b6e6ab0e 100644 --- a/test/snapshots/prefer-class-fields.mjs.md +++ b/test/snapshots/prefer-class-fields.mjs.md @@ -147,3 +147,69 @@ Generated by [AVA](https://avajs.dev). 5 | }␊ 6 | }␊ ` + +## invalid(5): 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(6): 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 | }␊ + ` diff --git a/test/snapshots/prefer-class-fields.mjs.snap b/test/snapshots/prefer-class-fields.mjs.snap index 708c7022c00767eb63068ae80f2e7d9b47cfa28e..fa6d236595b0f1ead9f1fab31ea1fadaa3cfb661 100644 GIT binary patch literal 726 zcmV;{0xA7LRzVH)9dFoOM?p=*0l|f9Pn@7U0FS|mD<`hJ0S|zj+DXRVO`NDD%`IzZzHerJlbx^L zkdGA3nfLRntEJ~ ze^E6d`yKV=OU}Mbd0UX(tkCw|MeW}%&XC#!r`3F*F&qq{&F{TS}ND_OOWyl zv$7`o@(b{91l!Lg7hsJgGdZhYgU8(LGR52CNQHRAt60u;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 Date: Fri, 20 Dec 2024 02:48:04 +0100 Subject: [PATCH 11/15] chore: replace existing field if present --- rules/prefer-class-fields.js | 124 ++++++++++++-------- test/prefer-class-fields.mjs | 10 +- test/snapshots/prefer-class-fields.mjs.md | 55 ++------- test/snapshots/prefer-class-fields.mjs.snap | Bin 726 -> 706 bytes 4 files changed, 89 insertions(+), 100 deletions(-) diff --git a/rules/prefer-class-fields.js b/rules/prefer-class-fields.js index f93dcad002..201e448467 100644 --- a/rules/prefer-class-fields.js +++ b/rules/prefer-class-fields.js @@ -27,27 +27,6 @@ const isThisAssignmentExpression = node => { return true; }; -/** -@param {import('estree').Expression | import('estree').PrivateIdentifier} node -*/ -const getPropertyName = node => { - if (node.type === 'Identifier') { - return node.name; - } - - if (node.type === 'Literal') { - return `[${node.raw}]`; - } - - if ( - node.type === 'TemplateLiteral' - && node.expressions.length === 0 - && node.quasis.length === 1 - ) { - return `[\`${node.quasis[0].value.raw}\`]`; - } -}; - /** @param {import('eslint').Rule.Node} node @param {import('eslint').Rule.RuleContext['sourceCode']} sourceCode @@ -67,6 +46,58 @@ const removeFieldAssignment = (node, sourceCode, fixer) => { : 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']} */ @@ -87,39 +118,34 @@ const create = context => { return; } - const classBodyStartRange = [classBody.range[0], classBody.range[0] + 1]; - const indent = getIndentString(constructor, sourceCode); - for (let i = constructorBody.length - 1; i >= 0; i--) { const node = constructorBody[i]; if ( - !isThisAssignmentExpression(node) - || node.expression.right?.type !== 'Literal' - || node.expression.operator !== '=' + isThisAssignmentExpression(node) + && node.expression.right?.type === 'Literal' + && node.expression.operator === '=' + && node.expression.left.property.type === 'Identifier' ) { - continue; + 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, + ); + }, + }; } - - const propertyName = getPropertyName(node.expression.left.property); - if (!propertyName) { - continue; - } - - return { - node, - messageId: MESSAGE_ID, - - /** - @param {import('eslint').Rule.RuleFixer} fixer - */ - * fix(fixer) { - yield removeFieldAssignment(node, sourceCode, fixer); - yield fixer.insertTextAfterRange( - classBodyStartRange, - `\n${indent}${propertyName} = ${node.expression.right.raw};`, - ); - }, - }; } }, }; diff --git a/test/prefer-class-fields.mjs b/test/prefer-class-fields.mjs index 52beae2b7f..823e55919d 100644 --- a/test/prefer-class-fields.mjs +++ b/test/prefer-class-fields.mjs @@ -75,15 +75,9 @@ test.snapshot({ `, outdent` class Foo { + foo = 'test'; constructor() { - this['foo'] = 'foo'; - } - } - `, - outdent` - class Foo { - constructor() { - this[\`foo\`] = 'foo'; + this.foo = 'foo'; } } `, diff --git a/test/snapshots/prefer-class-fields.mjs.md b/test/snapshots/prefer-class-fields.mjs.md index d3b6e6ab0e..c21d297e0e 100644 --- a/test/snapshots/prefer-class-fields.mjs.md +++ b/test/snapshots/prefer-class-fields.mjs.md @@ -148,56 +148,24 @@ Generated by [AVA](https://avajs.dev). 6 | }␊ ` -## invalid(5): class Foo { constructor() { this['foo'] = 'foo'; } } +## invalid(5): class Foo { foo = 'test'; constructor() { this.foo = 'foo'; } } > Input `␊ 1 | class Foo {␊ - 2 | constructor() {␊ - 3 | this['foo'] = 'foo';␊ - 4 | }␊ - 5 | }␊ - ` - -> Output - - `␊ - 1 | class Foo {␊ - 2 | ['foo'] = 'foo';␊ + 2 | foo = 'test';␊ 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(6): class Foo { constructor() { this[`foo`] = 'foo'; } } - -> Input - - `␊ - 1 | class Foo {␊ - 2 | constructor() {␊ - 3 | this[\`foo\`] = 'foo';␊ - 4 | }␊ - 5 | }␊ + 4 | this.foo = 'foo';␊ + 5 | }␊ + 6 | }␊ ` > Output `␊ 1 | class Foo {␊ - 2 | [\`foo\`] = 'foo';␊ + 2 | foo = 'foo'␊ 3 | constructor() {␊ 4 | }␊ 5 | }␊ @@ -207,9 +175,10 @@ Generated by [AVA](https://avajs.dev). `␊ 1 | class Foo {␊ - 2 | constructor() {␊ - > 3 | this[\`foo\`] = 'foo';␊ - | ^^^^^^^^^^^^^^^^^^^^ Prefer class field declaration over \`this\` assignment in constructor for static values.␊ - 4 | }␊ - 5 | }␊ + 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 index fa6d236595b0f1ead9f1fab31ea1fadaa3cfb661..445d2abffe984f19b39ed030016b2d0963a8766c 100644 GIT binary patch literal 706 zcmV;z0zLgfRzVIEN0@JkD!( zVFD8-12T|DT*-Imu1geo=3J>ZNEMdV8Uod;FI6xc0TOc%OJz};h&+p7<; zXqu4yg6i^d!MdchEy-?PYm&CAMsu9WX-aRK5dVxqI4cOjz&Li8V;l$DUMwW7Y}69_ z)(s^5y0wDbcMEgBiWbr)F5g1ONei`REwuaQ;ww5*G;k$c{K8Gur&5w0je1Ik13Ev5 zTP7fV3LV&pZEYC3<^!tSdyJexhpv`$IkQq`WZ^>Qk{+YEz7GB{{LcxJWTj+XGTbT3&eBa50~H(+8SP!-l(%{uV@|B-csfC oK1W&}Ep)s8)nfb;LHNCVo)kx)-BMP&EtlZ+8H)9dFoOM?p=*0l|f9Pn@7U0FS|mD<`hJ0S|zj+DXRVO`NDD%`IzZzHerJlbx^L zkdGA3nfLRntEJ~ ze^E6d`yKV=OU}Mbd0UX(tkCw|MeW}%&XC#!r`3F*F&qq{&F{TS}ND_OOWyl zv$7`o@(b{91l!Lg7hsJgGdZhYgU8(L Date: Fri, 20 Dec 2024 03:02:01 +0100 Subject: [PATCH 12/15] chore: lint fix --- rules/prefer-class-fields.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rules/prefer-class-fields.js b/rules/prefer-class-fields.js index 201e448467..f33cfc3c70 100644 --- a/rules/prefer-class-fields.js +++ b/rules/prefer-class-fields.js @@ -118,8 +118,8 @@ const create = context => { return; } - for (let i = constructorBody.length - 1; i >= 0; i--) { - const node = constructorBody[i]; + for (let index = constructorBody.length - 1; index >= 0; index--) { + const node = constructorBody[index]; if ( isThisAssignmentExpression(node) && node.expression.right?.type === 'Literal' From f9caf1b8fdf8f85b87c2036023b6bf2e7972e476 Mon Sep 17 00:00:00 2001 From: Jakub Freisler Date: Sat, 21 Dec 2024 13:53:02 +0100 Subject: [PATCH 13/15] feat: handle only non-computed properties --- rules/prefer-class-fields.js | 57 +++++++++++++++++++----------------- test/prefer-class-fields.mjs | 17 +++++++---- 2 files changed, 42 insertions(+), 32 deletions(-) diff --git a/rules/prefer-class-fields.js b/rules/prefer-class-fields.js index f33cfc3c70..b839b83241 100644 --- a/rules/prefer-class-fields.js +++ b/rules/prefer-class-fields.js @@ -1,26 +1,27 @@ -'use strict'; -const getIndentString = require('./utils/get-indent-string.js'); +"use strict"; +const getIndentString = require("./utils/get-indent-string.js"); -const MESSAGE_ID = 'prefer-class-fields/error'; +const MESSAGE_ID = "prefer-class-fields/error"; const messages = { - [MESSAGE_ID]: 'Prefer class field declaration over `this` assignment in constructor for static values.', + [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 => { +const isThisAssignmentExpression = (node) => { if ( - node.type !== 'ExpressionStatement' - || node.expression.type !== 'AssignmentExpression' + node.type !== "ExpressionStatement" || + node.expression.type !== "AssignmentExpression" ) { return false; } const lhs = node.expression.left; - if (!lhs.object || lhs.object.type !== 'ThisExpression') { + if (!lhs.object || lhs.object.type !== "ThisExpression") { return false; } @@ -33,16 +34,16 @@ const isThisAssignmentExpression = node => { @param {import('eslint').Rule.RuleFixer} fixer */ const removeFieldAssignment = (node, sourceCode, fixer) => { - const {line} = node.loc.start; + 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}), - ]) + sourceCode.getIndexFromLoc({ line, column: 0 }), + sourceCode.getIndexFromLoc({ line: line + 1, column: 0 }), + ]) : fixer.remove(node); }; @@ -53,9 +54,9 @@ const removeFieldAssignment = (node, sourceCode, fixer) => { const findClassFieldNamed = (propertyName, classBody) => { for (const classBodyChild of classBody.body) { if ( - classBodyChild.type === 'PropertyDefinition' - && classBodyChild.key.type === 'Identifier' - && classBodyChild.key.name === propertyName + classBodyChild.type === "PropertyDefinition" && + classBodyChild.key.type === "Identifier" && + classBodyChild.key.name === propertyName ) { return classBodyChild; } @@ -101,14 +102,14 @@ const addOrReplaceClassFieldDeclaration = ( /** @type {import('eslint').Rule.RuleModule['create']} */ -const create = context => { - const {sourceCode} = context; +const create = (context) => { + const { sourceCode } = context; return { ClassBody(classBody) { - const constructor = classBody.body.find(x => x.kind === 'constructor'); + const constructor = classBody.body.find((x) => x.kind === "constructor"); - if (!constructor || constructor.type !== 'MethodDefinition') { + if (!constructor || constructor.type !== "MethodDefinition") { return; } @@ -121,10 +122,11 @@ const create = context => { for (let index = constructorBody.length - 1; index >= 0; index--) { const node = constructorBody[index]; if ( - isThisAssignmentExpression(node) - && node.expression.right?.type === 'Literal' - && node.expression.operator === '=' - && node.expression.left.property.type === 'Identifier' + isThisAssignmentExpression(node) && + node.expression.right?.type === "Literal" && + node.expression.operator === "=" && + node.expression.left.property.type === "Identifier" && + !node.expression.left.computed ) { return { node, @@ -133,7 +135,7 @@ const create = context => { /** @param {import('eslint').Rule.RuleFixer} fixer */ - * fix(fixer) { + *fix(fixer) { yield removeFieldAssignment(node, sourceCode, fixer); yield addOrReplaceClassFieldDeclaration( node.expression.left.property.name, @@ -155,12 +157,13 @@ const create = context => { module.exports = { create, meta: { - type: 'suggestion', + type: "suggestion", docs: { - description: 'Prefer class field declarations over assigning static values in constructor using `this`.', + description: + "Prefer class field declarations over assigning static values in constructor using `this`.", recommended: true, }, - fixable: 'code', + fixable: "code", hasSuggestions: false, messages, }, diff --git a/test/prefer-class-fields.mjs b/test/prefer-class-fields.mjs index 823e55919d..a96585ad2c 100644 --- a/test/prefer-class-fields.mjs +++ b/test/prefer-class-fields.mjs @@ -1,9 +1,9 @@ -import outdent from 'outdent'; -import {getTester} from './utils/test.mjs'; +import outdent from "outdent"; +import { getTester } from "./utils/test.mjs"; -const {test} = getTester(import.meta); +const { test } = getTester(import.meta); -const MESSAGE_ID = 'prefer-class-fields/error'; +const MESSAGE_ID = "prefer-class-fields/error"; test.snapshot({ valid: [ @@ -31,6 +31,13 @@ test.snapshot({ } } `, + outdent` + class Foo { + constructor() { + this[foo] = 'foo'; + } + } + `, outdent` class Foo { something = 'a'; @@ -107,7 +114,7 @@ test.typescript({ } } `, - errors: [{messageId: MESSAGE_ID}], + errors: [{ messageId: MESSAGE_ID }], output: outdent` class MyError extends Error { name = "MyError"; From 3376845a24b906cc6d5fec99524b99570397136c Mon Sep 17 00:00:00 2001 From: Jakub Freisler Date: Sun, 22 Dec 2024 18:53:16 +0100 Subject: [PATCH 14/15] chore: stop static analysis on unsupported cases --- rules/prefer-class-fields.js | 72 +++++++++++++++++++++--------------- test/prefer-class-fields.mjs | 18 ++++++--- 2 files changed, 55 insertions(+), 35 deletions(-) diff --git a/rules/prefer-class-fields.js b/rules/prefer-class-fields.js index b839b83241..2c90a46c81 100644 --- a/rules/prefer-class-fields.js +++ b/rules/prefer-class-fields.js @@ -1,27 +1,27 @@ -"use strict"; -const getIndentString = require("./utils/get-indent-string.js"); +'use strict'; +const getIndentString = require('./utils/get-indent-string.js'); -const MESSAGE_ID = "prefer-class-fields/error"; +const MESSAGE_ID = 'prefer-class-fields/error'; const messages = { [MESSAGE_ID]: - "Prefer class field declaration over `this` assignment in constructor for static values.", + '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) => { +const isThisAssignmentExpression = node => { if ( - node.type !== "ExpressionStatement" || - node.expression.type !== "AssignmentExpression" + node.type !== 'ExpressionStatement' + || node.expression.type !== 'AssignmentExpression' ) { return false; } const lhs = node.expression.left; - if (!lhs.object || lhs.object.type !== "ThisExpression") { + if (!lhs.object || lhs.object.type !== 'ThisExpression') { return false; } @@ -34,16 +34,16 @@ const isThisAssignmentExpression = (node) => { @param {import('eslint').Rule.RuleFixer} fixer */ const removeFieldAssignment = (node, sourceCode, fixer) => { - const { line } = node.loc.start; + 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 }), - ]) + sourceCode.getIndexFromLoc({line, column: 0}), + sourceCode.getIndexFromLoc({line: line + 1, column: 0}), + ]) : fixer.remove(node); }; @@ -54,9 +54,9 @@ const removeFieldAssignment = (node, sourceCode, fixer) => { const findClassFieldNamed = (propertyName, classBody) => { for (const classBodyChild of classBody.body) { if ( - classBodyChild.type === "PropertyDefinition" && - classBodyChild.key.type === "Identifier" && - classBodyChild.key.name === propertyName + classBodyChild.type === 'PropertyDefinition' + && classBodyChild.key.type === 'Identifier' + && classBodyChild.key.name === propertyName ) { return classBodyChild; } @@ -102,14 +102,14 @@ const addOrReplaceClassFieldDeclaration = ( /** @type {import('eslint').Rule.RuleModule['create']} */ -const create = (context) => { - const { sourceCode } = context; +const create = context => { + const {sourceCode} = context; return { ClassBody(classBody) { - const constructor = classBody.body.find((x) => x.kind === "constructor"); + const constructor = classBody.body.find(x => x.kind === 'constructor'); - if (!constructor || constructor.type !== "MethodDefinition") { + if (!constructor || constructor.type !== 'MethodDefinition') { return; } @@ -119,14 +119,26 @@ const create = (context) => { return; } - for (let index = constructorBody.length - 1; index >= 0; index--) { - const node = constructorBody[index]; + 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 + isThisAssignmentExpression(node) + && node.expression.right?.type === 'Literal' + && node.expression.operator === '=' + && node.expression.left.property.type === 'Identifier' + && !node.expression.left.computed ) { return { node, @@ -135,7 +147,7 @@ const create = (context) => { /** @param {import('eslint').Rule.RuleFixer} fixer */ - *fix(fixer) { + * fix(fixer) { yield removeFieldAssignment(node, sourceCode, fixer); yield addOrReplaceClassFieldDeclaration( node.expression.left.property.name, @@ -157,13 +169,13 @@ const create = (context) => { module.exports = { create, meta: { - type: "suggestion", + type: 'suggestion', docs: { description: - "Prefer class field declarations over assigning static values in constructor using `this`.", + 'Prefer class field declarations over assigning static values in constructor using `this`.', recommended: true, }, - fixable: "code", + fixable: 'code', hasSuggestions: false, messages, }, diff --git a/test/prefer-class-fields.mjs b/test/prefer-class-fields.mjs index a96585ad2c..dfe06842ba 100644 --- a/test/prefer-class-fields.mjs +++ b/test/prefer-class-fields.mjs @@ -1,9 +1,9 @@ -import outdent from "outdent"; -import { getTester } from "./utils/test.mjs"; +import outdent from 'outdent'; +import {getTester} from './utils/test.mjs'; -const { test } = getTester(import.meta); +const {test} = getTester(import.meta); -const MESSAGE_ID = "prefer-class-fields/error"; +const MESSAGE_ID = 'prefer-class-fields/error'; test.snapshot({ valid: [ @@ -46,6 +46,14 @@ test.snapshot({ } } `, + outdent` + class Foo { + constructor() { + if (something) { return; } + this.elo = 'foo'; + } + } + `, ], invalid: [ outdent` @@ -114,7 +122,7 @@ test.typescript({ } } `, - errors: [{ messageId: MESSAGE_ID }], + errors: [{messageId: MESSAGE_ID}], output: outdent` class MyError extends Error { name = "MyError"; From d9078277b8e50b26459148bc1fddb09ce03fec84 Mon Sep 17 00:00:00 2001 From: Jakub Freisler Date: Wed, 25 Dec 2024 13:39:21 +0100 Subject: [PATCH 15/15] chore: add missing semicolon --- rules/prefer-class-fields.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/prefer-class-fields.js b/rules/prefer-class-fields.js index 2c90a46c81..74c332354a 100644 --- a/rules/prefer-class-fields.js +++ b/rules/prefer-class-fields.js @@ -87,7 +87,7 @@ const addOrReplaceClassFieldDeclaration = ( if (alreadyExistingDeclaration) { return fixer.replaceText( alreadyExistingDeclaration, - `${propertyName} = ${propertyValue}`, + `${propertyName} = ${propertyValue};`, ); }