raml-org / datatype-expansion

(deprecated) Utility tool to expand a given RAML type and create a canonical form
Apache License 2.0
6 stars 7 forks source link

Unions of arrays are broken #106

Open psixdev opened 6 years ago

psixdev commented 6 years ago

I have the context:

const typesContext = {
    U1: {type: 'integer[] | string[]'},
    U2: {type: 'integer[] | string[] | nil'}
};

And I output expanded and canonical forms for types:

U1

expanded

{
  "type": "array",
  "items": {
    "anyOf": [
      {
        "type": "array",
        "items": {
          "type": "integer"
        }
      },
      {
        "type": "string"
      }
    ],
    "type": "union"
  }
}

canonical

{
  "type": "array",
  "items": {
    "anyOf": [
      {
        "type": "array",
        "items": {
          "type": "integer"
        }
      },
      {
        "type": "string"
      }
    ],
    "type": "union"
  }
}

U2

expanded

{
  "type": "union",
  "anyOf": [
    {
      "type": "array",
      "items": {
        "type": "integer"
      }
    },
    {
      "type": "array",
      "items": {
        "type": "string"
      }
    },
    {
      "type": "nil"
    }
  ]
}

canonical

{
  "type": "union",
  "anyOf": [
    {
      "type": "array",
      "items": {
        "type": "integer"
      }
    },
    {
      "type": "array",
      "items": {
        "type": "string"
      }
    },
    {
      "type": "nil"
    }
  ]
}

As you can see, the array of strings in the first union is turned into a string. It looks like a bug.

trevorr commented 6 years ago

IIRC, the precedence of the array and union operators is not what you'd expect. Based on your spacing, it's obvious to a human that you want a union of arrays (and possibly nil). However, the type parser doesn't consider spacing and applies different rules. Could you try wrapping your arrays in parentheses and see if that gives the result you expect?

trevorr commented 6 years ago

BTW, the code for that is here. As you can probably see, it's an ad hoc parser, and it doesn't follow consistent precedence rules. Unfortunately, AFAIK the RAML spec does not provide a grammar or precedences rules for type expressions.

psixdev commented 6 years ago

If I try to use type (integer[]) | (string[]) the tools.expandedForm throws an error:

Error: could not resolve: integer[])
    at expandForm (/node_modules/datatype-expansion/src/expanded.js:134:11)
    at form.anyOf.form.anyOf.map.elem (/node_modules/datatype-expansion/src/expanded.js:226:39)
    at Array.map (<anonymous>)
    at expandUnion (/node_modules/datatype-expansion/src/expanded.js:226:27)
    at expandForm (/node_modules/datatype-expansion/src/expanded.js:104:14)
    at expandForm (/node_modules/datatype-expansion/src/expanded.js:96:16)
    at expandForm (/node_modules/datatype-expansion/src/expanded.js:165:36)
    at Object.expandedForm (/node_modules/datatype-expansion/src/expanded.js:33:12)

Is this the library bug?

But I can use type ((integer[]) | (string[])) and get the result I expect, thank you.