Recursive union type silently breaks union structure about amf

[looseale]

Hi @deiteris
We are already working on a refactor in the transformation code that resolves inheritance because we also detected some cases of inheritance mixed with recursion that have this same problem.
We still don't have an ETA for this, but it probably will be in the late May release.

[tomsfernandez]

Hi @deiteris! This refactor was released in version 5.3.0. The latest version has some fixes on that inheritance resolution also. Do you want to test it to see if this still applies?

[deiteris]

Hi @tomsfernandez! The code I mentioned in the issue produces identical result in 5.3.0-0 and 5.4.0-1.

I've tried different transformation pipelines as well which also produce the same result (even though a more readable one). For example, this is the result of the Default pipeline:

http://a.ml/amf/default_document#/declares/union/Value
http://a.ml/amf/default_document#/declares/shape/TypeOne
http://a.ml/amf/default_document#/declares/union/Value/inherits/union/default-union/anyOf/union/default-union
http://a.ml/amf/default_document#/declares/shape/TypeTwo
http://a.ml/amf/default_document#/declares/union/Value/inherits/union/default-union/anyOf/union/default-union/anyOf/union/default-union   
http://a.ml/amf/default_document#/declares/scalar/TypeThree
http://a.ml/amf/default_document#/declares/shape/TypeMap

What I also find concerning, is that this nesting seems to happen with any custom type even without recursive type (I guess might need to rename the issue). So, scalar TypeOne | TypeTwo | TypeThree (that are just string | number | integer) will also produce this sequence with nesting:

http://a.ml/amf/default_document#/declares/union/Value
http://a.ml/amf/default_document#/declares/scalar/TypeOne
http://a.ml/amf/default_document#/declares/union/Value/inherits/union/default-union/anyOf/union/default-union
http://a.ml/amf/default_document#/declares/scalar/TypeTwo
http://a.ml/amf/default_document#/declares/scalar/TypeThree

While union of plain string | number | integer produces correct sequence:

http://a.ml/amf/default_document#/declares/union/PlainUnion
http://a.ml/amf/default_document#/declares/union/PlainUnion/inherits/union/default-union/anyOf/scalar/default-scalar
http://a.ml/amf/default_document#/declares/union/PlainUnion/inherits/union/default-union/anyOf/scalar/default-scalar_1
http://a.ml/amf/default_document#/declares/union/PlainUnion/inherits/union/default-union/anyOf/scalar/default-scalar_2

I've been making my own expression parser and it seems to me as if CST which was used to generate AST is as follows:

(Value: union 
  (null: union (TypeOne: custom_type)  (TypeTwo: custom_type)) 
  (TypeThree: custom_type)
)

Which may happen in case | is treated as an operator and associated to left/right value.

While what I expect (and managed to achieve) is:

(Value: union (TypeOne: custom_type) (TypeTwo: custom_type) (TypeThree: custom_type)))

[deiteris]

As a workaround, I've flatten such union structure in my own code (v5.4.0-1) in the flattenUnion function, just in case someone finds it useful:

import amf, { Module, Shape, TypeIRI, TypeUtil, UnionShape } from 'amf-client-js'

const raml = `
#%RAML 1.0 Library

types:
  TypeOne:
    properties:
      prop3: string | integer

  TypeTwo:
    properties:
      prop1: string
      prop2?: string

  TypeThree:
    properties:
      prop5: boolean

  TypeMap:
    properties:
      prop4: Value

  Value:
    type: TypeOne | TypeTwo | TypeThree | TypeMap
`;

(async () => {
  const client = amf.RAMLConfiguration.RAML10().baseUnitClient()

  const data = await client.parseContent(raml)

  //const result = client.transform(data.baseUnit, amf.PipelineId.Default)

  function flattenUnion(shape: UnionShape, shapes: Shape[] = []): Shape[] {
    for (const t of shape.anyOf) {
      if (TypeUtil.isTypeOf(t, TypeIRI.UnionShape) && t.name.isNull) {
        flattenUnion(t as UnionShape, shapes)
      } else {
        shapes.push(t)
      }
    }
    return shapes
  }

  function traverse(type: Shape) {
    console.log(type.isLink ? type.linkLabel.value() : type.name.value())
    for (const parent of type.inherits) {
      traverse(parent)
    }
    if (TypeUtil.isTypeOf(type, TypeIRI.UnionShape)) {
      const shapes = flattenUnion(type as UnionShape)
      for (const t of shapes) {
        traverse(t)
      }
    }
  }
  traverse(((data.baseUnit as Module).declares as Shape[]).find(shape => shape.name.value() === 'Value')!)
})();

Outputs:

Value
null
TypeOne
TypeTwo
TypeThree
TypeMap

Without this workaround, the output is:

Value
null
TypeOne
null
TypeTwo
null
TypeThree
TypeMap