From ef3147cf73767ddece91ce57f6028a83ce074b60 Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Mon, 17 Jan 2022 21:59:04 +1300 Subject: [PATCH] fix(type-utils): check IndexSignature internals when checking isTypeReadonly (#4417) * fix(type-utils): make isTypeReadonly's options param optional fix #4410 * test(type-utils): add basic tests for isTypeReadonly * test(type-utils): add test for IndexSignature internals * fix(type-utils): check IndexSignature internals when checking isTypeReadonly fix #3714 * perf(type-utils): don't test IndexSignature key for readonlyness as it will always be readonly Co-authored-by: Josh Goldberg --- packages/type-utils/src/isTypeReadonly.ts | 56 +++++++++++++------ .../type-utils/tests/isTypeReadonly.test.ts | 28 ++++++++++ 2 files changed, 68 insertions(+), 16 deletions(-) diff --git a/packages/type-utils/src/isTypeReadonly.ts b/packages/type-utils/src/isTypeReadonly.ts index d5d05e94a0e..632108cd6b3 100644 --- a/packages/type-utils/src/isTypeReadonly.ts +++ b/packages/type-utils/src/isTypeReadonly.ts @@ -107,9 +107,16 @@ function isTypeReadonlyObject( function checkIndexSignature(kind: ts.IndexKind): Readonlyness { const indexInfo = checker.getIndexInfoOfType(type, kind); if (indexInfo) { - return indexInfo.isReadonly - ? Readonlyness.Readonly - : Readonlyness.Mutable; + if (!indexInfo.isReadonly) { + return Readonlyness.Mutable; + } + + return isTypeReadonlyRecurser( + checker, + indexInfo.type, + options, + seenTypes, + ); } return Readonlyness.UnknownType; @@ -119,20 +126,37 @@ function isTypeReadonlyObject( if (properties.length) { // ensure the properties are marked as readonly for (const property of properties) { - if ( - !( - isPropertyReadonlyInType(type, property.getEscapedName(), checker) || - (options.treatMethodsAsReadonly && - property.valueDeclaration !== undefined && - hasSymbol(property.valueDeclaration) && - isSymbolFlagSet( - property.valueDeclaration.symbol, - ts.SymbolFlags.Method, - )) - ) - ) { - return Readonlyness.Mutable; + if (options.treatMethodsAsReadonly) { + if ( + property.valueDeclaration !== undefined && + hasSymbol(property.valueDeclaration) && + isSymbolFlagSet( + property.valueDeclaration.symbol, + ts.SymbolFlags.Method, + ) + ) { + continue; + } + + const declarations = property.getDeclarations(); + const lastDeclaration = + declarations !== undefined && declarations.length > 0 + ? declarations[declarations.length - 1] + : undefined; + if ( + lastDeclaration !== undefined && + hasSymbol(lastDeclaration) && + isSymbolFlagSet(lastDeclaration.symbol, ts.SymbolFlags.Method) + ) { + continue; + } + } + + if (isPropertyReadonlyInType(type, property.getEscapedName(), checker)) { + continue; } + + return Readonlyness.Mutable; } // all properties were readonly diff --git a/packages/type-utils/tests/isTypeReadonly.test.ts b/packages/type-utils/tests/isTypeReadonly.test.ts index 5afc3b6e59c..d63ecf422d9 100644 --- a/packages/type-utils/tests/isTypeReadonly.test.ts +++ b/packages/type-utils/tests/isTypeReadonly.test.ts @@ -109,6 +109,34 @@ describe('isTypeReadonly', () => { ])('handles non fully readonly sets and maps', runTests); }); }); + + describe('IndexSignature', () => { + describe('is readonly', () => { + const runTests = runTestIsReadonly; + + it.each([ + ['type Test = { readonly [key: string]: string };'], + [ + 'type Test = { readonly [key: string]: { readonly foo: readonly string[]; }; };', + ], + ])( + 'handles readonly PropertySignature inside a readonly IndexSignature', + runTests, + ); + }); + + describe('is not readonly', () => { + const runTests = runTestIsNotReadonly; + + it.each([ + ['type Test = { [key: string]: string };'], + ['type Test = { readonly [key: string]: { foo: string[]; }; };'], + ])( + 'handles mutable PropertySignature inside a readonly IndexSignature', + runTests, + ); + }); + }); }); describe('treatMethodsAsReadonly', () => {