Typescript: 'in' should not operate on primitive types

[millsp]

TypeScript Version: 4.1.0-dev

Search Terms:
operator in exception crash unhandled

Code

const hasKey = <A, K extends string | number | symbol>(
    thing: A,
    key: K,
): boolean => {
    return key in thing;
};

hasKey(123, 'hello'); // TypeError exception

Expected behavior:

TS should expect A to be an object

Actual behavior:

TS didn't detect the potential crash

Playground Link: https://www.typescriptlang.org/play?ts=4.1.0-beta#code/MYewdgzgLgBAFgQwgaQKYE8YF4YB4CCANDMjKgB5SpgAmEM0ATgJZgDmMAPjGAK4C2AI1SMuDdEJAAbAHwAKAFAxlMKHFZsAXDCJKVAawzbkhBQEptgkNNQIw2GTADee5Y1RReje4cytV6uwA3AoAviEKiCgYcgCMAEwAzMQA5HCoUlIgKWZBMAD0+TAAKugADqgAooyMIKIUwKhlUMzgCkA

Related Issues:

[DanielRosenwasser]

This is weird because we'd potentially allow String but not string - but that's probably okay.

[DanielRosenwasser]

Approved.

To avoid breakage on unconstrained type parameters, we're thinking to check if the type is assignable to the union of all primitive types - or if it contains any primitive type (e.g. number, string, boolean, bigint, symbol, null, undefined). isTypeAssignableToKind might be one thing to use here, if not, just use isAssignableTo.

If that's still too breaky, maybeTypeOfKind is a helper that can do this pretty quickly.

The more thorough version of this would be to check whether the type on the right side of the in has to be assignable to object. We could run that on the RWC.

[DanielRosenwasser]

To be explicit, this is all about the type on the right of the in operator. The left can be unconstrained, and maybe we should reconsider that at a later date.

[orange4glace]

If possible, I would like to make a PR for this.

[orange4glace]

Which line should emit error, 1) or 2)?

const hasKey = <A, K extends string | number | symbol>(
    thing: A,
    key: K,
): boolean => {
    return key in thing;
                  ~~~~~~
                  1) TypeError exception
};

hasKey(123, 'hello');
       ~~~
       2) TypeError exception

[andrewbranch]

@orange4glace the former. Are you still interested in doing this? The change would be in checker.ts around line 29940:

function checkInExpression(left: Expression, right: Expression, leftType: Type, rightType: Type): Type {
    if (leftType === silentNeverType || rightType === silentNeverType) {
        return silentNeverType;
    }
    leftType = checkNonNullType(leftType, left);
    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.
    // 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);
    }
    // 👇 here
    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);
    }
    return booleanType;

[orange4glace]

@andrewbranch Sure! It would be very happy if I can do this.
Thanks for the detailed guidance! I'll investgate on this ;)