From 06f9c782e3da9531848fe81a2106ea4f073f4bb1 Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Sat, 7 Aug 2021 01:41:39 +1200 Subject: [PATCH] feat(prefer-readonly-type-alias): setup fixer --- src/rules/prefer-readonly-type-alias.ts | 380 +++++++++++++++--- src/util/tree.ts | 17 + src/util/typeguard.ts | 6 + .../prefer-readonly-type-alias/ts/invalid.ts | 298 +++++++++++++- 4 files changed, 635 insertions(+), 66 deletions(-) diff --git a/src/rules/prefer-readonly-type-alias.ts b/src/rules/prefer-readonly-type-alias.ts index 815266dde..137a102da 100644 --- a/src/rules/prefer-readonly-type-alias.ts +++ b/src/rules/prefer-readonly-type-alias.ts @@ -3,6 +3,14 @@ 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 { + isIdentifier, + isTSArrayType, + isTSParameterProperty, + isTSTupleType, + isTSTypeOperator, +} from "~/util/typeguard"; // The name of this rule. export const name = "prefer-readonly-type-alias" as const; @@ -87,12 +95,17 @@ const defaultOptions: Options = { // The possible error messages. const errorMessages = { - mutable: "Mutable types should not be fully readonly.", - readonly: "Readonly types should not be mutable at all.", - mutableReadonly: + typeAliasShouldBeMutable: "Mutable types should not be fully readonly.", + typeAliasShouldBeReadonly: "Readonly types should not be mutable at all.", + typeAliasErrorMutableReadonly: "Configuration error - this type must be marked as both readonly and mutable.", - needsExplicitMarking: + typeAliasNeedsExplicitMarking: "Type must be explicity marked as either readonly or mutable.", + propertyShouldBeReadonly: + "A readonly modifier is required for this property.", + typeShouldBeReadonly: "Type should be readonly.", + tupleShouldBeReadonly: "Tuple should be readonly.", + arrayShouldBeReadonly: "Array should be readonly.", } as const; // The meta data for this rule. @@ -108,14 +121,56 @@ const meta: RuleMetaData = { schema, }; +const mutableToImmutableTypes: ReadonlyMap = new Map< + string, + string +>([ + ["Array", "ReadonlyArray"], + ["Map", "ReadonlyMap"], + ["Set", "ReadonlySet"], +]); + +enum TypeAliasDeclarationDetails { + ERROR_MUTABLE_READONLY, + NEEDS_EXPLICIT_MARKING, + IGNORE, + MUTABLE_OK, + MUTABLE_NOT_OK, + READONLY_OK, + READONLY_NOT_OK, +} + +const cachedTypeAliasDeclarationsDetails = new WeakMap< + TSESTree.TSTypeAliasDeclaration, + TypeAliasDeclarationDetails +>(); + /** - * Check if the given TypeReference violates this rule. + * Get the details for the given type alias. */ -function checkTypeAliasDeclaration( +function getTypeAliasDeclarationDetails( node: TSESTree.TSTypeAliasDeclaration, context: RuleContext, options: Options -): RuleResult { +): TypeAliasDeclarationDetails { + const cached = cachedTypeAliasDeclarationsDetails.get(node); + if (cached !== undefined) { + return cached; + } + + const result = getTypeAliasDeclarationDetailsInternal(node, context, options); + cachedTypeAliasDeclarationsDetails.set(node, result); + return result; +} + +/** + * Get the details for the given type alias. + */ +function getTypeAliasDeclarationDetailsInternal( + node: TSESTree.TSTypeAliasDeclaration, + context: RuleContext, + options: Options +): TypeAliasDeclarationDetails { const blacklistPatterns = ( Array.isArray(options.blacklist) ? options.blacklist : [options.blacklist] ).map((pattern) => new RegExp(pattern, "u")); @@ -125,10 +180,7 @@ function checkTypeAliasDeclaration( ); if (blacklisted) { - return { - context, - descriptors: [], - }; + return TypeAliasDeclarationDetails.IGNORE; } const mustBeReadonlyPatterns = ( @@ -151,15 +203,7 @@ function checkTypeAliasDeclaration( ); if (patternStatesReadonly && patternStatesMutable) { - return { - context, - descriptors: [ - { - node: node.id, - messageId: "mutableReadonly", - }, - ], - }; + return TypeAliasDeclarationDetails.ERROR_MUTABLE_READONLY; } if ( @@ -168,15 +212,7 @@ function checkTypeAliasDeclaration( options.mustBeReadonly.requireOthersToBeMutable && options.mustBeMutable.requireOthersToBeReadonly ) { - return { - context, - descriptors: [ - { - node: node.id, - messageId: "needsExplicitMarking", - }, - ], - }; + return TypeAliasDeclarationDetails.NEEDS_EXPLICIT_MARKING; } const requiredReadonlyness = @@ -189,52 +225,291 @@ function checkTypeAliasDeclaration( ? RequiredReadonlyness.MUTABLE : RequiredReadonlyness.EITHER; - return checkRequiredReadonlyness( - node, - context, - options, - requiredReadonlyness - ); + if (requiredReadonlyness === RequiredReadonlyness.EITHER) { + return TypeAliasDeclarationDetails.IGNORE; + } + + const readonly = isReadonly(node.typeAnnotation, context); + + if (requiredReadonlyness === RequiredReadonlyness.MUTABLE) { + return readonly + ? TypeAliasDeclarationDetails.MUTABLE_NOT_OK + : TypeAliasDeclarationDetails.MUTABLE_OK; + } + + return readonly + ? TypeAliasDeclarationDetails.READONLY_OK + : TypeAliasDeclarationDetails.READONLY_NOT_OK; } -function checkRequiredReadonlyness( +/** + * Check if the given TypeReference violates this rule. + */ +function checkTypeAliasDeclaration( node: TSESTree.TSTypeAliasDeclaration, context: RuleContext, - options: Options, - requiredReadonlyness: RequiredReadonlyness + options: Options ): RuleResult { - if (requiredReadonlyness !== RequiredReadonlyness.EITHER) { - const readonly = isReadonly(node.typeAnnotation, context); + const details = getTypeAliasDeclarationDetails(node, context, options); - if (readonly && requiredReadonlyness === RequiredReadonlyness.MUTABLE) { + switch (details) { + case TypeAliasDeclarationDetails.NEEDS_EXPLICIT_MARKING: { return { context, descriptors: [ { node: node.id, - messageId: "readonly", + messageId: "typeAliasNeedsExplicitMarking", }, ], }; } - - if (!readonly && requiredReadonlyness === RequiredReadonlyness.READONLY) { + case TypeAliasDeclarationDetails.ERROR_MUTABLE_READONLY: { return { context, descriptors: [ { node: node.id, - messageId: "mutable", + messageId: "typeAliasErrorMutableReadonly", }, ], }; } + case TypeAliasDeclarationDetails.MUTABLE_NOT_OK: { + return { + context, + descriptors: [ + { + node: node.id, + messageId: "typeAliasShouldBeMutable", + }, + ], + }; + } + case TypeAliasDeclarationDetails.READONLY_NOT_OK: { + return { + context, + descriptors: [ + { + node: node.id, + messageId: "typeAliasShouldBeReadonly", + }, + ], + }; + } + default: { + return { + context, + descriptors: [], + }; + } } +} - return { - context, - descriptors: [], - }; +/** + * Check if the given ArrayType or TupleType violates this rule. + */ +function checkArrayOrTupleType( + node: TSESTree.TSArrayType | TSESTree.TSTupleType, + context: RuleContext, + options: Options +): RuleResult { + const typeAlias = getParentTypeAliasDeclaration(node); + + if (typeAlias === null) { + return { + context, + descriptors: [], + }; + } + + const details = getTypeAliasDeclarationDetails(typeAlias, context, options); + + switch (details) { + case TypeAliasDeclarationDetails.READONLY_NOT_OK: { + return { + context, + descriptors: + node.parent === undefined || + !isTSTypeOperator(node.parent) || + node.parent.operator !== "readonly" + ? [ + { + 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: [], + }; + } + } +} + +/** + * Check if the given TSMappedType violates this rule. + */ +function checkMappedType( + node: TSESTree.TSMappedType, + context: RuleContext, + options: Options +): RuleResult { + const typeAlias = getParentTypeAliasDeclaration(node); + + if (typeAlias === null) { + return { + context, + descriptors: [], + }; + } + + const details = getTypeAliasDeclarationDetails(typeAlias, context, options); + + switch (details) { + case TypeAliasDeclarationDetails.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: [], + }; + } + } +} + +/** + * Check if the given TypeReference violates this rule. + */ +function checkTypeReference( + node: TSESTree.TSTypeReference, + context: RuleContext, + options: Options +): RuleResult { + if (!isIdentifier(node.typeName)) { + return { + context, + descriptors: [], + }; + } + + const typeAlias = getParentTypeAliasDeclaration(node); + + if (typeAlias === null) { + return { + context, + descriptors: [], + }; + } + + const details = getTypeAliasDeclarationDetails(typeAlias, context, options); + + switch (details) { + case TypeAliasDeclarationDetails.READONLY_NOT_OK: { + const immutableType = mutableToImmutableTypes.get(node.typeName.name); + + return { + context, + descriptors: + immutableType === undefined || immutableType.length === 0 + ? [] + : [ + { + node, + messageId: "typeShouldBeReadonly", + fix: (fixer) => + fixer.replaceText(node.typeName, immutableType), + }, + ], + }; + } + default: { + return { + context, + descriptors: [], + }; + } + } +} + +/** + * Check if the given property/signature node violates this rule. + */ +function checkProperty( + node: + | TSESTree.TSIndexSignature + | TSESTree.TSParameterProperty + | TSESTree.TSPropertySignature, + context: RuleContext, + options: Options +): RuleResult { + const typeAlias = getParentTypeAliasDeclaration(node); + + if (typeAlias === null) { + return { + context, + descriptors: [], + }; + } + + const details = getTypeAliasDeclarationDetails(typeAlias, context, options); + + switch (details) { + case TypeAliasDeclarationDetails.READONLY_NOT_OK: { + return { + context, + descriptors: + node.readonly !== true + ? [ + { + node, + messageId: "propertyShouldBeReadonly", + fix: isTSParameterProperty(node) + ? (fixer) => + fixer.insertTextBefore(node.parameter, "readonly ") + : (fixer) => fixer.insertTextBefore(node, "readonly "), + }, + ] + : [], + }; + } + default: { + return { + context, + descriptors: [], + }; + } + } } // Create the rule. @@ -243,6 +518,13 @@ export const rule = createRule( meta, defaultOptions, { + TSArrayType: checkArrayOrTupleType, + TSIndexSignature: checkProperty, + TSMappedType: checkMappedType, + TSParameterProperty: checkProperty, + TSPropertySignature: checkProperty, + TSTupleType: checkArrayOrTupleType, TSTypeAliasDeclaration: checkTypeAliasDeclaration, + TSTypeReference: checkTypeReference, } ); diff --git a/src/util/tree.ts b/src/util/tree.ts index 88cf73dae..621e7bb21 100644 --- a/src/util/tree.ts +++ b/src/util/tree.ts @@ -10,6 +10,7 @@ import { isMethodDefinition, isProperty, isTSInterfaceBody, + isTSTypeAliasDeclaration, } from "./typeguard"; /** @@ -81,6 +82,22 @@ 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 0b7ff9e0f..79769eaa2 100644 --- a/src/util/typeguard.ts +++ b/src/util/typeguard.ts @@ -210,6 +210,12 @@ export function isTSInterfaceBody( 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-alias/ts/invalid.ts b/tests/rules/prefer-readonly-type-alias/ts/invalid.ts index 2940f122b..0810115d7 100644 --- a/tests/rules/prefer-readonly-type-alias/ts/invalid.ts +++ b/tests/rules/prefer-readonly-type-alias/ts/invalid.ts @@ -21,17 +21,23 @@ const tests: ReadonlyArray = [ a: string; };`, optionsSet: [[]], - // output: dedent` - // type MyType = { - // readonly a: string; - // };`, + output: dedent` + type MyType = { + readonly a: string; + };`, errors: [ { - messageId: "mutable", + messageId: "typeAliasShouldBeReadonly", type: "Identifier", line: 1, column: 6, }, + { + messageId: "propertyShouldBeReadonly", + type: "TSPropertySignature", + line: 2, + column: 3, + }, ], }, // Mutable types should not be readonly. @@ -41,13 +47,9 @@ const tests: ReadonlyArray = [ readonly a: string; };`, optionsSet: [optionsReversedDefault], - // output: dedent` - // type MyType = { - // a: string; - // };`, errors: [ { - messageId: "readonly", + messageId: "typeAliasShouldBeMutable", type: "Identifier", line: 1, column: 6, @@ -61,13 +63,9 @@ const tests: ReadonlyArray = [ readonly a: string; };`, optionsSet: [[]], - // output: dedent` - // type MutableMyType = { - // a: string; - // };`, errors: [ { - messageId: "readonly", + messageId: "typeAliasShouldBeMutable", type: "Identifier", line: 1, column: 6, @@ -92,7 +90,7 @@ const tests: ReadonlyArray = [ ], errors: [ { - messageId: "needsExplicitMarking", + messageId: "typeAliasNeedsExplicitMarking", type: "Identifier", line: 1, column: 6, @@ -117,11 +115,277 @@ const tests: ReadonlyArray = [ ], errors: [ { - messageId: "mutableReadonly", + messageId: "typeAliasErrorMutableReadonly", + type: "Identifier", + line: 1, + column: 6, + }, + ], + }, + // Index Signatures. + { + code: dedent` + type MyType1 = { + [key: string]: string + } + type MyType2 = { + [key: string]: { prop: string } + }`, + optionsSet: [[]], + output: dedent` + type MyType1 = { + readonly [key: string]: string + } + type MyType2 = { + readonly [key: string]: { readonly prop: string } + }`, + errors: [ + { + messageId: "typeAliasShouldBeReadonly", type: "Identifier", line: 1, column: 6, }, + { + messageId: "propertyShouldBeReadonly", + type: "TSIndexSignature", + line: 2, + column: 3, + }, + { + messageId: "typeAliasShouldBeReadonly", + type: "Identifier", + line: 4, + column: 6, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSIndexSignature", + line: 5, + column: 3, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSPropertySignature", + line: 5, + column: 20, + }, + ], + }, + // Type literal in property template parameter without readonly should produce failures. + { + code: dedent` + type MyType = ReadonlyArray<{ + type: string, + code: string, + }>;`, + optionsSet: [[]], + output: dedent` + type MyType = ReadonlyArray<{ + readonly type: string, + readonly code: 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, + }, + ], + }, + // Computed properties. + { + code: dedent` + const propertyName = 'myProperty'; + type MyType = { + [propertyName]: string; + };`, + optionsSet: [[]], + output: dedent` + const propertyName = 'myProperty'; + type MyType = { + readonly [propertyName]: string; + };`, + errors: [ + { + messageId: "typeAliasShouldBeReadonly", + type: "Identifier", + line: 2, + column: 6, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSPropertySignature", + line: 3, + column: 3, + }, + ], + }, + // Mapped type without readonly. + { + code: dedent` + type MyType = { [key in string]: number }`, + optionsSet: [[]], + output: dedent` + type MyType = { readonly [key in string]: number }`, + errors: [ + { + messageId: "typeAliasShouldBeReadonly", + type: "Identifier", + line: 1, + column: 6, + }, + { + messageId: "propertyShouldBeReadonly", + type: "TSMappedType", + line: 1, + column: 15, + }, + ], + }, + // Should fail on array in type alias. + { + code: `type MyType = string[];`, + optionsSet: [[]], + output: `type MyType = readonly string[];`, + errors: [ + { + messageId: "typeAliasShouldBeReadonly", + type: "Identifier", + line: 1, + column: 6, + }, + { + messageId: "arrayShouldBeReadonly", + type: "TSArrayType", + line: 1, + column: 15, + }, + ], + }, + // Should fail on array as type member. + { + code: dedent` + type MyType = { + readonly bar: string[] + }`, + optionsSet: [[]], + output: dedent` + type MyType = { + readonly bar: readonly string[] + }`, + errors: [ + { + messageId: "typeAliasShouldBeReadonly", + type: "Identifier", + line: 1, + column: 6, + }, + { + messageId: "arrayShouldBeReadonly", + type: "TSArrayType", + line: 2, + column: 17, + }, + ], + }, + // Should fail on array type being used as template param. + { + code: `type MyType = Promise;`, + optionsSet: [[]], + output: `type MyType = Promise;`, + errors: [ + { + messageId: "typeAliasShouldBeReadonly", + type: "Identifier", + line: 1, + column: 6, + }, + { + messageId: "arrayShouldBeReadonly", + type: "TSArrayType", + line: 1, + column: 23, + }, + ], + }, + // Should fail on Array type alias. + { + code: `type MyType = Array;`, + optionsSet: [[]], + output: `type MyType = ReadonlyArray;`, + errors: [ + { + messageId: "typeAliasShouldBeReadonly", + type: "Identifier", + line: 1, + column: 6, + }, + { + messageId: "typeShouldBeReadonly", + type: "TSTypeReference", + line: 1, + column: 15, + }, + ], + }, + // Should fail on Array as type member. + { + code: dedent` + type MyType = { + readonly bar: Array + }`, + optionsSet: [[]], + output: dedent` + type MyType = { + readonly bar: ReadonlyArray + }`, + errors: [ + { + messageId: "typeAliasShouldBeReadonly", + type: "Identifier", + line: 1, + column: 6, + }, + { + messageId: "typeShouldBeReadonly", + type: "TSTypeReference", + line: 2, + column: 17, + }, + ], + }, + // Should fail on Array type being used as template param. + { + code: `type MyType = Promise>;`, + optionsSet: [[]], + output: `type MyType = Promise>;`, + errors: [ + { + messageId: "typeAliasShouldBeReadonly", + type: "Identifier", + line: 1, + column: 6, + }, + { + messageId: "typeShouldBeReadonly", + type: "TSTypeReference", + line: 1, + column: 23, + }, ], }, ];