From e7ae0395378e996e39041651da8a9568b130c8f1 Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Mon, 9 Aug 2021 01:20:13 +1200 Subject: [PATCH 1/4] refactor!(prefer-readonly-type): change checkImplicit option to be more expicit of what it does BREAKING CHANGE: option checkImplicit changed to checkForImplicitMutableArrays --- src/rules/prefer-readonly-type.ts | 18 +++++++++--------- tests/rules/prefer-readonly-type/ts/invalid.ts | 2 +- tests/rules/prefer-readonly-type/ts/valid.ts | 10 +++++++--- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/rules/prefer-readonly-type.ts b/src/rules/prefer-readonly-type.ts index 82fb28086..7c68f0356 100644 --- a/src/rules/prefer-readonly-type.ts +++ b/src/rules/prefer-readonly-type.ts @@ -39,7 +39,7 @@ type Options = AllowLocalMutationOption & IgnoreInterfaceOption & IgnorePatternOption & { readonly allowMutableReturnType: boolean; - readonly checkImplicit: boolean; + readonly checkForImplicitMutableArrays: boolean; readonly ignoreCollections: boolean; }; @@ -56,7 +56,7 @@ const schema: JSONSchema4 = [ allowMutableReturnType: { type: "boolean", }, - checkImplicit: { + checkForImplicitMutableArrays: { type: "boolean", }, ignoreCollections: { @@ -70,7 +70,7 @@ const schema: JSONSchema4 = [ // The default options for the rule. const defaultOptions: Options = { - checkImplicit: false, + checkForImplicitMutableArrays: false, ignoreClass: false, ignoreInterface: false, ignoreCollections: false, @@ -255,7 +255,7 @@ function checkProperty( /** * Check if the given TypeReference violates this rule. */ -function checkImplicitType( +function checkForImplicitMutableArray( node: | TSESTree.ArrowFunctionExpression | TSESTree.FunctionDeclaration @@ -264,7 +264,7 @@ function checkImplicitType( context: RuleContext, options: Options ): RuleResult { - if (options.checkImplicit) { + if (options.checkForImplicitMutableArrays) { type Declarator = { readonly id: TSESTree.Node; readonly init: TSESTree.Node | null; @@ -324,10 +324,10 @@ export const rule = createRule( meta, defaultOptions, { - ArrowFunctionExpression: checkImplicitType, + ArrowFunctionExpression: checkForImplicitMutableArray, ClassProperty: checkProperty, - FunctionDeclaration: checkImplicitType, - FunctionExpression: checkImplicitType, + FunctionDeclaration: checkForImplicitMutableArray, + FunctionExpression: checkForImplicitMutableArray, TSArrayType: checkArrayOrTupleType, TSIndexSignature: checkProperty, TSParameterProperty: checkProperty, @@ -335,6 +335,6 @@ export const rule = createRule( TSTupleType: checkArrayOrTupleType, TSMappedType: checkMappedType, TSTypeReference: checkTypeReference, - VariableDeclaration: checkImplicitType, + VariableDeclaration: checkForImplicitMutableArray, } ); diff --git a/tests/rules/prefer-readonly-type/ts/invalid.ts b/tests/rules/prefer-readonly-type/ts/invalid.ts index 26be07fda..9277990eb 100644 --- a/tests/rules/prefer-readonly-type/ts/invalid.ts +++ b/tests/rules/prefer-readonly-type/ts/invalid.ts @@ -507,7 +507,7 @@ const tests: ReadonlyArray = [ code: dedent` const foo = [1, 2, 3] function bar(param = [1, 2, 3]) {}`, - optionsSet: [[{ checkImplicit: true }]], + optionsSet: [[{ checkForImplicitMutableArrays: true }]], output: dedent` const foo: readonly unknown[] = [1, 2, 3] function bar(param: readonly unknown[] = [1, 2, 3]) {}`, diff --git a/tests/rules/prefer-readonly-type/ts/valid.ts b/tests/rules/prefer-readonly-type/ts/valid.ts index 88954c8aa..fdc4e250f 100644 --- a/tests/rules/prefer-readonly-type/ts/valid.ts +++ b/tests/rules/prefer-readonly-type/ts/valid.ts @@ -142,7 +142,7 @@ const tests: ReadonlyArray = [ { code: dedent` const foo = [1, 2, 3] as const`, - optionsSet: [[{ checkImplicit: true }]], + optionsSet: [[{ checkForImplicitMutableArrays: true }]], }, // Should not fail on implicit Array. { @@ -357,7 +357,9 @@ const tests: ReadonlyArray = [ }, { code: dedent`const Foo = []`, - optionsSet: [[{ ignoreCollections: true, checkImplicit: true }]], + optionsSet: [ + [{ ignoreCollections: true, checkForImplicitMutableArrays: true }], + ], }, { code: dedent`type Foo = [string, string];`, @@ -369,7 +371,9 @@ const tests: ReadonlyArray = [ }, { code: dedent`const Foo = ['foo', 'bar'];`, - optionsSet: [[{ ignoreCollections: true, checkImplicit: true }]], + optionsSet: [ + [{ ignoreCollections: true, checkForImplicitMutableArrays: true }], + ], }, { code: dedent`type Foo = Set;`, From 112d147518655c9ba7bef5ac09456945c21ba414 Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Mon, 26 Jul 2021 05:44:35 +1200 Subject: [PATCH 2/4] feat!(prefer-readonly-type): turn option "allowMutableReturnType" on by default BREAKING CHANGE: allowMutableReturnType is now on by default re #153 --- docs/rules/prefer-readonly-type.md | 2 +- src/rules/prefer-readonly-type.ts | 2 +- .../rules/prefer-readonly-type/ts/invalid.ts | 316 ++++++++++++++---- tests/rules/prefer-readonly-type/ts/valid.ts | 40 +-- 4 files changed, 273 insertions(+), 87 deletions(-) diff --git a/docs/rules/prefer-readonly-type.md b/docs/rules/prefer-readonly-type.md index 1043c89c2..c7db101a2 100644 --- a/docs/rules/prefer-readonly-type.md +++ b/docs/rules/prefer-readonly-type.md @@ -106,7 +106,7 @@ The default options: ```ts { allowLocalMutation: false, - allowMutableReturnType: false, + allowMutableReturnType: true, checkImplicit: false, ignoreClass: false, ignoreInterface: false, diff --git a/src/rules/prefer-readonly-type.ts b/src/rules/prefer-readonly-type.ts index 7c68f0356..60f91d3a9 100644 --- a/src/rules/prefer-readonly-type.ts +++ b/src/rules/prefer-readonly-type.ts @@ -75,7 +75,7 @@ const defaultOptions: Options = { ignoreInterface: false, ignoreCollections: false, allowLocalMutation: false, - allowMutableReturnType: false, + allowMutableReturnType: true, }; // The possible error messages. diff --git a/tests/rules/prefer-readonly-type/ts/invalid.ts b/tests/rules/prefer-readonly-type/ts/invalid.ts index 9277990eb..6a28365e2 100644 --- a/tests/rules/prefer-readonly-type/ts/invalid.ts +++ b/tests/rules/prefer-readonly-type/ts/invalid.ts @@ -125,18 +125,12 @@ const tests: ReadonlyArray = [ }`, optionsSet: [[]], output: dedent` - function foo(): ReadonlyArray { + function foo(): Array { interface Foo { readonly bar: ReadonlyArray } }`, errors: [ - { - messageId: "type", - type: "TSTypeReference", - line: 1, - column: 17, - }, { messageId: "type", type: "TSTypeReference", @@ -155,18 +149,12 @@ const tests: ReadonlyArray = [ }`, optionsSet: [[]], output: dedent` - const foo = (): ReadonlyArray => { + const foo = (): Array => { interface Foo { readonly bar: ReadonlyArray } }`, errors: [ - { - messageId: "type", - type: "TSTypeReference", - line: 1, - column: 17, - }, { messageId: "type", type: "TSTypeReference", @@ -175,38 +163,6 @@ const tests: ReadonlyArray = [ }, ], }, - // Should fail on shorthand syntax Array type as return type. - { - code: dedent` - function foo(): number[] { - }`, - optionsSet: [[]], - output: dedent` - function foo(): readonly number[] { - }`, - errors: [ - { - messageId: "array", - type: "TSArrayType", - line: 1, - column: 17, - }, - ], - }, - // Should fail on shorthand syntax Array type as return type. - { - code: `const foo = (): number[] => {}`, - optionsSet: [[]], - output: `const foo = (): readonly number[] => {}`, - errors: [ - { - messageId: "array", - type: "TSArrayType", - line: 1, - column: 17, - }, - ], - }, // Should fail inside function. { code: dedent` @@ -331,7 +287,7 @@ const tests: ReadonlyArray = [ readonly baz: ReadonlyArray } ): { - readonly bar: ReadonlyArray, + readonly bar: Array, readonly baz: ReadonlyArray } { let foo: { @@ -350,12 +306,6 @@ const tests: ReadonlyArray = [ line: 3, column: 19, }, - { - messageId: "type", - type: "TSTypeReference", - line: 7, - column: 17, - }, { messageId: "type", type: "TSTypeReference", @@ -651,17 +601,11 @@ const tests: ReadonlyArray = [ // Function Index Signatures. { code: dedent` - function foo(): { [source: string]: string } { - return undefined; - } function bar(param: { [source: string]: string }): void { return undefined; }`, optionsSet: [[]], output: dedent` - function foo(): { readonly [source: string]: string } { - return undefined; - } function bar(param: { readonly [source: string]: string }): void { return undefined; }`, @@ -670,12 +614,6 @@ const tests: ReadonlyArray = [ messageId: "property", type: "TSIndexSignature", line: 1, - column: 19, - }, - { - messageId: "property", - type: "TSIndexSignature", - line: 4, column: 23, }, ], @@ -902,6 +840,254 @@ const tests: ReadonlyArray = [ }, ], }, + // Don't allow mutable return type. + { + code: dedent` + function foo(...numbers: ReadonlyArray): Array {} + function bar(...numbers: readonly number[]): number[] {}`, + optionsSet: [[{ allowMutableReturnType: false }]], + output: dedent` + function foo(...numbers: ReadonlyArray): ReadonlyArray {} + function bar(...numbers: readonly number[]): readonly number[] {}`, + errors: [ + { + messageId: "type", + type: "TSTypeReference", + line: 1, + column: 50, + }, + { + messageId: "array", + type: "TSArrayType", + line: 2, + column: 46, + }, + ], + }, + // Don't allow mutable return type. + { + code: dedent` + const foo = function(...numbers: ReadonlyArray): Array {} + const bar = function(...numbers: readonly number[]): number[] {}`, + optionsSet: [[{ allowMutableReturnType: false }]], + output: dedent` + const foo = function(...numbers: ReadonlyArray): ReadonlyArray {} + const bar = function(...numbers: readonly number[]): readonly number[] {}`, + errors: [ + { + messageId: "type", + type: "TSTypeReference", + line: 1, + column: 58, + }, + { + messageId: "array", + type: "TSArrayType", + line: 2, + column: 54, + }, + ], + }, + // Don't allow mutable return type. + { + code: dedent` + const foo = (...numbers: ReadonlyArray): Array => {} + const bar = (...numbers: readonly number[]): number[] => {}`, + optionsSet: [[{ allowMutableReturnType: false }]], + output: dedent` + const foo = (...numbers: ReadonlyArray): ReadonlyArray => {} + const bar = (...numbers: readonly number[]): readonly number[] => {}`, + errors: [ + { + messageId: "type", + type: "TSTypeReference", + line: 1, + column: 50, + }, + { + messageId: "array", + type: "TSArrayType", + line: 2, + column: 46, + }, + ], + }, + // Don't allow mutable return type. + { + code: dedent` + class Foo { + foo(...numbers: ReadonlyArray): Array { + } + } + class Bar { + foo(...numbers: readonly number[]): number[] { + } + }`, + optionsSet: [[{ allowMutableReturnType: false }]], + output: dedent` + class Foo { + foo(...numbers: ReadonlyArray): ReadonlyArray { + } + } + class Bar { + foo(...numbers: readonly number[]): readonly number[] { + } + }`, + errors: [ + { + messageId: "type", + type: "TSTypeReference", + line: 2, + column: 43, + }, + { + messageId: "array", + type: "TSArrayType", + line: 6, + column: 39, + }, + ], + }, + // Don't allow mutable return type with Type Arguments. + { + code: dedent` + function foo(...numbers: ReadonlyArray): Promise> {} + function foo(...numbers: ReadonlyArray): Promise {}`, + optionsSet: [[{ allowMutableReturnType: false }]], + output: dedent` + function foo(...numbers: ReadonlyArray): Promise> {} + function foo(...numbers: ReadonlyArray): Promise {}`, + errors: [ + { + messageId: "type", + type: "TSTypeReference", + line: 1, + column: 58, + }, + { + messageId: "array", + type: "TSArrayType", + line: 2, + column: 58, + }, + ], + }, + // Don't allow mutable return type with deep Type Arguments. + { + code: dedent` + type Foo = { readonly x: T; }; + function foo(...numbers: ReadonlyArray): Promise>> {} + function foo(...numbers: ReadonlyArray): Promise> {}`, + optionsSet: [[{ allowMutableReturnType: false }]], + output: dedent` + type Foo = { readonly x: T; }; + function foo(...numbers: ReadonlyArray): Promise>> {} + function foo(...numbers: ReadonlyArray): Promise> {}`, + errors: [ + { + messageId: "type", + type: "TSTypeReference", + line: 2, + column: 62, + }, + { + messageId: "array", + type: "TSArrayType", + line: 3, + column: 62, + }, + ], + }, + // Don't allow mutable return type with Type Arguments in a tuple. + { + code: dedent` + function foo(...numbers: ReadonlyArray): readonly [number, Array, number] {} + function foo(...numbers: ReadonlyArray): readonly [number, number[], number] {}`, + optionsSet: [[{ allowMutableReturnType: false }]], + output: dedent` + function foo(...numbers: ReadonlyArray): readonly [number, ReadonlyArray, number] {} + function foo(...numbers: ReadonlyArray): readonly [number, readonly number[], number] {}`, + errors: [ + { + messageId: "type", + type: "TSTypeReference", + line: 1, + column: 68, + }, + { + messageId: "array", + type: "TSArrayType", + line: 2, + column: 68, + }, + ], + }, + // Don't allow mutable return type with Type Arguments Union. + { + code: dedent` + function foo(...numbers: ReadonlyArray): { readonly a: Array } | { readonly b: string[] } {}`, + optionsSet: [[{ allowMutableReturnType: false }]], + output: dedent` + function foo(...numbers: ReadonlyArray): { readonly a: ReadonlyArray } | { readonly b: readonly string[] } {}`, + errors: [ + { + messageId: "type", + type: "TSTypeReference", + line: 1, + column: 64, + }, + { + messageId: "array", + type: "TSArrayType", + line: 1, + column: 96, + }, + ], + }, + // Don't allow mutable return type with Type Arguments Intersection. + { + code: dedent` + function foo(...numbers: ReadonlyArray): { readonly a: Array } & { readonly b: string[] } {}`, + optionsSet: [[{ allowMutableReturnType: false }]], + output: dedent` + function foo(...numbers: ReadonlyArray): { readonly a: ReadonlyArray } & { readonly b: readonly string[] } {}`, + errors: [ + { + messageId: "type", + type: "TSTypeReference", + line: 1, + column: 64, + }, + { + messageId: "array", + type: "TSArrayType", + line: 1, + column: 96, + }, + ], + }, + // Don't allow mutable return type with Type Arguments Conditional. + { + code: dedent` + function foo(x: T): T extends Array ? string : number[] {}`, + optionsSet: [[{ allowMutableReturnType: false }]], + output: dedent` + function foo(x: T): T extends ReadonlyArray ? string : readonly number[] {}`, + errors: [ + { + messageId: "type", + type: "TSTypeReference", + line: 1, + column: 34, + }, + { + messageId: "array", + type: "TSArrayType", + line: 1, + column: 59, + }, + ], + }, ]; export default tests; diff --git a/tests/rules/prefer-readonly-type/ts/valid.ts b/tests/rules/prefer-readonly-type/ts/valid.ts index fdc4e250f..4bee1a203 100644 --- a/tests/rules/prefer-readonly-type/ts/valid.ts +++ b/tests/rules/prefer-readonly-type/ts/valid.ts @@ -56,28 +56,28 @@ const tests: ReadonlyArray = [ code: `const foo: ReadonlyArray = [];`, optionsSet: [[]], }, - // Allow return type. + // Allow mutable return type. { code: dedent` function foo(...numbers: ReadonlyArray): Array {} function bar(...numbers: readonly number[]): number[] {}`, - optionsSet: [[{ allowMutableReturnType: true }]], + optionsSet: [], }, - // Allow return type. + // Allow mutable return type. { code: dedent` const foo = function(...numbers: ReadonlyArray): Array {} const bar = function(...numbers: readonly number[]): number[] {}`, - optionsSet: [[{ allowMutableReturnType: true }]], + optionsSet: [], }, - // Allow return type. + // Allow mutable return type. { code: dedent` const foo = (...numbers: ReadonlyArray): Array => {} const bar = (...numbers: readonly number[]): number[] => {}`, - optionsSet: [[{ allowMutableReturnType: true }]], + optionsSet: [], }, - // Allow return type. + // Allow mutable return type. { code: dedent` class Foo { @@ -88,47 +88,47 @@ const tests: ReadonlyArray = [ foo(...numbers: readonly number[]): number[] { } }`, - optionsSet: [[{ allowMutableReturnType: true }]], + optionsSet: [], }, - // Allow return type with Type Arguments. + // Allow mutable return type with Type Arguments. { code: dedent` function foo(...numbers: ReadonlyArray): Promise> {} function foo(...numbers: ReadonlyArray): Promise {}`, - optionsSet: [[{ allowMutableReturnType: true }]], + optionsSet: [], }, - // Allow return type with deep Type Arguments. + // Allow mutable return type with deep Type Arguments. { code: dedent` type Foo = { readonly x: T; }; function foo(...numbers: ReadonlyArray): Promise>> {} function foo(...numbers: ReadonlyArray): Promise> {}`, - optionsSet: [[{ allowMutableReturnType: true }]], + optionsSet: [], }, - // Allow return type with Type Arguments in a tuple. + // Allow mutable return type with Type Arguments in a tuple. { code: dedent` function foo(...numbers: ReadonlyArray): readonly [number, Array, number] {} function foo(...numbers: ReadonlyArray): readonly [number, number[], number] {}`, - optionsSet: [[{ allowMutableReturnType: true }]], + optionsSet: [], }, - // Allow return type with Type Arguments Union. + // Allow mutable return type with Type Arguments Union. { code: dedent` function foo(...numbers: ReadonlyArray): { readonly a: Array } | { readonly b: string[] } {}`, - optionsSet: [[{ allowMutableReturnType: true }]], + optionsSet: [], }, - // Allow return type with Type Arguments Intersection. + // Allow mutable return type with Type Arguments Intersection. { code: dedent` function foo(...numbers: ReadonlyArray): { readonly a: Array } & { readonly b: string[] } {}`, - optionsSet: [[{ allowMutableReturnType: true }]], + optionsSet: [], }, - // Allow return type with Type Arguments Conditional. + // Allow mutable return type with Type Arguments Conditional. { code: dedent` function foo(x: T): T extends Array ? string : number[] {}`, - optionsSet: [[{ allowMutableReturnType: true }]], + optionsSet: [], }, // Allow inline mutable return type. { From 780a6c8e923f3f9f517df459facb84931281ec13 Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Mon, 9 Aug 2021 02:08:08 +1200 Subject: [PATCH 3/4] refactor(prefer-readonly-type): give error message ids more explicit names --- src/rules/prefer-readonly-type.ts | 21 +-- .../rules/prefer-readonly-type/ts/invalid.ts | 148 +++++++++--------- 2 files changed, 85 insertions(+), 84 deletions(-) diff --git a/src/rules/prefer-readonly-type.ts b/src/rules/prefer-readonly-type.ts index 60f91d3a9..9bd5959cb 100644 --- a/src/rules/prefer-readonly-type.ts +++ b/src/rules/prefer-readonly-type.ts @@ -80,11 +80,10 @@ const defaultOptions: Options = { // The possible error messages. const errorMessages = { - array: "Only readonly arrays allowed.", - implicit: "Implicitly a mutable array. Only readonly arrays allowed.", - property: "A readonly modifier is required.", - tuple: "Only readonly tuples allowed.", - type: "Only readonly types allowed.", + arrayShouldBeReadonly: "Array should be readonly.", + propertyShouldBeReadonly: "This property should be readonly.", + tupleShouldBeReadonly: "Tuple should be readonly.", + typeShouldBeReadonly: "Type should be readonly.", } as const; // The meta data for this rule. @@ -137,7 +136,9 @@ function checkArrayOrTupleType( ? [ { node, - messageId: isTSTupleType(node) ? "tuple" : "array", + messageId: isTSTupleType(node) + ? "tupleShouldBeReadonly" + : "arrayShouldBeReadonly", fix: node.parent !== undefined && isTSArrayType(node.parent) ? (fixer) => [ @@ -166,7 +167,7 @@ function checkMappedType( : [ { node, - messageId: "property", + messageId: "propertyShouldBeReadonly", fix: (fixer) => fixer.insertTextBeforeRange( [node.range[0] + 1, node.range[1]], @@ -205,7 +206,7 @@ function checkTypeReference( ? [ { node, - messageId: "type", + messageId: "typeShouldBeReadonly", fix: (fixer) => fixer.replaceText(node.typeName, immutableType), }, ] @@ -238,7 +239,7 @@ function checkProperty( ? [ { node, - messageId: "property", + messageId: "propertyShouldBeReadonly", fix: isTSIndexSignature(node) || isTSPropertySignature(node) ? (fixer) => fixer.insertTextBefore(node, "readonly ") @@ -303,7 +304,7 @@ function checkForImplicitMutableArray( ? [ { node: declarator.node, - messageId: "implicit", + messageId: "arrayShouldBeReadonly", fix: (fixer) => fixer.insertTextAfter(declarator.id, ": readonly unknown[]"), }, diff --git a/tests/rules/prefer-readonly-type/ts/invalid.ts b/tests/rules/prefer-readonly-type/ts/invalid.ts index 6a28365e2..eca2b8e7d 100644 --- a/tests/rules/prefer-readonly-type/ts/invalid.ts +++ b/tests/rules/prefer-readonly-type/ts/invalid.ts @@ -13,7 +13,7 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "array", + messageId: "arrayShouldBeReadonly", type: "TSArrayType", line: 1, column: 26, @@ -30,7 +30,7 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 1, column: 26, @@ -47,7 +47,7 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 1, column: 23, @@ -64,7 +64,7 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 1, column: 23, @@ -84,7 +84,7 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 2, column: 17, @@ -108,7 +108,7 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 3, column: 22, @@ -132,7 +132,7 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 3, column: 19, @@ -156,7 +156,7 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 3, column: 19, @@ -176,7 +176,7 @@ const tests: ReadonlyArray = [ };`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 2, column: 12, @@ -194,7 +194,7 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "tuple", + messageId: "tupleShouldBeReadonly", type: "TSTupleType", line: 1, column: 21, @@ -211,13 +211,13 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "tuple", + messageId: "tupleShouldBeReadonly", type: "TSTupleType", line: 1, column: 21, }, { - messageId: "tuple", + messageId: "tupleShouldBeReadonly", type: "TSTupleType", line: 1, column: 38, @@ -234,7 +234,7 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "tuple", + messageId: "tupleShouldBeReadonly", type: "TSTupleType", line: 1, column: 47, @@ -251,7 +251,7 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "tuple", + messageId: "tupleShouldBeReadonly", type: "TSTupleType", line: 1, column: 21, @@ -301,13 +301,13 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 3, column: 19, }, { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 11, column: 19, @@ -321,7 +321,7 @@ const tests: ReadonlyArray = [ output: `type Foo = ReadonlyArray;`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 1, column: 12, @@ -345,7 +345,7 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 3, column: 19, @@ -365,7 +365,7 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 2, column: 14, @@ -389,7 +389,7 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 3, column: 19, @@ -403,7 +403,7 @@ const tests: ReadonlyArray = [ output: `const foo: ReadonlyArray = [];`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 1, column: 12, @@ -417,7 +417,7 @@ const tests: ReadonlyArray = [ output: `const foo: readonly number[] = [1, 2, 3];`, errors: [ { - messageId: "array", + messageId: "arrayShouldBeReadonly", type: "TSArrayType", line: 1, column: 12, @@ -431,7 +431,7 @@ const tests: ReadonlyArray = [ output: `let x: Foo>;`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 1, column: 12, @@ -445,7 +445,7 @@ const tests: ReadonlyArray = [ output: `let x: readonly (readonly string[])[];`, errors: [ { - messageId: "array", + messageId: "arrayShouldBeReadonly", type: "TSArrayType", line: 1, column: 17, @@ -463,13 +463,13 @@ const tests: ReadonlyArray = [ function bar(param: readonly unknown[] = [1, 2, 3]) {}`, errors: [ { - messageId: "implicit", + messageId: "arrayShouldBeReadonly", type: "VariableDeclarator", line: 1, column: 7, }, { - messageId: "implicit", + messageId: "arrayShouldBeReadonly", type: "AssignmentPattern", line: 2, column: 14, @@ -495,25 +495,25 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "ClassProperty", line: 2, column: 3, }, { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "ClassProperty", line: 3, column: 3, }, { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "ClassProperty", line: 4, column: 3, }, { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "ClassProperty", line: 5, column: 3, @@ -541,19 +541,19 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSParameterProperty", line: 3, column: 5, }, { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSParameterProperty", line: 4, column: 5, }, { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSParameterProperty", line: 5, column: 5, @@ -579,19 +579,19 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSIndexSignature", line: 2, column: 3, }, { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSIndexSignature", line: 5, column: 3, }, { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSPropertySignature", line: 5, column: 20, @@ -611,7 +611,7 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSIndexSignature", line: 1, column: 23, @@ -625,7 +625,7 @@ const tests: ReadonlyArray = [ output: `let foo: { readonly [key: string]: number };`, errors: [ { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSIndexSignature", line: 1, column: 12, @@ -647,13 +647,13 @@ const tests: ReadonlyArray = [ }>;`, errors: [ { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSPropertySignature", line: 2, column: 3, }, { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSPropertySignature", line: 3, column: 3, @@ -696,61 +696,61 @@ const tests: ReadonlyArray = [ };`, errors: [ { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSPropertySignature", line: 2, column: 3, }, { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSPropertySignature", line: 3, column: 3, }, { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSPropertySignature", line: 4, column: 3, }, { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSPropertySignature", line: 5, column: 3, }, { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSIndexSignature", line: 6, column: 3, }, { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSPropertySignature", line: 8, column: 5, }, { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSPropertySignature", line: 9, column: 5, }, { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSPropertySignature", line: 10, column: 5, }, { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSPropertySignature", line: 11, column: 5, }, { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSIndexSignature", line: 12, column: 5, @@ -767,7 +767,7 @@ const tests: ReadonlyArray = [ };`, errors: [ { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSPropertySignature", line: 1, column: 21, @@ -783,7 +783,7 @@ const tests: ReadonlyArray = [ const func = (x: { readonly [key in string]: number }) => {}`, errors: [ { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSMappedType", line: 1, column: 18, @@ -811,7 +811,7 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSPropertySignature", line: 4, column: 7, @@ -833,7 +833,7 @@ const tests: ReadonlyArray = [ };`, errors: [ { - messageId: "property", + messageId: "propertyShouldBeReadonly", type: "TSPropertySignature", line: 3, column: 3, @@ -851,13 +851,13 @@ const tests: ReadonlyArray = [ function bar(...numbers: readonly number[]): readonly number[] {}`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 1, column: 50, }, { - messageId: "array", + messageId: "arrayShouldBeReadonly", type: "TSArrayType", line: 2, column: 46, @@ -875,13 +875,13 @@ const tests: ReadonlyArray = [ const bar = function(...numbers: readonly number[]): readonly number[] {}`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 1, column: 58, }, { - messageId: "array", + messageId: "arrayShouldBeReadonly", type: "TSArrayType", line: 2, column: 54, @@ -899,13 +899,13 @@ const tests: ReadonlyArray = [ const bar = (...numbers: readonly number[]): readonly number[] => {}`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 1, column: 50, }, { - messageId: "array", + messageId: "arrayShouldBeReadonly", type: "TSArrayType", line: 2, column: 46, @@ -935,13 +935,13 @@ const tests: ReadonlyArray = [ }`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 2, column: 43, }, { - messageId: "array", + messageId: "arrayShouldBeReadonly", type: "TSArrayType", line: 6, column: 39, @@ -959,13 +959,13 @@ const tests: ReadonlyArray = [ function foo(...numbers: ReadonlyArray): Promise {}`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 1, column: 58, }, { - messageId: "array", + messageId: "arrayShouldBeReadonly", type: "TSArrayType", line: 2, column: 58, @@ -985,13 +985,13 @@ const tests: ReadonlyArray = [ function foo(...numbers: ReadonlyArray): Promise> {}`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 2, column: 62, }, { - messageId: "array", + messageId: "arrayShouldBeReadonly", type: "TSArrayType", line: 3, column: 62, @@ -1009,13 +1009,13 @@ const tests: ReadonlyArray = [ function foo(...numbers: ReadonlyArray): readonly [number, readonly number[], number] {}`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 1, column: 68, }, { - messageId: "array", + messageId: "arrayShouldBeReadonly", type: "TSArrayType", line: 2, column: 68, @@ -1031,13 +1031,13 @@ const tests: ReadonlyArray = [ function foo(...numbers: ReadonlyArray): { readonly a: ReadonlyArray } | { readonly b: readonly string[] } {}`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 1, column: 64, }, { - messageId: "array", + messageId: "arrayShouldBeReadonly", type: "TSArrayType", line: 1, column: 96, @@ -1053,13 +1053,13 @@ const tests: ReadonlyArray = [ function foo(...numbers: ReadonlyArray): { readonly a: ReadonlyArray } & { readonly b: readonly string[] } {}`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 1, column: 64, }, { - messageId: "array", + messageId: "arrayShouldBeReadonly", type: "TSArrayType", line: 1, column: 96, @@ -1075,13 +1075,13 @@ const tests: ReadonlyArray = [ function foo(x: T): T extends ReadonlyArray ? string : readonly number[] {}`, errors: [ { - messageId: "type", + messageId: "typeShouldBeReadonly", type: "TSTypeReference", line: 1, column: 34, }, { - messageId: "array", + messageId: "arrayShouldBeReadonly", type: "TSArrayType", line: 1, column: 59, From a292ceff9cdb990c29cd44f9bdb67323076bf75a Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Mon, 9 Aug 2021 02:15:58 +1200 Subject: [PATCH 4/4] feat(prefer-readonly-type): add support for enforcing type alias readonlyness --- src/rules/prefer-readonly-type.ts | 618 ++++++++++++++---- src/util/rule.ts | 15 + src/util/tree.ts | 42 ++ src/util/typeguard.ts | 12 + .../rules/prefer-readonly-type/ts/invalid.ts | 302 ++++++++- tests/rules/prefer-readonly-type/ts/valid.ts | 125 +++- 6 files changed, 921 insertions(+), 193 deletions(-) diff --git a/src/rules/prefer-readonly-type.ts b/src/rules/prefer-readonly-type.ts index 9bd5959cb..7b627826f 100644 --- a/src/rules/prefer-readonly-type.ts +++ b/src/rules/prefer-readonly-type.ts @@ -15,8 +15,12 @@ import { ignorePatternOptionSchema, } from "~/common/ignore-options"; import type { RuleContext, RuleMetaData, RuleResult } from "~/util/rule"; -import { createRule, getTypeOfNode } from "~/util/rule"; -import { isInReturnType } from "~/util/tree"; +import { isReadonly, createRule, getTypeOfNode } from "~/util/rule"; +import { + getParentIndexSignature, + getTypeDeclaration, + isInReturnType, +} from "~/util/tree"; import { isArrayType, isAssignmentPattern, @@ -24,9 +28,11 @@ import { isIdentifier, isTSArrayType, isTSIndexSignature, + isTSInterfaceDeclaration, isTSParameterProperty, isTSPropertySignature, isTSTupleType, + isTSTypeAliasDeclaration, isTSTypeOperator, } from "~/util/typeguard"; @@ -41,6 +47,17 @@ type Options = AllowLocalMutationOption & readonly allowMutableReturnType: boolean; readonly checkForImplicitMutableArrays: boolean; readonly ignoreCollections: boolean; + readonly aliases: { + readonly mustBeReadonly: { + readonly pattern: ReadonlyArray | string; + readonly requireOthersToBeMutable: boolean; + }; + readonly mustBeMutable: { + readonly pattern: ReadonlyArray | string; + readonly requireOthersToBeReadonly: boolean; + }; + readonly blacklist: ReadonlyArray | string; + }; }; // The schema for the rule options. @@ -62,6 +79,54 @@ const schema: JSONSchema4 = [ ignoreCollections: { type: "boolean", }, + aliases: { + type: "object", + properties: { + mustBeReadonly: { + type: "object", + properties: { + pattern: { + type: ["string", "array"], + items: { + type: "string", + }, + }, + requireOthersToBeMutable: { + type: "boolean", + }, + }, + additionalProperties: false, + }, + mustBeMutable: { + type: "object", + properties: { + pattern: { + type: ["string", "array"], + items: { + type: "string", + }, + }, + requireOthersToBeReadonly: { + type: "boolean", + }, + }, + additionalProperties: false, + }, + blacklist: { + type: "array", + items: { + type: ["string", "array"], + items: { + type: "string", + }, + }, + }, + ignoreInterface: { + type: "boolean", + }, + }, + additionalProperties: false, + }, }, additionalProperties: false, }, @@ -76,10 +141,27 @@ const defaultOptions: Options = { ignoreCollections: false, allowLocalMutation: false, allowMutableReturnType: true, + aliases: { + blacklist: "^Mutable$", + mustBeReadonly: { + pattern: "^(I?)Readonly", + requireOthersToBeMutable: false, + }, + mustBeMutable: { + pattern: "^(I?)Mutable", + requireOthersToBeReadonly: true, + }, + }, }; // The possible error messages. const errorMessages = { + aliasConfigErrorMutableReadonly: + "Configuration error - this type must be marked as both readonly and mutable.", + aliasNeedsExplicitMarking: + "Type must be explicity marked as either readonly or mutable.", + aliasShouldBeMutable: "Mutable types should not be fully readonly.", + aliasShouldBeReadonly: "Readonly types should not be mutable at all.", arrayShouldBeReadonly: "Array should be readonly.", propertyShouldBeReadonly: "This property should be readonly.", tupleShouldBeReadonly: "Tuple should be readonly.", @@ -90,7 +172,7 @@ const errorMessages = { const meta: RuleMetaData = { type: "suggestion", docs: { - description: "Prefer readonly array over mutable arrays.", + description: "Prefer readonly types over mutable one and enforce patterns.", category: "Best Practices", recommended: "error", }, @@ -112,6 +194,213 @@ const mutableTypeRegex = new RegExp( "u" ); +const enum RequiredReadonlyness { + READONLY, + MUTABLE, + EITHER, +} + +const enum TypeReadonlynessDetails { + NONE, + ERROR_MUTABLE_READONLY, + NEEDS_EXPLICIT_MARKING, + IGNORE, + MUTABLE_OK, + MUTABLE_NOT_OK, + READONLY_OK, + READONLY_NOT_OK, +} + +const cachedDetails = new WeakMap< + TSESTree.TSInterfaceDeclaration | TSESTree.TSTypeAliasDeclaration, + TypeReadonlynessDetails +>(); + +/** + * Get the details for the given type alias. + */ +function getTypeAliasDeclarationDetails( + node: TSESTree.Node, + context: RuleContext, + options: Options +): TypeReadonlynessDetails { + const typeDeclaration = getTypeDeclaration(node); + if (typeDeclaration === null) { + return TypeReadonlynessDetails.NONE; + } + + const indexSignature = getParentIndexSignature(node); + if (indexSignature !== null && getTypeDeclaration(indexSignature) !== null) { + return TypeReadonlynessDetails.IGNORE; + } + + if (options.ignoreInterface && isTSInterfaceDeclaration(typeDeclaration)) { + return TypeReadonlynessDetails.IGNORE; + } + + const cached = cachedDetails.get(typeDeclaration); + if (cached !== undefined) { + return cached; + } + + const result = getTypeAliasDeclarationDetailsInternal( + typeDeclaration, + context, + options + ); + cachedDetails.set(typeDeclaration, result); + return result; +} + +/** + * Get the details for the given type alias. + */ +function getTypeAliasDeclarationDetailsInternal( + node: TSESTree.TSInterfaceDeclaration | TSESTree.TSTypeAliasDeclaration, + context: RuleContext, + options: Options +): TypeReadonlynessDetails { + const blacklistPatterns = ( + Array.isArray(options.aliases.blacklist) + ? options.aliases.blacklist + : [options.aliases.blacklist] + ).map((pattern) => new RegExp(pattern, "u")); + + const blacklisted = blacklistPatterns.some((pattern) => + pattern.test(node.id.name) + ); + + if (blacklisted) { + return TypeReadonlynessDetails.IGNORE; + } + + const mustBeReadonlyPatterns = ( + Array.isArray(options.aliases.mustBeReadonly.pattern) + ? options.aliases.mustBeReadonly.pattern + : [options.aliases.mustBeReadonly.pattern] + ).map((pattern) => new RegExp(pattern, "u")); + + const mustBeMutablePatterns = ( + Array.isArray(options.aliases.mustBeMutable.pattern) + ? options.aliases.mustBeMutable.pattern + : [options.aliases.mustBeMutable.pattern] + ).map((pattern) => new RegExp(pattern, "u")); + + const patternStatesReadonly = mustBeReadonlyPatterns.some((pattern) => + pattern.test(node.id.name) + ); + const patternStatesMutable = mustBeMutablePatterns.some((pattern) => + pattern.test(node.id.name) + ); + + if (patternStatesReadonly && patternStatesMutable) { + return TypeReadonlynessDetails.ERROR_MUTABLE_READONLY; + } + + if ( + !patternStatesReadonly && + !patternStatesMutable && + options.aliases.mustBeReadonly.requireOthersToBeMutable && + options.aliases.mustBeMutable.requireOthersToBeReadonly + ) { + return TypeReadonlynessDetails.NEEDS_EXPLICIT_MARKING; + } + + const requiredReadonlyness = + patternStatesReadonly || + (!patternStatesMutable && + options.aliases.mustBeMutable.requireOthersToBeReadonly) + ? RequiredReadonlyness.READONLY + : patternStatesMutable || + (!patternStatesReadonly && + options.aliases.mustBeReadonly.requireOthersToBeMutable) + ? RequiredReadonlyness.MUTABLE + : RequiredReadonlyness.EITHER; + + if (requiredReadonlyness === RequiredReadonlyness.EITHER) { + return TypeReadonlynessDetails.IGNORE; + } + + const readonly = isReadonly( + isTSTypeAliasDeclaration(node) ? node.typeAnnotation : node.body, + context + ); + + if (requiredReadonlyness === RequiredReadonlyness.MUTABLE) { + return readonly + ? TypeReadonlynessDetails.MUTABLE_NOT_OK + : TypeReadonlynessDetails.MUTABLE_OK; + } + + return readonly + ? TypeReadonlynessDetails.READONLY_OK + : TypeReadonlynessDetails.READONLY_NOT_OK; +} + +/** + * Check if the given Interface or Type Alias violates this rule. + */ +function checkTypeDeclaration( + node: TSESTree.TSInterfaceDeclaration | TSESTree.TSTypeAliasDeclaration, + context: RuleContext, + options: Options +): RuleResult { + const details = getTypeAliasDeclarationDetails(node, context, options); + + switch (details) { + case TypeReadonlynessDetails.NEEDS_EXPLICIT_MARKING: { + return { + context, + descriptors: [ + { + node: node.id, + messageId: "aliasNeedsExplicitMarking", + }, + ], + }; + } + case TypeReadonlynessDetails.ERROR_MUTABLE_READONLY: { + return { + context, + descriptors: [ + { + node: node.id, + messageId: "aliasConfigErrorMutableReadonly", + }, + ], + }; + } + case TypeReadonlynessDetails.MUTABLE_NOT_OK: { + return { + context, + descriptors: [ + { + node: node.id, + messageId: "aliasShouldBeMutable", + }, + ], + }; + } + case TypeReadonlynessDetails.READONLY_NOT_OK: { + return { + context, + descriptors: [ + { + node: node.id, + messageId: "aliasShouldBeReadonly", + }, + ], + }; + } + default: { + return { + context, + descriptors: [], + }; + } + } +} + /** * Check if the given ArrayType or TupleType violates this rule. */ @@ -126,30 +415,44 @@ function checkArrayOrTupleType( descriptors: [], }; } - return { - context, - descriptors: - (node.parent === undefined || - !isTSTypeOperator(node.parent) || - node.parent.operator !== "readonly") && - (!options.allowMutableReturnType || !isInReturnType(node)) - ? [ - { - node, - messageId: isTSTupleType(node) - ? "tupleShouldBeReadonly" - : "arrayShouldBeReadonly", - fix: - node.parent !== undefined && isTSArrayType(node.parent) - ? (fixer) => [ - fixer.insertTextBefore(node, "(readonly "), - fixer.insertTextAfter(node, ")"), - ] - : (fixer) => fixer.insertTextBefore(node, "readonly "), - }, - ] - : [], - }; + + const aliasDetails = getTypeAliasDeclarationDetails(node, context, options); + + switch (aliasDetails) { + case TypeReadonlynessDetails.NONE: + case TypeReadonlynessDetails.READONLY_NOT_OK: { + return { + context, + descriptors: + (node.parent === undefined || + !isTSTypeOperator(node.parent) || + node.parent.operator !== "readonly") && + (!options.allowMutableReturnType || !isInReturnType(node)) + ? [ + { + node, + messageId: isTSTupleType(node) + ? "tupleShouldBeReadonly" + : "arrayShouldBeReadonly", + fix: + node.parent !== undefined && isTSArrayType(node.parent) + ? (fixer) => [ + fixer.insertTextBefore(node, "(readonly "), + fixer.insertTextAfter(node, ")"), + ] + : (fixer) => fixer.insertTextBefore(node, "readonly "), + }, + ] + : [], + }; + } + default: { + return { + context, + descriptors: [], + }; + } + } } /** @@ -157,25 +460,39 @@ function checkArrayOrTupleType( */ function checkMappedType( node: TSESTree.TSMappedType, - context: RuleContext + context: RuleContext, + options: Options ): RuleResult { - return { - context, - descriptors: - node.readonly === true || node.readonly === "+" - ? [] - : [ - { - node, - messageId: "propertyShouldBeReadonly", - fix: (fixer) => - fixer.insertTextBeforeRange( - [node.range[0] + 1, node.range[1]], - " readonly" - ), - }, - ], - }; + const aliasDetails = getTypeAliasDeclarationDetails(node, context, options); + + switch (aliasDetails) { + case TypeReadonlynessDetails.NONE: + case TypeReadonlynessDetails.READONLY_NOT_OK: { + return { + context, + descriptors: + node.readonly === true || node.readonly === "+" + ? [] + : [ + { + node, + messageId: "propertyShouldBeReadonly", + fix: (fixer) => + fixer.insertTextBeforeRange( + [node.range[0] + 1, node.range[1]], + " readonly" + ), + }, + ], + }; + } + default: { + return { + context, + descriptors: [], + }; + } + } } /** @@ -186,37 +503,47 @@ function checkTypeReference( context: RuleContext, options: Options ): RuleResult { - if (isIdentifier(node.typeName)) { - if ( - options.ignoreCollections && - mutableTypeRegex.test(node.typeName.name) - ) { + if ( + !isIdentifier(node.typeName) || + (options.ignoreCollections && mutableTypeRegex.test(node.typeName.name)) + ) { + return { + context, + descriptors: [], + }; + } + + const aliasDetails = getTypeAliasDeclarationDetails(node, context, options); + + switch (aliasDetails) { + case TypeReadonlynessDetails.NONE: + case TypeReadonlynessDetails.READONLY_NOT_OK: { + const immutableType = mutableToImmutableTypes.get(node.typeName.name); + + return { + context, + descriptors: + immutableType === undefined || + immutableType.length === 0 || + (options.allowMutableReturnType && isInReturnType(node)) + ? [] + : [ + { + node, + messageId: "typeShouldBeReadonly", + fix: (fixer) => + fixer.replaceText(node.typeName, immutableType), + }, + ], + }; + } + default: { return { context, descriptors: [], }; } - const immutableType = mutableToImmutableTypes.get(node.typeName.name); - return { - context, - descriptors: - immutableType !== undefined && - immutableType.length > 0 && - (!options.allowMutableReturnType || !isInReturnType(node)) - ? [ - { - node, - messageId: "typeShouldBeReadonly", - fix: (fixer) => fixer.replaceText(node.typeName, immutableType), - }, - ] - : [], - }; } - return { - context, - descriptors: [], - }; } /** @@ -231,30 +558,44 @@ function checkProperty( context: RuleContext, options: Options ): RuleResult { - return { - context, - descriptors: - node.readonly !== true && - (!options.allowMutableReturnType || !isInReturnType(node)) - ? [ - { - node, - messageId: "propertyShouldBeReadonly", - fix: - isTSIndexSignature(node) || isTSPropertySignature(node) - ? (fixer) => fixer.insertTextBefore(node, "readonly ") - : isTSParameterProperty(node) - ? (fixer) => - fixer.insertTextBefore(node.parameter, "readonly ") - : (fixer) => fixer.insertTextBefore(node.key, "readonly "), - }, - ] - : [], - }; + const aliasDetails = getTypeAliasDeclarationDetails(node, context, options); + + switch (aliasDetails) { + case TypeReadonlynessDetails.NONE: + case TypeReadonlynessDetails.READONLY_NOT_OK: { + return { + context, + descriptors: + node.readonly !== true && + (!options.allowMutableReturnType || !isInReturnType(node)) + ? [ + { + node, + messageId: "propertyShouldBeReadonly", + fix: + isTSIndexSignature(node) || isTSPropertySignature(node) + ? (fixer) => fixer.insertTextBefore(node, "readonly ") + : isTSParameterProperty(node) + ? (fixer) => + fixer.insertTextBefore(node.parameter, "readonly ") + : (fixer) => + fixer.insertTextBefore(node.key, "readonly "), + }, + ] + : [], + }; + } + default: { + return { + context, + descriptors: [], + }; + } + } } /** - * Check if the given TypeReference violates this rule. + * Check if the given Implicit Type violates this rule. */ function checkForImplicitMutableArray( node: @@ -265,57 +606,60 @@ function checkForImplicitMutableArray( context: RuleContext, options: Options ): RuleResult { - if (options.checkForImplicitMutableArrays) { - type Declarator = { - readonly id: TSESTree.Node; - readonly init: TSESTree.Node | null; - readonly node: TSESTree.Node; - }; - - const declarators: ReadonlyArray = isFunctionLike(node) - ? node.params - .map((param) => - isAssignmentPattern(param) - ? ({ - id: param.left, - init: param.right, - node: param, - } as Declarator) - : undefined - ) - .filter((param): param is Declarator => param !== undefined) - : node.declarations.map( - (declaration) => - ({ - id: declaration.id, - init: declaration.init, - node: declaration, - } as Declarator) - ); + type Declarator = { + readonly id: TSESTree.Node; + readonly init: TSESTree.Node | null; + readonly node: TSESTree.Node; + }; + if ( + options.checkForImplicitMutableArrays === false || + options.ignoreCollections + ) { return { context, - descriptors: declarators.flatMap((declarator) => - isIdentifier(declarator.id) && - declarator.id.typeAnnotation === undefined && - declarator.init !== null && - isArrayType(getTypeOfNode(declarator.init, context)) && - !options.ignoreCollections - ? [ - { - node: declarator.node, - messageId: "arrayShouldBeReadonly", - fix: (fixer) => - fixer.insertTextAfter(declarator.id, ": readonly unknown[]"), - }, - ] - : [] - ), + descriptors: [], }; } + + const declarators: ReadonlyArray = isFunctionLike(node) + ? node.params + .map((param) => + isAssignmentPattern(param) + ? ({ + id: param.left, + init: param.right, + node: param, + } as Declarator) + : undefined + ) + .filter((param): param is Declarator => param !== undefined) + : node.declarations.map( + (declaration) => + ({ + id: declaration.id, + init: declaration.init, + node: declaration, + } as Declarator) + ); + return { context, - descriptors: [], + descriptors: declarators.flatMap((declarator) => + isIdentifier(declarator.id) && + declarator.id.typeAnnotation === undefined && + declarator.init !== null && + isArrayType(getTypeOfNode(declarator.init, context)) + ? [ + { + node: declarator.node, + messageId: "arrayShouldBeReadonly", + fix: (fixer) => + fixer.insertTextAfter(declarator.id, ": readonly unknown[]"), + }, + ] + : [] + ), }; } @@ -331,10 +675,12 @@ export const rule = createRule( FunctionExpression: checkForImplicitMutableArray, TSArrayType: checkArrayOrTupleType, TSIndexSignature: checkProperty, + TSInterfaceDeclaration: checkTypeDeclaration, + TSMappedType: checkMappedType, TSParameterProperty: checkProperty, TSPropertySignature: checkProperty, TSTupleType: checkArrayOrTupleType, - TSMappedType: checkMappedType, + TSTypeAliasDeclaration: checkTypeDeclaration, TSTypeReference: checkTypeReference, VariableDeclaration: checkForImplicitMutableArray, } diff --git a/src/util/rule.ts b/src/util/rule.ts index b736f1a61..c1ce3924a 100644 --- a/src/util/rule.ts +++ b/src/util/rule.ts @@ -138,6 +138,21 @@ export function getTypeOfNode>( return constrained ?? nodeType; } +export function isReadonly>( + node: TSESTree.Node, + context: Context +): boolean { + const { parserServices } = context; + + if (parserServices === undefined || parserServices.program === undefined) { + return false; + } + + const checker = parserServices.program.getTypeChecker(); + const type = getTypeOfNode(node, context); + return ESLintUtils.isTypeReadonly(checker, type!); +} + /** * Get the es tree node from the given ts node. */ diff --git a/src/util/tree.ts b/src/util/tree.ts index 88cf73dae..a4ccb26fa 100644 --- a/src/util/tree.ts +++ b/src/util/tree.ts @@ -9,7 +9,10 @@ import { isMemberExpression, isMethodDefinition, isProperty, + isTSIndexSignature, isTSInterfaceBody, + isTSInterfaceDeclaration, + isTSTypeAliasDeclaration, } from "./typeguard"; /** @@ -39,6 +42,45 @@ export function inFunctionBody(node: TSESTree.Node): boolean { ); } +/** + * Get the type alias or interface that the given node is in. + */ +export function getTypeDeclaration( + node: TSESTree.Node +): TSESTree.TSInterfaceDeclaration | TSESTree.TSTypeAliasDeclaration | null { + if (isTSTypeAliasDeclaration(node) || isTSInterfaceDeclaration(node)) { + return node; + } + + return (getAncestorOfType( + (n): n is TSESTree.Node => + n.parent !== undefined && + n.parent !== null && + ((isTSTypeAliasDeclaration(n.parent) && n.parent.typeAnnotation === n) || + (isTSInterfaceDeclaration(n.parent) && n.parent.body === n)), + node + )?.parent ?? null) as + | TSESTree.TSInterfaceDeclaration + | TSESTree.TSTypeAliasDeclaration + | null; +} + +/** + * Get the parent Index Signature that the given node is in. + */ +export function getParentIndexSignature( + node: TSESTree.Node +): TSESTree.TSIndexSignature | null { + return (getAncestorOfType( + (n): n is TSESTree.Node => + n.parent !== undefined && + n.parent !== null && + isTSIndexSignature(n.parent) && + n.parent.typeAnnotation === n, + node + )?.parent ?? null) as TSESTree.TSIndexSignature | null; +} + /** * Test if the given node is in a class. */ diff --git a/src/util/typeguard.ts b/src/util/typeguard.ts index 0b7ff9e0f..30f9d5671 100644 --- a/src/util/typeguard.ts +++ b/src/util/typeguard.ts @@ -204,12 +204,24 @@ export function isTSIndexSignature( return node.type === AST_NODE_TYPES.TSIndexSignature; } +export function isTSInterfaceDeclaration( + node: TSESTree.Node +): node is TSESTree.TSInterfaceDeclaration { + return node.type === AST_NODE_TYPES.TSInterfaceDeclaration; +} + export function isTSInterfaceBody( node: TSESTree.Node ): node is TSESTree.TSInterfaceBody { return node.type === AST_NODE_TYPES.TSInterfaceBody; } +export function isTSTypeAliasDeclaration( + node: TSESTree.Node +): node is TSESTree.TSTypeAliasDeclaration { + return node.type === AST_NODE_TYPES.TSTypeAliasDeclaration; +} + export function isTSNullKeyword( node: TSESTree.Node ): node is TSESTree.TSNullKeyword { diff --git a/tests/rules/prefer-readonly-type/ts/invalid.ts b/tests/rules/prefer-readonly-type/ts/invalid.ts index eca2b8e7d..e66502b7b 100644 --- a/tests/rules/prefer-readonly-type/ts/invalid.ts +++ b/tests/rules/prefer-readonly-type/ts/invalid.ts @@ -83,6 +83,12 @@ const tests: ReadonlyArray = [ readonly bar: ReadonlyArray }`, errors: [ + { + messageId: "aliasShouldBeReadonly", + type: "Identifier", + line: 1, + column: 11, + }, { messageId: "typeShouldBeReadonly", type: "TSTypeReference", @@ -92,29 +98,36 @@ const tests: ReadonlyArray = [ ], }, // Should fail on Array type in index interface. - { - code: dedent` - interface Foo { - readonly [key: string]: { - readonly groups: Array - } - }`, - optionsSet: [[]], - output: dedent` - interface Foo { - readonly [key: string]: { - readonly groups: ReadonlyArray - } - }`, - errors: [ - { - messageId: "typeShouldBeReadonly", - type: "TSTypeReference", - line: 3, - column: 22, - }, - ], - }, + // https://github.com/typescript-eslint/typescript-eslint/issues/3714 + // { + // code: dedent` + // interface Foo { + // readonly [key: string]: { + // readonly groups: Array + // } + // }`, + // optionsSet: [[]], + // output: dedent` + // interface Foo { + // readonly [key: string]: { + // readonly groups: ReadonlyArray + // } + // }`, + // errors: [ + // { + // messageId: "aliasShouldBeReadonly", + // type: "Identifier", + // line: 1, + // column: 11, + // }, + // { + // messageId: "typeShouldBeReadonly", + // type: "TSTypeReference", + // line: 3, + // column: 22, + // }, + // ], + // }, // Should fail on Array type as function return type and in local interface. { code: dedent` @@ -131,6 +144,12 @@ const tests: ReadonlyArray = [ } }`, errors: [ + { + messageId: "aliasShouldBeReadonly", + type: "Identifier", + line: 2, + column: 13, + }, { messageId: "typeShouldBeReadonly", type: "TSTypeReference", @@ -155,6 +174,12 @@ const tests: ReadonlyArray = [ } }`, errors: [ + { + messageId: "aliasShouldBeReadonly", + type: "Identifier", + line: 2, + column: 13, + }, { messageId: "typeShouldBeReadonly", type: "TSTypeReference", @@ -163,6 +188,38 @@ const tests: ReadonlyArray = [ }, ], }, + // Should fail on shorthand syntax Array type as return type. + { + code: dedent` + function foo(): number[] { + }`, + optionsSet: [[{ allowMutableReturnType: false }]], + output: dedent` + function foo(): readonly number[] { + }`, + errors: [ + { + messageId: "arrayShouldBeReadonly", + type: "TSArrayType", + line: 1, + column: 17, + }, + ], + }, + // Should fail on shorthand syntax Array type as return type. + { + code: `const foo = (): number[] => {}`, + optionsSet: [[{ allowMutableReturnType: false }]], + output: `const foo = (): readonly number[] => {}`, + errors: [ + { + messageId: "arrayShouldBeReadonly", + type: "TSArrayType", + line: 1, + column: 17, + }, + ], + }, // Should fail inside function. { code: dedent` @@ -320,6 +377,12 @@ const tests: ReadonlyArray = [ optionsSet: [[]], output: `type Foo = ReadonlyArray;`, errors: [ + { + messageId: "aliasShouldBeReadonly", + type: "Identifier", + line: 1, + column: 6, + }, { messageId: "typeShouldBeReadonly", type: "TSTypeReference", @@ -344,6 +407,12 @@ const tests: ReadonlyArray = [ } }`, errors: [ + { + messageId: "aliasShouldBeReadonly", + type: "Identifier", + line: 2, + column: 8, + }, { messageId: "typeShouldBeReadonly", type: "TSTypeReference", @@ -364,6 +433,12 @@ const tests: ReadonlyArray = [ type Foo = ReadonlyArray; }`, errors: [ + { + messageId: "aliasShouldBeReadonly", + type: "Identifier", + line: 2, + column: 8, + }, { messageId: "typeShouldBeReadonly", type: "TSTypeReference", @@ -388,6 +463,12 @@ const tests: ReadonlyArray = [ } }`, errors: [ + { + messageId: "aliasShouldBeReadonly", + type: "Identifier", + line: 2, + column: 8, + }, { messageId: "typeShouldBeReadonly", type: "TSTypeReference", @@ -455,12 +536,10 @@ const tests: ReadonlyArray = [ // Should fail on implicit Array type in variable declaration. { code: dedent` - const foo = [1, 2, 3] - function bar(param = [1, 2, 3]) {}`, + const foo = [1, 2, 3]`, optionsSet: [[{ checkForImplicitMutableArrays: true }]], output: dedent` - const foo: readonly unknown[] = [1, 2, 3] - function bar(param: readonly unknown[] = [1, 2, 3]) {}`, + const foo: readonly unknown[] = [1, 2, 3]`, errors: [ { messageId: "arrayShouldBeReadonly", @@ -468,10 +547,20 @@ const tests: ReadonlyArray = [ line: 1, column: 7, }, + ], + }, + // Should fail on implicit Array type in function declaration. + { + code: dedent` + function bar(param = [1, 2, 3]) {}`, + optionsSet: [[{ checkForImplicitMutableArrays: true }]], + output: dedent` + function bar(param: readonly unknown[] = [1, 2, 3]) {}`, + errors: [ { messageId: "arrayShouldBeReadonly", type: "AssignmentPattern", - line: 2, + line: 1, column: 14, }, ], @@ -575,9 +664,15 @@ const tests: ReadonlyArray = [ readonly [key: string]: string } interface Bar { - readonly [key: string]: { readonly prop: string } + readonly [key: string]: { prop: string } }`, errors: [ + { + messageId: "aliasShouldBeReadonly", + type: "Identifier", + line: 1, + column: 11, + }, { messageId: "propertyShouldBeReadonly", type: "TSIndexSignature", @@ -585,16 +680,16 @@ const tests: ReadonlyArray = [ column: 3, }, { - messageId: "propertyShouldBeReadonly", - type: "TSIndexSignature", - line: 5, - column: 3, + messageId: "aliasShouldBeReadonly", + type: "Identifier", + line: 4, + column: 11, }, { messageId: "propertyShouldBeReadonly", - type: "TSPropertySignature", + type: "TSIndexSignature", line: 5, - column: 20, + column: 3, }, ], }, @@ -646,6 +741,12 @@ const tests: ReadonlyArray = [ readonly code: string, }>;`, errors: [ + { + messageId: "aliasShouldBeReadonly", + type: "Identifier", + line: 1, + column: 6, + }, { messageId: "propertyShouldBeReadonly", type: "TSPropertySignature", @@ -832,6 +933,12 @@ const tests: ReadonlyArray = [ readonly [propertyName]: string; };`, errors: [ + { + messageId: "aliasShouldBeReadonly", + type: "Identifier", + line: 2, + column: 6, + }, { messageId: "propertyShouldBeReadonly", type: "TSPropertySignature", @@ -1088,6 +1195,131 @@ const tests: ReadonlyArray = [ }, ], }, + // Readonly types should not be mutable. + { + code: dedent` + type MyType = { + a: string; + };`, + optionsSet: [[]], + output: dedent` + type MyType = { + readonly a: string; + };`, + errors: [ + { + messageId: "aliasShouldBeReadonly", + type: "Identifier", + line: 1, + column: 6, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSPropertySignature", + line: 2, + column: 3, + }, + ], + }, + // Mutable types should not be readonly. + { + code: dedent` + type MyType = { + readonly a: string; + };`, + optionsSet: [ + [ + { + aliases: { + mustBeReadonly: { + requireOthersToBeMutable: true, + }, + mustBeMutable: { + requireOthersToBeReadonly: false, + }, + }, + }, + ], + ], + errors: [ + { + messageId: "aliasShouldBeMutable", + type: "Identifier", + line: 1, + column: 6, + }, + ], + }, + // Mutable types should not be readonly. + { + code: dedent` + type MutableMyType = { + readonly a: string; + };`, + optionsSet: [[]], + errors: [ + { + messageId: "aliasShouldBeMutable", + type: "Identifier", + line: 1, + column: 6, + }, + ], + }, + // Needs Explicit Marking. + { + code: dedent` + type MyType = {};`, + optionsSet: [ + [ + { + aliases: { + mustBeReadonly: { + requireOthersToBeMutable: true, + }, + mustBeMutable: { + requireOthersToBeReadonly: true, + }, + }, + }, + ], + ], + errors: [ + { + messageId: "aliasNeedsExplicitMarking", + type: "Identifier", + line: 1, + column: 6, + }, + ], + }, + // Both Mutable and Readonly error. + { + code: dedent` + type MyType = {};`, + optionsSet: [ + [ + { + aliases: { + mustBeReadonly: { + pattern: ".*", + }, + mustBeMutable: { + pattern: ".*", + }, + }, + }, + ], + ], + errors: [ + { + messageId: "aliasConfigErrorMutableReadonly", + type: "Identifier", + line: 1, + column: 6, + }, + ], + }, ]; export default tests; diff --git a/tests/rules/prefer-readonly-type/ts/valid.ts b/tests/rules/prefer-readonly-type/ts/valid.ts index 4bee1a203..b8bd599a2 100644 --- a/tests/rules/prefer-readonly-type/ts/valid.ts +++ b/tests/rules/prefer-readonly-type/ts/valid.ts @@ -199,14 +199,21 @@ const tests: ReadonlyArray = [ }, // CallSignature and MethodSignature cannot have readonly modifiers and should // not produce failures. - { - code: dedent` - interface Foo { - (): void - foo(): void - }`, - optionsSet: [[]], - }, + // Waiting on https://github.com/typescript-eslint/typescript-eslint/issues/1758 + // { + // code: dedent` + // interface Foo { + // (): void + // foo(): void + // }`, + // optionsSet: [ + // [ + // { + // treatMethodsAsReadonly: true, + // }, + // ], + // ], + // }, // The literal with indexer with readonly modifier should not produce failures. { code: `let foo: { readonly [key: string]: number };`, @@ -347,10 +354,6 @@ const tests: ReadonlyArray = [ optionsSet: [[{ ignorePattern: "^mutable" }]], }, // Ignore Mutable Collections (Array, Tuple, Set, Map) - { - code: dedent`type Foo = Array;`, - optionsSet: [[{ ignoreCollections: true }]], - }, { code: dedent`const Foo: number[] = [];`, optionsSet: [[{ ignoreCollections: true }]], @@ -361,10 +364,6 @@ const tests: ReadonlyArray = [ [{ ignoreCollections: true, checkForImplicitMutableArrays: true }], ], }, - { - code: dedent`type Foo = [string, string];`, - optionsSet: [[{ ignoreCollections: true }]], - }, { code: dedent`const Foo: [string, string] = ['foo', 'bar'];`, optionsSet: [[{ ignoreCollections: true }]], @@ -376,20 +375,102 @@ const tests: ReadonlyArray = [ ], }, { - code: dedent`type Foo = Set;`, + code: dedent`const Foo: Set = new Set();`, optionsSet: [[{ ignoreCollections: true }]], }, { - code: dedent`const Foo: Set = new Set();`, + code: dedent`const Foo: Map = new Map();`, optionsSet: [[{ ignoreCollections: true }]], }, + // Readonly types should be readonly. { - code: dedent`type Foo = Map;`, - optionsSet: [[{ ignoreCollections: true }]], + code: dedent` + type MyType = { + readonly a: string; + };`, + optionsSet: [[]], }, { - code: dedent`const Foo: Map = new Map();`, - optionsSet: [[{ ignoreCollections: true }]], + code: dedent` + type ReadonlyMyType = { + readonly a: string; + };`, + optionsSet: [ + [ + { + aliases: { + mustBeReadonly: { + requireOthersToBeMutable: true, + }, + mustBeMutable: { + requireOthersToBeReadonly: false, + }, + }, + }, + ], + ], + }, + // Readonly types should be readonly and mutable types mutable. + { + code: dedent` + type MutableMyType = { + a: string; + }; + type MyType = Readonly;`, + optionsSet: [[]], + }, + { + code: dedent` + type MyType = { + a: string; + }; + type ReadonlyMyType = Readonly;`, + optionsSet: [ + [ + { + aliases: { + mustBeReadonly: { + requireOthersToBeMutable: true, + }, + mustBeMutable: { + requireOthersToBeReadonly: false, + }, + }, + }, + ], + ], + }, + // Readonly types should be readonly and mutable types mutable. + { + code: dedent` + type Mutable = { -readonly[P in keyof T]: T[P] }; + type MyType = { + readonly a: string; + }; + type MutableMyType = Mutable;`, + optionsSet: [[]], + }, + { + code: dedent` + type Mutable = { -readonly[P in keyof T]: T[P] }; + type ReadonlyMyType = { + readonly a: string; + }; + type MyType = Mutable;`, + optionsSet: [ + [ + { + aliases: { + mustBeReadonly: { + requireOthersToBeMutable: true, + }, + mustBeMutable: { + requireOthersToBeReadonly: false, + }, + }, + }, + ], + ], }, ];