Skip to content

Commit

Permalink
Improve checking of in operator (#50666)
Browse files Browse the repository at this point in the history 
* Improve checking of `in` operator

* Accept new baselines

* Add tests

* Delete old and accept new baselines

* Disallow right operand of type '{}'

* Accept new baselines

* Support number and symbol literals

* Add tests

* Disallow {} typed right operand only in strictNullChecks mode

* Accept new baselines

* Detect {} resulting from intersections

* Accept new baselines

* Don't attempt to reduce intersections with Record<K, unknown>

* Accept new baselines

* Return undefined instead of unknownSymbol from getGlobalRecordSymbol()
  • Loading branch information
@ahejlsberg
ahejlsberg committed Sep 19, 2022
1 parent 67f2b62 commit a11c416
Show file tree
Hide file tree
Showing 31 changed files with 5,090 additions and 936 deletions.
93 changes: 44 additions & 49 deletions src/compiler/checker.ts
View file
Expand Up @@ -858,7 +858,8 @@ namespace ts {
emptyTypeLiteralSymbol.members = createSymbolTable();
const emptyTypeLiteralType = createAnonymousType(emptyTypeLiteralSymbol, emptySymbols, emptyArray, emptyArray, emptyArray);

const unknownUnionType = strictNullChecks ? getUnionType([undefinedType, nullType, createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, emptyArray)]) : unknownType;
const unknownEmptyObjectType = createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, emptyArray);
const unknownUnionType = strictNullChecks ? getUnionType([undefinedType, nullType, unknownEmptyObjectType]) : unknownType;

