diff --git a/packages/eslint-plugin/src/rules/class-literal-property-style.ts b/packages/eslint-plugin/src/rules/class-literal-property-style.ts index 483ad9a4080..416267d2835 100644 --- a/packages/eslint-plugin/src/rules/class-literal-property-style.ts +++ b/packages/eslint-plugin/src/rules/class-literal-property-style.ts @@ -1,7 +1,13 @@ import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; -import { createRule, getStaticStringValue } from '../util'; +import { + createRule, + getStaticStringValue, + isAssignee, + isFunction, + nullThrows, +} from '../util'; type Options = ['fields' | 'getters']; type MessageIds = @@ -15,6 +21,11 @@ interface NodeWithModifiers { static: boolean; } +interface PropertiesInfo { + properties: TSESTree.PropertyDefinition[]; + excludeSet: Set; +} + const printNodeModifiers = ( node: NodeWithModifiers, final: 'get' | 'readonly', @@ -65,10 +76,71 @@ export default createRule({ }, defaultOptions: ['fields'], create(context, [style]) { - function getMethodName(node: TSESTree.MethodDefinition): string { - return ( - getStaticStringValue(node.key) ?? context.sourceCode.getText(node.key) + const propertiesInfoStack: PropertiesInfo[] = []; + + function getStringValue(node: TSESTree.Node): string { + return getStaticStringValue(node) ?? context.sourceCode.getText(node); + } + + function enterClassBody(): void { + propertiesInfoStack.push({ + properties: [], + excludeSet: new Set(), + }); + } + + function exitClassBody(): void { + const { properties, excludeSet } = nullThrows( + propertiesInfoStack.pop(), + 'Stack should exist on class exit', ); + + properties.forEach(node => { + const { value } = node; + if (!value || !isSupportedLiteral(value)) { + return; + } + + const name = getStringValue(node.key); + if (excludeSet.has(name)) { + return; + } + + context.report({ + node: node.key, + messageId: 'preferGetterStyle', + suggest: [ + { + messageId: 'preferGetterStyleSuggestion', + fix(fixer): TSESLint.RuleFix { + const name = context.sourceCode.getText(node.key); + + let text = ''; + text += printNodeModifiers(node, 'get'); + text += node.computed ? `[${name}]` : name; + text += `() { return ${context.sourceCode.getText(value)}; }`; + + return fixer.replaceText(node, text); + }, + }, + ], + }); + }); + } + + function excludeAssignedProperty(node: TSESTree.MemberExpression): void { + if (isAssignee(node)) { + const { excludeSet } = + propertiesInfoStack[propertiesInfoStack.length - 1]; + + const name = + getStaticStringValue(node.property) ?? + context.sourceCode.getText(node.property); + + if (name) { + excludeSet.add(name); + } + } } return { @@ -94,14 +166,14 @@ export default createRule({ return; } - const name = getMethodName(node); + const name = getStringValue(node.key); if (node.parent.type === AST_NODE_TYPES.ClassBody) { const hasDuplicateKeySetter = node.parent.body.some(element => { return ( element.type === AST_NODE_TYPES.MethodDefinition && element.kind === 'set' && - getMethodName(element) === name + getStringValue(element.key) === name ); }); if (hasDuplicateKeySetter) { @@ -132,37 +204,33 @@ export default createRule({ }, }), ...(style === 'getters' && { + ClassBody: enterClassBody, + 'ClassBody:exit': exitClassBody, + 'MethodDefinition[kind="constructor"] ThisExpression'( + node: TSESTree.ThisExpression, + ): void { + if (node.parent.type === AST_NODE_TYPES.MemberExpression) { + let parent: TSESTree.Node | undefined = node.parent; + + while (!isFunction(parent)) { + parent = parent.parent; + } + + if ( + parent.parent.type === AST_NODE_TYPES.MethodDefinition && + parent.parent.kind === 'constructor' + ) { + excludeAssignedProperty(node.parent); + } + } + }, PropertyDefinition(node): void { if (!node.readonly || node.declare) { return; } - - const { value } = node; - - if (!value || !isSupportedLiteral(value)) { - return; - } - - context.report({ - node: node.key, - messageId: 'preferGetterStyle', - suggest: [ - { - messageId: 'preferGetterStyleSuggestion', - fix(fixer): TSESLint.RuleFix { - const name = context.sourceCode.getText(node.key); - - let text = ''; - - text += printNodeModifiers(node, 'get'); - text += node.computed ? `[${name}]` : name; - text += `() { return ${context.sourceCode.getText(value)}; }`; - - return fixer.replaceText(node, text); - }, - }, - ], - }); + const { properties } = + propertiesInfoStack[propertiesInfoStack.length - 1]; + properties.push(node); }, }), }; diff --git a/packages/eslint-plugin/src/rules/prefer-for-of.ts b/packages/eslint-plugin/src/rules/prefer-for-of.ts index 0f6bba8d543..18622a04e2e 100644 --- a/packages/eslint-plugin/src/rules/prefer-for-of.ts +++ b/packages/eslint-plugin/src/rules/prefer-for-of.ts @@ -1,7 +1,7 @@ import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; -import { createRule } from '../util'; +import { createRule, isAssignee } from '../util'; export default createRule({ name: 'prefer-for-of', @@ -102,60 +102,6 @@ export default createRule({ ); } - function isAssignee(node: TSESTree.Node): boolean { - const parent = node.parent; - if (!parent) { - return false; - } - - // a[i] = 1, a[i] += 1, etc. - if ( - parent.type === AST_NODE_TYPES.AssignmentExpression && - parent.left === node - ) { - return true; - } - - // delete a[i] - if ( - parent.type === AST_NODE_TYPES.UnaryExpression && - parent.operator === 'delete' && - parent.argument === node - ) { - return true; - } - - // a[i]++, --a[i], etc. - if ( - parent.type === AST_NODE_TYPES.UpdateExpression && - parent.argument === node - ) { - return true; - } - - // [a[i]] = [0] - if (parent.type === AST_NODE_TYPES.ArrayPattern) { - return true; - } - - // [...a[i]] = [0] - if (parent.type === AST_NODE_TYPES.RestElement) { - return true; - } - - // ({ foo: a[i] }) = { foo: 0 } - if ( - parent.type === AST_NODE_TYPES.Property && - parent.value === node && - parent.parent.type === AST_NODE_TYPES.ObjectExpression && - isAssignee(parent.parent) - ) { - return true; - } - - return false; - } - function isIndexOnlyUsedWithArray( body: TSESTree.Statement, indexVar: TSESLint.Scope.Variable, diff --git a/packages/eslint-plugin/src/util/index.ts b/packages/eslint-plugin/src/util/index.ts index f6b583000ce..1f8d657cd54 100644 --- a/packages/eslint-plugin/src/util/index.ts +++ b/packages/eslint-plugin/src/util/index.ts @@ -15,6 +15,7 @@ export * from './isUndefinedIdentifier'; export * from './misc'; export * from './objectIterators'; export * from './types'; +export * from './isAssignee'; // this is done for convenience - saves migrating all of the old rules export * from '@typescript-eslint/type-utils'; diff --git a/packages/eslint-plugin/src/util/isAssignee.ts b/packages/eslint-plugin/src/util/isAssignee.ts new file mode 100644 index 00000000000..39024315250 --- /dev/null +++ b/packages/eslint-plugin/src/util/isAssignee.ts @@ -0,0 +1,56 @@ +import type { TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; + +export function isAssignee(node: TSESTree.Node): boolean { + const parent = node.parent; + if (!parent) { + return false; + } + + // a[i] = 1, a[i] += 1, etc. + if ( + parent.type === AST_NODE_TYPES.AssignmentExpression && + parent.left === node + ) { + return true; + } + + // delete a[i] + if ( + parent.type === AST_NODE_TYPES.UnaryExpression && + parent.operator === 'delete' && + parent.argument === node + ) { + return true; + } + + // a[i]++, --a[i], etc. + if ( + parent.type === AST_NODE_TYPES.UpdateExpression && + parent.argument === node + ) { + return true; + } + + // [a[i]] = [0] + if (parent.type === AST_NODE_TYPES.ArrayPattern) { + return true; + } + + // [...a[i]] = [0] + if (parent.type === AST_NODE_TYPES.RestElement) { + return true; + } + + // ({ foo: a[i] }) = { foo: 0 } + if ( + parent.type === AST_NODE_TYPES.Property && + parent.value === node && + parent.parent.type === AST_NODE_TYPES.ObjectExpression && + isAssignee(parent.parent) + ) { + return true; + } + + return false; +} diff --git a/packages/eslint-plugin/tests/rules/class-literal-property-style.test.ts b/packages/eslint-plugin/tests/rules/class-literal-property-style.test.ts index 1959b1cdaa8..9e6519c10db 100644 --- a/packages/eslint-plugin/tests/rules/class-literal-property-style.test.ts +++ b/packages/eslint-plugin/tests/rules/class-literal-property-style.test.ts @@ -220,6 +220,45 @@ class Mx { `, options: ['getters'], }, + { + code: ` + class A { + private readonly foo: string = 'bar'; + constructor(foo: string) { + this.foo = foo; + } + } + `, + options: ['getters'], + }, + { + code: ` + class A { + private readonly foo: string = 'bar'; + constructor(foo: string) { + this['foo'] = foo; + } + } + `, + options: ['getters'], + }, + { + code: ` + class A { + private readonly foo: string = 'bar'; + constructor(foo: string) { + const bar = new (class { + private readonly foo: string = 'baz'; + constructor() { + this.foo = 'qux'; + } + })(); + this['foo'] = foo; + } + } + `, + options: ['getters'], + }, ], invalid: [ { @@ -660,5 +699,126 @@ class Mx { ], options: ['getters'], }, + { + code: ` +class A { + private readonly foo: string = 'bar'; + constructor(foo: string) { + const bar = new (class { + private readonly foo: string = 'baz'; + constructor() { + this.foo = 'qux'; + } + })(); + } +} + `, + options: ['getters'], + errors: [ + { + messageId: 'preferGetterStyle', + column: 20, + line: 3, + suggestions: [ + { + messageId: 'preferGetterStyleSuggestion', + output: ` +class A { + private get foo() { return 'bar'; } + constructor(foo: string) { + const bar = new (class { + private readonly foo: string = 'baz'; + constructor() { + this.foo = 'qux'; + } + })(); + } +} + `, + }, + ], + }, + ], + }, + { + code: ` +class A { + private readonly ['foo']: string = 'bar'; + constructor(foo: string) { + const bar = new (class { + private readonly foo: string = 'baz'; + constructor() {} + })(); + + if (bar) { + this.foo = 'baz'; + } + } +} + `, + options: ['getters'], + errors: [ + { + messageId: 'preferGetterStyle', + column: 24, + line: 6, + suggestions: [ + { + messageId: 'preferGetterStyleSuggestion', + output: ` +class A { + private readonly ['foo']: string = 'bar'; + constructor(foo: string) { + const bar = new (class { + private get foo() { return 'baz'; } + constructor() {} + })(); + + if (bar) { + this.foo = 'baz'; + } + } +} + `, + }, + ], + }, + ], + }, + { + code: ` +class A { + private readonly foo: string = 'bar'; + constructor(foo: string) { + function func() { + this.foo = 'aa'; + } + } +} + `, + options: ['getters'], + errors: [ + { + messageId: 'preferGetterStyle', + column: 20, + line: 3, + suggestions: [ + { + messageId: 'preferGetterStyleSuggestion', + output: ` +class A { + private get foo() { return 'bar'; } + constructor(foo: string) { + function func() { + this.foo = 'aa'; + } + } +} + `, + }, + ], + }, + ], + }, ], });