The Wayback Machine - https://web.archive.org/web/20201124045625/https://github.com/microsoft/TypeScript/issues/41317
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.

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 #41317

Open
millsp opened this issue Oct 29, 2020 · 5 comments
Open

'in' should not operate on primitive types #41317

millsp opened this issue Oct 29, 2020 · 5 comments

Comments

@millsp
Copy link
Contributor

@millsp millsp commented Oct 29, 2020

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:

@millsp millsp changed the title Potential crash of operator `in` could be avoided Potential crash of `in` operator could be avoided Oct 29, 2020
@millsp millsp changed the title Potential crash of `in` operator could be avoided Potential crash of `in` operator Oct 29, 2020
@DanielRosenwasser DanielRosenwasser changed the title Potential crash of `in` operator 'in' should not operate on primitive types Oct 29, 2020
@DanielRosenwasser
Copy link
Member

@DanielRosenwasser DanielRosenwasser commented Oct 29, 2020

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

@DanielRosenwasser
Copy link
Member

@DanielRosenwasser DanielRosenwasser commented Nov 2, 2020

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
Copy link
Member

@DanielRosenwasser DanielRosenwasser commented Nov 3, 2020

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
Copy link

@orange4glace orange4glace commented Nov 6, 2020

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

@orange4glace
Copy link

@orange4glace orange4glace commented Nov 7, 2020

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.