Skip to content

Commit

Permalink
fix(eslint-plugin): [prefer-readonly] support modifiers of unions and…
Browse files Browse the repository at this point in the history
… intersections (#8169)
  • Loading branch information
auvred committed Jan 8, 2024
1 parent 10c0530 commit 3a219c0
Show file tree
Hide file tree
Showing 2 changed files with 225 additions and 13 deletions.
79 changes: 66 additions & 13 deletions packages/eslint-plugin/src/rules/prefer-readonly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,13 @@ type ParameterOrPropertyDeclaration =
const OUTSIDE_CONSTRUCTOR = -1;
const DIRECTLY_INSIDE_CONSTRUCTOR = 0;

enum TypeToClassRelation {
ClassAndInstance,
Class,
Instance,
None,
}

class ClassScope {
private readonly privateModifiableMembers = new Map<
string,
Expand Down Expand Up @@ -312,29 +319,75 @@ class ClassScope {
).set(node.name.getText(), node);
}

public getTypeToClassRelation(type: ts.Type): TypeToClassRelation {
if (type.isIntersection()) {
let result: TypeToClassRelation = TypeToClassRelation.None;
for (const subType of type.types) {
const subTypeResult = this.getTypeToClassRelation(subType);
switch (subTypeResult) {
case TypeToClassRelation.Class:
if (result === TypeToClassRelation.Instance) {
return TypeToClassRelation.ClassAndInstance;
}
result = TypeToClassRelation.Class;
break;
case TypeToClassRelation.Instance:
if (result === TypeToClassRelation.Class) {
return TypeToClassRelation.ClassAndInstance;
}
result = TypeToClassRelation.Instance;
break;
}
}
return result;
}
if (type.isUnion()) {
// any union of class/instance and something else will prevent access to
// private members, so we assume that union consists only of classes
// or class instances, because otherwise tsc will report an error
return this.getTypeToClassRelation(type.types[0]);
}

if (!type.getSymbol() || !typeIsOrHasBaseType(type, this.classType)) {
return TypeToClassRelation.None;
}

const typeIsClass =
tsutils.isObjectType(type) &&
tsutils.isObjectFlagSet(type, ts.ObjectFlags.Anonymous);

if (typeIsClass) {
return TypeToClassRelation.Class;
}

return TypeToClassRelation.Instance;
}

public addVariableModification(node: ts.PropertyAccessExpression): void {
const modifierType = this.checker.getTypeAtLocation(node.expression);

const relationOfModifierTypeToClass =
this.getTypeToClassRelation(modifierType);

if (
!modifierType.getSymbol() ||
!typeIsOrHasBaseType(modifierType, this.classType)
relationOfModifierTypeToClass === TypeToClassRelation.Instance &&
this.constructorScopeDepth === DIRECTLY_INSIDE_CONSTRUCTOR
) {
return;
}

const modifyingStatic =
tsutils.isObjectType(modifierType) &&
tsutils.isObjectFlagSet(modifierType, ts.ObjectFlags.Anonymous);
if (
!modifyingStatic &&
this.constructorScopeDepth === DIRECTLY_INSIDE_CONSTRUCTOR
relationOfModifierTypeToClass === TypeToClassRelation.Instance ||
relationOfModifierTypeToClass === TypeToClassRelation.ClassAndInstance
) {
return;
this.memberVariableModifications.add(node.name.text);
}
if (
relationOfModifierTypeToClass === TypeToClassRelation.Class ||
relationOfModifierTypeToClass === TypeToClassRelation.ClassAndInstance
) {
this.staticVariableModifications.add(node.name.text);
}

(modifyingStatic
? this.staticVariableModifications
: this.memberVariableModifications
).add(node.name.text);
}

public enterConstructor(
Expand Down
159 changes: 159 additions & 0 deletions packages/eslint-plugin/tests/rules/prefer-readonly.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,72 @@ class Foo {
}
`,
},
`
class TestIntersection {
private prop: number = 3;
test() {
const that = {} as this & { _foo: 'bar' };
that.prop = 1;
}
}
`,
`
class TestUnion {
private prop: number = 3;
test() {
const that = {} as this | (this & { _foo: 'bar' });
that.prop = 1;
}
}
`,
`
class TestStaticIntersection {
private static prop: number;
test() {
const that = {} as typeof TestStaticIntersection & { _foo: 'bar' };
that.prop = 1;
}
}
`,
`
class TestStaticUnion {
private static prop: number = 1;
test() {
const that = {} as
| typeof TestStaticUnion
| (typeof TestStaticUnion & { _foo: 'bar' });
that.prop = 1;
}
}
`,
`
class TestBothIntersection {
private prop1: number = 1;
private static prop2: number;
test() {
const that = {} as typeof TestBothIntersection & this;
that.prop1 = 1;
that.prop2 = 1;
}
}
`,
`
class TestBothIntersection {
private prop1: number = 1;
private static prop2: number;
test() {
const that = {} as this & typeof TestBothIntersection;
that.prop1 = 1;
that.prop2 = 1;
}
}
`,
],
invalid: [
{
Expand Down Expand Up @@ -1978,5 +2044,98 @@ function ClassWithName<TBase extends new (...args: any[]) => {}>(Base: TBase) {
},
],
},
{
code: `
class Test {
private prop: number = 3;
test() {
const that = {} as this & { _foo: 'bar' };
that._foo = 1;
}
}
`,
output: `
class Test {
private readonly prop: number = 3
test() {
const that = {} as this & {_foo: 'bar'}
that._foo = 1
}
}
`,
errors: [
{
data: {
name: 'prop',
},
line: 3,
messageId: 'preferReadonly',
},
],
},
{
code: `
class Test {
private prop: number = 3;
test() {
const that = {} as this | (this & { _foo: 'bar' });
that.prop;
}
}
`,
output: `
class Test {
private readonly prop: number = 3
test() {
const that = {} as this | (this & {_foo: 'bar'})
that.prop
}
}
`,
errors: [
{
data: {
name: 'prop',
},
line: 3,
messageId: 'preferReadonly',
},
],
},
{
code: `
class Test {
private prop: number;
constructor() {
const that = {} as this & { _foo: 'bar' };
that.prop = 1;
}
}
`,
output: `
class Test {
private readonly prop: number
constructor() {
const that = {} as this & {_foo: 'bar'}
that.prop = 1
}
}
`,
errors: [
{
data: {
name: 'prop',
},
line: 3,
messageId: 'preferReadonly',
},
],
},
],
});

0 comments on commit 3a219c0

Please sign in to comment.