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

Incorrect test in __rest helper for Object rest #12227

Closed
rbuckton opened this issue Nov 14, 2016 · 1 comment
Closed

Incorrect test in __rest helper for Object rest #12227

rbuckton opened this issue Nov 14, 2016 · 1 comment
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@rbuckton
Copy link
Member

TypeScript Version: 2.2.0-dev.20161114
The emit for __rest uses !e.indexOf(p) to check as to whether a property name should be excluded. This test is incorrect as Array.prototype.indexOf return -1 if an element is not found, or a zero-bounded index if an element is found, for example:

const e = ["a", "b"];
!e.indexOf("a") === !(0)  === true  // fail, we want 'false' here`
!e.indexOf("b") === !(1)  === false // ok, we want 'false' here`
!e.indexOf("c") === !(-1) === false // fail, we want 'true' here`

A concise and correct test should instead be !~e.indexOf(p), as unary ~ will give the twos-complement of e.indexOf(p):

const e = ["a", "b"];
!~e.indexOf("a") === !~(0)  === !(-1) === false // ok, we want 'false' here
!~e.indexOf("b") === !~(1)  === !(-2) === false // ok, we want 'false' here
!~e.indexOf("c") === !~(-1) === !(0)  === true  // ok, we want 'true' here

Expected emit:

var __rest = (this && this.__rest) || function (s, e) {
    var t = {};
    for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p) && !~e.indexOf(p))
        t[p] = s[p];
    return t;
};

Actual emit:

var __rest = (this && this.__rest) || function (s, e) {
    var t = {};
    for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p) && !e.indexOf(p))
        t[p] = s[p];
    return t;
};
@mhegazy mhegazy added the Bug A bug in TypeScript label Nov 14, 2016
@mhegazy mhegazy added this to the TypeScript 2.1.3 milestone Nov 14, 2016
@sandersn
Copy link
Member

what about e.indexOf(p) === -1 ? That's easier to read I think, and is probably as fast.

@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Nov 15, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

3 participants