More specific JSON parse reviver argument in lib.d.ts

[avonwyss]

Currently, the JSON interface in lib.d.ts declares the following:

parse(text: string, reviver?: (key: any, value: any) => any): any;

However, my understanding is that the spec guaratees a string for the key (see http://www.ecma-international.org/ecma-262/5.1/#sec-15.12.2 paragraph "The abstract operation Walk ... String name ..."), in consequence it could be more specific:

parse(text: string, reviver?: (key: string, value: any) => any): any;

[mhegazy]

The problem here is similar to for..in issues, see the fix in TS 1.8 in #6379.

The value is a string, but it could be a numeric string in cases of array. it is not a number value (i.e. can not call toExponent on it) but it can still be used to index into an array (since at the end all index operations first do a toString in the key anyways). TypeScript, tries to strongly type index operations using index signatures. there are two, string and numeric. and if you index into an object using a type that it does not have an index signature for, you get an any. ideally, this

so:

var list = [0, 1];
JSON.parse("[3, 4]", (k:string, v) => {
    var e = list[k]; // e is any, as list has no string indexer,
                          // also an error with --noImplicitAny
})

JSON.parse("[3, 4]", (k:number, v) => {
    var e = list[k]; // e number, as expected
    k.toExponential() // runtime error, k is not a number
})

in short, this was intentional. but maybe we should reconsider.

[avonwyss]

I understand your reasoning, but how is this different from the stringify replacer which is kind of a sibling to the reviver?

stringify(value: any, replacer: (key: string, value: any) => any): string;

I see this as a callback with clearly defined arguments, the first being a string. Nothing prevents you from passing in a function which accepts (any, any), but it would prevent you from passing in (number, any) which is currently allowed but wrong to my understanding:

function B(name: number, value: any): any {
    ...
} 

JSON.parse("", B); // no error as of now!

[RyanCavanaugh]

PR welcomed on this one (be sure to read CONTRIBUTING.md on how to submit a lib.d.ts PR)

[AnyhowStep]

Hey,

I've been wanting to get more involved with the TypeScript code base but simply lack the knowledge about the current implementation (code base is huge and intimidating, I've only written toy interpreters).

This issue looks simple enough, I think. Just seems to be updating .d.ts files?

---

I looked at DefinitelyTyped/DefinitelyTyped#15744 and their Reviver type,

export type Reviver = (this: {}, key: string, value: any) => any;

I think the this parameter should be of type any or the following code will fail in strict mode,

JSON.parse(`{ "x":42 }`, function (k, v) {
    //Element implicitly has an 'any' type because type '{}' has no index signature.
    const x = this[k];
});

If we use this : any, then x will be of type any in the above code.

---

It also looks like JSON.stringify() needs to be updated.
The replacer parameter looks like it could benefit from having the same signature as Reviver

---

• Should JSON.parse()'s reviver be (this: any, key: string, value: any) => any?
• Should JSON.stringify()'s replacer be (this: any, key: string, value: any) => any?

---

I ran grep -nri reviver\? ./ and grep -nri replacer\? ./ in the lib/ directory and only found results in lib.es5.d.ts, I assume that's the only file that needs to be updated?

[AnyhowStep]

I also just noticed there are trailing spaces in 4 lines of comments in lib.es5.d.ts.

I assume no one wants these removed.

---

I made the changes in the two places and git diff gives me this,

---

Assuming this is all good, do I just need to run jake tests, and jake runtests before submitting a PR?

---

Also, I tried signing the CLA (Contributor License Agreement) at https://cla.opensource.microsoft.com/ but I think it's broken.
I signed in to GitHub and authorized the site to access my public account info and it's still asking me to sign in. Clicking "Sign In" again doesn't ask me to authorize the site again; so I assume authorization has been given but something else is broken.

Unless signing in was the same as signing the CLA...