microsoft / vscode-json-languageservice

JSON language service extracted from VSCode to be reused, e.g in the Monaco editor.
Other
265 stars 113 forks source link

It’s not longer possible to reference schema paths using hashes #123

Open remcohaszing opened 2 years ago

remcohaszing commented 2 years ago

Trying to figure out the cause of redhat-developer/yaml-language-server#585 I narrowed it down to a change in vscode-json-languageservice@4.1.9.

Based on the diff between 4.1.8 and 4.1.9 I suspect https://github.com/microsoft/vscode-json-languageservice/commit/6a3406c5f41c8fbba4ed25fbd1ab5938529cf86c is the commit which causes the issue.

When I use the latest version of Visual Studio Code (1.63.2) I can reproduce this issue by creating the following JSON file:

{
  "$schema": "https://appsemble.app/api.json#/components/schemas/AppDefinition",
  "name": 123
}

This should display a number of errors, but instead it silently fails to load the JSON schema.

gorkem commented 2 years ago

@aeschli This is blocking vscode-yaml to update to newer versions.

aeschli commented 2 years ago

Schema URIs with fragments are new to me. Can you describe how this used to work? Was the request service called with the full URI (including fragment) and then extract the schema at the given path?

aeschli commented 2 years ago

@remcohaszing We never intentionally supported $schema with fragment path that points to a sub schema. I'd say that never worked, but maybe you know more. Is the original issue https://github.com/redhat-developer/yaml-language-server/issues/585, the error is about $ref. That would make more sense as ref supports that syntax. I think we had a encoding issue that got fixed. If not, can you provide a reproducible example that is based on $ref?

remcohaszing commented 2 years ago

$schema with fragment support may have been unintentional, but it did work. It still works with the VSCode YAML 1.7.0 (latest at the moment of writing).

Just create a file named app-definition.yaml. It will pull in the schema based on a url with a fragment defined in SchemaStore (permalink). You will get schema validation, completion, and hovers effects. This is also what the original issue is about. $ref is only mentioned in the VSCode output message.

I only nailed it down to this issue, because I troubleshooted the issue in yaml-language-server and found out the version bump of vscode-json-languageservice causes the issue. I haven’t used vscode-json-languageservice directly.

aeschli commented 2 years ago

So maybe the request service of the yaml language server knows how to load the schema with fragment, parses the schema resource and returns the sub schema. If that's broken, this should be debugged and maybe fixed in the yaml language service. As said it's not a use we thought of in the json-language-service. I definitely like to get some background there, if you have anything.

We can think of adding that feature to the json-languageservice, I implemented it in https://github.com/microsoft/vscode-json-languageservice/tree/aeschli/schemaWithFragment. Why I hesitate is, the use of $schema outside of schemas is actually a VS Code thing. It's not part of the spec. The spec only defines $schema to be used in schemas, for meta schemas. I'd rather not add any additional unspeced features, unless the use of fragments is also defined for meta schemas. The question is if we support #/foo/bar, what about #foo (pointing to an anchor). That would involve more work.

remcohaszing commented 2 years ago

I’m not familiar with meta schemas, so I trust you know more about this than I do. :)

From my perspective it just used to work, until it didn’t at some point. If this isn’t spec compliant, I understand you’d rather not include this feature.

However, since using $schema on a JSON schema is actually not spec compliant, I also see no harm in supporting this. In fact, I think it’s a powerful feature to be able to reference schemas from OpenAPI documents, even if this feature was previously unintentional.

Also schemas aren’t only referenced through $schema. In my case, it’s referenced from schemastore or the uri property in the schemas array in monaco-yaml (which maps exactly to the official monaco-json integration). Neither of these need to be spec compliant.

bfelaco commented 3 months ago

The use of a JSON Pointer in a URL hash to reference a sub-schema is explained here and here.

The problem is unrelated to the $schema tag. The YAML extension does not actually require that convention. I can reproduce the problem by explicitly mapping file paths to a sub-schema within the JSON Schema for the OpenAPI spec (https://raw.githubusercontent.com/OAI/OpenAPI-Specification/main/schemas/v3.1/schema.json#/$defs/parameter) in the settings.json for the RedHat YAML extension as explained here.

I did find a workaround:

  1. Create a local YAML or JSON file within the workspace.
  2. Within the local file, add a single $ref property containing the URL of the target subschema, with URL hash.
  3. In the settings.json, instead of the target URL, use a local file reference to the local YAML or JSON file.

settings.json:

{
    "yaml.schemas": {
        "schemas/parameter.yaml": [
            "**/components/parameters/**/*.yaml"
        ]
    }
}

schemas/parameter.yaml:

$ref: https://raw.githubusercontent.com/OAI/OpenAPI-Specification/main/schemas/v3.1/schema.json#/$defs/parameter

My assumption is that this delegates to the JSON Schema parser which properly handles the $ref. Perhaps a simple fix is to dynamically construct a JSON Schema document on the fly when encountering a URL with a hash, and hand that to the JSON Schema to parse and resolve all references?