From c146fdae6d6b8dccf3a217c564f5466251a9d4ec Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Wed, 4 Aug 2021 21:24:29 +1200 Subject: [PATCH 1/3] feat(prefer-readonly-type-alias): initial creation of rule --- src/configs/all.ts | 1 + src/configs/no-mutations.ts | 1 + src/rules/index.ts | 2 + src/rules/prefer-readonly-type-alias.ts | 248 ++++++++++++++++++ src/util/rule.ts | 15 ++ .../prefer-readonly-type-alias/index.test.ts | 6 + .../prefer-readonly-type-alias/ts/index.ts | 7 + .../prefer-readonly-type-alias/ts/invalid.ts | 129 +++++++++ .../prefer-readonly-type-alias/ts/valid.ts | 70 +++++ 9 files changed, 479 insertions(+) create mode 100644 src/rules/prefer-readonly-type-alias.ts create mode 100644 tests/rules/prefer-readonly-type-alias/index.test.ts create mode 100644 tests/rules/prefer-readonly-type-alias/ts/index.ts create mode 100644 tests/rules/prefer-readonly-type-alias/ts/invalid.ts create mode 100644 tests/rules/prefer-readonly-type-alias/ts/valid.ts diff --git a/src/configs/all.ts b/src/configs/all.ts index 0db73d9d0..e4f73acaa 100644 --- a/src/configs/all.ts +++ b/src/configs/all.ts @@ -22,6 +22,7 @@ const config: Linter.Config = { "functional/no-method-signature": "error", "functional/no-mixed-type": "error", "functional/prefer-readonly-type": "error", + "functional/prefer-readonly-type-alias": "error", "functional/prefer-tacit": ["error", { assumeTypes: false }], "functional/no-return-void": "error", }, diff --git a/src/configs/no-mutations.ts b/src/configs/no-mutations.ts index bc5cc4600..8509943a4 100644 --- a/src/configs/no-mutations.ts +++ b/src/configs/no-mutations.ts @@ -11,6 +11,7 @@ const config: Linter.Config = { rules: { "functional/no-method-signature": "warn", "functional/prefer-readonly-type": "error", + "functional/prefer-readonly-type-alias": "error", }, }, ], diff --git a/src/rules/index.ts b/src/rules/index.ts index 06eb7baef..9ed1cb619 100644 --- a/src/rules/index.ts +++ b/src/rules/index.ts @@ -13,6 +13,7 @@ import * as noThisExpression from "./no-this-expression"; import * as noThrowStatement from "./no-throw-statement"; import * as noTryStatement from "./no-try-statement"; import * as preferReadonlyTypes from "./prefer-readonly-type"; +import * as preferReadonlyTypeAlias from "./prefer-readonly-type-alias"; import * as preferTacit from "./prefer-tacit"; /** @@ -34,5 +35,6 @@ export const rules = { [noThrowStatement.name]: noThrowStatement.rule, [noTryStatement.name]: noTryStatement.rule, [preferReadonlyTypes.name]: preferReadonlyTypes.rule, + [preferReadonlyTypeAlias.name]: preferReadonlyTypeAlias.rule, [preferTacit.name]: preferTacit.rule, }; diff --git a/src/rules/prefer-readonly-type-alias.ts b/src/rules/prefer-readonly-type-alias.ts new file mode 100644 index 000000000..815266dde --- /dev/null +++ b/src/rules/prefer-readonly-type-alias.ts @@ -0,0 +1,248 @@ +import type { TSESTree } from "@typescript-eslint/experimental-utils"; +import type { JSONSchema4 } from "json-schema"; + +import type { RuleContext, RuleMetaData, RuleResult } from "~/util/rule"; +import { createRule, isReadonly } from "~/util/rule"; + +// The name of this rule. +export const name = "prefer-readonly-type-alias" as const; + +const enum RequiredReadonlyness { + READONLY, + MUTABLE, + EITHER, +} + +// The options this rule can take. +type Options = { + readonly mustBeReadonly: { + readonly pattern: ReadonlyArray | string; + readonly requireOthersToBeMutable: boolean; + }; + readonly mustBeMutable: { + readonly pattern: ReadonlyArray | string; + readonly requireOthersToBeReadonly: boolean; + }; + readonly blacklist: ReadonlyArray; +}; + +// The schema for the rule options. +const schema: JSONSchema4 = [ + { + 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", + }, + }, + }, + additionalProperties: false, + }, +]; + +// The default options for the rule. +const defaultOptions: Options = { + mustBeReadonly: { + pattern: "^Readonly", + requireOthersToBeMutable: false, + }, + mustBeMutable: { + pattern: "^Mutable", + requireOthersToBeReadonly: true, + }, + blacklist: ["^Mutable$"], +}; + +// The possible error messages. +const errorMessages = { + mutable: "Mutable types should not be fully readonly.", + readonly: "Readonly types should not be mutable at all.", + mutableReadonly: + "Configuration error - this type must be marked as both readonly and mutable.", + needsExplicitMarking: + "Type must be explicity marked as either readonly or mutable.", +} as const; + +// The meta data for this rule. +const meta: RuleMetaData = { + type: "suggestion", + docs: { + description: "Prefer readonly type alias over mutable one.", + category: "Best Practices", + recommended: "error", + }, + messages: errorMessages, + fixable: "code", + schema, +}; + +/** + * Check if the given TypeReference violates this rule. + */ +function checkTypeAliasDeclaration( + node: TSESTree.TSTypeAliasDeclaration, + context: RuleContext, + options: Options +): RuleResult { + const blacklistPatterns = ( + Array.isArray(options.blacklist) ? options.blacklist : [options.blacklist] + ).map((pattern) => new RegExp(pattern, "u")); + + const blacklisted = blacklistPatterns.some((pattern) => + pattern.test(node.id.name) + ); + + if (blacklisted) { + return { + context, + descriptors: [], + }; + } + + const mustBeReadonlyPatterns = ( + Array.isArray(options.mustBeReadonly.pattern) + ? options.mustBeReadonly.pattern + : [options.mustBeReadonly.pattern] + ).map((pattern) => new RegExp(pattern, "u")); + + const mustBeMutablePatterns = ( + Array.isArray(options.mustBeMutable.pattern) + ? options.mustBeMutable.pattern + : [options.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 { + context, + descriptors: [ + { + node: node.id, + messageId: "mutableReadonly", + }, + ], + }; + } + + if ( + !patternStatesReadonly && + !patternStatesMutable && + options.mustBeReadonly.requireOthersToBeMutable && + options.mustBeMutable.requireOthersToBeReadonly + ) { + return { + context, + descriptors: [ + { + node: node.id, + messageId: "needsExplicitMarking", + }, + ], + }; + } + + const requiredReadonlyness = + patternStatesReadonly || + (!patternStatesMutable && options.mustBeMutable.requireOthersToBeReadonly) + ? RequiredReadonlyness.READONLY + : patternStatesMutable || + (!patternStatesReadonly && + options.mustBeReadonly.requireOthersToBeMutable) + ? RequiredReadonlyness.MUTABLE + : RequiredReadonlyness.EITHER; + + return checkRequiredReadonlyness( + node, + context, + options, + requiredReadonlyness + ); +} + +function checkRequiredReadonlyness( + node: TSESTree.TSTypeAliasDeclaration, + context: RuleContext, + options: Options, + requiredReadonlyness: RequiredReadonlyness +): RuleResult { + if (requiredReadonlyness !== RequiredReadonlyness.EITHER) { + const readonly = isReadonly(node.typeAnnotation, context); + + if (readonly && requiredReadonlyness === RequiredReadonlyness.MUTABLE) { + return { + context, + descriptors: [ + { + node: node.id, + messageId: "readonly", + }, + ], + }; + } + + if (!readonly && requiredReadonlyness === RequiredReadonlyness.READONLY) { + return { + context, + descriptors: [ + { + node: node.id, + messageId: "mutable", + }, + ], + }; + } + } + + return { + context, + descriptors: [], + }; +} + +// Create the rule. +export const rule = createRule( + name, + meta, + defaultOptions, + { + TSTypeAliasDeclaration: checkTypeAliasDeclaration, + } +); 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/tests/rules/prefer-readonly-type-alias/index.test.ts b/tests/rules/prefer-readonly-type-alias/index.test.ts new file mode 100644 index 000000000..31f16a60c --- /dev/null +++ b/tests/rules/prefer-readonly-type-alias/index.test.ts @@ -0,0 +1,6 @@ +import { name, rule } from "~/rules/prefer-readonly-type-alias"; +import { testUsing } from "~/tests/helpers/testers"; + +import tsTests from "./ts"; + +testUsing.typescript(name, rule, tsTests); diff --git a/tests/rules/prefer-readonly-type-alias/ts/index.ts b/tests/rules/prefer-readonly-type-alias/ts/index.ts new file mode 100644 index 000000000..40a005f71 --- /dev/null +++ b/tests/rules/prefer-readonly-type-alias/ts/index.ts @@ -0,0 +1,7 @@ +import invalid from "./invalid"; +import valid from "./valid"; + +export default { + valid, + invalid, +}; diff --git a/tests/rules/prefer-readonly-type-alias/ts/invalid.ts b/tests/rules/prefer-readonly-type-alias/ts/invalid.ts new file mode 100644 index 000000000..2940f122b --- /dev/null +++ b/tests/rules/prefer-readonly-type-alias/ts/invalid.ts @@ -0,0 +1,129 @@ +import dedent from "dedent"; + +import type { InvalidTestCase } from "~/tests/helpers/util"; + +const optionsReversedDefault = [ + { + mustBeReadonly: { + requireOthersToBeMutable: true, + }, + mustBeMutable: { + requireOthersToBeReadonly: false, + }, + }, +]; + +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: "mutable", + type: "Identifier", + line: 1, + column: 6, + }, + ], + }, + // Mutable types should not be readonly. + { + code: dedent` + type MyType = { + readonly a: string; + };`, + optionsSet: [optionsReversedDefault], + // output: dedent` + // type MyType = { + // a: string; + // };`, + errors: [ + { + messageId: "readonly", + type: "Identifier", + line: 1, + column: 6, + }, + ], + }, + // Mutable types should not be readonly. + { + code: dedent` + type MutableMyType = { + readonly a: string; + };`, + optionsSet: [[]], + // output: dedent` + // type MutableMyType = { + // a: string; + // };`, + errors: [ + { + messageId: "readonly", + type: "Identifier", + line: 1, + column: 6, + }, + ], + }, + // Needs Explicit Marking. + { + code: dedent` + type MyType = {};`, + optionsSet: [ + [ + { + mustBeReadonly: { + requireOthersToBeMutable: true, + }, + mustBeMutable: { + requireOthersToBeReadonly: true, + }, + }, + ], + ], + errors: [ + { + messageId: "needsExplicitMarking", + type: "Identifier", + line: 1, + column: 6, + }, + ], + }, + // Both Mutable and Readonly error. + { + code: dedent` + type MyType = {};`, + optionsSet: [ + [ + { + mustBeReadonly: { + pattern: ".*", + }, + mustBeMutable: { + pattern: ".*", + }, + }, + ], + ], + errors: [ + { + messageId: "mutableReadonly", + type: "Identifier", + line: 1, + column: 6, + }, + ], + }, +]; + +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 new file mode 100644 index 000000000..252aa630e --- /dev/null +++ b/tests/rules/prefer-readonly-type-alias/ts/valid.ts @@ -0,0 +1,70 @@ +import dedent from "dedent"; + +import type { ValidTestCase } from "~/tests/helpers/util"; + +const optionsReversedDefault = [ + { + mustBeReadonly: { + requireOthersToBeMutable: true, + }, + mustBeMutable: { + requireOthersToBeReadonly: false, + }, + }, +]; + +const tests: ReadonlyArray = [ + // Readonly types should be readonly. + { + code: dedent` + type MyType = { + readonly a: string; + };`, + optionsSet: [[]], + }, + { + code: dedent` + type ReadonlyMyType = { + readonly a: string; + };`, + optionsSet: [optionsReversedDefault], + }, + // 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: [optionsReversedDefault], + }, + // 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: [optionsReversedDefault], + }, +]; + +export default tests; From f7a6cb24efd4d7a7f5cef4bb3c7b6a04184b24bd Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Sat, 7 Aug 2021 01:41:39 +1200 Subject: [PATCH 2/3] 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, + }, ], }, ]; From aa3d4cf782591ae908874e03a4b64a055a34ad22 Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Sat, 7 Aug 2021 18:01:38 +1200 Subject: [PATCH 3/3] feat(prefer-readonly-type-alias): add support for interfaces --- src/rules/prefer-readonly-type-alias.ts | 180 ++++++++------ src/util/tree.ts | 19 +- src/util/typeguard.ts | 6 + .../prefer-readonly-type-alias/ts/invalid.ts | 234 +++++++++++++++++- .../prefer-readonly-type-alias/ts/valid.ts | 82 ++++++ 5 files changed, 424 insertions(+), 97 deletions(-) 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;