diff --git a/packages/eslint-plugin/docs/rules/prefer-readonly.md b/packages/eslint-plugin/docs/rules/prefer-readonly.md index 774b55b39ffa..fb4bff187d7b 100644 --- a/packages/eslint-plugin/docs/rules/prefer-readonly.md +++ b/packages/eslint-plugin/docs/rules/prefer-readonly.md @@ -6,7 +6,7 @@ description: "Require private members to be marked as `readonly` if they're neve > > See **https://typescript-eslint.io/rules/prefer-readonly** for documentation. -Member variables with the privacy `private` are never permitted to be modified outside of their declaring class. +Private member variables (whether using the `private` modifier or private `#` fields) are never permitted to be modified outside of their declaring class. If that class never modifies their value, they may safely be marked as `readonly`. This rule reports on private members are marked as `readonly` if they're never modified outside of the constructor. @@ -22,6 +22,7 @@ class Container { // These member variables could be marked as readonly private neverModifiedMember = true; private onlyModifiedInConstructor: number; + #neverModifiedPrivateField = 3; public constructor( onlyModifiedInConstructor: number, @@ -49,6 +50,13 @@ class Container { public mutate() { this.modifiedLater = 'mutated'; } + + // This is modified later on by the class + #modifiedLaterPrivateField = 'unchanged'; + + public mutatePrivateField() { + this.#modifiedLaterPrivateField = 'mutated'; + } } ``` diff --git a/packages/eslint-plugin/src/rules/prefer-readonly.ts b/packages/eslint-plugin/src/rules/prefer-readonly.ts index f5fe608bb15b..b9980536fb23 100644 --- a/packages/eslint-plugin/src/rules/prefer-readonly.ts +++ b/packages/eslint-plugin/src/rules/prefer-readonly.ts @@ -287,7 +287,10 @@ class ClassScope { public addDeclaredVariable(node: ParameterOrPropertyDeclaration): void { if ( - !tsutils.isModifierFlagSet(node, ts.ModifierFlags.Private) || + !( + tsutils.isModifierFlagSet(node, ts.ModifierFlags.Private) || + node.name.kind === ts.SyntaxKind.PrivateIdentifier + ) || tsutils.isModifierFlagSet(node, ts.ModifierFlags.Readonly) || ts.isComputedPropertyName(node.name) ) { diff --git a/packages/eslint-plugin/tests/rules/prefer-readonly.test.ts b/packages/eslint-plugin/tests/rules/prefer-readonly.test.ts index f1b57419adc8..d35ffa66edb6 100644 --- a/packages/eslint-plugin/tests/rules/prefer-readonly.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-readonly.test.ts @@ -52,6 +52,11 @@ ruleTester.run('prefer-readonly', rule, { private static readonly correctlyReadonlyStatic = 7; } `, + ` + class TestReadonlyStatic { + static readonly #correctlyReadonlyStatic = 7; + } + `, ` class TestModifiableStatic { private static correctlyModifiableStatic = 7; @@ -61,6 +66,15 @@ ruleTester.run('prefer-readonly', rule, { } } `, + ` + class TestModifiableStatic { + static #correctlyModifiableStatic = 7; + + public constructor() { + TestModifiableStatic.#correctlyModifiableStatic += 1; + } + } + `, ` class TestModifiableByParameterProperty { private static readonly correctlyModifiableByParameterProperty = 7; @@ -72,11 +86,27 @@ ruleTester.run('prefer-readonly', rule, { ) {} } `, + ` + class TestModifiableByParameterProperty { + static readonly #correctlyModifiableByParameterProperty = 7; + + public constructor( + public correctlyModifiablePublicParameter: number = (() => { + return (TestModifiableStatic.#correctlyModifiableByParameterProperty += 1); + })(), + ) {} + } + `, ` class TestReadonlyInline { private readonly correctlyReadonlyInline = 7; } `, + ` + class TestReadonlyInline { + readonly #correctlyReadonlyInline = 7; + } + `, ` class TestReadonlyDelayed { private readonly correctlyReadonlyDelayed = 7; @@ -86,6 +116,15 @@ ruleTester.run('prefer-readonly', rule, { } } `, + ` + class TestReadonlyDelayed { + readonly #correctlyReadonlyDelayed = 7; + + public constructor() { + this.#correctlyReadonlyDelayed += 1; + } + } + `, ` class TestModifiableInline { private correctlyModifiableInline = 7; @@ -103,6 +142,23 @@ ruleTester.run('prefer-readonly', rule, { } } `, + ` + class TestModifiableInline { + #correctlyModifiableInline = 7; + + public mutate() { + this.#correctlyModifiableInline += 1; + + return class { + #correctlyModifiableInline = 7; + + mutate() { + this.#correctlyModifiableInline += 1; + } + }; + } + } + `, ` class TestModifiableDelayed { private correctlyModifiableDelayed = 7; @@ -112,6 +168,15 @@ ruleTester.run('prefer-readonly', rule, { } } `, + ` + class TestModifiableDelayed { + #correctlyModifiableDelayed = 7; + + public mutate() { + this.#correctlyModifiableDelayed += 1; + } + } + `, ` class TestModifiableDeleted { private correctlyModifiableDeleted = 7; @@ -132,6 +197,17 @@ ruleTester.run('prefer-readonly', rule, { } } `, + ` + class TestModifiableWithinConstructor { + #correctlyModifiableWithinConstructor = 7; + + public constructor() { + (() => { + this.#correctlyModifiableWithinConstructor += 1; + })(); + } + } + `, ` class TestModifiableWithinConstructorArrowFunction { private correctlyModifiableWithinConstructorArrowFunction = 7; @@ -143,6 +219,17 @@ ruleTester.run('prefer-readonly', rule, { } } `, + ` + class TestModifiableWithinConstructorArrowFunction { + #correctlyModifiableWithinConstructorArrowFunction = 7; + + public constructor() { + (() => { + this.#correctlyModifiableWithinConstructorArrowFunction += 1; + })(); + } + } + `, ` class TestModifiableWithinConstructorInFunctionExpression { private correctlyModifiableWithinConstructorInFunctionExpression = 7; @@ -156,6 +243,19 @@ ruleTester.run('prefer-readonly', rule, { } } `, + ` + class TestModifiableWithinConstructorInFunctionExpression { + #correctlyModifiableWithinConstructorInFunctionExpression = 7; + + public constructor() { + const self = this; + + (() => { + self.#correctlyModifiableWithinConstructorInFunctionExpression += 1; + })(); + } + } + `, ` class TestModifiableWithinConstructorInGetAccessor { private correctlyModifiableWithinConstructorInGetAccessor = 7; @@ -171,6 +271,21 @@ ruleTester.run('prefer-readonly', rule, { } } `, + ` + class TestModifiableWithinConstructorInGetAccessor { + #correctlyModifiableWithinConstructorInGetAccessor = 7; + + public constructor() { + const self = this; + + const confusingObject = { + get accessor() { + return (self.#correctlyModifiableWithinConstructorInGetAccessor += 1); + }, + }; + } + } + `, ` class TestModifiableWithinConstructorInMethodDeclaration { private correctlyModifiableWithinConstructorInMethodDeclaration = 7; @@ -186,6 +301,21 @@ ruleTester.run('prefer-readonly', rule, { } } `, + ` + class TestModifiableWithinConstructorInMethodDeclaration { + #correctlyModifiableWithinConstructorInMethodDeclaration = 7; + + public constructor() { + const self = this; + + const confusingObject = { + methodDeclaration() { + self.#correctlyModifiableWithinConstructorInMethodDeclaration = 7; + }, + }; + } + } + `, ` class TestModifiableWithinConstructorInSetAccessor { private correctlyModifiableWithinConstructorInSetAccessor = 7; @@ -201,6 +331,21 @@ ruleTester.run('prefer-readonly', rule, { } } `, + ` + class TestModifiableWithinConstructorInSetAccessor { + #correctlyModifiableWithinConstructorInSetAccessor = 7; + + public constructor() { + const self = this; + + const confusingObject = { + set accessor(value: number) { + self.#correctlyModifiableWithinConstructorInSetAccessor += value; + }, + }; + } + } + `, ` class TestModifiablePostDecremented { private correctlyModifiablePostDecremented = 7; @@ -210,6 +355,15 @@ ruleTester.run('prefer-readonly', rule, { } } `, + ` + class TestModifiablePostDecremented { + #correctlyModifiablePostDecremented = 7; + + public mutate() { + this.#correctlyModifiablePostDecremented -= 1; + } + } + `, ` class TestyModifiablePostIncremented { private correctlyModifiablePostIncremented = 7; @@ -219,6 +373,15 @@ ruleTester.run('prefer-readonly', rule, { } } `, + ` + class TestyModifiablePostIncremented { + #correctlyModifiablePostIncremented = 7; + + public mutate() { + this.#correctlyModifiablePostIncremented += 1; + } + } + `, ` class TestModifiablePreDecremented { private correctlyModifiablePreDecremented = 7; @@ -228,6 +391,15 @@ ruleTester.run('prefer-readonly', rule, { } } `, + ` + class TestModifiablePreDecremented { + #correctlyModifiablePreDecremented = 7; + + public mutate() { + --this.#correctlyModifiablePreDecremented; + } + } + `, ` class TestModifiablePreIncremented { private correctlyModifiablePreIncremented = 7; @@ -237,6 +409,15 @@ ruleTester.run('prefer-readonly', rule, { } } `, + ` + class TestModifiablePreIncremented { + #correctlyModifiablePreIncremented = 7; + + public mutate() { + ++this.#correctlyModifiablePreIncremented; + } + } + `, ` class TestProtectedModifiable { protected protectedModifiable = 7; @@ -273,6 +454,18 @@ ruleTester.run('prefer-readonly', rule, { }, ], }, + { + code: ` + class TestCorrectlyNonInlineLambdas { + #correctlyNonInlineLambda = 7; + } + `, + options: [ + { + onlyInlineLambdas: true, + }, + ], + }, ` class TestComputedParameter { public mutate() { @@ -294,6 +487,18 @@ class Foo { }, { code: ` +class Foo { + #value: number = 0; + + bar(newValue: { value: number }) { + ({ value: this.#value } = newValue); + return this.#value; + } +} + `, + }, + { + code: ` function ClassWithName {}>(Base: TBase) { return class extends Base { private _name: string; @@ -307,6 +512,19 @@ function ClassWithName {}>(Base: TBase) { }, { code: ` +function ClassWithName {}>(Base: TBase) { + return class extends Base { + #name: string; + + public test(value: string) { + this.#name = value; + } + }; +} + `, + }, + { + code: ` class Foo { private value: Record = {}; @@ -319,6 +537,18 @@ class Foo { }, { code: ` +class Foo { + #value: Record = {}; + + bar(newValue: Record) { + ({ ...this.#value } = newValue); + return this.#value; + } +} + `, + }, + { + code: ` class Foo { private value: number[] = []; @@ -331,6 +561,18 @@ class Foo { }, { code: ` +class Foo { + #value: number[] = []; + + bar(newValue: number[]) { + [...this.#value] = newValue; + return this.#value; + } +} + `, + }, + { + code: ` class Foo { private value: number = 0; @@ -338,6 +580,18 @@ class Foo { [this.value] = newValue; return this.value; } +} + `, + }, + { + code: ` +class Foo { + #value: number = 0; + + bar(newValue: number[]) { + [this.#value] = newValue; + return this.#value; + } } `, }, @@ -354,6 +608,19 @@ class Foo { } `, }, + { + code: ` + class Test { + #testObj = { + prop: '', + }; + + public test(): void { + this.#testObj = ''; + } + } + `, + }, { code: ` class TestObject { @@ -369,6 +636,21 @@ class Foo { } `, }, + { + code: ` + class TestObject { + public prop: number; + } + + class Test { + #testObj = new TestObject(); + + public test(): void { + this.#testObj = new TestObject(); + } + } + `, + }, ], invalid: [ { @@ -391,6 +673,26 @@ class Foo { } `, }, + { + code: ` + class TestIncorrectlyModifiableStatic { + static #incorrectlyModifiableStatic = 7; + } + `, + errors: [ + { + data: { + name: '#incorrectlyModifiableStatic', + }, + messageId: 'preferReadonly', + }, + ], + output: ` + class TestIncorrectlyModifiableStatic { + static readonly #incorrectlyModifiableStatic = 7; + } + `, + }, { code: ` class TestIncorrectlyModifiableStaticArrow { @@ -413,20 +715,40 @@ class Foo { }, { code: ` - class TestIncorrectlyModifiableInline { - private incorrectlyModifiableInline = 7; - - public createConfusingChildClass() { - return class { - private incorrectlyModifiableInline = 7; - }; - } + class TestIncorrectlyModifiableStaticArrow { + static #incorrectlyModifiableStaticArrow = () => 7; } `, errors: [ { data: { - name: 'incorrectlyModifiableInline', + name: '#incorrectlyModifiableStaticArrow', + }, + messageId: 'preferReadonly', + }, + ], + output: ` + class TestIncorrectlyModifiableStaticArrow { + static readonly #incorrectlyModifiableStaticArrow = () => 7; + } + `, + }, + { + code: ` + class TestIncorrectlyModifiableInline { + private incorrectlyModifiableInline = 7; + + public createConfusingChildClass() { + return class { + private incorrectlyModifiableInline = 7; + }; + } + } + `, + errors: [ + { + data: { + name: 'incorrectlyModifiableInline', }, line: 3, messageId: 'preferReadonly', @@ -451,6 +773,46 @@ class Foo { } `, }, + { + code: ` + class TestIncorrectlyModifiableInline { + #incorrectlyModifiableInline = 7; + + public createConfusingChildClass() { + return class { + #incorrectlyModifiableInline = 7; + }; + } + } + `, + errors: [ + { + data: { + name: '#incorrectlyModifiableInline', + }, + line: 3, + messageId: 'preferReadonly', + }, + { + data: { + name: '#incorrectlyModifiableInline', + }, + line: 7, + messageId: 'preferReadonly', + }, + ], + output: ` + class TestIncorrectlyModifiableInline { + readonly #incorrectlyModifiableInline = 7; + + public createConfusingChildClass() { + return class { + readonly #incorrectlyModifiableInline = 7; + }; + } + } + `, + }, { code: ` class TestIncorrectlyModifiableDelayed { @@ -479,6 +841,34 @@ class Foo { } `, }, + { + code: ` + class TestIncorrectlyModifiableDelayed { + #incorrectlyModifiableDelayed = 7; + + public constructor() { + this.#incorrectlyModifiableDelayed = 7; + } + } + `, + errors: [ + { + data: { + name: '#incorrectlyModifiableDelayed', + }, + messageId: 'preferReadonly', + }, + ], + output: ` + class TestIncorrectlyModifiableDelayed { + readonly #incorrectlyModifiableDelayed = 7; + + public constructor() { + this.#incorrectlyModifiableDelayed = 7; + } + } + `, + }, { code: ` class TestChildClassExpressionModifiable { @@ -520,6 +910,47 @@ class Foo { } `, }, + { + code: ` + class TestChildClassExpressionModifiable { + #childClassExpressionModifiable = 7; + + public createConfusingChildClass() { + return class { + #childClassExpressionModifiable = 7; + + mutate() { + this.#childClassExpressionModifiable += 1; + } + }; + } + } + `, + errors: [ + { + data: { + name: '#childClassExpressionModifiable', + }, + line: 3, + messageId: 'preferReadonly', + }, + ], + output: ` + class TestChildClassExpressionModifiable { + readonly #childClassExpressionModifiable = 7; + + public createConfusingChildClass() { + return class { + #childClassExpressionModifiable = 7; + + mutate() { + this.#childClassExpressionModifiable += 1; + } + }; + } + } + `, + }, { code: ` class TestIncorrectlyModifiablePostMinus { @@ -549,6 +980,35 @@ class Foo { } `, }, + { + code: ` + class TestIncorrectlyModifiablePostMinus { + #incorrectlyModifiablePostMinus = 7; + + public mutate() { + this.#incorrectlyModifiablePostMinus - 1; + } + } + `, + errors: [ + { + data: { + name: '#incorrectlyModifiablePostMinus', + }, + line: 3, + messageId: 'preferReadonly', + }, + ], + output: ` + class TestIncorrectlyModifiablePostMinus { + readonly #incorrectlyModifiablePostMinus = 7; + + public mutate() { + this.#incorrectlyModifiablePostMinus - 1; + } + } + `, + }, { code: ` class TestIncorrectlyModifiablePostPlus { @@ -578,6 +1038,35 @@ class Foo { } `, }, + { + code: ` + class TestIncorrectlyModifiablePostPlus { + #incorrectlyModifiablePostPlus = 7; + + public mutate() { + this.#incorrectlyModifiablePostPlus + 1; + } + } + `, + errors: [ + { + data: { + name: '#incorrectlyModifiablePostPlus', + }, + line: 3, + messageId: 'preferReadonly', + }, + ], + output: ` + class TestIncorrectlyModifiablePostPlus { + readonly #incorrectlyModifiablePostPlus = 7; + + public mutate() { + this.#incorrectlyModifiablePostPlus + 1; + } + } + `, + }, { code: ` class TestIncorrectlyModifiablePreMinus { @@ -607,6 +1096,35 @@ class Foo { } `, }, + { + code: ` + class TestIncorrectlyModifiablePreMinus { + #incorrectlyModifiablePreMinus = 7; + + public mutate() { + -this.#incorrectlyModifiablePreMinus; + } + } + `, + errors: [ + { + data: { + name: '#incorrectlyModifiablePreMinus', + }, + line: 3, + messageId: 'preferReadonly', + }, + ], + output: ` + class TestIncorrectlyModifiablePreMinus { + readonly #incorrectlyModifiablePreMinus = 7; + + public mutate() { + -this.#incorrectlyModifiablePreMinus; + } + } + `, + }, { code: ` class TestIncorrectlyModifiablePrePlus { @@ -636,6 +1154,35 @@ class Foo { } `, }, + { + code: ` + class TestIncorrectlyModifiablePrePlus { + #incorrectlyModifiablePrePlus = 7; + + public mutate() { + +this.#incorrectlyModifiablePrePlus; + } + } + `, + errors: [ + { + data: { + name: '#incorrectlyModifiablePrePlus', + }, + line: 3, + messageId: 'preferReadonly', + }, + ], + output: ` + class TestIncorrectlyModifiablePrePlus { + readonly #incorrectlyModifiablePrePlus = 7; + + public mutate() { + +this.#incorrectlyModifiablePrePlus; + } + } + `, + }, { code: ` class TestOverlappingClassVariable { @@ -772,6 +1319,31 @@ function ClassWithName {}>(Base: TBase) { }, ], }, + { + code: ` +function ClassWithName {}>(Base: TBase) { + return class extends Base { + #name: string; + }; +} + `, + output: ` +function ClassWithName {}>(Base: TBase) { + return class extends Base { + readonly #name: string; + }; +} + `, + errors: [ + { + data: { + name: '#name', + }, + line: 4, + messageId: 'preferReadonly', + }, + ], + }, { code: ` class Test { @@ -805,6 +1377,39 @@ function ClassWithName {}>(Base: TBase) { }, ], }, + { + code: ` + class Test { + #testObj = { + prop: '', + }; + + public test(): void { + this.#testObj.prop = ''; + } + } + `, + output: ` + class Test { + readonly #testObj = { + prop: '', + }; + + public test(): void { + this.#testObj.prop = ''; + } + } + `, + errors: [ + { + data: { + name: '#testObj', + }, + line: 3, + messageId: 'preferReadonly', + }, + ], + }, { code: ` class TestObject { @@ -842,6 +1447,43 @@ function ClassWithName {}>(Base: TBase) { }, ], }, + { + code: ` + class TestObject { + public prop: number; + } + + class Test { + #testObj = new TestObject(); + + public test(): void { + this.#testObj.prop = 10; + } + } + `, + output: ` + class TestObject { + public prop: number; + } + + class Test { + readonly #testObj = new TestObject(); + + public test(): void { + this.#testObj.prop = 10; + } + } + `, + errors: [ + { + data: { + name: '#testObj', + }, + line: 7, + messageId: 'preferReadonly', + }, + ], + }, { code: ` class Test { @@ -873,6 +1515,37 @@ function ClassWithName {}>(Base: TBase) { }, ], }, + { + code: ` + class Test { + #testObj = { + prop: '', + }; + public test(): void { + this.#testObj.prop; + } + } + `, + output: ` + class Test { + readonly #testObj = { + prop: '', + }; + public test(): void { + this.#testObj.prop; + } + } + `, + errors: [ + { + data: { + name: '#testObj', + }, + line: 3, + messageId: 'preferReadonly', + }, + ], + }, { code: ` class Test { @@ -900,6 +1573,33 @@ function ClassWithName {}>(Base: TBase) { }, ], }, + { + code: ` + class Test { + #testObj = {}; + public test(): void { + this.#testObj?.prop; + } + } + `, + output: ` + class Test { + readonly #testObj = {}; + public test(): void { + this.#testObj?.prop; + } + } + `, + errors: [ + { + data: { + name: '#testObj', + }, + line: 3, + messageId: 'preferReadonly', + }, + ], + }, { code: ` class Test { @@ -927,6 +1627,33 @@ function ClassWithName {}>(Base: TBase) { }, ], }, + { + code: ` + class Test { + #testObj = {}; + public test(): void { + this.#testObj!.prop; + } + } + `, + output: ` + class Test { + readonly #testObj = {}; + public test(): void { + this.#testObj!.prop; + } + } + `, + errors: [ + { + data: { + name: '#testObj', + }, + line: 3, + messageId: 'preferReadonly', + }, + ], + }, { code: ` class Test { @@ -954,6 +1681,33 @@ function ClassWithName {}>(Base: TBase) { }, ], }, + { + code: ` + class Test { + #testObj = {}; + public test(): void { + this.#testObj.prop.prop = ''; + } + } + `, + output: ` + class Test { + readonly #testObj = {}; + public test(): void { + this.#testObj.prop.prop = ''; + } + } + `, + errors: [ + { + data: { + name: '#testObj', + }, + line: 3, + messageId: 'preferReadonly', + }, + ], + }, { code: ` class Test { @@ -981,6 +1735,33 @@ function ClassWithName {}>(Base: TBase) { }, ], }, + { + code: ` + class Test { + #testObj = {}; + public test(): void { + this.#testObj.prop.doesSomething(); + } + } + `, + output: ` + class Test { + readonly #testObj = {}; + public test(): void { + this.#testObj.prop.doesSomething(); + } + } + `, + errors: [ + { + data: { + name: '#testObj', + }, + line: 3, + messageId: 'preferReadonly', + }, + ], + }, { code: ` class Test { @@ -1008,6 +1789,33 @@ function ClassWithName {}>(Base: TBase) { }, ], }, + { + code: ` + class Test { + #testObj = {}; + public test(): void { + this.#testObj?.prop.prop; + } + } + `, + output: ` + class Test { + readonly #testObj = {}; + public test(): void { + this.#testObj?.prop.prop; + } + } + `, + errors: [ + { + data: { + name: '#testObj', + }, + line: 3, + messageId: 'preferReadonly', + }, + ], + }, { code: ` class Test { @@ -1035,6 +1843,33 @@ function ClassWithName {}>(Base: TBase) { }, ], }, + { + code: ` + class Test { + #testObj = {}; + public test(): void { + this.#testObj?.prop?.prop; + } + } + `, + output: ` + class Test { + readonly #testObj = {}; + public test(): void { + this.#testObj?.prop?.prop; + } + } + `, + errors: [ + { + data: { + name: '#testObj', + }, + line: 3, + messageId: 'preferReadonly', + }, + ], + }, { code: ` class Test { @@ -1062,6 +1897,33 @@ function ClassWithName {}>(Base: TBase) { }, ], }, + { + code: ` + class Test { + #testObj = {}; + public test(): void { + this.#testObj.prop?.prop; + } + } + `, + output: ` + class Test { + readonly #testObj = {}; + public test(): void { + this.#testObj.prop?.prop; + } + } + `, + errors: [ + { + data: { + name: '#testObj', + }, + line: 3, + messageId: 'preferReadonly', + }, + ], + }, { code: ` class Test { @@ -1089,5 +1951,32 @@ function ClassWithName {}>(Base: TBase) { }, ], }, + { + code: ` + class Test { + #testObj = {}; + public test(): void { + this.#testObj!.prop?.prop; + } + } + `, + output: ` + class Test { + readonly #testObj = {}; + public test(): void { + this.#testObj!.prop?.prop; + } + } + `, + errors: [ + { + data: { + name: '#testObj', + }, + line: 3, + messageId: 'preferReadonly', + }, + ], + }, ], });