From 236f675f67f3ad50fdce1e1680bb1bbd77c7d0a6 Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Mon, 9 Aug 2021 02:15:58 +1200 Subject: [PATCH] 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, + }, + }, + }, + ], + ], }, ];