Skip to content

Commit

Permalink
Use missingType in --noUncheckedIndexedAccess mode (#51653)
Browse files Browse the repository at this point in the history
* Use missingType in noUncheckedIndexedAccess mode

* Accept new baselines

* Add tests

* Optimizing searching for undefinedType and missingType
  • Loading branch information
ahejlsberg committed Dec 7, 2022
1 parent 91f89b9 commit d43112a
Show file tree
Hide file tree
Showing 5 changed files with 234 additions and 21 deletions.
45 changes: 24 additions & 21 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1813,6 +1813,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const unresolvedSymbols = new Map<string, TransientSymbol>();
const errorTypes = new Map<string, Type>();

// 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");
Expand All @@ -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");
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
});
}
}
Expand All @@ -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,
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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<string, string>) {
>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<string, string>) {
>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<string, string>) {
>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<string, string>) {
>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))
}
}

Original file line number Diff line number Diff line change
@@ -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<string, string>) {
>f1 : (obj: Record<string, string>) => string
>obj : Record<string, string>

invariant("test" in obj);
>invariant("test" in obj) : void
>invariant : (condition: boolean) => asserts condition
>"test" in obj : boolean
>"test" : "test"
>obj : Record<string, string>

return obj.test; // string
>obj.test : string
>obj : Record<string, string>
>test : string
}

function f2(obj: Record<string, string>) {
>f2 : (obj: Record<string, string>) => string
>obj : Record<string, string>

if ("test" in obj) {
>"test" in obj : boolean
>"test" : "test"
>obj : Record<string, string>

return obj.test; // string
>obj.test : string
>obj : Record<string, string>
>test : string
}
return "default";
>"default" : "default"
}

function f3(obj: Record<string, string>) {
>f3 : (obj: Record<string, string>) => void
>obj : Record<string, string>

obj.test; // string | undefined
>obj.test : string | undefined
>obj : Record<string, string>
>test : string | undefined

if ("test" in obj) {
>"test" in obj : boolean
>"test" : "test"
>obj : Record<string, string>

obj.test; // string
>obj.test : string
>obj : Record<string, string>
>test : string
}
else {
obj.test; // undefined
>obj.test : undefined
>obj : Record<string, string>
>test : undefined
}
}

function f4(obj: Record<string, string>) {
>f4 : (obj: Record<string, string>) => void
>obj : Record<string, string>

obj.test; // string | undefined
>obj.test : string | undefined
>obj : Record<string, string>
>test : string | undefined

if (obj.hasOwnProperty("test")) {
>obj.hasOwnProperty("test") : boolean
>obj.hasOwnProperty : (v: PropertyKey) => boolean
>obj : Record<string, string>
>hasOwnProperty : (v: PropertyKey) => boolean
>"test" : "test"

obj.test; // string
>obj.test : string
>obj : Record<string, string>
>test : string
}
else {
obj.test; // undefined
>obj.test : undefined
>obj : Record<string, string>
>test : undefined
}
}

2 changes: 2 additions & 0 deletions tests/baselines/reference/noUncheckedIndexedAccess.errors.txt
Original file line number Diff line number Diff line change
@@ -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'.
Expand Down Expand Up @@ -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'.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// @strict: true
// @noEmit: true
// @noUncheckedIndexedAccess: true

declare function invariant(condition: boolean): asserts condition;

function f1(obj: Record<string, string>) {
invariant("test" in obj);
return obj.test; // string
}

function f2(obj: Record<string, string>) {
if ("test" in obj) {
return obj.test; // string
}
return "default";
}

function f3(obj: Record<string, string>) {
obj.test; // string | undefined
if ("test" in obj) {
obj.test; // string
}
else {
obj.test; // undefined
}
}

function f4(obj: Record<string, string>) {
obj.test; // string | undefined
if (obj.hasOwnProperty("test")) {
obj.test; // string
}
else {
obj.test; // undefined
}
}

0 comments on commit d43112a

Please sign in to comment.