Skip to content

Commit

Permalink
'in' should not operate on primitive types (microsoft#41928 + @andrew…
Browse files Browse the repository at this point in the history
…branch) (microsoft#42288)

* 'in' should not operate on primitive types

* accept baselines of failing tests

* review

* update error message

* check if constraint of right type is assignable to a non primitive or instantiable non primitive

* do not throw errors where narrowing is impossible

* accept baselines

* fix test case failures

* Add more accurate comment discussion and document failing edge case in test

* Update baselines

Co-authored-by: Jonas Hübotter <[email protected]>
  • Loading branch information
2 people authored and Zzzen committed Jan 16, 2021
1 parent 3bd1ef6 commit 323e19a
Show file tree
Hide file tree
Showing 10 changed files with 656 additions and 26 deletions.
28 changes: 25 additions & 3 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30310,14 +30310,36 @@ namespace ts {
rightType = checkNonNullType(rightType, right);
// TypeScript 1.0 spec (April 2014): 4.15.5
// The in operator requires the left operand to be of type Any, the String primitive type, or the Number primitive type,
// and the right operand to be of type Any, an object type, or a type parameter type.
// and 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.
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_of_type_any_string_number_or_symbol);
}
if (!allTypesAssignableToKind(rightType, TypeFlags.NonPrimitive | TypeFlags.InstantiableNonPrimitive)) {
error(right, Diagnostics.The_right_hand_side_of_an_in_expression_must_be_of_type_any_an_object_type_or_a_type_parameter);
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
2 changes: 1 addition & 1 deletion src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1625,7 +1625,7 @@
"category": "Error",
"code": 2360
},
"The right-hand side of an 'in' expression must be of type 'any', an object type or a type parameter.": {
"The right-hand side of an 'in' expression must not be a primitive.": {
"category": "Error",
"code": 2361
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
tests/cases/compiler/inDoesNotOperateOnPrimitiveTypes.ts(19,17): error TS2361: The right-hand side of an 'in' expression must not be a primitive.
tests/cases/compiler/inDoesNotOperateOnPrimitiveTypes.ts(23,12): error TS2361: The right-hand side of an 'in' expression must not be a primitive.
tests/cases/compiler/inDoesNotOperateOnPrimitiveTypes.ts(27,12): error TS2361: The right-hand side of an 'in' expression must not be a primitive.
tests/cases/compiler/inDoesNotOperateOnPrimitiveTypes.ts(34,12): error TS2361: The right-hand side of an 'in' expression must not be a primitive.
tests/cases/compiler/inDoesNotOperateOnPrimitiveTypes.ts(53,14): error TS2361: The right-hand side of an 'in' expression must not be a primitive.
tests/cases/compiler/inDoesNotOperateOnPrimitiveTypes.ts(55,18): error TS2361: The right-hand side of an 'in' expression must not be a primitive.
tests/cases/compiler/inDoesNotOperateOnPrimitiveTypes.ts(60,12): error TS2361: The right-hand side of an 'in' expression must not be a primitive.
tests/cases/compiler/inDoesNotOperateOnPrimitiveTypes.ts(64,12): error TS2361: The right-hand side of an 'in' expression must not be a primitive.


==== tests/cases/compiler/inDoesNotOperateOnPrimitiveTypes.ts (8 errors) ====
const validHasKey = <T extends object>(
thing: T,
key: string,
): boolean => {
return key in thing; // Ok
};

const alsoValidHasKey = <T>(
thing: T,
key: string,
): boolean => {
return key in thing; // Ok (as T may be instantiated with a valid type)
};

function invalidHasKey<T extends string | number>(
thing: T,
key: string,
): boolean {
return key in thing; // Error (because all possible instantiations are errors)
~~~~~
!!! error TS2361: The right-hand side of an 'in' expression must not be a primitive.
}

function union1<T extends string | number, U extends boolean>(thing: T | U) {
"key" in thing; // Error (because all possible instantiations are errors)
~~~~~
!!! error TS2361: The right-hand side of an 'in' expression must not be a primitive.
}

function union2<T extends object, U extends string | number>(thing: T | U) {
"key" in thing; // Error (because narrowing is possible)
~~~~~
!!! error TS2361: The right-hand side of an 'in' expression must not be a primitive.
if (typeof thing === "object") {
"key" in thing; // Ok
}
}

function union3<T>(thing: T | string | number) {
"key" in thing; // Error (because narrowing is possible)
~~~~~
!!! error TS2361: The right-hand side of an 'in' expression must not be a primitive.
if (typeof thing !== "string" && typeof thing !== "number") {
"key" in thing; // Ok (because further narrowing is impossible)
}
}

function union4<T extends object | "hello">(thing: T) {
"key" in thing; // Ok (because narrowing is impossible)
}

function union5<T extends object | string, U extends object | number>(p: T | U) {
// For consistency, this should probably not be an error, because useful
// narrowing is impossible. However, this is exceptionally strange input,
// and it adds a lot of complexity to distinguish between a `T | U` where
// one constraint is non-primitive and the other is primitive and a `T | U`
// like this where both constraints have primitive and non-primitive
// constitutents. Also, the strictly sound behavior would be to error
// here, which is what's happening, so "fixing" this by suppressing the
// error seems very low-value.
"key" in p;
~
!!! error TS2361: The right-hand side of an 'in' expression must not be a primitive.
if (typeof p === "object") {
"key" in p;
~
!!! error TS2361: The right-hand side of an 'in' expression must not be a primitive.
}
}

function intersection1<T extends number, U extends 0 | 1 | 2>(thing: T & U) {
"key" in thing; // Error (because all possible instantiations are errors)
~~~~~
!!! error TS2361: The right-hand side of an 'in' expression must not be a primitive.
}

function intersection2<T>(thing: T & (0 | 1 | 2)) {
"key" in thing; // Error (because all possible instantations are errors)
~~~~~
!!! error TS2361: The right-hand side of an 'in' expression must not be a primitive.
}

116 changes: 116 additions & 0 deletions tests/baselines/reference/inDoesNotOperateOnPrimitiveTypes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
//// [inDoesNotOperateOnPrimitiveTypes.ts]
const validHasKey = <T extends object>(
thing: T,
key: string,
): boolean => {
return key in thing; // Ok
};

const alsoValidHasKey = <T>(
thing: T,
key: string,
): boolean => {
return key in thing; // Ok (as T may be instantiated with a valid type)
};

function invalidHasKey<T extends string | number>(
thing: T,
key: string,
): boolean {
return key in thing; // Error (because all possible instantiations are errors)
}

function union1<T extends string | number, U extends boolean>(thing: T | U) {
"key" in thing; // Error (because all possible instantiations are errors)
}

function union2<T extends object, U extends string | number>(thing: T | U) {
"key" in thing; // Error (because narrowing is possible)
if (typeof thing === "object") {
"key" in thing; // Ok
}
}

function union3<T>(thing: T | string | number) {
"key" in thing; // Error (because narrowing is possible)
if (typeof thing !== "string" && typeof thing !== "number") {
"key" in thing; // Ok (because further narrowing is impossible)
}
}

function union4<T extends object | "hello">(thing: T) {
"key" in thing; // Ok (because narrowing is impossible)
}

function union5<T extends object | string, U extends object | number>(p: T | U) {
// For consistency, this should probably not be an error, because useful
// narrowing is impossible. However, this is exceptionally strange input,
// and it adds a lot of complexity to distinguish between a `T | U` where
// one constraint is non-primitive and the other is primitive and a `T | U`
// like this where both constraints have primitive and non-primitive
// constitutents. Also, the strictly sound behavior would be to error
// here, which is what's happening, so "fixing" this by suppressing the
// error seems very low-value.
"key" in p;
if (typeof p === "object") {
"key" in p;
}
}

function intersection1<T extends number, U extends 0 | 1 | 2>(thing: T & U) {
"key" in thing; // Error (because all possible instantiations are errors)
}

function intersection2<T>(thing: T & (0 | 1 | 2)) {
"key" in thing; // Error (because all possible instantations are errors)
}


//// [inDoesNotOperateOnPrimitiveTypes.js]
var validHasKey = function (thing, key) {
return key in thing; // Ok
};
var alsoValidHasKey = function (thing, key) {
return key in thing; // Ok (as T may be instantiated with a valid type)
};
function invalidHasKey(thing, key) {
return key in thing; // Error (because all possible instantiations are errors)
}
function union1(thing) {
"key" in thing; // Error (because all possible instantiations are errors)
}
function union2(thing) {
"key" in thing; // Error (because narrowing is possible)
if (typeof thing === "object") {
"key" in thing; // Ok
}
}
function union3(thing) {
"key" in thing; // Error (because narrowing is possible)
if (typeof thing !== "string" && typeof thing !== "number") {
"key" in thing; // Ok (because further narrowing is impossible)
}
}
function union4(thing) {
"key" in thing; // Ok (because narrowing is impossible)
}
function union5(p) {
// For consistency, this should probably not be an error, because useful
// narrowing is impossible. However, this is exceptionally strange input,
// and it adds a lot of complexity to distinguish between a `T | U` where
// one constraint is non-primitive and the other is primitive and a `T | U`
// like this where both constraints have primitive and non-primitive
// constitutents. Also, the strictly sound behavior would be to error
// here, which is what's happening, so "fixing" this by suppressing the
// error seems very low-value.
"key" in p;
if (typeof p === "object") {
"key" in p;
}
}
function intersection1(thing) {
"key" in thing; // Error (because all possible instantiations are errors)
}
function intersection2(thing) {
"key" in thing; // Error (because all possible instantations are errors)
}
Loading

0 comments on commit 323e19a

Please sign in to comment.