From 846eb88a7bcaf8b6ade60d8802acc3fb942d1e2c Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Thu, 23 Jan 2020 20:15:00 -0800 Subject: [PATCH 01/11] feat(eslint-plugin): add prefer-readonly-parameters --- .../src/rules/prefer-readonly-parameters.ts | 173 ++++++++++++++++++ .../rules/prefer-readonly-parameters.test.ts | 77 ++++++++ .../eslint-plugin/typings/typescript.d.ts | 4 +- 3 files changed, 252 insertions(+), 2 deletions(-) create mode 100644 packages/eslint-plugin/src/rules/prefer-readonly-parameters.ts create mode 100644 packages/eslint-plugin/tests/rules/prefer-readonly-parameters.test.ts diff --git a/packages/eslint-plugin/src/rules/prefer-readonly-parameters.ts b/packages/eslint-plugin/src/rules/prefer-readonly-parameters.ts new file mode 100644 index 00000000000..59deef88983 --- /dev/null +++ b/packages/eslint-plugin/src/rules/prefer-readonly-parameters.ts @@ -0,0 +1,173 @@ +import { + TSESTree, + AST_NODE_TYPES, +} from '@typescript-eslint/experimental-utils'; +import { isObjectType, isUnionType, unionTypeParts } from 'tsutils'; +import * as ts from 'typescript'; +import * as util from '../util'; + +export default util.createRule({ + name: 'prefer-readonly-parameters', + meta: { + type: 'suggestion', + docs: { + description: 'TODO', + category: 'Possible Errors', + recommended: false, + requiresTypeChecking: true, + }, + schema: [ + { + type: 'string', + enum: ['prefer-readonly', 'prefer-mutable', 'ignore'], + }, + ], + messages: { + shouldBeReadonly: 'Parameter should be a read only type', + }, + }, + defaultOptions: [], + create(context) { + const { esTreeNodeToTSNodeMap, program } = util.getParserServices(context); + const checker = program.getTypeChecker(); + + /** + * 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(type: ts.Type): boolean | null { + if (checker.isArrayType(type)) { + const symbol = util.nullThrows( + type.getSymbol(), + util.NullThrowsReasons.MissingToken('symbol', 'array type'), + ); + const escapedName = symbol.getEscapedName(); + if (escapedName === 'ReadonlyArray') { + return true; + } + if (escapedName === 'Array') { + return false; + } + } + + if (checker.isTupleType(type)) { + if (type.target.readonly) { + return true; + } + return false; + } + + 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(type: ts.Type): boolean | null { + function checkIndex(kind: ts.IndexKind): boolean | null { + const indexInfo = checker.getIndexInfoOfType(type, kind); + if (indexInfo) { + return indexInfo.isReadonly ? true : false; + } + + return null; + } + + const isStringIndexReadonly = checkIndex(ts.IndexKind.String); + if (isStringIndexReadonly !== null) { + return isStringIndexReadonly; + } + + const isNumberIndexReadonly = checkIndex(ts.IndexKind.Number); + if (isNumberIndexReadonly !== null) { + return isNumberIndexReadonly; + } + + const properties = type.getProperties(); + if (properties.length) { + // NOTES: + // - port isReadonlySymbol - https://github.com/Microsoft/TypeScript/blob/4212484ae18163df867f53dab19a8cc0c6000793/src/compiler/checker.ts#L26285 + // eslint-disable-next-line @typescript-eslint/ban-ts-ignore + // @ts-ignore + declare function isReadonlySymbol(symbol: ts.Symbol): boolean; + + for (const property of properties) { + if (!isReadonlySymbol(property)) { + return false; + } + } + + // all properties were readonly + return true; + } + + return false; + } + + /** + * Checks if the given type is readonly + */ + function isTypeReadonly(type: ts.Type): boolean { + if (isUnionType(type)) { + return unionTypeParts(type).every(t => isTypeReadonly(t)); + } + + // all non-object types are readonly + if (!isObjectType(type)) { + return true; + } + + // pure function types are readonly + if ( + type.getCallSignatures().length > 0 && + type.getProperties().length === 0 + ) { + return true; + } + + const isReadonlyArray = isTypeReadonlyArrayOrTuple(type); + if (isReadonlyArray !== null) { + return isReadonlyArray; + } + + const isReadonlyObject = isTypeReadonlyObject(type); + if (isReadonlyObject !== null) { + return isReadonlyObject; + } + + throw new Error('Unhandled type'); + } + + return { + 'ArrowFunctionExpression, FunctionDeclaration, FunctionExpression, TSEmptyBodyFunctionExpression'( + node: + | TSESTree.ArrowFunctionExpression + | TSESTree.FunctionDeclaration + | TSESTree.FunctionExpression + | TSESTree.TSEmptyBodyFunctionExpression, + ): void { + for (const param of node.params) { + const actualParam = + param.type === AST_NODE_TYPES.TSParameterProperty + ? param.parameter + : param; + const tsNode = esTreeNodeToTSNodeMap.get(actualParam); + const type = checker.getTypeAtLocation(tsNode); + const isReadOnly = isTypeReadonly(type); + + if (!isReadOnly) { + return context.report({ + node: actualParam, + messageId: 'shouldBeReadonly', + }); + } + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/prefer-readonly-parameters.test.ts b/packages/eslint-plugin/tests/rules/prefer-readonly-parameters.test.ts new file mode 100644 index 00000000000..e65fb107b14 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/prefer-readonly-parameters.test.ts @@ -0,0 +1,77 @@ +import rule from '../../src/rules/prefer-readonly-parameters'; +import { RuleTester, getFixturesRootDir } from '../RuleTester'; + +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', + 'any', + 'unknown', + 'never', + 'null', + 'undefined', +]; + +ruleTester.run('prefer-readonly-parameters', rule, { + valid: [ + // primitives + ...primitives.map(type => `function foo(arg: ${type}) {}`), + + // arrays + 'function foo(arg: readonly string[]) {}', + 'function foo(arg: Readonly) {}', + 'function foo(arg: ReadonlyArray) {}', + 'function foo(arg: ReadonlyArray | ReadonlyArray) {}', + + // functions + 'function foo(arg: () => void) {}', + ` + interface ImmutablePropFunction { + (): void + readonly immutable: boolean + } + function foo(arg: ImmutablePropFunction) {} + `, + + // unions + 'function foo(arg: string | null) {}', + 'function foo(arg: string | ReadonlyArray) {}', + 'function foo(arg: string | (() => void)) {}', + ], + invalid: [ + { + code: 'function foo(arg: string[]) {}', + errors: [ + { + messageId: 'shouldBeReadonly', + data: { + type: 'Parameters', + }, + }, + ], + }, + { + code: ` + interface MutablePropFunction { + (): void + mutable: boolean + } + function foo(arg: MutablePropFunction) {} + `, + errors: [], + }, + ], +}); diff --git a/packages/eslint-plugin/typings/typescript.d.ts b/packages/eslint-plugin/typings/typescript.d.ts index 6d9a098b538..bf379a245b8 100644 --- a/packages/eslint-plugin/typings/typescript.d.ts +++ b/packages/eslint-plugin/typings/typescript.d.ts @@ -11,11 +11,11 @@ declare module 'typescript' { * - foo[] * - readonly foo[] */ - isArrayType(type: Type): boolean; + isArrayType(type: Type): type is TypeReference; /** * @returns `true` if the given type is a tuple type: * - [foo] */ - isTupleType(type: Type): boolean; + isTupleType(type: Type): type is TupleTypeReference; } } From dfb288fd366ea93fb32edd209c44498a5af05a6c Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Fri, 24 Jan 2020 13:04:17 -0800 Subject: [PATCH 02/11] wip --- ...donly-parameters.ts => prefer-readonly-parameter-types.ts} | 2 +- ...meters.test.ts => prefer-readonly-parameter-types.test.ts} | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) rename packages/eslint-plugin/src/rules/{prefer-readonly-parameters.ts => prefer-readonly-parameter-types.ts} (99%) rename packages/eslint-plugin/tests/rules/{prefer-readonly-parameters.test.ts => prefer-readonly-parameter-types.test.ts} (92%) diff --git a/packages/eslint-plugin/src/rules/prefer-readonly-parameters.ts b/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts similarity index 99% rename from packages/eslint-plugin/src/rules/prefer-readonly-parameters.ts rename to packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts index 59deef88983..49dd35c7841 100644 --- a/packages/eslint-plugin/src/rules/prefer-readonly-parameters.ts +++ b/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts @@ -7,7 +7,7 @@ import * as ts from 'typescript'; import * as util from '../util'; export default util.createRule({ - name: 'prefer-readonly-parameters', + name: 'prefer-readonly-parameter-types', meta: { type: 'suggestion', docs: { diff --git a/packages/eslint-plugin/tests/rules/prefer-readonly-parameters.test.ts b/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts similarity index 92% rename from packages/eslint-plugin/tests/rules/prefer-readonly-parameters.test.ts rename to packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts index e65fb107b14..99e04612385 100644 --- a/packages/eslint-plugin/tests/rules/prefer-readonly-parameters.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts @@ -1,4 +1,4 @@ -import rule from '../../src/rules/prefer-readonly-parameters'; +import rule from '../../src/rules/prefer-readonly-parameter-types'; import { RuleTester, getFixturesRootDir } from '../RuleTester'; const rootPath = getFixturesRootDir(); @@ -25,7 +25,7 @@ const primitives = [ 'undefined', ]; -ruleTester.run('prefer-readonly-parameters', rule, { +ruleTester.run('prefer-readonly-parameter-types', rule, { valid: [ // primitives ...primitives.map(type => `function foo(arg: ${type}) {}`), From 7dad3bf8469a92c1fed281be6ac424a11e9a4ed5 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Fri, 24 Jan 2020 17:30:54 -0800 Subject: [PATCH 03/11] feat: nested array checks --- .../rules/prefer-readonly-parameter-types.ts | 30 +++- .../prefer-readonly-parameter-types.test.ts | 152 +++++++++++++++--- 2 files changed, 155 insertions(+), 27 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts b/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts index 49dd35c7841..fdf021f31a8 100644 --- a/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts +++ b/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts @@ -38,25 +38,41 @@ export default util.createRule({ * - false if the type is a mutable array or mutable tuple. */ function isTypeReadonlyArrayOrTuple(type: ts.Type): boolean | null { + function checkTypeArguments(arrayType: ts.TypeReference): boolean { + const typeArguments = checker.getTypeArguments(arrayType); + 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(typeArg))) { + return false; + } + return true; + } + if (checker.isArrayType(type)) { const symbol = util.nullThrows( type.getSymbol(), util.NullThrowsReasons.MissingToken('symbol', 'array type'), ); const escapedName = symbol.getEscapedName(); - if (escapedName === 'ReadonlyArray') { - return true; - } - if (escapedName === 'Array') { + if (escapedName === 'Array' && escapedName !== 'ReadonlyArray') { return false; } + + return checkTypeArguments(type); } if (checker.isTupleType(type)) { - if (type.target.readonly) { - return true; + if (!type.target.readonly) { + return false; } - return false; + + return checkTypeArguments(type); } return null; 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 index 99e04612385..1b48d070fde 100644 --- a/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts @@ -1,5 +1,13 @@ import rule from '../../src/rules/prefer-readonly-parameter-types'; import { RuleTester, getFixturesRootDir } from '../RuleTester'; +import { TSESLint } from '@typescript-eslint/experimental-utils'; +import { + InferMessageIdsTypeFromRule, + InferOptionsTypeFromRule, +} from '../../src/util'; + +type MessageIds = InferMessageIdsTypeFromRule; +type Options = InferOptionsTypeFromRule; const rootPath = getFixturesRootDir(); @@ -24,54 +32,158 @@ const primitives = [ 'null', 'undefined', ]; +const arrays = [ + 'readonly string[]', + 'Readonly', + 'ReadonlyArray', + 'readonly [string]', + 'Readonly<[string]>', +]; +const weirdIntersections = [ + ` + interface Test extends ReadonlyArray { + readonly property: boolean + } + function foo(arg: Test) {} + `, + ` + type Test = (readonly string[]) & { + readonly property: boolean + }; + function foo(arg: Test) {} + `, + ` + 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}) {}`), // arrays - 'function foo(arg: readonly string[]) {}', - 'function foo(arg: Readonly) {}', - 'function foo(arg: ReadonlyArray) {}', - 'function foo(arg: ReadonlyArray | ReadonlyArray) {}', + ...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) {}', - ` - interface ImmutablePropFunction { - (): void - readonly immutable: boolean - } - function foo(arg: ImmutablePropFunction) {} - `, // unions 'function foo(arg: string | null) {}', 'function foo(arg: string | ReadonlyArray) {}', 'function foo(arg: string | (() => void)) {}', + 'function foo(arg: ReadonlyArray | ReadonlyArray) {}', + + // objects + + // weird intersections + ...weirdIntersections.map(code => code), ], 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: string[]) {}', + code: 'function foo(arg: readonly (string[])[]) {}', errors: [ { messageId: 'shouldBeReadonly', - data: { - type: 'Parameters', - }, + 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 + // { + // code: ` + // interface MutablePropFunction { + // (): void + // mutable: boolean + // } + // function foo(arg: MutablePropFunction) {} + // `, + // errors: [], + // }, + + // 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' }], + }, { code: ` - interface MutablePropFunction { - (): void - mutable: boolean + interface Test extends Array { + property: boolean } - function foo(arg: MutablePropFunction) {} + function foo(arg: Test) {} `, - errors: [], + errors: [{ messageId: 'shouldBeReadonly' }], }, ], }); From e77d9779e875e9af6f90dba7becdc7ba84c0ac1e Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Tue, 28 Jan 2020 19:11:20 -0800 Subject: [PATCH 04/11] wip --- .../rules/prefer-readonly-parameter-types.ts | 132 +--------------- packages/eslint-plugin/src/util/index.ts | 1 + .../src/util/isTypeReadonly/index.ts | 147 ++++++++++++++++++ .../util/isTypeReadonly/isReadonlySymbol.ts | 82 ++++++++++ .../prefer-readonly-parameter-types.test.ts | 37 ++++- .../eslint-plugin/typings/typescript.d.ts | 2 +- 6 files changed, 268 insertions(+), 133 deletions(-) create mode 100644 packages/eslint-plugin/src/util/isTypeReadonly/index.ts create mode 100644 packages/eslint-plugin/src/util/isTypeReadonly/isReadonlySymbol.ts diff --git a/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts b/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts index fdf021f31a8..65a11484bf3 100644 --- a/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts +++ b/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts @@ -2,8 +2,6 @@ import { TSESTree, AST_NODE_TYPES, } from '@typescript-eslint/experimental-utils'; -import { isObjectType, isUnionType, unionTypeParts } from 'tsutils'; -import * as ts from 'typescript'; import * as util from '../util'; export default util.createRule({ @@ -31,134 +29,6 @@ export default util.createRule({ const { esTreeNodeToTSNodeMap, program } = util.getParserServices(context); const checker = program.getTypeChecker(); - /** - * 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(type: ts.Type): boolean | null { - function checkTypeArguments(arrayType: ts.TypeReference): boolean { - const typeArguments = checker.getTypeArguments(arrayType); - 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(typeArg))) { - return false; - } - return true; - } - - if (checker.isArrayType(type)) { - const symbol = util.nullThrows( - type.getSymbol(), - util.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(type: ts.Type): boolean | null { - function checkIndex(kind: ts.IndexKind): boolean | null { - const indexInfo = checker.getIndexInfoOfType(type, kind); - if (indexInfo) { - return indexInfo.isReadonly ? true : false; - } - - return null; - } - - const isStringIndexReadonly = checkIndex(ts.IndexKind.String); - if (isStringIndexReadonly !== null) { - return isStringIndexReadonly; - } - - const isNumberIndexReadonly = checkIndex(ts.IndexKind.Number); - if (isNumberIndexReadonly !== null) { - return isNumberIndexReadonly; - } - - const properties = type.getProperties(); - if (properties.length) { - // NOTES: - // - port isReadonlySymbol - https://github.com/Microsoft/TypeScript/blob/4212484ae18163df867f53dab19a8cc0c6000793/src/compiler/checker.ts#L26285 - // eslint-disable-next-line @typescript-eslint/ban-ts-ignore - // @ts-ignore - declare function isReadonlySymbol(symbol: ts.Symbol): boolean; - - for (const property of properties) { - if (!isReadonlySymbol(property)) { - return false; - } - } - - // all properties were readonly - return true; - } - - return false; - } - - /** - * Checks if the given type is readonly - */ - function isTypeReadonly(type: ts.Type): boolean { - if (isUnionType(type)) { - return unionTypeParts(type).every(t => isTypeReadonly(t)); - } - - // all non-object types are readonly - if (!isObjectType(type)) { - return true; - } - - // pure function types are readonly - if ( - type.getCallSignatures().length > 0 && - type.getProperties().length === 0 - ) { - return true; - } - - const isReadonlyArray = isTypeReadonlyArrayOrTuple(type); - if (isReadonlyArray !== null) { - return isReadonlyArray; - } - - const isReadonlyObject = isTypeReadonlyObject(type); - if (isReadonlyObject !== null) { - return isReadonlyObject; - } - - throw new Error('Unhandled type'); - } - return { 'ArrowFunctionExpression, FunctionDeclaration, FunctionExpression, TSEmptyBodyFunctionExpression'( node: @@ -174,7 +44,7 @@ export default util.createRule({ : param; const tsNode = esTreeNodeToTSNodeMap.get(actualParam); const type = checker.getTypeAtLocation(tsNode); - const isReadOnly = isTypeReadonly(type); + const isReadOnly = util.isTypeReadonly(checker, type); if (!isReadOnly) { return context.report({ 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/index.ts b/packages/eslint-plugin/src/util/isTypeReadonly/index.ts new file mode 100644 index 00000000000..06eae5475d5 --- /dev/null +++ b/packages/eslint-plugin/src/util/isTypeReadonly/index.ts @@ -0,0 +1,147 @@ +import { + isObjectType, + isUnionType, + isUnionOrIntersectionType, + unionTypeParts, +} from 'tsutils'; +import * as ts from 'typescript'; +import { nullThrows, NullThrowsReasons } from '..'; +import { isReadonlySymbol } from './isReadonlySymbol'; + +/** + * 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); + 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) { + for (const property of properties) { + // eslint-disable-next-line @typescript-eslint/ban-ts-ignore + // @ts-ignore + const x = checker.isReadonlySymbol(property); + const y = isReadonlySymbol(property); + if (x !== y) { + throw new Error('FUCK'); + } + if (!isReadonlySymbol(property)) { + return false; + } + } + + // all properties were readonly + } + + 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); + if (isReadonlyObject !== null) { + return isReadonlyObject; + } + + throw new Error('Unhandled type'); +} + +export { isTypeReadonly }; diff --git a/packages/eslint-plugin/src/util/isTypeReadonly/isReadonlySymbol.ts b/packages/eslint-plugin/src/util/isTypeReadonly/isReadonlySymbol.ts new file mode 100644 index 00000000000..69b55945938 --- /dev/null +++ b/packages/eslint-plugin/src/util/isTypeReadonly/isReadonlySymbol.ts @@ -0,0 +1,82 @@ +// - this code is ported from typescript's type checker +// Starting at https://github.com/Microsoft/TypeScript/blob/4212484ae18163df867f53dab19a8cc0c6000793/src/compiler/checker.ts#L26285 + +import * as ts from 'typescript'; + +// #region internal types used for isReadonlySymbol + +// we can't use module augmentation because typescript uses export = ts +/* eslint-disable @typescript-eslint/ban-ts-ignore */ + +// CheckFlags is actually const enum +// https://github.com/Microsoft/TypeScript/blob/236012e47b26fee210caa9cbd2e072ef9e99f9ae/src/compiler/types.ts#L4038 +const enum CheckFlags { + Readonly = 1 << 3, +} +type GetCheckFlags = (symbol: ts.Symbol) => CheckFlags; +// @ts-ignore +const getCheckFlags: GetCheckFlags = ts.getCheckFlags; + +type GetDeclarationModifierFlagsFromSymbol = (s: ts.Symbol) => ts.ModifierFlags; +const getDeclarationModifierFlagsFromSymbol: GetDeclarationModifierFlagsFromSymbol = + // @ts-ignore + ts.getDeclarationModifierFlagsFromSymbol; + +/* eslint-enable @typescript-eslint/ban-ts-ignore */ + +// function getDeclarationNodeFlagsFromSymbol(s: ts.Symbol): ts.NodeFlags { +// return s.valueDeclaration ? ts.getCombinedNodeFlags(s.valueDeclaration) : 0; +// } + +// #endregion + +function isReadonlySymbol(symbol: ts.Symbol): boolean { + // The following symbols are considered read-only: + // Properties with a 'readonly' modifier + // Variables declared with 'const' + // Get accessors without matching set accessors + // Enum members + // Unions and intersections of the above (unions and intersections eagerly set isReadonly on creation) + + // transient readonly property + if (getCheckFlags(symbol) & CheckFlags.Readonly) { + console.log('check flags is truthy'); + return true; + } + + // Properties with a 'readonly' modifier + if ( + symbol.flags & ts.SymbolFlags.Property && + getDeclarationModifierFlagsFromSymbol(symbol) & ts.ModifierFlags.Readonly + ) { + return true; + } + + // Variables declared with 'const' + // if ( + // symbol.flags & ts.SymbolFlags.Variable && + // getDeclarationNodeFlagsFromSymbol(symbol) & ts.NodeFlags.Const + // ) { + // return true; + // } + + // Get accessors without matching set accessors + if ( + symbol.flags & ts.SymbolFlags.Accessor && + !(symbol.flags & ts.SymbolFlags.SetAccessor) + ) { + return true; + } + + // Enum members + if (symbol.flags & ts.SymbolFlags.EnumMember) { + return true; + } + + return false; + + // TODO - maybe add this check? + // || symbol.declarations.some(isReadonlyAssignmentDeclaration) +} + +export { isReadonlySymbol }; 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 index 1b48d070fde..8d85b687b6a 100644 --- a/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts @@ -65,10 +65,27 @@ const weirdIntersections = [ }; function foo(arg: Test) {} `, + ` + type Test = string & number; + function foo(arg: Test) {} + `, ]; ruleTester.run('prefer-readonly-parameter-types', rule, { valid: [ + ` + type Test = (readonly string[]) & { + property: boolean + }; + function foo(arg: Test) {} + `, + ` + interface Test extends ReadonlyArray { + property: boolean + } + function foo(arg: Test) {} + `, + 'function foo(arg: { readonly a: string }) {}', 'function foo() {}', // primitives @@ -92,8 +109,26 @@ ruleTester.run('prefer-readonly-parameter-types', rule, { // objects - // weird intersections + // weird other cases ...weirdIntersections.map(code => code), + ` + class Foo { + readonly bang = 1; + } + interface Foo { + readonly prop: string; + } + interface Foo { + readonly prop2: string; + } + function foo(arg: Foo) {} + `, + ` + class Foo { + method() {} + } + function foo(arg: Readonly) {} + `, ], invalid: [ // arrays diff --git a/packages/eslint-plugin/typings/typescript.d.ts b/packages/eslint-plugin/typings/typescript.d.ts index bf379a245b8..12b12c88953 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 { From 9396235f2a2eed2118a5a306583ea20d5d3544b4 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Thu, 30 Jan 2020 21:17:28 -0800 Subject: [PATCH 05/11] feat: object checks --- .../index.ts => isTypeReadonly.ts} | 32 ++++-- .../util/isTypeReadonly/isReadonlySymbol.ts | 82 -------------- .../prefer-readonly-parameter-types.test.ts | 100 +++++++++++------- .../eslint-plugin/typings/typescript.d.ts | 15 ++- 4 files changed, 94 insertions(+), 135 deletions(-) rename packages/eslint-plugin/src/util/{isTypeReadonly/index.ts => isTypeReadonly.ts} (80%) delete mode 100644 packages/eslint-plugin/src/util/isTypeReadonly/isReadonlySymbol.ts diff --git a/packages/eslint-plugin/src/util/isTypeReadonly/index.ts b/packages/eslint-plugin/src/util/isTypeReadonly.ts similarity index 80% rename from packages/eslint-plugin/src/util/isTypeReadonly/index.ts rename to packages/eslint-plugin/src/util/isTypeReadonly.ts index 06eae5475d5..eea0a8785df 100644 --- a/packages/eslint-plugin/src/util/isTypeReadonly/index.ts +++ b/packages/eslint-plugin/src/util/isTypeReadonly.ts @@ -3,10 +3,10 @@ import { isUnionType, isUnionOrIntersectionType, unionTypeParts, + isPropertyReadonlyInType, } from 'tsutils'; import * as ts from 'typescript'; -import { nullThrows, NullThrowsReasons } from '..'; -import { isReadonlySymbol } from './isReadonlySymbol'; +import { nullThrows, NullThrowsReasons } from '.'; /** * Returns: @@ -79,20 +79,32 @@ function isTypeReadonlyObject( const properties = type.getProperties(); if (properties.length) { + // ensure the properties are marked as readonly for (const property of properties) { - // eslint-disable-next-line @typescript-eslint/ban-ts-ignore - // @ts-ignore - const x = checker.isReadonlySymbol(property); - const y = isReadonlySymbol(property); - if (x !== y) { - throw new Error('FUCK'); - } - if (!isReadonlySymbol(property)) { + 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); diff --git a/packages/eslint-plugin/src/util/isTypeReadonly/isReadonlySymbol.ts b/packages/eslint-plugin/src/util/isTypeReadonly/isReadonlySymbol.ts deleted file mode 100644 index 69b55945938..00000000000 --- a/packages/eslint-plugin/src/util/isTypeReadonly/isReadonlySymbol.ts +++ /dev/null @@ -1,82 +0,0 @@ -// - this code is ported from typescript's type checker -// Starting at https://github.com/Microsoft/TypeScript/blob/4212484ae18163df867f53dab19a8cc0c6000793/src/compiler/checker.ts#L26285 - -import * as ts from 'typescript'; - -// #region internal types used for isReadonlySymbol - -// we can't use module augmentation because typescript uses export = ts -/* eslint-disable @typescript-eslint/ban-ts-ignore */ - -// CheckFlags is actually const enum -// https://github.com/Microsoft/TypeScript/blob/236012e47b26fee210caa9cbd2e072ef9e99f9ae/src/compiler/types.ts#L4038 -const enum CheckFlags { - Readonly = 1 << 3, -} -type GetCheckFlags = (symbol: ts.Symbol) => CheckFlags; -// @ts-ignore -const getCheckFlags: GetCheckFlags = ts.getCheckFlags; - -type GetDeclarationModifierFlagsFromSymbol = (s: ts.Symbol) => ts.ModifierFlags; -const getDeclarationModifierFlagsFromSymbol: GetDeclarationModifierFlagsFromSymbol = - // @ts-ignore - ts.getDeclarationModifierFlagsFromSymbol; - -/* eslint-enable @typescript-eslint/ban-ts-ignore */ - -// function getDeclarationNodeFlagsFromSymbol(s: ts.Symbol): ts.NodeFlags { -// return s.valueDeclaration ? ts.getCombinedNodeFlags(s.valueDeclaration) : 0; -// } - -// #endregion - -function isReadonlySymbol(symbol: ts.Symbol): boolean { - // The following symbols are considered read-only: - // Properties with a 'readonly' modifier - // Variables declared with 'const' - // Get accessors without matching set accessors - // Enum members - // Unions and intersections of the above (unions and intersections eagerly set isReadonly on creation) - - // transient readonly property - if (getCheckFlags(symbol) & CheckFlags.Readonly) { - console.log('check flags is truthy'); - return true; - } - - // Properties with a 'readonly' modifier - if ( - symbol.flags & ts.SymbolFlags.Property && - getDeclarationModifierFlagsFromSymbol(symbol) & ts.ModifierFlags.Readonly - ) { - return true; - } - - // Variables declared with 'const' - // if ( - // symbol.flags & ts.SymbolFlags.Variable && - // getDeclarationNodeFlagsFromSymbol(symbol) & ts.NodeFlags.Const - // ) { - // return true; - // } - - // Get accessors without matching set accessors - if ( - symbol.flags & ts.SymbolFlags.Accessor && - !(symbol.flags & ts.SymbolFlags.SetAccessor) - ) { - return true; - } - - // Enum members - if (symbol.flags & ts.SymbolFlags.EnumMember) { - return true; - } - - return false; - - // TODO - maybe add this check? - // || symbol.declarations.some(isReadonlyAssignmentDeclaration) -} - -export { isReadonlySymbol }; 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 index 8d85b687b6a..894f6de6245 100644 --- a/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts @@ -39,19 +39,12 @@ const arrays = [ 'readonly [string]', 'Readonly<[string]>', ]; +const objects = [ + '{ foo: "" }', + '{ foo: readonly string[] }', + '{ foo(): void }', +]; const weirdIntersections = [ - ` - interface Test extends ReadonlyArray { - readonly property: boolean - } - function foo(arg: Test) {} - `, - ` - type Test = (readonly string[]) & { - readonly property: boolean - }; - function foo(arg: Test) {} - `, ` interface Test { (): void @@ -65,26 +58,10 @@ const weirdIntersections = [ }; function foo(arg: Test) {} `, - ` - type Test = string & number; - function foo(arg: Test) {} - `, ]; ruleTester.run('prefer-readonly-parameter-types', rule, { valid: [ - ` - type Test = (readonly string[]) & { - property: boolean - }; - function foo(arg: Test) {} - `, - ` - interface Test extends ReadonlyArray { - property: boolean - } - function foo(arg: Test) {} - `, 'function foo(arg: { readonly a: string }) {}', 'function foo() {}', @@ -108,9 +85,35 @@ ruleTester.run('prefer-readonly-parameter-types', rule, { 'function foo(arg: ReadonlyArray | ReadonlyArray) {}', // objects + ...objects.map(type => `function foo(arg: Readonly<${type}>) {}`), + ` + function foo(arg: { + readonly foo: { + readonly bar: string + } + }) {} + `, // 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; @@ -123,6 +126,7 @@ ruleTester.run('prefer-readonly-parameter-types', rule, { } function foo(arg: Foo) {} `, + // method made readonly via Readonly ` class Foo { method() {} @@ -181,16 +185,36 @@ ruleTester.run('prefer-readonly-parameter-types', rule, { }, // objects - // { - // code: ` - // interface MutablePropFunction { - // (): void - // mutable: boolean - // } - // function foo(arg: MutablePropFunction) {} - // `, - // errors: [], - // }, + ...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, + }, + ], + }, // weird intersections ...weirdIntersections.map>( diff --git a/packages/eslint-plugin/typings/typescript.d.ts b/packages/eslint-plugin/typings/typescript.d.ts index 12b12c88953..7c9089158b4 100644 --- a/packages/eslint-plugin/typings/typescript.d.ts +++ b/packages/eslint-plugin/typings/typescript.d.ts @@ -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): type is TypeReference; /** * @returns `true` if the given type is a tuple type: - * - [foo] + * - `[foo]` + * - `readonly [foo]` */ 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; } } From 5c70d6e292299552b7e39066b5c4c80899102fa5 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Thu, 30 Jan 2020 21:47:08 -0800 Subject: [PATCH 06/11] fix: format --- packages/eslint-plugin/src/util/isTypeReadonly.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/eslint-plugin/src/util/isTypeReadonly.ts b/packages/eslint-plugin/src/util/isTypeReadonly.ts index eea0a8785df..32b4c55b176 100644 --- a/packages/eslint-plugin/src/util/isTypeReadonly.ts +++ b/packages/eslint-plugin/src/util/isTypeReadonly.ts @@ -81,11 +81,7 @@ function isTypeReadonlyObject( if (properties.length) { // ensure the properties are marked as readonly for (const property of properties) { - if (!isPropertyReadonlyInType( - type, - property.getEscapedName(), - checker, - )) { + if (!isPropertyReadonlyInType(type, property.getEscapedName(), checker)) { return false; } } From 24702cb889a6917d77aaaf01e5a634f4e47a1655 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Mon, 3 Feb 2020 10:32:44 -0800 Subject: [PATCH 07/11] wip --- packages/eslint-plugin/README.md | 1 + packages/eslint-plugin/src/rules/index.ts | 24 ++++++++++++----------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 0f55ace3d5d..58ab5f07192 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -141,6 +141,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) | TODO | | | :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/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 16a4c705ca5..5af31e4ed98 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'; @@ -28,10 +28,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'; @@ -40,8 +40,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'; @@ -50,7 +50,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'; @@ -67,6 +67,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'; @@ -90,8 +91,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, 'brace-style': braceStyle, camelcase: camelcase, @@ -123,28 +124,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, @@ -155,6 +156,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, From 6eaa9f343d8ab479631bc9b0242fb12d01dfdb1c Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Thu, 6 Feb 2020 20:58:35 -0800 Subject: [PATCH 08/11] docs: add readme --- packages/eslint-plugin/README.md | 2 +- packages/eslint-plugin/src/configs/all.json | 1 + .../src/rules/prefer-readonly-parameter-types.ts | 3 ++- .../rules/prefer-readonly-parameter-types.test.ts | 10 +++++++++- 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 58ab5f07192..d2053ec7fa5 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -141,7 +141,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) | TODO | | | :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/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index 23e9db2d974..d9075e47265 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -78,6 +78,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/prefer-readonly-parameter-types.ts b/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts index 65a11484bf3..e03af55c713 100644 --- a/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts +++ b/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts @@ -9,7 +9,8 @@ export default util.createRule({ meta: { type: 'suggestion', docs: { - description: 'TODO', + description: + 'Requires that function parameters are typed as readonly to prevent accidental mutation of inputs', category: 'Possible Errors', recommended: false, requiresTypeChecking: true, 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 index 894f6de6245..f7e7235a6ae 100644 --- a/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts @@ -26,6 +26,7 @@ const primitives = [ "'a'", 'number', '1', + 'symbol', 'any', 'unknown', 'never', @@ -62,11 +63,18 @@ const weirdIntersections = [ ruleTester.run('prefer-readonly-parameter-types', rule, { valid: [ - 'function foo(arg: { readonly a: string }) {}', '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}) {}`), From 7bf7e3e813439c32a6a7259bd0ab24091d798d83 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Thu, 6 Feb 2020 21:01:08 -0800 Subject: [PATCH 09/11] docs: docs --- .../rules/prefer-readonly-parameter-types.md | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/prefer-readonly-parameter-types.md 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..5b9500539a9 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/prefer-readonly-parameter-types.md @@ -0,0 +1,91 @@ +# 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 +``` + +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) {} +``` From 7b3a14524ad62807cac45b67e6f2ee5ce63af5a4 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Fri, 7 Feb 2020 10:06:49 -0800 Subject: [PATCH 10/11] test: more tests --- .../rules/prefer-readonly-parameter-types.ts | 35 +++- .../eslint-plugin/src/util/isTypeReadonly.ts | 4 +- .../prefer-readonly-parameter-types.test.ts | 183 +++++++++++++++++- 3 files changed, 210 insertions(+), 12 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts b/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts index e03af55c713..5fedd24d755 100644 --- a/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts +++ b/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts @@ -4,7 +4,14 @@ import { } from '@typescript-eslint/experimental-utils'; import * as util from '../util'; -export default util.createRule({ +type Options = [ + { + checkParameterProperties?: boolean; + }, +]; +type MessageIds = 'shouldBeReadonly'; + +export default util.createRule({ name: 'prefer-readonly-parameter-types', meta: { type: 'suggestion', @@ -17,16 +24,25 @@ export default util.createRule({ }, schema: [ { - type: 'string', - enum: ['prefer-readonly', 'prefer-mutable', 'ignore'], + type: 'object', + additionalProperties: false, + properties: { + checkParameterProperties: { + type: 'boolean', + }, + }, }, ], messages: { shouldBeReadonly: 'Parameter should be a read only type', }, }, - defaultOptions: [], - create(context) { + defaultOptions: [ + { + checkParameterProperties: true, + }, + ], + create(context, [{ checkParameterProperties }]) { const { esTreeNodeToTSNodeMap, program } = util.getParserServices(context); const checker = program.getTypeChecker(); @@ -39,6 +55,13 @@ export default util.createRule({ | TSESTree.TSEmptyBodyFunctionExpression, ): 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 @@ -48,7 +71,7 @@ export default util.createRule({ const isReadOnly = util.isTypeReadonly(checker, type); if (!isReadOnly) { - return context.report({ + context.report({ node: actualParam, messageId: 'shouldBeReadonly', }); diff --git a/packages/eslint-plugin/src/util/isTypeReadonly.ts b/packages/eslint-plugin/src/util/isTypeReadonly.ts index 32b4c55b176..99df07b153a 100644 --- a/packages/eslint-plugin/src/util/isTypeReadonly.ts +++ b/packages/eslint-plugin/src/util/isTypeReadonly.ts @@ -20,7 +20,7 @@ function isTypeReadonlyArrayOrTuple( ): boolean | null { function checkTypeArguments(arrayType: ts.TypeReference): boolean { const typeArguments = checker.getTypeArguments(arrayType); - if (typeArguments.length === 0) { + /* 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 @@ -145,7 +145,7 @@ function isTypeReadonly(checker: ts.TypeChecker, type: ts.Type): boolean { } const isReadonlyObject = isTypeReadonlyObject(checker, type); - if (isReadonlyObject !== null) { + /* istanbul ignore else */ if (isReadonlyObject !== null) { return isReadonlyObject; } 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 index f7e7235a6ae..c5fa5233de7 100644 --- a/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts @@ -1,6 +1,6 @@ -import rule from '../../src/rules/prefer-readonly-parameter-types'; -import { RuleTester, getFixturesRootDir } from '../RuleTester'; import { TSESLint } from '@typescript-eslint/experimental-utils'; +import { RuleTester, getFixturesRootDir } from '../RuleTester'; +import rule from '../../src/rules/prefer-readonly-parameter-types'; import { InferMessageIdsTypeFromRule, InferOptionsTypeFromRule, @@ -101,6 +101,20 @@ ruleTester.run('prefer-readonly-parameter-types', rule, { } }) {} `, + ` + 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), @@ -141,6 +155,42 @@ ruleTester.run('prefer-readonly-parameter-types', rule, { } 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, + }, + ], + }, ], invalid: [ // arrays @@ -223,6 +273,39 @@ ruleTester.run('prefer-readonly-parameter-types', rule, { }, ], }, + // 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>( @@ -241,7 +324,15 @@ ruleTester.run('prefer-readonly-parameter-types', rule, { } function foo(arg: Test) {} `, - errors: [{ messageId: 'shouldBeReadonly' }], + errors: [ + { + messageId: 'shouldBeReadonly', + line: 5, + column: 22, + endLine: 5, + endColumn: 31, + }, + ], }, { code: ` @@ -250,7 +341,91 @@ ruleTester.run('prefer-readonly-parameter-types', rule, { } function foo(arg: Test) {} `, - errors: [{ messageId: 'shouldBeReadonly' }], + 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, + }, + ], }, ], }); From fb5c338a25ab3187eba894dfd1f9111f37ab322e Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Fri, 28 Feb 2020 17:46:05 -0800 Subject: [PATCH 11/11] feat: support function types --- .../rules/prefer-readonly-parameter-types.md | 72 ++++++++++++++++++ .../rules/prefer-readonly-parameter-types.ts | 19 ++++- .../prefer-readonly-parameter-types.test.ts | 76 +++++++++++++++++++ 3 files changed, 165 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/prefer-readonly-parameter-types.md b/packages/eslint-plugin/docs/rules/prefer-readonly-parameter-types.md index 5b9500539a9..c69b3b21c34 100644 --- a/packages/eslint-plugin/docs/rules/prefer-readonly-parameter-types.md +++ b/packages/eslint-plugin/docs/rules/prefer-readonly-parameter-types.md @@ -41,6 +41,20 @@ interface CustomFunction { 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: @@ -88,4 +102,62 @@ 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/rules/prefer-readonly-parameter-types.ts b/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts index 5fedd24d755..07e8e69940c 100644 --- a/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts +++ b/packages/eslint-plugin/src/rules/prefer-readonly-parameter-types.ts @@ -47,12 +47,27 @@ export default util.createRule({ const checker = program.getTypeChecker(); return { - 'ArrowFunctionExpression, FunctionDeclaration, FunctionExpression, TSEmptyBodyFunctionExpression'( + [[ + 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.TSEmptyBodyFunctionExpression, + | TSESTree.TSCallSignatureDeclaration + | TSESTree.TSConstructSignatureDeclaration + | TSESTree.TSDeclareFunction + | TSESTree.TSEmptyBodyFunctionExpression + | TSESTree.TSFunctionType + | TSESTree.TSMethodSignature, ): void { for (const param of node.params) { if ( 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 index c5fa5233de7..b869e0c15ed 100644 --- a/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-readonly-parameter-types.test.ts @@ -191,6 +191,14 @@ ruleTester.run('prefer-readonly-parameter-types', rule, { }, ], }, + + // 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 @@ -427,5 +435,73 @@ ruleTester.run('prefer-readonly-parameter-types', rule, { }, ], }, + + // 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, + }, + ], + }, ], });