From 974858a82236c3e6c5bb16759c4e0d969d9172ff Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Sun, 9 Jan 2022 19:07:35 +1300 Subject: [PATCH 1/5] fix(type-utils): make isTypeReadonly's options param optional fix #4410 --- packages/type-utils/src/isTypeReadonly.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/type-utils/src/isTypeReadonly.ts b/packages/type-utils/src/isTypeReadonly.ts index c6bc0f5761a..7f689b5b7df 100644 --- a/packages/type-utils/src/isTypeReadonly.ts +++ b/packages/type-utils/src/isTypeReadonly.ts @@ -243,7 +243,7 @@ function isTypeReadonlyRecurser( function isTypeReadonly( checker: ts.TypeChecker, type: ts.Type, - options: ReadonlynessOptions, + options: ReadonlynessOptions = readonlynessOptionsDefaults, ): boolean { return ( isTypeReadonlyRecurser(checker, type, options, new Set()) === From bb209f41e5961931fd27e5bc26b39136efeba13d Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Sun, 9 Jan 2022 19:59:56 +1300 Subject: [PATCH 2/5] test(type-utils): add basic tests for isTypeReadonly --- .../type-utils/tests/isTypeReadonly.test.ts | 138 ++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 packages/type-utils/tests/isTypeReadonly.test.ts diff --git a/packages/type-utils/tests/isTypeReadonly.test.ts b/packages/type-utils/tests/isTypeReadonly.test.ts new file mode 100644 index 00000000000..5ecfc078f98 --- /dev/null +++ b/packages/type-utils/tests/isTypeReadonly.test.ts @@ -0,0 +1,138 @@ +import * as ts from 'typescript'; +import { TSESTree } from '@typescript-eslint/experimental-utils'; +import { parseForESLint } from '@typescript-eslint/parser'; +import { + isTypeReadonly, + type ReadonlynessOptions, +} from '../src/isTypeReadonly'; +import path from 'path'; + +describe('isTypeReadonly', () => { + const rootDir = path.join(__dirname, 'fixtures'); + + describe('TSTypeAliasDeclaration ', () => { + function getType(code: string): { + type: ts.Type; + checker: ts.TypeChecker; + } { + const { ast, services } = parseForESLint(code, { + project: './tsconfig.json', + filePath: path.join(rootDir, 'file.ts'), + tsconfigRootDir: rootDir, + }); + const checker = services.program.getTypeChecker(); + const esTreeNodeToTSNodeMap = services.esTreeNodeToTSNodeMap; + + const declaration = ast.body[0] as TSESTree.TSTypeAliasDeclaration; + return { + type: checker.getTypeAtLocation( + esTreeNodeToTSNodeMap.get(declaration.id), + ), + checker, + }; + } + + function runTestForAliasDeclaration( + code: string, + options: ReadonlynessOptions | undefined, + expected: boolean, + ): void { + const { type, checker } = getType(code); + + const result = isTypeReadonly(checker, type, options); + expect(result).toBe(expected); + } + + describe('default options', () => { + const options = undefined; + + function runTestIsReadonly(code: string): void { + runTestForAliasDeclaration(code, options, true); + } + + function runTestIsNotReadonly(code: string): void { + runTestForAliasDeclaration(code, options, false); + } + + describe('basics', () => { + describe('is readonly', () => { + const runTests = runTestIsReadonly; + + // Record. + it.each([ + ['type Test = { readonly bar: string; };'], + ['type Test = Readonly<{ bar: string; }>;'], + ])('handles fully readonly records', runTests); + + // Array. + it.each([ + ['type Test = Readonly;'], + ['type Test = Readonly>;'], + ])('handles fully readonly arrays', runTests); + + // Array - special case. + // Note: Methods are mutable but arrays are treated special; hence no failure. + it.each([ + ['type Test = readonly string[];'], + ['type Test = ReadonlyArray;'], + ])('treats readonly arrays as fully readonly', runTests); + + // Set and Map. + it.each([ + ['type Test = Readonly>;'], + ['type Test = Readonly>;'], + ])('handles fully readonly sets and maps', runTests); + }); + + describe('is not readonly', () => { + const runTests = runTestIsNotReadonly; + + // Record. + it.each([ + ['type Test = { foo: string; };'], + ['type Test = { foo: string; readonly bar: number; };'], + ])('handles non fully readonly records', runTests); + + // Array. + it.each([['type Test = string[]'], ['type Test = Array']])( + 'handles non fully readonly arrays', + runTests, + ); + + // Set and Map. + // Note: Methods are mutable for ReadonlySet and ReadonlyMet; hence failure. + it.each([ + ['type Test = Set;'], + ['type Test = Map;'], + ['type Test = ReadonlySet;'], + ['type Test = ReadonlyMap;'], + ])('handles non fully readonly sets and maps', runTests); + }); + }); + }); + + describe('treatMethodsAsReadonly', () => { + const options: ReadonlynessOptions = { + treatMethodsAsReadonly: true, + }; + + function runTestIsReadonly(code: string): void { + runTestForAliasDeclaration(code, options, true); + } + + // function runTestIsNotReadonly(code: string): void { + // runTestForAliasDeclaration(code, options, false); + // } + + describe('is readonly', () => { + const runTests = runTestIsReadonly; + + // Set and Map. + it.each([ + ['type Test = ReadonlySet;'], + ['type Test = ReadonlyMap;'], + ])('handles non fully readonly sets and maps', runTests); + }); + }); + }); +}); From 8047fa902f2fe35971ce6e74d80a216348ac3cf8 Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Sun, 9 Jan 2022 20:12:40 +1300 Subject: [PATCH 3/5] test(type-utils): add test for IndexSignature internals --- .../type-utils/tests/isTypeReadonly.test.ts | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/packages/type-utils/tests/isTypeReadonly.test.ts b/packages/type-utils/tests/isTypeReadonly.test.ts index 5ecfc078f98..ed0c36ea72c 100644 --- a/packages/type-utils/tests/isTypeReadonly.test.ts +++ b/packages/type-utils/tests/isTypeReadonly.test.ts @@ -109,6 +109,32 @@ 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]: { readonly foo: readonly string[]; }; };', + ], + ])( + 'handles readonly PropertySignature inside a readonly IndexSignature', + runTests, + ); + }); + + describe('is not readonly', () => { + const runTests = runTestIsNotReadonly; + + it.each([ + ['type Test = { readonly [key: string]: { foo: string[]; }; };'], + ])( + 'handles mutable PropertySignature inside a readonly IndexSignature', + runTests, + ); + }); + }); }); describe('treatMethodsAsReadonly', () => { From 957eed43848fe6622e807f14f99f70e159bc7e5e Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Sun, 9 Jan 2022 20:39:14 +1300 Subject: [PATCH 4/5] fix(type-utils): check IndexSignature internals when checking isTypeReadonly fix #3714 --- packages/type-utils/src/isTypeReadonly.ts | 58 ++++++++++++++++------- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/packages/type-utils/src/isTypeReadonly.ts b/packages/type-utils/src/isTypeReadonly.ts index 7f689b5b7df..b27c39a7b67 100644 --- a/packages/type-utils/src/isTypeReadonly.ts +++ b/packages/type-utils/src/isTypeReadonly.ts @@ -107,9 +107,18 @@ 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.keyType, + options, + seenTypes, + ) && isTypeReadonlyRecurser(checker, indexInfo.type, options, seenTypes) + ); } return Readonlyness.UnknownType; @@ -119,20 +128,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 From 1de3f351a90d9caed929fb585cedfe3e043eb512 Mon Sep 17 00:00:00 2001 From: Rebecca Stevens Date: Mon, 17 Jan 2022 21:02:23 +1300 Subject: [PATCH 5/5] perf(type-utils): don't test IndexSignature key for readonlyness as it will always be readonly --- packages/type-utils/src/isTypeReadonly.ts | 12 +++++------- packages/type-utils/tests/isTypeReadonly.test.ts | 2 ++ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/type-utils/src/isTypeReadonly.ts b/packages/type-utils/src/isTypeReadonly.ts index 02a251966f6..632108cd6b3 100644 --- a/packages/type-utils/src/isTypeReadonly.ts +++ b/packages/type-utils/src/isTypeReadonly.ts @@ -111,13 +111,11 @@ function isTypeReadonlyObject( return Readonlyness.Mutable; } - return ( - isTypeReadonlyRecurser( - checker, - indexInfo.keyType, - options, - seenTypes, - ) && isTypeReadonlyRecurser(checker, indexInfo.type, options, seenTypes) + return isTypeReadonlyRecurser( + checker, + indexInfo.type, + options, + seenTypes, ); } diff --git a/packages/type-utils/tests/isTypeReadonly.test.ts b/packages/type-utils/tests/isTypeReadonly.test.ts index ed0c36ea72c..6d30ef2bdb7 100644 --- a/packages/type-utils/tests/isTypeReadonly.test.ts +++ b/packages/type-utils/tests/isTypeReadonly.test.ts @@ -115,6 +115,7 @@ describe('isTypeReadonly', () => { const runTests = runTestIsReadonly; it.each([ + ['type Test = { readonly [key: string]: string };'], [ 'type Test = { readonly [key: string]: { readonly foo: readonly string[]; }; };', ], @@ -128,6 +129,7 @@ describe('isTypeReadonly', () => { const runTests = runTestIsNotReadonly; it.each([ + ['type Test = { [key: string]: string };'], ['type Test = { readonly [key: string]: { foo: string[]; }; };'], ])( 'handles mutable PropertySignature inside a readonly IndexSignature',