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.

Sign up for GitHub

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

'in' should not operate on primitive types #41928

Closed
wants to merge 8 commits into from
Closed

'in' should not operate on primitive types #41928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
669e96b
'in' should not operate on primitive types
jonhue Dec 11, 2020
8c7ca65
accept baselines of failing tests
jonhue Dec 11, 2020
d2cd9b1
review
jonhue Dec 16, 2020
4e7ba7b
update error message
jonhue Dec 22, 2020
37268fc
check if constraint of right type is assignable to a non primitive or…
jonhue Dec 22, 2020
4989346
do not throw errors where narrowing is impossible
jonhue Dec 28, 2020
cb430eb
accept baselines
jonhue Dec 28, 2020
afcf859
fix test case failures
jonhue Dec 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29996,14 +29996,22 @@ 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 not to extend a primitive type.
andrewbranch marked this conversation as resolved.
Show resolved Hide resolved
// 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 && (
// If constraint is a union/intersection, ensure no types are primitive.
isTypeAssignableToKind(rightType, TypeFlags.UnionOrIntersection) && !allTypesAssignableToKind(rightTypeConstraint, TypeFlags.NonPrimitive | TypeFlags.InstantiableNonPrimitive) ||
// Otherwise, ensure that at least one type is not a primitive.
andrewbranch marked this conversation as resolved.
Show resolved Hide resolved
!maybeTypeOfKind(rightTypeConstraint, TypeFlags.NonPrimitive | TypeFlags.InstantiableNonPrimitive | TypeFlags.Object)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maybeTypeOfKind is correct here, but everything else uses assignability to NonPrimitive, and this isn't any kind of base case. I'd feel happier if the whole thing used assignability.

Copy link
Member

@andrewbranch andrewbranch Jan 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pasting from discussion in chat:

...applying your suggestion breaks a test case I designed—it adds new errors that are strictly more correct but more breaky, and a bigger divergence from existing behavior.

What maybeTypeOfKind allows for is this:

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

I asserted that this should remain errorless, because we can't track the narrowing of thing if you type guarded that it was an object or not a string. Swapping maybeTypeOfKind for isTypeAssignableToKind makes it an error.

)
) {
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1621,7 +1621,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
72 changes: 72 additions & 0 deletions tests/baselines/reference/inDoesNotOperateOnPrimitiveTypes.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
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(45,12): error TS2361: The right-hand side of an 'in' expression must not be a primitive.
tests/cases/compiler/inDoesNotOperateOnPrimitiveTypes.ts(49,12): error TS2361: The right-hand side of an 'in' expression must not be a primitive.


==== tests/cases/compiler/inDoesNotOperateOnPrimitiveTypes.ts (6 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 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.
}

87 changes: 87 additions & 0 deletions tests/baselines/reference/inDoesNotOperateOnPrimitiveTypes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
//// [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 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 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)
}
135 changes: 135 additions & 0 deletions tests/baselines/reference/inDoesNotOperateOnPrimitiveTypes.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
=== tests/cases/compiler/inDoesNotOperateOnPrimitiveTypes.ts ===
const validHasKey = <T extends object>(
>validHasKey : Symbol(validHasKey, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 0, 5))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 0, 21))

thing: T,
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 0, 39))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 0, 21))

key: string,
>key : Symbol(key, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 1, 11))

): boolean => {
return key in thing; // Ok
>key : Symbol(key, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 1, 11))
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 0, 39))

};

const alsoValidHasKey = <T>(
>alsoValidHasKey : Symbol(alsoValidHasKey, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 7, 5))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 7, 25))

thing: T,
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 7, 28))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 7, 25))

key: string,
>key : Symbol(key, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 8, 11))

): boolean => {
return key in thing; // Ok (as T may be instantiated with a valid type)
>key : Symbol(key, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 8, 11))
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 7, 28))

};

function invalidHasKey<T extends string | number>(
>invalidHasKey : Symbol(invalidHasKey, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 12, 2))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 14, 23))

thing: T,
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 14, 50))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 14, 23))

key: string,
>key : Symbol(key, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 15, 11))

): boolean {
return key in thing; // Error (because all possible instantiations are errors)
>key : Symbol(key, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 15, 11))
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 14, 50))
}

function union1<T extends string | number, U extends boolean>(thing: T | U) {
>union1 : Symbol(union1, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 19, 1))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 21, 16))
>U : Symbol(U, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 21, 42))
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 21, 62))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 21, 16))
>U : Symbol(U, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 21, 42))

"key" in thing; // Error (because all possible instantiations are errors)
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 21, 62))
}

function union2<T extends object, U extends string | number>(thing: T | U) {
>union2 : Symbol(union2, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 23, 1))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 25, 16))
>U : Symbol(U, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 25, 33))
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 25, 61))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 25, 16))
>U : Symbol(U, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 25, 33))

"key" in thing; // Error (because narrowing is possible)
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 25, 61))

if (typeof thing === "object") {
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 25, 61))

"key" in thing; // Ok
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 25, 61))
}
}

function union3<T>(thing: T | string | number) {
>union3 : Symbol(union3, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 30, 1))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 32, 16))
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 32, 19))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 32, 16))

"key" in thing; // Error (because narrowing is possible)
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 32, 19))

if (typeof thing !== "string" && typeof thing !== "number") {
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 32, 19))
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 32, 19))

"key" in thing; // Ok (because further narrowing is impossible)
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 32, 19))
}
}

function union4<T extends object | "hello">(thing: T) {
>union4 : Symbol(union4, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 37, 1))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 39, 16))
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 39, 44))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 39, 16))

"key" in thing; // Ok (because narrowing is impossible)
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 39, 44))
}

function intersection1<T extends number, U extends 0 | 1 | 2>(thing: T & U) {
>intersection1 : Symbol(intersection1, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 41, 1))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 43, 23))
>U : Symbol(U, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 43, 40))
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 43, 62))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 43, 23))
>U : Symbol(U, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 43, 40))

"key" in thing; // Error (because all possible instantiations are errors)
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 43, 62))
}

function intersection2<T>(thing: T & (0 | 1 | 2)) {
>intersection2 : Symbol(intersection2, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 45, 1))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 47, 23))
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 47, 26))
>T : Symbol(T, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 47, 23))

"key" in thing; // Error (because all possible instantations are errors)
>thing : Symbol(thing, Decl(inDoesNotOperateOnPrimitiveTypes.ts, 47, 26))
}

Loading