Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues and warnings caused by profiles.schema.json #16103

Merged
merged 1 commit into from
Oct 13, 2023

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Oct 5, 2023

This addresses the following issues:

  • The JSON Schema spec doesn't actually define whether objects with
    a "properties" key still require "type": "object" or not.
    VS Code for instance largely pretends as if it's implied, but when it
    encounters them inside a oneOf tree, then it behaves as if it isn't.
    In other words, we need to always set "type": "object".
  • Declaring an oneOf containing a "type": "string" and an enum
    doesn't work, because if one of the enum cases is given, it results
    in both variants to match, since any enum is also a string.
    We have to use anyOf instead.
  • SuggestionSource used "BuiltinSuggestionSource" inside a type
    key which doesn't work. We have to use $ref for that.

Closes #13387

Validation Steps Performed

@KalleOlaviNiemitalo
Copy link

KalleOlaviNiemitalo commented Oct 5, 2023

IIUC, "properties" does not imply "type": "object"; if the instance is not an object, then "properties" in the schema has no effect. (The non-object instance always validates OK against "properties" because the instance cannot have any properties that would fail validation against the schemas in "properties".) https://json-schema.org/draft/2020-12/json-schema-core#section-10.3.2.1

When "properties" is used without "type" inside "oneOf",

{
    "oneOf": [
        {
            "type": "string"
        },
        {
            "properties": {
                "name": {
                    "type": "string"
                }
            }
        }
    ]
}

the string "hello" is valid instance for both subschemas for "oneOf", and thus invalid for the whole schema.

Does VS Code behave differently?

@lhecker
Copy link
Member Author

lhecker commented Oct 5, 2023

If you use "properties" without "type", VS Code still offers the given properties up for auto-completion and it does use it for validation too. For instance this is part of our default config:

{
    "command":
    {
        "action": "splitPane",
        "split": "auto",
        "splitMode": "duplicate"
    },
    "keys": "alt+shift+d"
}

It does auto-complete the "splitMode" value to "duplicate". And if you fix the schema to say this:

"splitMode": {
    "default": "duplicate",
    "description": "...",
    "type": "string",         // added
    "enum": ["duplicate"]     // added
},

...then it even complains about "splitMode" being invalid if you use anything but "duplicate".
Even though we have defined that object without "type": "object".

@KalleOlaviNiemitalo
Copy link

If the schema does not have "type": "object", then the instance does not have to be an object. But if the instance is an object and the schema has "properties" with the same property names as in the instance, then the property values must match those schemas.

For the following schema, null, {}, and {"name": "smirk", "length": 5} are all valid. {"name": null}, 42, and [true] are not valid.

{
    "type": [ "null", "object" ],
    "properties": {
        "name": {
            "type": "string"
        }
    }
}

If you then remove "type": [ "null", "object" ] from the schema, 42, and [true] become valid. {"name": null} is still not valid.

In an editor, if you have already typed the opening brace, thus making the instance an object, then it seems sensible to auto-complete the names listed in '"properties"`.

@zadjii-msft zadjii-msft merged commit 0b9f041 into main Oct 13, 2023
9 checks passed
@zadjii-msft zadjii-msft deleted the dev/lhecker/13387-schema branch October 13, 2023 20:44
@microsoft-github-policy-service microsoft-github-policy-service bot added Issue-Bug It either shouldn't be doing this or needs an investigation. Area-Schema Things that have to do with the json schema. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Schema Things that have to do with the json schema. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Recurring vscode JSON validation issues
3 participants