Flow: Is there a reason JSON.parse returns any

[jcready]

Right now the definition for the JSON global is as follows:

declare class JSON {
    static parse(text: string, reviver: (key: any, value: any) => any): any;
    static stringify(
      value: any,
      replacer?: ?((key: string, value: any) => any) | Array<any>,
      space?: string | number
    ): string;
}

But is there a reason JSON.parse couldn't return a more refined JSONValue type like this?

type JSONValue = | string | number | boolean | null | JSONObject | JSONArray
type JSONObject = { [key:string]: JSONValue }
type JSONArray = JSONValue[]

declare class JSON {
    static parse(text: string, ...args: []): JSONValue;
    static parse(text: string, reviver: (key: string, value: JSONValue) => any): mixed;
    static stringify(
      value: any,
      replacer?: ?((key: string, value: any) => any) | Array<any>,
      space?: string | number
    ): string;
}

99.99999% of the time that JSON.parse() is used it is only provided a single argument. And in those cases the only return type it can have would be a JSONValue (as defined above). For instance there's no way that it would ever return a function, a Buffer, undefined, etc. Is there a reason that JSON.parse() couldn't just be defined twice, one for the 99% case which returns a much more refined type JSONValue and another whose return type is mixed and accepts the reviver function?

[LoganBarnett]

My team ran into this because we have a requirement to have 100% type coverage in our code. It basically means any usage of JSON.parse causes the coverage check to fail. If it were the JSONValue or even mixed Flow would still require we validate whatever we got back, right?

[jcready]

@LoganBarnett from what I can tell, using the JSONValue counts as type coverage, although you probably will want to validate the parsed JSON regardless. Also, I recently discovered that you would have to change how JSONObject is defined to this:

type JSONObject = { [key:string]: JSONValue, $call?: empty }

Without the $call?: empty flow allows all of these to pass type checking:

const fn = () => {}
const notJSONa: JSONValue = fn
const notJSONb: JSONValue = { fn }
const notJSONc: JSONValue = [ fn ]

[LoganBarnett]

Ah, and JSON can't have functions in it.

We managed to work around the issue for now by copying core.js's libdef to our project's libdef. We changed the any to mixed and now we have proper coverage.

[dsilvasc]

It's odd that while this works:

type JSONValue = null | boolean | string | number | JSONValue[] | JSONObject
type JSONObject = { [key:string]: JSONValue, $call?: empty }

function read(input: string): JSONValue {
    return JSON.parse(input)
}

type Book = {
    id: number,
    published: boolean
}

function parse(x: JSONValue): ?Book {
  if (x && typeof x === 'object' && !Array.isArray(x) && typeof x.id === 'number' && typeof x.published === 'boolean') {
    return {
      id: x.id,
      published: x.published
    }
  } else return null
}

this other definition of parse doesn't:

function parse(x: JSONValue): ?Book {
  if (x && typeof x === 'object' && !Array.isArray(x) && typeof x.id === 'number' && typeof x.published === 'boolean') {
    return x
  } else return null
}

Flow complains:

15:     return x               ^ JSONObject. This type is incompatible with the expected return type of
13: function parse(x: JSONValue): ?Book {
                                   ^ Book
Property `id` is incompatible:
2: type JSONObject = { [key:string]: JSONValue, $call?: empty }
                                     ^ JSONObject. This type is incompatible with
9:     id: number,
           ^ number

[bwestergard]

This would be a great help in developing projects that are 100% typed.

[bwestergard]

Could the use of @jcready's declaration perhaps be made an option through a command line argument to flow?

[LoganBarnett]

@bwestergard Right now we have our own decls/core.js and our .flowconfig refers to decls as a source of libdefs. This seems to take precedence and allows us to maintain 100% type coverage. I don't have that configuration on my personal setup right now so let me know if you need more information and I'll provide it next time I'm at work.

[mrkev]

Is there a PR for this?