diff --git a/src/rules/prefer-readonly-type-alias.ts b/src/rules/prefer-readonly-type-alias.ts index 137a102da..ac6143ecd 100644 --- a/src/rules/prefer-readonly-type-alias.ts +++ b/src/rules/prefer-readonly-type-alias.ts @@ -3,12 +3,15 @@ import type { JSONSchema4 } from "json-schema"; import type { RuleContext, RuleMetaData, RuleResult } from "~/util/rule"; import { createRule, isReadonly } from "~/util/rule"; -import { getParentTypeAliasDeclaration } from "~/util/tree"; +import { getAncestorOfType } from "~/util/tree"; import { isIdentifier, isTSArrayType, + isTSIndexSignature, + isTSInterfaceDeclaration, isTSParameterProperty, isTSTupleType, + isTSTypeAliasDeclaration, isTSTypeOperator, } from "~/util/typeguard"; @@ -32,6 +35,7 @@ type Options = { readonly requireOthersToBeReadonly: boolean; }; readonly blacklist: ReadonlyArray; + readonly ignoreInterface: boolean; }; // The schema for the rule options. @@ -75,6 +79,9 @@ const schema: JSONSchema4 = [ type: "string", }, }, + ignoreInterface: { + type: "boolean", + }, }, additionalProperties: false, }, @@ -83,14 +90,15 @@ const schema: JSONSchema4 = [ // The default options for the rule. const defaultOptions: Options = { mustBeReadonly: { - pattern: "^Readonly", + pattern: "^(I?)Readonly", requireOthersToBeMutable: false, }, mustBeMutable: { - pattern: "^Mutable", + pattern: "^(I?)Mutable", requireOthersToBeReadonly: true, }, blacklist: ["^Mutable$"], + ignoreInterface: false, }; // The possible error messages. @@ -130,7 +138,7 @@ const mutableToImmutableTypes: ReadonlyMap = new Map< ["Set", "ReadonlySet"], ]); -enum TypeAliasDeclarationDetails { +enum TypeReadonlynessDetails { ERROR_MUTABLE_READONLY, NEEDS_EXPLICIT_MARKING, IGNORE, @@ -140,26 +148,40 @@ enum TypeAliasDeclarationDetails { READONLY_NOT_OK, } -const cachedTypeAliasDeclarationsDetails = new WeakMap< - TSESTree.TSTypeAliasDeclaration, - TypeAliasDeclarationDetails +const cachedDetails = new WeakMap< + TSESTree.TSInterfaceDeclaration | TSESTree.TSTypeAliasDeclaration, + TypeReadonlynessDetails >(); /** * Get the details for the given type alias. */ function getTypeAliasDeclarationDetails( - node: TSESTree.TSTypeAliasDeclaration, + node: TSESTree.Node, context: RuleContext, options: Options -): TypeAliasDeclarationDetails { - const cached = cachedTypeAliasDeclarationsDetails.get(node); +): TypeReadonlynessDetails { + const typeDeclaration = getTypeDeclaration(node); + if (typeDeclaration === null) { + return TypeReadonlynessDetails.IGNORE; + } + + const indexSignature = getParentIndexSignature(node); + if (indexSignature !== null && getTypeDeclaration(indexSignature) !== null) { + return TypeReadonlynessDetails.IGNORE; + } + + const cached = cachedDetails.get(typeDeclaration); if (cached !== undefined) { return cached; } - const result = getTypeAliasDeclarationDetailsInternal(node, context, options); - cachedTypeAliasDeclarationsDetails.set(node, result); + const result = getTypeAliasDeclarationDetailsInternal( + typeDeclaration, + context, + options + ); + cachedDetails.set(typeDeclaration, result); return result; } @@ -167,10 +189,10 @@ function getTypeAliasDeclarationDetails( * Get the details for the given type alias. */ function getTypeAliasDeclarationDetailsInternal( - node: TSESTree.TSTypeAliasDeclaration, + node: TSESTree.TSInterfaceDeclaration | TSESTree.TSTypeAliasDeclaration, context: RuleContext, options: Options -): TypeAliasDeclarationDetails { +): TypeReadonlynessDetails { const blacklistPatterns = ( Array.isArray(options.blacklist) ? options.blacklist : [options.blacklist] ).map((pattern) => new RegExp(pattern, "u")); @@ -180,7 +202,7 @@ function getTypeAliasDeclarationDetailsInternal( ); if (blacklisted) { - return TypeAliasDeclarationDetails.IGNORE; + return TypeReadonlynessDetails.IGNORE; } const mustBeReadonlyPatterns = ( @@ -203,7 +225,7 @@ function getTypeAliasDeclarationDetailsInternal( ); if (patternStatesReadonly && patternStatesMutable) { - return TypeAliasDeclarationDetails.ERROR_MUTABLE_READONLY; + return TypeReadonlynessDetails.ERROR_MUTABLE_READONLY; } if ( @@ -212,7 +234,7 @@ function getTypeAliasDeclarationDetailsInternal( options.mustBeReadonly.requireOthersToBeMutable && options.mustBeMutable.requireOthersToBeReadonly ) { - return TypeAliasDeclarationDetails.NEEDS_EXPLICIT_MARKING; + return TypeReadonlynessDetails.NEEDS_EXPLICIT_MARKING; } const requiredReadonlyness = @@ -226,34 +248,44 @@ function getTypeAliasDeclarationDetailsInternal( : RequiredReadonlyness.EITHER; if (requiredReadonlyness === RequiredReadonlyness.EITHER) { - return TypeAliasDeclarationDetails.IGNORE; + return TypeReadonlynessDetails.IGNORE; } - const readonly = isReadonly(node.typeAnnotation, context); + const readonly = isReadonly( + isTSTypeAliasDeclaration(node) ? node.typeAnnotation : node.body, + context + ); if (requiredReadonlyness === RequiredReadonlyness.MUTABLE) { return readonly - ? TypeAliasDeclarationDetails.MUTABLE_NOT_OK - : TypeAliasDeclarationDetails.MUTABLE_OK; + ? TypeReadonlynessDetails.MUTABLE_NOT_OK + : TypeReadonlynessDetails.MUTABLE_OK; } return readonly - ? TypeAliasDeclarationDetails.READONLY_OK - : TypeAliasDeclarationDetails.READONLY_NOT_OK; + ? TypeReadonlynessDetails.READONLY_OK + : TypeReadonlynessDetails.READONLY_NOT_OK; } /** * Check if the given TypeReference violates this rule. */ function checkTypeAliasDeclaration( - node: TSESTree.TSTypeAliasDeclaration, + node: TSESTree.TSInterfaceDeclaration | TSESTree.TSTypeAliasDeclaration, context: RuleContext, options: Options ): RuleResult { + if (options.ignoreInterface && isTSInterfaceDeclaration(node)) { + return { + context, + descriptors: [], + }; + } + const details = getTypeAliasDeclarationDetails(node, context, options); switch (details) { - case TypeAliasDeclarationDetails.NEEDS_EXPLICIT_MARKING: { + case TypeReadonlynessDetails.NEEDS_EXPLICIT_MARKING: { return { context, descriptors: [ @@ -264,7 +296,7 @@ function checkTypeAliasDeclaration( ], }; } - case TypeAliasDeclarationDetails.ERROR_MUTABLE_READONLY: { + case TypeReadonlynessDetails.ERROR_MUTABLE_READONLY: { return { context, descriptors: [ @@ -275,7 +307,7 @@ function checkTypeAliasDeclaration( ], }; } - case TypeAliasDeclarationDetails.MUTABLE_NOT_OK: { + case TypeReadonlynessDetails.MUTABLE_NOT_OK: { return { context, descriptors: [ @@ -286,7 +318,7 @@ function checkTypeAliasDeclaration( ], }; } - case TypeAliasDeclarationDetails.READONLY_NOT_OK: { + case TypeReadonlynessDetails.READONLY_NOT_OK: { return { context, descriptors: [ @@ -314,19 +346,10 @@ function checkArrayOrTupleType( context: RuleContext, options: Options ): RuleResult { - const typeAlias = getParentTypeAliasDeclaration(node); - - if (typeAlias === null) { - return { - context, - descriptors: [], - }; - } - - const details = getTypeAliasDeclarationDetails(typeAlias, context, options); + const details = getTypeAliasDeclarationDetails(node, context, options); switch (details) { - case TypeAliasDeclarationDetails.READONLY_NOT_OK: { + case TypeReadonlynessDetails.READONLY_NOT_OK: { return { context, descriptors: @@ -368,19 +391,10 @@ function checkMappedType( context: RuleContext, options: Options ): RuleResult { - const typeAlias = getParentTypeAliasDeclaration(node); - - if (typeAlias === null) { - return { - context, - descriptors: [], - }; - } - - const details = getTypeAliasDeclarationDetails(typeAlias, context, options); + const details = getTypeAliasDeclarationDetails(node, context, options); switch (details) { - case TypeAliasDeclarationDetails.READONLY_NOT_OK: { + case TypeReadonlynessDetails.READONLY_NOT_OK: { return { context, descriptors: @@ -423,19 +437,10 @@ function checkTypeReference( }; } - const typeAlias = getParentTypeAliasDeclaration(node); - - if (typeAlias === null) { - return { - context, - descriptors: [], - }; - } - - const details = getTypeAliasDeclarationDetails(typeAlias, context, options); + const details = getTypeAliasDeclarationDetails(node, context, options); switch (details) { - case TypeAliasDeclarationDetails.READONLY_NOT_OK: { + case TypeReadonlynessDetails.READONLY_NOT_OK: { const immutableType = mutableToImmutableTypes.get(node.typeName.name); return { @@ -473,19 +478,10 @@ function checkProperty( context: RuleContext, options: Options ): RuleResult { - const typeAlias = getParentTypeAliasDeclaration(node); - - if (typeAlias === null) { - return { - context, - descriptors: [], - }; - } - - const details = getTypeAliasDeclarationDetails(typeAlias, context, options); + const details = getTypeAliasDeclarationDetails(node, context, options); switch (details) { - case TypeAliasDeclarationDetails.READONLY_NOT_OK: { + case TypeReadonlynessDetails.READONLY_NOT_OK: { return { context, descriptors: @@ -512,6 +508,45 @@ function checkProperty( } } +/** + * Get the type alias or interface that the given node is in. + */ +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. + */ +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; +} + // Create the rule. export const rule = createRule( name, @@ -520,6 +555,7 @@ export const rule = createRule( { TSArrayType: checkArrayOrTupleType, TSIndexSignature: checkProperty, + TSInterfaceDeclaration: checkTypeAliasDeclaration, TSMappedType: checkMappedType, TSParameterProperty: checkProperty, TSPropertySignature: checkProperty, diff --git a/src/util/tree.ts b/src/util/tree.ts index 621e7bb21..f0e7e1f01 100644 --- a/src/util/tree.ts +++ b/src/util/tree.ts @@ -10,13 +10,12 @@ import { isMethodDefinition, isProperty, isTSInterfaceBody, - isTSTypeAliasDeclaration, } from "./typeguard"; /** * Return the first ancestor that meets the given check criteria. */ -function getAncestorOfType( +export function getAncestorOfType( checker: (node: TSESTree.Node, child: TSESTree.Node | null) => node is T, node: TSESTree.Node, child: TSESTree.Node | null = null @@ -82,22 +81,6 @@ export function isInReturnType(node: TSESTree.Node): boolean { ); } -/** - * Is the given node in a type alias. - */ -export function getParentTypeAliasDeclaration( - node: TSESTree.Node -): TSESTree.TSTypeAliasDeclaration | null { - return (getAncestorOfType( - (n): n is TSESTree.Node => - n.parent !== undefined && - n.parent !== null && - isTSTypeAliasDeclaration(n.parent) && - n.parent.typeAnnotation === n, - node - )?.parent ?? null) as TSESTree.TSTypeAliasDeclaration | null; -} - /** * Is the given identifier a property of an object? */ diff --git a/src/util/typeguard.ts b/src/util/typeguard.ts index 79769eaa2..30f9d5671 100644 --- a/src/util/typeguard.ts +++ b/src/util/typeguard.ts @@ -204,6 +204,12 @@ 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 { diff --git a/tests/rules/prefer-readonly-type-alias/ts/invalid.ts b/tests/rules/prefer-readonly-type-alias/ts/invalid.ts index 0810115d7..d000a5409 100644 --- a/tests/rules/prefer-readonly-type-alias/ts/invalid.ts +++ b/tests/rules/prefer-readonly-type-alias/ts/invalid.ts @@ -137,7 +137,7 @@ const tests: ReadonlyArray = [ readonly [key: string]: string } type MyType2 = { - readonly [key: string]: { readonly prop: string } + readonly [key: string]: { prop: string } }`, errors: [ { @@ -164,12 +164,6 @@ const tests: ReadonlyArray = [ line: 5, column: 3, }, - { - messageId: "propertyShouldBeReadonly", - type: "TSPropertySignature", - line: 5, - column: 20, - }, ], }, // Type literal in property template parameter without readonly should produce failures. @@ -388,6 +382,232 @@ const tests: ReadonlyArray = [ }, ], }, + // Tuples. + { + code: dedent` + type MyType = [number, string];`, + optionsSet: [[]], + output: dedent` + type MyType = readonly [number, string];`, + errors: [ + { + messageId: "typeAliasShouldBeReadonly", + type: "Identifier", + line: 1, + column: 6, + }, + { + messageId: "tupleShouldBeReadonly", + type: "TSTupleType", + line: 1, + column: 15, + }, + ], + }, + // Should fail on Array type in interface. + { + code: dedent` + interface Foo { + readonly bar: Array + }`, + optionsSet: [[]], + output: dedent` + interface Foo { + readonly bar: ReadonlyArray + }`, + errors: [ + { + messageId: "typeAliasShouldBeReadonly", + type: "Identifier", + line: 1, + column: 11, + }, + { + messageId: "typeShouldBeReadonly", + type: "TSTypeReference", + line: 2, + column: 17, + }, + ], + }, + // Should fail on mutable index signature but not on index signature internals. + // https://github.com/typescript-eslint/typescript-eslint/issues/3714 + { + code: dedent` + interface Foo { + [key: string]: { + readonly groups: Array + } + }`, + optionsSet: [[]], + output: dedent` + interface Foo { + readonly [key: string]: { + readonly groups: Array + } + }`, + errors: [ + { + messageId: "typeAliasShouldBeReadonly", + type: "Identifier", + line: 1, + column: 11, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSIndexSignature", + line: 2, + column: 3, + }, + ], + }, + // Interface Index Signatures. + { + code: dedent` + interface Foo { + [key: string]: string + } + interface Bar { + [key: string]: { prop: string } + }`, + optionsSet: [[]], + output: dedent` + interface Foo { + readonly [key: string]: string + } + interface Bar { + readonly [key: string]: { prop: string } + }`, + errors: [ + { + messageId: "typeAliasShouldBeReadonly", + type: "Identifier", + line: 1, + column: 11, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSIndexSignature", + line: 2, + column: 3, + }, + { + messageId: "typeAliasShouldBeReadonly", + type: "Identifier", + line: 4, + column: 11, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSIndexSignature", + line: 5, + column: 3, + }, + ], + }, + // Type literal without readonly on members should produce failures. + // Also verify that nested members are checked. + { + code: dedent` + type MyType = { + a: number, + b: ReadonlyArray, + c: () => string, + d: { readonly [key: string]: string }, + [key: string]: string, + readonly e: { + a: number, + b: ReadonlyArray, + c: () => string, + d: { readonly [key: string]: string }, + [key: string]: string, + } + };`, + optionsSet: [[]], + output: dedent` + type MyType = { + readonly a: number, + readonly b: ReadonlyArray, + readonly c: () => string, + readonly d: { readonly [key: string]: string }, + readonly [key: string]: string, + readonly e: { + readonly a: number, + readonly b: ReadonlyArray, + readonly c: () => string, + readonly d: { readonly [key: string]: string }, + readonly [key: string]: string, + } + };`, + errors: [ + { + messageId: "typeAliasShouldBeReadonly", + type: "Identifier", + line: 1, + column: 6, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSPropertySignature", + line: 2, + column: 3, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSPropertySignature", + line: 3, + column: 3, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSPropertySignature", + line: 4, + column: 3, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSPropertySignature", + line: 5, + column: 3, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSIndexSignature", + line: 6, + column: 3, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSPropertySignature", + line: 8, + column: 5, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSPropertySignature", + line: 9, + column: 5, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSPropertySignature", + line: 10, + column: 5, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSPropertySignature", + line: 11, + column: 5, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSIndexSignature", + line: 12, + column: 5, + }, + ], + }, ]; export default tests; diff --git a/tests/rules/prefer-readonly-type-alias/ts/valid.ts b/tests/rules/prefer-readonly-type-alias/ts/valid.ts index 252aa630e..a73883fa5 100644 --- a/tests/rules/prefer-readonly-type-alias/ts/valid.ts +++ b/tests/rules/prefer-readonly-type-alias/ts/valid.ts @@ -65,6 +65,88 @@ const tests: ReadonlyArray = [ type MyType = Mutable;`, optionsSet: [optionsReversedDefault], }, + // Readonly Tuple. + { + code: dedent` + type MyType = readonly [number, string, readonly [number, string]];`, + optionsSet: [[]], + }, + // Should not fail on ReadonlyArray type alias. + { + code: `type Foo = ReadonlyArray;`, + optionsSet: [[]], + }, + // Interface with readonly modifiers should not produce failures. + { + code: dedent` + interface Foo { + readonly a: number, + readonly b: ReadonlyArray, + readonly c: () => string, + readonly d: { readonly [key: string]: string }, + readonly [key: string]: string, + }`, + optionsSet: [[]], + }, + // PropertySignature and IndexSignature members without readonly modifier + // should produce failures. Also verify that nested members are checked. + { + code: dedent` + interface Foo { + readonly a: number, + readonly b: ReadonlyArray, + readonly c: () => string, + readonly d: { readonly [key: string]: string }, + readonly [key: string]: string, + readonly e: { + readonly a: number, + readonly b: ReadonlyArray, + readonly c: () => string, + readonly d: { readonly [key: string]: string }, + readonly [key: string]: string, + } + }`, + optionsSet: [[]], + }, + // CallSignature and MethodSignature cannot have readonly modifiers and should + // not produce failures. + // Waiting on https://github.com/typescript-eslint/typescript-eslint/issues/1758 + // { + // code: dedent` + // interface Foo { + // (): void + // foo(): void + // }`, + // optionsSet: [ + // [ + // { + // treatMethodsAsReadonly: true, + // }, + // ], + // ], + // }, + // Type literal in array template parameter with readonly should not produce failures. + { + code: `type foo = ReadonlyArray<{ readonly type: string, readonly code: string }>;`, + optionsSet: [[]], + }, + // Mapped types with readonly on members should not produce failures. + { + code: dedent` + type MyType = (x: { readonly [key in string]: number }) => {}`, + optionsSet: [[]], + }, + // Ignore Interfaces. + { + code: dedent` + interface Foo { + foo: number, + bar: ReadonlyArray, + baz: () => string, + qux: { [key: string]: string } + }`, + optionsSet: [[{ ignoreInterface: true }]], + }, ]; export default tests;