From d43112a75b06b263411940cef80ba52f3a01aed8 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Wed, 7 Dec 2022 13:38:33 -0800 Subject: [PATCH] Use `missingType` in `--noUncheckedIndexedAccess` mode (#51653) * Use missingType in noUncheckedIndexedAccess mode * Accept new baselines * Add tests * Optimizing searching for undefinedType and missingType --- src/compiler/checker.ts | 45 +++++---- ...rowingWithNoUncheckedIndexedAccess.symbols | 75 +++++++++++++++ ...arrowingWithNoUncheckedIndexedAccess.types | 96 +++++++++++++++++++ .../noUncheckedIndexedAccess.errors.txt | 2 + ...rdNarrowingWithNoUncheckedIndexedAccess.ts | 37 +++++++ 5 files changed, 234 insertions(+), 21 deletions(-) create mode 100644 tests/baselines/reference/inKeywordNarrowingWithNoUncheckedIndexedAccess.symbols create mode 100644 tests/baselines/reference/inKeywordNarrowingWithNoUncheckedIndexedAccess.types create mode 100644 tests/cases/compiler/inKeywordNarrowingWithNoUncheckedIndexedAccess.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 42211aa4f2a..1b46c0e0911 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -1813,6 +1813,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { const unresolvedSymbols = new Map(); const errorTypes = new Map(); + // We specifically create the `undefined` and `null` types before any other types that can occur in + // unions such that they are given low type IDs and occur first in the sorted list of union constituents. + // We can then just examine the first constituent(s) of a union to check for their presence. + const anyType = createIntrinsicType(TypeFlags.Any, "any"); const autoType = createIntrinsicType(TypeFlags.Any, "any", ObjectFlags.NonInferrableType); const wildcardType = createIntrinsicType(TypeFlags.Any, "any"); @@ -1824,8 +1828,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { const nonNullUnknownType = createIntrinsicType(TypeFlags.Unknown, "unknown"); const undefinedType = createIntrinsicType(TypeFlags.Undefined, "undefined"); const undefinedWideningType = strictNullChecks ? undefinedType : createIntrinsicType(TypeFlags.Undefined, "undefined", ObjectFlags.ContainsWideningType); + const missingType = createIntrinsicType(TypeFlags.Undefined, "undefined"); + const undefinedOrMissingType = exactOptionalPropertyTypes ? missingType : undefinedType; const optionalType = createIntrinsicType(TypeFlags.Undefined, "undefined"); - const missingType = exactOptionalPropertyTypes ? createIntrinsicType(TypeFlags.Undefined, "undefined") : undefinedType; const nullType = createIntrinsicType(TypeFlags.Null, "null"); const nullWideningType = strictNullChecks ? nullType : createIntrinsicType(TypeFlags.Null, "null", ObjectFlags.ContainsWideningType); const stringType = createIntrinsicType(TypeFlags.String, "string"); @@ -15952,10 +15957,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { includes & TypeFlags.IncludesWildcard ? wildcardType : anyType : includes & TypeFlags.Null || containsType(typeSet, unknownType) ? unknownType : nonNullUnknownType; } - if (exactOptionalPropertyTypes && includes & TypeFlags.Undefined) { - const missingIndex = binarySearch(typeSet, missingType, getTypeId, compareValues); - if (missingIndex >= 0 && containsType(typeSet, undefinedType)) { - orderedRemoveItemAt(typeSet, missingIndex); + if (includes & TypeFlags.Undefined) { + // If type set contains both undefinedType and missingType, remove missingType + if (typeSet.length >= 2 && typeSet[0] === undefinedType && typeSet[1] === missingType) { + orderedRemoveItemAt(typeSet, 1); } } if (includes & (TypeFlags.Literal | TypeFlags.UniqueESSymbol | TypeFlags.TemplateLiteral | TypeFlags.StringMapping) || includes & TypeFlags.Void && includes & TypeFlags.Undefined) { @@ -16096,7 +16101,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { if (type === wildcardType) includes |= TypeFlags.IncludesWildcard; } else if (strictNullChecks || !(flags & TypeFlags.Nullable)) { - if (exactOptionalPropertyTypes && type === missingType) { + if (type === missingType) { includes |= TypeFlags.IncludesMissingType; type = undefinedType; } @@ -16320,9 +16325,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { result = getIntersectionType(typeSet, aliasSymbol, aliasTypeArguments); } else if (eachIsUnionContaining(typeSet, TypeFlags.Undefined)) { - const undefinedOrMissingType = exactOptionalPropertyTypes && some(typeSet, t => containsType((t as UnionType).types, missingType)) ? missingType : undefinedType; + const containedUndefinedType = some(typeSet, containsMissingType) ? missingType : undefinedType; removeFromEach(typeSet, TypeFlags.Undefined); - result = getUnionType([getIntersectionType(typeSet), undefinedOrMissingType], UnionReduction.Literal, aliasSymbol, aliasTypeArguments); + result = getUnionType([getIntersectionType(typeSet), containedUndefinedType], UnionReduction.Literal, aliasSymbol, aliasTypeArguments); } else if (eachIsUnionContaining(typeSet, TypeFlags.Null)) { removeFromEach(typeSet, TypeFlags.Null); @@ -16853,7 +16858,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { errorIfWritingToReadonlyIndex(getIndexInfoOfType(objectType, numberType)); return mapType(objectType, t => { const restType = getRestTypeOfTupleType(t as TupleTypeReference) || undefinedType; - return accessFlags & AccessFlags.IncludeUndefined ? getUnionType([restType, undefinedType]) : restType; + return accessFlags & AccessFlags.IncludeUndefined ? getUnionType([restType, missingType]) : restType; }); } } @@ -16875,7 +16880,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { if (accessNode && indexInfo.keyType === stringType && !isTypeAssignableToKind(indexType, TypeFlags.String | TypeFlags.Number)) { const indexNode = getIndexNodeForAccessExpression(accessNode); error(indexNode, Diagnostics.Type_0_cannot_be_used_as_an_index_type, typeToString(indexType)); - return accessFlags & AccessFlags.IncludeUndefined ? getUnionType([indexInfo.type, undefinedType]) : indexInfo.type; + return accessFlags & AccessFlags.IncludeUndefined ? getUnionType([indexInfo.type, missingType]) : indexInfo.type; } errorIfWritingToReadonlyIndex(indexInfo); // When accessing an enum object with its own type, @@ -16887,7 +16892,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { (indexType.symbol && indexType.flags & TypeFlags.EnumLiteral && getParentOfSymbol(indexType.symbol) === objectType.symbol))) { - return getUnionType([indexInfo.type, undefinedType]); + return getUnionType([indexInfo.type, missingType]); } return indexInfo.type; } @@ -20421,8 +20426,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { } function getUndefinedStrippedTargetIfNeeded(source: Type, target: Type) { - // As a builtin type, `undefined` is a very low type ID - making it almsot always first, making this a very fast check to see - // if we need to strip `undefined` from the target if (source.flags & TypeFlags.Union && target.flags & TypeFlags.Union && !((source as UnionType).types[0].flags & TypeFlags.Undefined) && (target as UnionType).types[0].flags & TypeFlags.Undefined) { return extractTypesOfKind(target, ~TypeFlags.Undefined); @@ -22790,8 +22793,8 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { function getOptionalType(type: Type, isProperty = false): Type { Debug.assert(strictNullChecks); - const missingOrUndefined = isProperty ? missingType : undefinedType; - return type.flags & TypeFlags.Undefined || type.flags & TypeFlags.Union && (type as UnionType).types[0] === missingOrUndefined ? type : getUnionType([type, missingOrUndefined]); + const missingOrUndefined = isProperty ? undefinedOrMissingType : undefinedType; + return type === missingOrUndefined || type.flags & TypeFlags.Union && (type as UnionType).types[0] === missingOrUndefined ? type : getUnionType([type, missingOrUndefined]); } function getGlobalNonNullableTypeInstantiation(type: Type) { @@ -22830,7 +22833,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { } function containsMissingType(type: Type) { - return exactOptionalPropertyTypes && (type === missingType || type.flags & TypeFlags.Union && containsType((type as UnionType).types, missingType)); + return type === missingType || !!(type.flags & TypeFlags.Union) && (type as UnionType).types[0] === missingType; } function removeMissingOrUndefinedType(type: Type): Type { @@ -22983,7 +22986,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { if (cached) { return cached; } - const result = createSymbolWithType(prop, missingType); + const result = createSymbolWithType(prop, undefinedOrMissingType); result.flags |= SymbolFlags.Optional; undefinedProperties.set(prop.escapedName, result); return result; @@ -24388,7 +24391,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { } function isTypeOrBaseIdenticalTo(s: Type, t: Type) { - return exactOptionalPropertyTypes && t === missingType ? s === t : + return t === missingType ? s === t : (isTypeIdenticalTo(s, t) || !!(t.flags & TypeFlags.String && s.flags & TypeFlags.StringLiteral || t.flags & TypeFlags.Number && s.flags & TypeFlags.NumberLiteral)); } @@ -25072,7 +25075,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { function includeUndefinedInIndexSignature(type: Type | undefined): Type | undefined { if (!type) return type; return compilerOptions.noUncheckedIndexedAccess ? - getUnionType([type, undefinedType]) : + getUnionType([type, missingType]) : type; } @@ -29096,7 +29099,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { } else if (exactOptionalPropertyTypes && e.kind === SyntaxKind.OmittedExpression) { hasOmittedExpression = true; - elementTypes.push(missingType); + elementTypes.push(undefinedOrMissingType); elementFlags.push(ElementFlags.Optional); } else { @@ -30640,7 +30643,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker { error(node, Diagnostics.Index_signature_in_type_0_only_permits_reading, typeToString(apparentType)); } - propType = (compilerOptions.noUncheckedIndexedAccess && !isAssignmentTarget(node)) ? getUnionType([indexInfo.type, undefinedType]) : indexInfo.type; + propType = (compilerOptions.noUncheckedIndexedAccess && !isAssignmentTarget(node)) ? getUnionType([indexInfo.type, missingType]) : indexInfo.type; if (compilerOptions.noPropertyAccessFromIndexSignature && isPropertyAccessExpression(node)) { error(right, Diagnostics.Property_0_comes_from_an_index_signature_so_it_must_be_accessed_with_0, unescapeLeadingUnderscores(right.escapedText)); } diff --git a/tests/baselines/reference/inKeywordNarrowingWithNoUncheckedIndexedAccess.symbols b/tests/baselines/reference/inKeywordNarrowingWithNoUncheckedIndexedAccess.symbols new file mode 100644 index 00000000000..0b12667a974 --- /dev/null +++ b/tests/baselines/reference/inKeywordNarrowingWithNoUncheckedIndexedAccess.symbols @@ -0,0 +1,75 @@ +=== tests/cases/compiler/inKeywordNarrowingWithNoUncheckedIndexedAccess.ts === +declare function invariant(condition: boolean): asserts condition; +>invariant : Symbol(invariant, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 0, 0)) +>condition : Symbol(condition, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 0, 27)) +>condition : Symbol(condition, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 0, 27)) + +function f1(obj: Record) { +>f1 : Symbol(f1, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 0, 66)) +>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 2, 12)) +>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --)) + + invariant("test" in obj); +>invariant : Symbol(invariant, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 0, 0)) +>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 2, 12)) + + return obj.test; // string +>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 2, 12)) +} + +function f2(obj: Record) { +>f2 : Symbol(f2, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 5, 1)) +>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 7, 12)) +>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --)) + + if ("test" in obj) { +>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 7, 12)) + + return obj.test; // string +>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 7, 12)) + } + return "default"; +} + +function f3(obj: Record) { +>f3 : Symbol(f3, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 12, 1)) +>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 14, 12)) +>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --)) + + obj.test; // string | undefined +>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 14, 12)) + + if ("test" in obj) { +>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 14, 12)) + + obj.test; // string +>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 14, 12)) + } + else { + obj.test; // undefined +>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 14, 12)) + } +} + +function f4(obj: Record) { +>f4 : Symbol(f4, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 22, 1)) +>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 24, 12)) +>Record : Symbol(Record, Decl(lib.es5.d.ts, --, --)) + + obj.test; // string | undefined +>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 24, 12)) + + if (obj.hasOwnProperty("test")) { +>obj.hasOwnProperty : Symbol(Object.hasOwnProperty, Decl(lib.es5.d.ts, --, --)) +>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 24, 12)) +>hasOwnProperty : Symbol(Object.hasOwnProperty, Decl(lib.es5.d.ts, --, --)) + + obj.test; // string +>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 24, 12)) + } + else { + obj.test; // undefined +>obj : Symbol(obj, Decl(inKeywordNarrowingWithNoUncheckedIndexedAccess.ts, 24, 12)) + } +} + diff --git a/tests/baselines/reference/inKeywordNarrowingWithNoUncheckedIndexedAccess.types b/tests/baselines/reference/inKeywordNarrowingWithNoUncheckedIndexedAccess.types new file mode 100644 index 00000000000..1250758c74c --- /dev/null +++ b/tests/baselines/reference/inKeywordNarrowingWithNoUncheckedIndexedAccess.types @@ -0,0 +1,96 @@ +=== tests/cases/compiler/inKeywordNarrowingWithNoUncheckedIndexedAccess.ts === +declare function invariant(condition: boolean): asserts condition; +>invariant : (condition: boolean) => asserts condition +>condition : boolean + +function f1(obj: Record) { +>f1 : (obj: Record) => string +>obj : Record + + invariant("test" in obj); +>invariant("test" in obj) : void +>invariant : (condition: boolean) => asserts condition +>"test" in obj : boolean +>"test" : "test" +>obj : Record + + return obj.test; // string +>obj.test : string +>obj : Record +>test : string +} + +function f2(obj: Record) { +>f2 : (obj: Record) => string +>obj : Record + + if ("test" in obj) { +>"test" in obj : boolean +>"test" : "test" +>obj : Record + + return obj.test; // string +>obj.test : string +>obj : Record +>test : string + } + return "default"; +>"default" : "default" +} + +function f3(obj: Record) { +>f3 : (obj: Record) => void +>obj : Record + + obj.test; // string | undefined +>obj.test : string | undefined +>obj : Record +>test : string | undefined + + if ("test" in obj) { +>"test" in obj : boolean +>"test" : "test" +>obj : Record + + obj.test; // string +>obj.test : string +>obj : Record +>test : string + } + else { + obj.test; // undefined +>obj.test : undefined +>obj : Record +>test : undefined + } +} + +function f4(obj: Record) { +>f4 : (obj: Record) => void +>obj : Record + + obj.test; // string | undefined +>obj.test : string | undefined +>obj : Record +>test : string | undefined + + if (obj.hasOwnProperty("test")) { +>obj.hasOwnProperty("test") : boolean +>obj.hasOwnProperty : (v: PropertyKey) => boolean +>obj : Record +>hasOwnProperty : (v: PropertyKey) => boolean +>"test" : "test" + + obj.test; // string +>obj.test : string +>obj : Record +>test : string + } + else { + obj.test; // undefined +>obj.test : undefined +>obj : Record +>test : undefined + } +} + diff --git a/tests/baselines/reference/noUncheckedIndexedAccess.errors.txt b/tests/baselines/reference/noUncheckedIndexedAccess.errors.txt index 39a4e9aeb44..3d2411bee0a 100644 --- a/tests/baselines/reference/noUncheckedIndexedAccess.errors.txt +++ b/tests/baselines/reference/noUncheckedIndexedAccess.errors.txt @@ -1,6 +1,7 @@ tests/cases/conformance/pedantic/noUncheckedIndexedAccess.ts(3,32): error TS2344: Type 'boolean | undefined' does not satisfy the constraint 'boolean'. Type 'undefined' is not assignable to type 'boolean'. tests/cases/conformance/pedantic/noUncheckedIndexedAccess.ts(12,7): error TS2322: Type 'boolean | undefined' is not assignable to type 'boolean'. + Type 'undefined' is not assignable to type 'boolean'. tests/cases/conformance/pedantic/noUncheckedIndexedAccess.ts(13,7): error TS2322: Type 'boolean | undefined' is not assignable to type 'boolean'. tests/cases/conformance/pedantic/noUncheckedIndexedAccess.ts(14,7): error TS2322: Type 'boolean | undefined' is not assignable to type 'boolean'. tests/cases/conformance/pedantic/noUncheckedIndexedAccess.ts(15,7): error TS2322: Type 'boolean | undefined' is not assignable to type 'boolean'. @@ -54,6 +55,7 @@ tests/cases/conformance/pedantic/noUncheckedIndexedAccess.ts(99,11): error TS232 const e1: boolean = strMap["foo"]; ~~ !!! error TS2322: Type 'boolean | undefined' is not assignable to type 'boolean'. +!!! error TS2322: Type 'undefined' is not assignable to type 'boolean'. const e2: boolean = strMap.bar; ~~ !!! error TS2322: Type 'boolean | undefined' is not assignable to type 'boolean'. diff --git a/tests/cases/compiler/inKeywordNarrowingWithNoUncheckedIndexedAccess.ts b/tests/cases/compiler/inKeywordNarrowingWithNoUncheckedIndexedAccess.ts new file mode 100644 index 00000000000..c430ba18c17 --- /dev/null +++ b/tests/cases/compiler/inKeywordNarrowingWithNoUncheckedIndexedAccess.ts @@ -0,0 +1,37 @@ +// @strict: true +// @noEmit: true +// @noUncheckedIndexedAccess: true + +declare function invariant(condition: boolean): asserts condition; + +function f1(obj: Record) { + invariant("test" in obj); + return obj.test; // string +} + +function f2(obj: Record) { + if ("test" in obj) { + return obj.test; // string + } + return "default"; +} + +function f3(obj: Record) { + obj.test; // string | undefined + if ("test" in obj) { + obj.test; // string + } + else { + obj.test; // undefined + } +} + +function f4(obj: Record) { + obj.test; // string | undefined + if (obj.hasOwnProperty("test")) { + obj.test; // string + } + else { + obj.test; // undefined + } +}