jsonnext / codemirror-json-schema

A JSONSchema enabled mode for codemirror 6, for json4 and json5, inspired by monaco-json
https://codemirror-json-schema.netlify.app
MIT License
57 stars 9 forks source link

Expected type not working #69

Closed loganripplinger closed 1 month ago

loganripplinger commented 8 months ago

Repro steps

I am using schemas generated from protobufs via protoc-gen-jsonschema.

I am using JSON4 not JSON5.

Use this schema to reproduce the problem show below:

{
    "$schema": "http://json-schema.org/draft-04/schema#",
    "definitions": {
        "foo": {
            "properties": {
                "number": {
                    "oneOf": [
                        {
                            "type": "integer"
                        },
                        {
                            "type": "null"
                        }
                    ]
                }
            },
            "additionalProperties": false,
            "oneOf": [
                {
                    "type": "null"
                },
                {
                    "type": "object"
                }
            ]
        }},
    "properties": {
        "foo": {
            "$ref": "#/definitions/foo",
            "additionalProperties": false,
            "oneOf": [
                {
                    "type": "null"
                },
                {}
            ]
        }
    },
    "additionalProperties": false,
    "oneOf": [
        {
            "type": "null"
        },
        {
            "type": "object"
        }
    ]
}

I further narrowed it down to this schema (in JS/JSON5 instead of JSON4, though I am still using the JSON4 plugin):

{
  $schema: 'http://json-schema.org/draft-04/schema#',
  properties: {
    foo: {
      additionalProperties: false,
      properties: {
        number: {
          type: 'integer',
        },
      },
      oneOf: [
        {
          type: 'null',
        },
        {
          type: 'object'
        },
      ],
    },
  },
  additionalProperties: false,
  oneOf: [
    {
      type: 'null',
    },
    {
      type: 'object',
    },
  ],
}

Of note if you remove properties.oneOf and instead just use type: 'object' it will correctly show the error:

{
  $schema: 'http://json-schema.org/draft-04/schema#',
  properties: {
    foo: {
      additionalProperties: false,
      properties: {
        number: {
          type: 'integer',
        },
      },
      type: 'object'
    },
  },
  additionalProperties: false,
  oneOf: [
    {
      type: 'null',
    },
    {
      type: 'object',
    },
  ],
}

Expected Behavior

An error should appear.

image

Actual Behavior

No error appears.

image

acao commented 8 months ago

I will look into this bug - thanks for reporting! I didn't know oneOf could be represented this way tbh, haha! I see this is valid, but usually I would do this (I wonder if this also isn't working?)

{
  $schema: 'http://json-schema.org/draft-04/schema#',
  properties: {
    foo: {
      oneOf: [
        {
           type: 'object',
           additionalProperties: false,
            properties: {
              number: {
                 type: 'integer',
              },
           },
        {
          type: 'null'
        },
      ],
    },
  },
  additionalProperties: false,
  oneOf: [
    {
      type: 'null',
    },
    {
      type: 'object',
    },
  ],
}

but your generated schema def is also valid. I will see what I can do, starting with some tests!

acao commented 4 months ago

this one is going to take some time because it requires rewriting some of our pointer functions to pick up the root # as a pointer, which ends up breaking a lot of functionality that as it turns out was based on skipping # accidentally.

@imolorhe what do you think of making the object root a pointer - i think it will solve issues like these ultimately, and thus we can be in compatibility with some major json schema specs that have top level complex types or refs (top level $ref or top level complex types)

imolorhe commented 4 months ago

No issues there from me.

imolorhe commented 1 month ago

I dug into this a bit further, and I think the issue is upstream in json-schema-library. I've created an issue there.