Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(type-utils): check IndexSignature internals when checking isTypeReadonly #4417

Merged
merged 9 commits into from Jan 17, 2022
58 changes: 42 additions & 16 deletions packages/type-utils/src/isTypeReadonly.ts
Expand Up @@ -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,
RebeccaStevens marked this conversation as resolved.
Show resolved Hide resolved
options,
seenTypes,
) && isTypeReadonlyRecurser(checker, indexInfo.type, options, seenTypes)
);
}

return Readonlyness.UnknownType;
Expand All @@ -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
Expand Down
164 changes: 164 additions & 0 deletions packages/type-utils/tests/isTypeReadonly.test.ts
@@ -0,0 +1,164 @@
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<readonly string[]>;'],
['type Test = Readonly<ReadonlyArray<string>>;'],
])('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<string>;'],
])('treats readonly arrays as fully readonly', runTests);

// Set and Map.
it.each([
['type Test = Readonly<ReadonlySet<string>>;'],
['type Test = Readonly<ReadonlyMap<string, string>>;'],
])('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<string>']])(
'handles non fully readonly arrays',
runTests,
);

// Set and Map.
// Note: Methods are mutable for ReadonlySet and ReadonlyMet; hence failure.
it.each([
['type Test = Set<string>;'],
['type Test = Map<string, string>;'],
['type Test = ReadonlySet<string>;'],
['type Test = ReadonlyMap<string, string>;'],
])('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', () => {
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<string>;'],
['type Test = ReadonlyMap<string, string>;'],
])('handles non fully readonly sets and maps', runTests);
});
});
});
});