From 3be98542afd7553cbbec66c4be215173d7f7ffcf Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Sun, 1 Mar 2020 21:23:58 -0800 Subject: [PATCH] feat(eslint-plugin): add prefer-readonly-parameters (#1513) --- packages/eslint-plugin/README.md | 1 + .../rules/prefer-readonly-parameter-types.md | 163 ++++++ packages/eslint-plugin/src/configs/all.json | 1 + packages/eslint-plugin/src/rules/index.ts | 24 +- .../rules/prefer-readonly-parameter-types.ts | 98 ++++ packages/eslint-plugin/src/util/index.ts | 1 + .../eslint-plugin/src/util/isTypeReadonly.ts | 155 ++++++ .../prefer-readonly-parameter-types.test.ts | 507 ++++++++++++++++++ .../eslint-plugin/typings/typescript.d.ts | 21 +- 9 files changed, 952 insertions(+), 19 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/prefer-readonly-parameter-types.md create mode 100644 packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts create mode 100644 packages/eslint-plugin/src/util/isTypeReadonly.ts create mode 100644 packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 778e159d6ea..19ee2638a8e 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -142,6 +142,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/prefer-nullish-coalescing`](./docs/rules/prefer-nullish-coalescing.md) | Enforce the usage of the nullish coalescing operator instead of logical chaining | | :wrench: | :thought_balloon: | | [`@typescript-eslint/prefer-optional-chain`](./docs/rules/prefer-optional-chain.md) | Prefer using concise optional chain expressions instead of chained logical ands | | :wrench: | | | [`@typescript-eslint/prefer-readonly`](./docs/rules/prefer-readonly.md) | Requires that private members are marked as `readonly` if they're never modified outside of the constructor | | :wrench: | :thought_balloon: | +| [`@typescript-eslint/prefer-readonly-parameter-types`](./docs/rules/prefer-readonly-parameter-types.md) | Requires that function parameters are typed as readonly to prevent accidental mutation of inputs | | | :thought_balloon: | | [`@typescript-eslint/prefer-regexp-exec`](./docs/rules/prefer-regexp-exec.md) | Enforce that `RegExp#exec` is used instead of `String#match` if no global flag is provided | :heavy_check_mark: | | :thought_balloon: | | [`@typescript-eslint/prefer-string-starts-ends-with`](./docs/rules/prefer-string-starts-ends-with.md) | Enforce the use of `String#startsWith` and `String#endsWith` instead of other equivalent methods of checking substrings | :heavy_check_mark: | :wrench: | :thought_balloon: | | [`@typescript-eslint/promise-function-async`](./docs/rules/promise-function-async.md) | Requires any function or method that returns a Promise to be marked async | | | :thought_balloon: | diff --git a/packages/eslint-plugin/docs/rules/prefer-readonly-parameter-types.md b/packages/eslint-plugin/docs/rules/prefer-readonly-parameter-types.md new file mode 100644 index 00000000000..c69b3b21c34 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/prefer-readonly-parameter-types.md @@ -0,0 +1,163 @@ +# Requires that function parameters are typed as readonly to prevent accidental mutation of inputs (`prefer-readonly-parameter-types`) + +Mutating function arguments can lead to confusing, hard to debug behavior. +Whilst it's easy to implicitly remember to not modify function arguments, explicitly typing arguments as readonly provides clear contract to consumers. +This contract makes it easier for a consumer to reason about if a function has side-effects. + +## Rule Details + +This rule allows you to enforce that function parameters resolve to readonly types. +A type is considered readonly if: + +- it is a primitive type (`string`, `number`, `boolean`, `symbol`, or an enum), +- it is a function signature type, +- it is a readonly array type whose element type is considered readonly. +- it is a readonly tuple type whose elements are all considered readonly. +- it is an object type whose properties are all marked as readonly, and whose values are all considered readonly. + +Examples of **incorrect** code for this rule: + +```ts +function array1(arg: string[]) {} // array is not readonly +function array2(arg: readonly string[][]) {} // array element is not readonly +function array3(arg: [string, number]) {} // tuple is not readonly +function array4(arg: readonly [string[], number]) {} // tuple element is not readonly +// the above examples work the same if you use ReadonlyArray instead + +function object1(arg: { prop: string }) {} // property is not readonly +function object2(arg: { readonly prop: string; prop2: string }) {} // not all properties are readonly +function object3(arg: { readonly prop: { prop2: string } }) {} // nested property is not readonly +// the above examples work the same if you use Readonly instead + +interface CustomArrayType extends ReadonlyArray { + prop: string; // note: this property is mutable +} +function custom1(arg: CustomArrayType) {} + +interface CustomFunction { + (): void; + prop: string; // note: this property is mutable +} +function custom2(arg: CustomFunction) {} + +function union(arg: string[] | ReadonlyArray) {} // not all types are readonly + +// rule also checks function types +interface Foo { + (arg: string[]): void; +} +interface Foo { + new (arg: string[]): void; +} +const x = { foo(arg: string[]): void; }; +function foo(arg: string[]); +type Foo = (arg: string[]) => void; +interface Foo { + foo(arg: string[]): void; +} +``` + +Examples of **correct** code for this rule: + +```ts +function array1(arg: readonly string[]) {} +function array2(arg: readonly (readonly string[])[]) {} +function array3(arg: readonly [string, number]) {} +function array4(arg: readonly [readonly string[], number]) {} +// the above examples work the same if you use ReadonlyArray instead + +function object1(arg: { readonly prop: string }) {} +function object2(arg: { readonly prop: string; readonly prop2: string }) {} +function object3(arg: { readonly prop: { readonly prop2: string } }) {} +// the above examples work the same if you use Readonly instead + +interface CustomArrayType extends ReadonlyArray { + readonly prop: string; +} +function custom1(arg: CustomArrayType) {} + +interface CustomFunction { + (): void; + readonly prop: string; +} +function custom2(arg: CustomFunction) {} + +function union(arg: readonly string[] | ReadonlyArray) {} + +function primitive1(arg: string) {} +function primitive2(arg: number) {} +function primitive3(arg: boolean) {} +function primitive4(arg: unknown) {} +function primitive5(arg: null) {} +function primitive6(arg: undefined) {} +function primitive7(arg: any) {} +function primitive8(arg: never) {} +function primitive9(arg: string | number | undefined) {} + +function fnSig(arg: () => void) {} + +enum Foo { a, b } +function enum(arg: Foo) {} + +function symb1(arg: symbol) {} +const customSymbol = Symbol('a'); +function symb2(arg: typeof customSymbol) {} + +// function types +interface Foo { + (arg: readonly string[]): void; +} +interface Foo { + new (arg: readonly string[]): void; +} +const x = { foo(arg: readonly string[]): void; }; +function foo(arg: readonly string[]); +type Foo = (arg: readonly string[]) => void; +interface Foo { + foo(arg: readonly string[]): void; +} +``` + +## Options + +```ts +interface Options { + checkParameterProperties?: boolean; +} + +const defaultOptions: Options = { + checkParameterProperties: true, +}; +``` + +### `checkParameterProperties` + +This option allows you to enable or disable the checking of parameter properties. +Because parameter properties create properties on the class, it may be undesirable to force them to be readonly. + +Examples of **incorrect** code for this rule with `{checkParameterProperties: true}`: + +```ts +class Foo { + constructor(private paramProp: string[]) {} +} +``` + +Examples of **correct** code for this rule with `{checkParameterProperties: true}`: + +```ts +class Foo { + constructor(private paramProp: readonly string[]) {} +} +``` + +Examples of **correct** code for this rule with `{checkParameterProperties: false}`: + +```ts +class Foo { + constructor( + private paramProp1: string[], + private paramProp2: readonly string[], + ) {} +} +``` diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index bf5844653ef..590ef81df8d 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -79,6 +79,7 @@ "@typescript-eslint/prefer-nullish-coalescing": "error", "@typescript-eslint/prefer-optional-chain": "error", "@typescript-eslint/prefer-readonly": "error", + "@typescript-eslint/prefer-readonly-parameter-types": "error", "@typescript-eslint/prefer-regexp-exec": "error", "@typescript-eslint/prefer-string-starts-ends-with": "error", "@typescript-eslint/promise-function-async": "error", diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 0c3a5d9f0d1..3568057787b 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -1,8 +1,8 @@ import adjacentOverloadSignatures from './adjacent-overload-signatures'; import arrayType from './array-type'; import awaitThenable from './await-thenable'; -import banTsIgnore from './ban-ts-ignore'; import banTsComment from './ban-ts-comment'; +import banTsIgnore from './ban-ts-ignore'; import banTypes from './ban-types'; import braceStyle from './brace-style'; import camelcase from './camelcase'; @@ -29,10 +29,10 @@ import noDynamicDelete from './no-dynamic-delete'; import noEmptyFunction from './no-empty-function'; import noEmptyInterface from './no-empty-interface'; import noExplicitAny from './no-explicit-any'; +import noExtraneousClass from './no-extraneous-class'; import noExtraNonNullAssertion from './no-extra-non-null-assertion'; import noExtraParens from './no-extra-parens'; import noExtraSemi from './no-extra-semi'; -import noExtraneousClass from './no-extraneous-class'; import noFloatingPromises from './no-floating-promises'; import noForInArray from './no-for-in-array'; import noImpliedEval from './no-implied-eval'; @@ -41,8 +41,8 @@ import noMagicNumbers from './no-magic-numbers'; import noMisusedNew from './no-misused-new'; import noMisusedPromises from './no-misused-promises'; import noNamespace from './no-namespace'; -import noNonNullAssertion from './no-non-null-assertion'; import noNonNullAssertedOptionalChain from './no-non-null-asserted-optional-chain'; +import noNonNullAssertion from './no-non-null-assertion'; import noParameterProperties from './no-parameter-properties'; import noRequireImports from './no-require-imports'; import noThisAlias from './no-this-alias'; @@ -51,7 +51,7 @@ import noTypeAlias from './no-type-alias'; import noUnnecessaryBooleanLiteralCompare from './no-unnecessary-boolean-literal-compare'; import noUnnecessaryCondition from './no-unnecessary-condition'; import noUnnecessaryQualifier from './no-unnecessary-qualifier'; -import useDefaultTypeParameter from './no-unnecessary-type-arguments'; +import noUnnecessaryTypeArguments from './no-unnecessary-type-arguments'; import noUnnecessaryTypeAssertion from './no-unnecessary-type-assertion'; import noUntypedPublicSignature from './no-untyped-public-signature'; import noUnusedExpressions from './no-unused-expressions'; @@ -68,6 +68,7 @@ import preferNamespaceKeyword from './prefer-namespace-keyword'; import preferNullishCoalescing from './prefer-nullish-coalescing'; import preferOptionalChain from './prefer-optional-chain'; import preferReadonly from './prefer-readonly'; +import preferReadonlyParameterTypes from './prefer-readonly-parameter-types'; import preferRegexpExec from './prefer-regexp-exec'; import preferStringStartsEndsWith from './prefer-string-starts-ends-with'; import promiseFunctionAsync from './promise-function-async'; @@ -91,8 +92,8 @@ export default { 'adjacent-overload-signatures': adjacentOverloadSignatures, 'array-type': arrayType, 'await-thenable': awaitThenable, - 'ban-ts-ignore': banTsIgnore, 'ban-ts-comment': banTsComment, + 'ban-ts-ignore': banTsIgnore, 'ban-types': banTypes, 'no-base-to-string': noBaseToString, 'brace-style': braceStyle, @@ -125,28 +126,28 @@ export default { 'no-extraneous-class': noExtraneousClass, 'no-floating-promises': noFloatingPromises, 'no-for-in-array': noForInArray, - 'no-inferrable-types': noInferrableTypes, 'no-implied-eval': noImpliedEval, + 'no-inferrable-types': noInferrableTypes, 'no-magic-numbers': noMagicNumbers, 'no-misused-new': noMisusedNew, 'no-misused-promises': noMisusedPromises, 'no-namespace': noNamespace, - 'no-non-null-assertion': noNonNullAssertion, 'no-non-null-asserted-optional-chain': noNonNullAssertedOptionalChain, + 'no-non-null-assertion': noNonNullAssertion, 'no-parameter-properties': noParameterProperties, 'no-require-imports': noRequireImports, 'no-this-alias': noThisAlias, - 'no-type-alias': noTypeAlias, 'no-throw-literal': noThrowLiteral, + 'no-type-alias': noTypeAlias, 'no-unnecessary-boolean-literal-compare': noUnnecessaryBooleanLiteralCompare, 'no-unnecessary-condition': noUnnecessaryCondition, 'no-unnecessary-qualifier': noUnnecessaryQualifier, - 'no-unnecessary-type-arguments': useDefaultTypeParameter, + 'no-unnecessary-type-arguments': noUnnecessaryTypeArguments, 'no-unnecessary-type-assertion': noUnnecessaryTypeAssertion, 'no-untyped-public-signature': noUntypedPublicSignature, - 'no-unused-vars': noUnusedVars, - 'no-unused-vars-experimental': noUnusedVarsExperimental, 'no-unused-expressions': noUnusedExpressions, + 'no-unused-vars-experimental': noUnusedVarsExperimental, + 'no-unused-vars': noUnusedVars, 'no-use-before-define': noUseBeforeDefine, 'no-useless-constructor': noUselessConstructor, 'no-var-requires': noVarRequires, @@ -157,6 +158,7 @@ export default { 'prefer-namespace-keyword': preferNamespaceKeyword, 'prefer-nullish-coalescing': preferNullishCoalescing, 'prefer-optional-chain': preferOptionalChain, + 'prefer-readonly-parameter-types': preferReadonlyParameterTypes, 'prefer-readonly': preferReadonly, 'prefer-regexp-exec': preferRegexpExec, 'prefer-string-starts-ends-with': preferStringStartsEndsWith, diff --git a/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts b/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts new file mode 100644 index 00000000000..07e8e69940c --- /dev/null +++ b/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts @@ -0,0 +1,98 @@ +import { + TSESTree, + AST_NODE_TYPES, +} from '@typescript-eslint/experimental-utils'; +import * as util from '../util'; + +type Options = [ + { + checkParameterProperties?: boolean; + }, +]; +type MessageIds = 'shouldBeReadonly'; + +export default util.createRule({ + name: 'prefer-readonly-parameter-types', + meta: { + type: 'suggestion', + docs: { + description: + 'Requires that function parameters are typed as readonly to prevent accidental mutation of inputs', + category: 'Possible Errors', + recommended: false, + requiresTypeChecking: true, + }, + schema: [ + { + type: 'object', + additionalProperties: false, + properties: { + checkParameterProperties: { + type: 'boolean', + }, + }, + }, + ], + messages: { + shouldBeReadonly: 'Parameter should be a read only type', + }, + }, + defaultOptions: [ + { + checkParameterProperties: true, + }, + ], + create(context, [{ checkParameterProperties }]) { + const { esTreeNodeToTSNodeMap, program } = util.getParserServices(context); + const checker = program.getTypeChecker(); + + return { + [[ + AST_NODE_TYPES.ArrowFunctionExpression, + AST_NODE_TYPES.FunctionDeclaration, + AST_NODE_TYPES.FunctionExpression, + AST_NODE_TYPES.TSCallSignatureDeclaration, + AST_NODE_TYPES.TSConstructSignatureDeclaration, + AST_NODE_TYPES.TSDeclareFunction, + AST_NODE_TYPES.TSEmptyBodyFunctionExpression, + AST_NODE_TYPES.TSFunctionType, + AST_NODE_TYPES.TSMethodSignature, + ].join(', ')]( + node: + | TSESTree.ArrowFunctionExpression + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression + | TSESTree.TSCallSignatureDeclaration + | TSESTree.TSConstructSignatureDeclaration + | TSESTree.TSDeclareFunction + | TSESTree.TSEmptyBodyFunctionExpression + | TSESTree.TSFunctionType + | TSESTree.TSMethodSignature, + ): void { + for (const param of node.params) { + if ( + !checkParameterProperties && + param.type === AST_NODE_TYPES.TSParameterProperty + ) { + continue; + } + + const actualParam = + param.type === AST_NODE_TYPES.TSParameterProperty + ? param.parameter + : param; + const tsNode = esTreeNodeToTSNodeMap.get(actualParam); + const type = checker.getTypeAtLocation(tsNode); + const isReadOnly = util.isTypeReadonly(checker, type); + + if (!isReadOnly) { + context.report({ + node: actualParam, + messageId: 'shouldBeReadonly', + }); + } + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/src/util/index.ts b/packages/eslint-plugin/src/util/index.ts index 7fde0f41e71..381012acfec 100644 --- a/packages/eslint-plugin/src/util/index.ts +++ b/packages/eslint-plugin/src/util/index.ts @@ -2,6 +2,7 @@ import { ESLintUtils } from '@typescript-eslint/experimental-utils'; export * from './astUtils'; export * from './createRule'; +export * from './isTypeReadonly'; export * from './misc'; export * from './nullThrows'; export * from './types'; diff --git a/packages/eslint-plugin/src/util/isTypeReadonly.ts b/packages/eslint-plugin/src/util/isTypeReadonly.ts new file mode 100644 index 00000000000..99df07b153a --- /dev/null +++ b/packages/eslint-plugin/src/util/isTypeReadonly.ts @@ -0,0 +1,155 @@ +import { + isObjectType, + isUnionType, + isUnionOrIntersectionType, + unionTypeParts, + isPropertyReadonlyInType, +} from 'tsutils'; +import * as ts from 'typescript'; +import { nullThrows, NullThrowsReasons } from '.'; + +/** + * Returns: + * - null if the type is not an array or tuple, + * - true if the type is a readonly array or readonly tuple, + * - false if the type is a mutable array or mutable tuple. + */ +function isTypeReadonlyArrayOrTuple( + checker: ts.TypeChecker, + type: ts.Type, +): boolean | null { + function checkTypeArguments(arrayType: ts.TypeReference): boolean { + const typeArguments = checker.getTypeArguments(arrayType); + /* istanbul ignore if */ if (typeArguments.length === 0) { + // this shouldn't happen in reality as: + // - tuples require at least 1 type argument + // - ReadonlyArray requires at least 1 type argument + return true; + } + + // validate the element types are also readonly + if (typeArguments.some(typeArg => !isTypeReadonly(checker, typeArg))) { + return false; + } + return true; + } + + if (checker.isArrayType(type)) { + const symbol = nullThrows( + type.getSymbol(), + NullThrowsReasons.MissingToken('symbol', 'array type'), + ); + const escapedName = symbol.getEscapedName(); + if (escapedName === 'Array' && escapedName !== 'ReadonlyArray') { + return false; + } + + return checkTypeArguments(type); + } + + if (checker.isTupleType(type)) { + if (!type.target.readonly) { + return false; + } + + return checkTypeArguments(type); + } + + return null; +} + +/** + * Returns: + * - null if the type is not an object, + * - true if the type is an object with only readonly props, + * - false if the type is an object with at least one mutable prop. + */ +function isTypeReadonlyObject( + checker: ts.TypeChecker, + type: ts.Type, +): boolean | null { + function checkIndexSignature(kind: ts.IndexKind): boolean | null { + const indexInfo = checker.getIndexInfoOfType(type, kind); + if (indexInfo) { + return indexInfo.isReadonly ? true : false; + } + + return null; + } + + const properties = type.getProperties(); + if (properties.length) { + // ensure the properties are marked as readonly + for (const property of properties) { + if (!isPropertyReadonlyInType(type, property.getEscapedName(), checker)) { + return false; + } + } + + // all properties were readonly + // now ensure that all of the values are readonly also. + + // do this after checking property readonly-ness as a perf optimization, + // as we might be able to bail out early due to a mutable property before + // doing this deep, potentially expensive check. + for (const property of properties) { + const propertyType = nullThrows( + checker.getTypeOfPropertyOfType(type, property.getName()), + NullThrowsReasons.MissingToken(`property "${property.name}"`, 'type'), + ); + if (!isTypeReadonly(checker, propertyType)) { + return false; + } + } + } + + const isStringIndexSigReadonly = checkIndexSignature(ts.IndexKind.String); + if (isStringIndexSigReadonly === false) { + return isStringIndexSigReadonly; + } + + const isNumberIndexSigReadonly = checkIndexSignature(ts.IndexKind.Number); + if (isNumberIndexSigReadonly === false) { + return isNumberIndexSigReadonly; + } + + return true; +} + +/** + * Checks if the given type is readonly + */ +function isTypeReadonly(checker: ts.TypeChecker, type: ts.Type): boolean { + if (isUnionType(type)) { + // all types in the union must be readonly + return unionTypeParts(type).every(t => isTypeReadonly(checker, t)); + } + + // all non-object, non-intersection types are readonly. + // this should only be primitive types + if (!isObjectType(type) && !isUnionOrIntersectionType(type)) { + return true; + } + + // pure function types are readonly + if ( + type.getCallSignatures().length > 0 && + type.getProperties().length === 0 + ) { + return true; + } + + const isReadonlyArray = isTypeReadonlyArrayOrTuple(checker, type); + if (isReadonlyArray !== null) { + return isReadonlyArray; + } + + const isReadonlyObject = isTypeReadonlyObject(checker, type); + /* istanbul ignore else */ if (isReadonlyObject !== null) { + return isReadonlyObject; + } + + throw new Error('Unhandled type'); +} + +export { isTypeReadonly }; diff --git a/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts b/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts new file mode 100644 index 00000000000..b869e0c15ed --- /dev/null +++ b/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts @@ -0,0 +1,507 @@ +import { TSESLint } from '@typescript-eslint/experimental-utils'; +import { RuleTester, getFixturesRootDir } from '../RuleTester'; +import rule from '../../src/rules/prefer-readonly-parameter-types'; +import { + InferMessageIdsTypeFromRule, + InferOptionsTypeFromRule, +} from '../../src/util'; + +type MessageIds = InferMessageIdsTypeFromRule; +type Options = InferOptionsTypeFromRule; + +const rootPath = getFixturesRootDir(); + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + tsconfigRootDir: rootPath, + project: './tsconfig.json', + }, +}); + +const primitives = [ + 'boolean', + 'true', + 'string', + "'a'", + 'number', + '1', + 'symbol', + 'any', + 'unknown', + 'never', + 'null', + 'undefined', +]; +const arrays = [ + 'readonly string[]', + 'Readonly', + 'ReadonlyArray', + 'readonly [string]', + 'Readonly<[string]>', +]; +const objects = [ + '{ foo: "" }', + '{ foo: readonly string[] }', + '{ foo(): void }', +]; +const weirdIntersections = [ + ` + interface Test { + (): void + readonly property: boolean + } + function foo(arg: Test) {} + `, + ` + type Test = (() => void) & { + readonly property: boolean + }; + function foo(arg: Test) {} + `, +]; + +ruleTester.run('prefer-readonly-parameter-types', rule, { + valid: [ + 'function foo() {}', + + // primitives + ...primitives.map(type => `function foo(arg: ${type}) {}`), + ` + const symb = Symbol('a'); + function foo(arg: typeof symb) {} + `, + ` + enum Enum { a, b } + function foo(arg: Enum) {} + `, + + // arrays + ...arrays.map(type => `function foo(arg: ${type}) {}`), + // nested arrays + 'function foo(arg: readonly (readonly string[])[]) {}', + 'function foo(arg: Readonly[]>) {}', + 'function foo(arg: ReadonlyArray>) {}', + + // functions + 'function foo(arg: () => void) {}', + + // unions + 'function foo(arg: string | null) {}', + 'function foo(arg: string | ReadonlyArray) {}', + 'function foo(arg: string | (() => void)) {}', + 'function foo(arg: ReadonlyArray | ReadonlyArray) {}', + + // objects + ...objects.map(type => `function foo(arg: Readonly<${type}>) {}`), + ` + function foo(arg: { + readonly foo: { + readonly bar: string + } + }) {} + `, + ` + function foo(arg: { + readonly [k: string]: string, + }) {} + `, + ` + function foo(arg: { + readonly [k: number]: string, + }) {} + `, + ` + interface Empty {} + function foo(arg: Empty) {} + `, + + // weird other cases + ...weirdIntersections.map(code => code), + ` + interface Test extends ReadonlyArray { + readonly property: boolean + } + function foo(arg: Readonly) {} + `, + ` + type Test = (readonly string[]) & { + readonly property: boolean + }; + function foo(arg: Readonly) {} + `, + ` + type Test = string & number; + function foo(arg: Test) {} + `, + + // declaration merging + ` + class Foo { + readonly bang = 1; + } + interface Foo { + readonly prop: string; + } + interface Foo { + readonly prop2: string; + } + function foo(arg: Foo) {} + `, + // method made readonly via Readonly + ` + class Foo { + method() {} + } + function foo(arg: Readonly) {} + `, + + // parameter properties should work fine + { + code: ` + class Foo { + constructor( + private arg1: readonly string[], + public arg2: readonly string[], + protected arg3: readonly string[], + readonly arg4: readonly string[], + ) {} + } + `, + options: [ + { + checkParameterProperties: true, + }, + ], + }, + { + code: ` + class Foo { + constructor( + private arg1: string[], + public arg2: string[], + protected arg3: string[], + readonly arg4: string[], + ) {} + } + `, + options: [ + { + checkParameterProperties: false, + }, + ], + }, + + // type functions + 'interface Foo { (arg: readonly string[]): void }', // TSCallSignatureDeclaration + 'interface Foo { new (arg: readonly string[]): void }', // TSConstructSignatureDeclaration + 'const x = { foo(arg: readonly string[]): void }', // TSEmptyBodyFunctionExpression + 'function foo(arg: readonly string[]);', // TSDeclareFunction + 'type Foo = (arg: readonly string[]) => void;', // TSFunctionType + 'interface Foo { foo(arg: readonly string[]): void }', // TSMethodSignature + ], + invalid: [ + // arrays + ...arrays.map>(baseType => { + const type = baseType + .replace(/readonly /g, '') + .replace(/Readonly<(.+?)>/g, '$1') + .replace(/ReadonlyArray/g, 'Array'); + return { + code: `function foo(arg: ${type}) {}`, + errors: [ + { + messageId: 'shouldBeReadonly', + column: 14, + endColumn: 19 + type.length, + }, + ], + }; + }), + // nested arrays + { + code: 'function foo(arg: readonly (string[])[]) {}', + errors: [ + { + messageId: 'shouldBeReadonly', + column: 14, + endColumn: 40, + }, + ], + }, + { + code: 'function foo(arg: Readonly) {}', + errors: [ + { + messageId: 'shouldBeReadonly', + column: 14, + endColumn: 39, + }, + ], + }, + { + code: 'function foo(arg: ReadonlyArray>) {}', + errors: [ + { + messageId: 'shouldBeReadonly', + column: 14, + endColumn: 47, + }, + ], + }, + + // objects + ...objects.map>(type => { + return { + code: `function foo(arg: ${type}) {}`, + errors: [ + { + messageId: 'shouldBeReadonly', + column: 14, + endColumn: 19 + type.length, + }, + ], + }; + }), + { + code: ` + function foo(arg: { + readonly foo: { + bar: string + } + }) {} + `, + errors: [ + { + messageId: 'shouldBeReadonly', + line: 2, + column: 22, + endLine: 6, + endColumn: 10, + }, + ], + }, + // object index signatures + { + code: ` + function foo(arg: { + [key: string]: string + }) {} + `, + errors: [ + { + messageId: 'shouldBeReadonly', + line: 2, + column: 22, + endLine: 4, + endColumn: 10, + }, + ], + }, + { + code: ` + function foo(arg: { + [key: number]: string + }) {} + `, + errors: [ + { + messageId: 'shouldBeReadonly', + line: 2, + column: 22, + endLine: 4, + endColumn: 10, + }, + ], + }, + + // weird intersections + ...weirdIntersections.map>( + baseCode => { + const code = baseCode.replace(/readonly /g, ''); + return { + code, + errors: [{ messageId: 'shouldBeReadonly' }], + }; + }, + ), + { + code: ` + interface Test extends Array { + readonly property: boolean + } + function foo(arg: Test) {} + `, + errors: [ + { + messageId: 'shouldBeReadonly', + line: 5, + column: 22, + endLine: 5, + endColumn: 31, + }, + ], + }, + { + code: ` + interface Test extends Array { + property: boolean + } + function foo(arg: Test) {} + `, + errors: [ + { + messageId: 'shouldBeReadonly', + line: 5, + column: 22, + endLine: 5, + endColumn: 31, + }, + ], + }, + + // parameter properties should work fine + { + code: ` + class Foo { + constructor( + private arg1: string[], + public arg2: string[], + protected arg3: string[], + readonly arg4: string[], + ) {} + } + `, + options: [ + { + checkParameterProperties: true, + }, + ], + errors: [ + { + messageId: 'shouldBeReadonly', + line: 4, + column: 23, + endLine: 4, + endColumn: 37, + }, + { + messageId: 'shouldBeReadonly', + line: 5, + column: 23, + endLine: 5, + endColumn: 37, + }, + { + messageId: 'shouldBeReadonly', + line: 6, + column: 23, + endLine: 6, + endColumn: 37, + }, + { + messageId: 'shouldBeReadonly', + line: 7, + column: 23, + endLine: 7, + endColumn: 37, + }, + ], + }, + { + code: ` + class Foo { + constructor( + private arg1: readonly string[], + public arg2: readonly string[], + protected arg3: readonly string[], + readonly arg4: readonly string[], + arg5: string[], + ) {} + } + `, + options: [ + { + checkParameterProperties: false, + }, + ], + errors: [ + { + messageId: 'shouldBeReadonly', + line: 8, + column: 13, + endLine: 8, + endColumn: 27, + }, + ], + }, + + // type functions + { + // TSCallSignatureDeclaration + code: 'interface Foo { (arg: string[]): void }', + errors: [ + { + messageId: 'shouldBeReadonly', + column: 18, + endColumn: 31, + }, + ], + }, + { + // TSConstructSignatureDeclaration + code: 'interface Foo { new (arg: string[]): void }', + errors: [ + { + messageId: 'shouldBeReadonly', + column: 22, + endColumn: 35, + }, + ], + }, + { + // TSEmptyBodyFunctionExpression + code: 'const x = { foo(arg: string[]): void }', + errors: [ + { + messageId: 'shouldBeReadonly', + column: 17, + endColumn: 30, + }, + ], + }, + { + // TSDeclareFunction + code: 'function foo(arg: string[]);', + errors: [ + { + messageId: 'shouldBeReadonly', + column: 14, + endColumn: 27, + }, + ], + }, + { + // TSFunctionType + code: 'type Foo = (arg: string[]) => void', + errors: [ + { + messageId: 'shouldBeReadonly', + column: 13, + endColumn: 26, + }, + ], + }, + { + // TSMethodSignature + code: 'interface Foo { foo(arg: string[]): void }', + errors: [ + { + messageId: 'shouldBeReadonly', + column: 21, + endColumn: 34, + }, + ], + }, + ], +}); diff --git a/packages/eslint-plugin/typings/typescript.d.ts b/packages/eslint-plugin/typings/typescript.d.ts index 6d9a098b538..7c9089158b4 100644 --- a/packages/eslint-plugin/typings/typescript.d.ts +++ b/packages/eslint-plugin/typings/typescript.d.ts @@ -1,4 +1,4 @@ -import { TypeChecker, Type } from 'typescript'; +import 'typescript'; declare module 'typescript' { interface TypeChecker { @@ -6,16 +6,21 @@ declare module 'typescript' { /** * @returns `true` if the given type is an array type: - * - Array - * - ReadonlyArray - * - foo[] - * - readonly foo[] + * - `Array` + * - `ReadonlyArray` + * - `foo[]` + * - `readonly foo[]` */ - isArrayType(type: Type): boolean; + isArrayType(type: Type): type is TypeReference; /** * @returns `true` if the given type is a tuple type: - * - [foo] + * - `[foo]` + * - `readonly [foo]` */ - isTupleType(type: Type): boolean; + isTupleType(type: Type): type is TupleTypeReference; + /** + * Return the type of the given property in the given type, or undefined if no such property exists + */ + getTypeOfPropertyOfType(type: Type, propertyName: string): Type | undefined; } }