const emptyGenericType = createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, emptyArray) as ObjectType as GenericType;
emptyGenericType.instantiations = new Map<string, TypeReference>();
Expand Down Expand Up @@ -998,6 +999,7 @@ namespace ts {
let deferredGlobalOmitSymbol: Symbol | undefined;
let deferredGlobalAwaitedSymbol: Symbol | undefined;
let deferredGlobalBigIntType: ObjectType | undefined;
let deferredGlobalRecordSymbol: Symbol | undefined;

const allPotentiallyUnusedIdentifiers = new Map<Path, PotentiallyUnusedIdentifier[]>(); // key is file name

Expand Down Expand Up @@ -14314,6 +14316,11 @@ namespace ts {
return (deferredGlobalBigIntType ||= getGlobalType("BigInt" as __String, /*arity*/ 0, /*reportErrors*/ false)) || emptyObjectType;
}

function getGlobalRecordSymbol(): Symbol | undefined {
deferredGlobalRecordSymbol ||= getGlobalTypeAliasSymbol("Record" as __String, /*arity*/ 2, /*reportErrors*/ true) || unknownSymbol;
return deferredGlobalRecordSymbol === unknownSymbol ? undefined : deferredGlobalRecordSymbol;
}

/**
* Instantiates a global type that is generic with some element type, and returns that instantiation.
*/
Expand Down Expand Up @@ -25199,19 +25206,27 @@ namespace ts {

function isTypePresencePossible(type: Type, propName: __String, assumeTrue: boolean) {
const prop = getPropertyOfType(type, propName);
if (prop) {
return prop.flags & SymbolFlags.Optional ? true : assumeTrue;
}
return getApplicableIndexInfoForName(type, propName) ? true : !assumeTrue;
return prop ?
!!(prop.flags & SymbolFlags.Optional) || assumeTrue :
!!getApplicableIndexInfoForName(type, propName) || !assumeTrue;
}

function narrowByInKeyword(type: Type, name: __String, assumeTrue: boolean) {
if (type.flags & TypeFlags.Union
|| type.flags & TypeFlags.Object && declaredType !== type && !(declaredType === unknownType && isEmptyAnonymousObjectType(type))
|| isThisTypeParameter(type)
|| type.flags & TypeFlags.Intersection && every((type as IntersectionType).types, t => t.symbol !== globalThisSymbol)) {
function narrowByInKeyword(type: Type, nameType: StringLiteralType | NumberLiteralType | UniqueESSymbolType, assumeTrue: boolean) {
const name = getPropertyNameFromType(nameType);
const isKnownProperty = someType(type, t => isTypePresencePossible(t, name, /*assumeTrue*/ true));
if (isKnownProperty) {
// If the check is for a known property (i.e. a property declared in some constituent of
// the target type), we filter the target type by presence of absence of the property.
return filterType(type, t => isTypePresencePossible(t, name, assumeTrue));
}
if (assumeTrue) {
// If the check is for an unknown property, we intersect the target type with `Record<X, unknown>`,
// where X is the name of the property.
const recordSymbol = getGlobalRecordSymbol();
if (recordSymbol) {
return getIntersectionType([type, getTypeAliasInstantiation(recordSymbol, [nameType, unknownType])]);
}
}
return type;
}

Expand Down Expand Up @@ -25271,15 +25286,14 @@ namespace ts {
return narrowTypeByPrivateIdentifierInInExpression(type, expr, assumeTrue);
}
const target = getReferenceCandidate(expr.right);
const leftType = getTypeOfNode(expr.left);
if (leftType.flags & TypeFlags.StringLiteral) {
const name = escapeLeadingUnderscores((leftType as StringLiteralType).value);
const leftType = getTypeOfExpression(expr.left);
if (leftType.flags & TypeFlags.StringOrNumberLiteralOrUnique) {
if (containsMissingType(type) && isAccessExpression(reference) && isMatchingReference(reference.expression, target) &&
getAccessedPropertyName(reference) === name) {
getAccessedPropertyName(reference) === getPropertyNameFromType(leftType as StringLiteralType | NumberLiteralType | UniqueESSymbolType)) {
return getTypeWithFacts(type, assumeTrue ? TypeFacts.NEUndefined : TypeFacts.EQUndefined);
}
if (isMatchingReference(reference, target)) {
return narrowByInKeyword(type, name, assumeTrue);
return narrowByInKeyword(type, leftType as StringLiteralType | NumberLiteralType | UniqueESSymbolType, assumeTrue);
}
}
break;
Expand Down Expand Up @@ -33848,6 +33862,10 @@ namespace ts {
return booleanType;
}

function hasEmptyObjectIntersection(type: Type): boolean {
return someType(type, t => t === unknownEmptyObjectType || !!(t.flags & TypeFlags.Intersection) && some((t as IntersectionType).types, isEmptyAnonymousObjectType));
}

function checkInExpression(left: Expression, right: Expression, leftType: Type, rightType: Type): Type {
if (leftType === silentNeverType || rightType === silentNeverType) {
return silentNeverType;
Expand All @@ -33864,43 +33882,20 @@ namespace ts {
}
}
else {
leftType = checkNonNullType(leftType, left);
// TypeScript 1.0 spec (April 2014): 4.15.5
// Require the left operand to be of type Any, the String primitive type, or the Number primitive type.
if (!(allTypesAssignableToKind(leftType, TypeFlags.StringLike | TypeFlags.NumberLike | TypeFlags.ESSymbolLike) ||
isTypeAssignableToKind(leftType, TypeFlags.Index | TypeFlags.TemplateLiteral | TypeFlags.StringMapping | TypeFlags.TypeParameter))) {
error(left, Diagnostics.The_left_hand_side_of_an_in_expression_must_be_a_private_identifier_or_of_type_any_string_number_or_symbol);
// The type of the lef operand must be assignable to string, number, or symbol.
checkTypeAssignableTo(checkNonNullType(leftType, left), stringNumberSymbolType, left);
}
// The type of the right operand must be assignable to 'object'.
if (checkTypeAssignableTo(checkNonNullType(rightType, right), nonPrimitiveType, right)) {
// The {} type is assignable to the object type, yet {} might represent a primitive type. Here we
// detect and error on {} that results from narrowing the unknown type, as well as intersections
// that include {} (we know that the other types in such intersections are assignable to object
// since we already checked for that).
if (hasEmptyObjectIntersection(rightType)) {
error(right, Diagnostics.Type_0_may_represent_a_primitive_value_which_is_not_permitted_as_the_right_operand_of_the_in_operator, typeToString(rightType));
}
}
rightType = checkNonNullType(rightType, right);
// TypeScript 1.0 spec (April 2014): 4.15.5
// The in operator requires the right operand to be
//
// 1. assignable to the non-primitive type,
// 2. an unconstrained type parameter,
// 3. a union or intersection including one or more type parameters, whose constituents are all assignable to the
// the non-primitive type, or are unconstrainted type parameters, or have constraints assignable to the
// non-primitive type, or
// 4. a type parameter whose constraint is
// i. an object type,
// ii. the non-primitive type, or
// iii. a union or intersection with at least one constituent assignable to an object or non-primitive type.
//
// The divergent behavior for type parameters and unions containing type parameters is a workaround for type
// parameters not being narrowable. If the right operand is a concrete type, we can error if there is any chance
// it is a primitive. But if the operand is a type parameter, it cannot be narrowed, so we don't issue an error
// unless *all* instantiations would result in an error.
//
// The result is always of the Boolean primitive type.
const rightTypeConstraint = getConstraintOfType(rightType);
if (!allTypesAssignableToKind(rightType, TypeFlags.NonPrimitive | TypeFlags.InstantiableNonPrimitive) ||
rightTypeConstraint && (
isTypeAssignableToKind(rightType, TypeFlags.UnionOrIntersection) && !allTypesAssignableToKind(rightTypeConstraint, TypeFlags.NonPrimitive | TypeFlags.InstantiableNonPrimitive) ||
!maybeTypeOfKind(rightTypeConstraint, TypeFlags.NonPrimitive | TypeFlags.InstantiableNonPrimitive | TypeFlags.Object)
)
) {
error(right, Diagnostics.The_right_hand_side_of_an_in_expression_must_not_be_a_primitive);
}
return booleanType;
}

Expand Down
12 changes: 4 additions & 8 deletions src/compiler/diagnosticMessages.json
View file
Expand Up @@ -1844,14 +1844,6 @@
"category": "Error",
"code": 2359
},
"The left-hand side of an 'in' expression must be a private identifier or of type 'any', 'string', 'number', or 'symbol'.": {
"category": "Error",
"code": 2360
},
"The right-hand side of an 'in' expression must not be a primitive.": {
"category": "Error",
"code": 2361
},
"The left-hand side of an arithmetic operation must be of type 'any', 'number', 'bigint' or an enum type.": {
"category": "Error",
"code": 2362
Expand Down Expand Up @@ -2845,6 +2837,10 @@
"category": "Error",
"code": 2637
},
"Type '{0}' may represent a primitive value, which is not permitted as the right operand of the 'in' operator.": {
"category": "Error",
"code": 2638
},

"Cannot augment module '{0}' with value exports because it resolves to a non-module entity.": {
"category": "Error",
Expand Down

0 comments on commit a11c416

Please sign in to comment.