support specifying schema via $schema key

[robberphex]

What does this PR do?

Using $schema key to specify schema

What issues does this PR fix or reference?

Is it tested? How?

CI: Test getSchemaFromFileContent

---

There is a json file with $schema key:

{
    "$schema": "https://json.schemastore.org/package",
    "name": "name"
}

Auto-completion works based on schema (at VSCode).

But when I convert json to yaml:

$schema: https://json.schemastore.org/package
name: name

Auto-completion doesn't works. So, could yaml-language-server support $schema key? That what this PR do.

[evidolob]

@robberphex Thx for the contribution.
It would be nice if you fix prettier/eslint errors, to let CI run tests.

[coveralls]

Coverage increased (+0.08%) to 79.094% when pulling c2596fc on RobberPhex:schema-tag into 2bae915 on redhat-developer:master.

[evidolob]

@JPinkney Can you look on this PR?

[JPinkney]

I won't have time to look at it until next week. But I have a quick question. Since the $schema key is non-standard what happens if a user has a schema with additionalProperties: false? Does the $schema key report an error?

[evidolob]

@JPinkney Thx, that's what I thinking about.
@robberphex yaml-language-server supports comment based alternative for this feature.

[JPinkney]

I just tested it out and it seems to work well. I suppose this could be addressing a similar but slightly different use case then: https://github.com/redhat-developer/yaml-language-server#using-inlined-schema. With the inline $schema you can set the schema per yaml document and as long as whatever you are submitting the yaml to accepts additional properties then it should be fine. With the modeline it's only being set in the first document and works well in cases when the yaml cannot have additional properties.

I think a check would need to be added to the parser to not throw an error on $schema though. If I try and use the composer schema which doesn't allow additional properties:

$schema: http://json.schemastore.org/composer
abandoned: false

the autocompletion and hover work but I get: Property $schema is not allowed.yaml-schema: http://json.schemastore.org/composer on the $schema key

[shsuman]

Do you guys have any plan to merge this PR ?

[evidolob]

@shsuman We need to have changes in yaml parser/validator to do not throw error like Property $schema is not allowed. before we can merge this. I can look on it next week.

[shsuman]

Hello @evidolob ,
Thanks for your reply. We are planning to fork your repo and we would really want this $schema property to work. So, if you could give us a timeline for this feature to be available so that we don't have to do duplicate work

[JPinkney]

@shsuman Just out of curiosity is there any particular reason why you would be forking?

cc @gorkem

[gorkem]

@shsuman Do not fork or publish the yaml LSP with your own extension such things usually breaks our functionality. We have already had similar cases with different MSFT teams. I think we have an understanding that this is not a good practice.

[joshuawilson]

@shsuman PRs are welcome.

[shsuman]

@gorkem Thank you for your response. We are currently in discussion with our team regarding this issue. I will get back to you on the same

[shsuman]

@gorkem Can you briefly describe what problems have you seen in the past with other extensions shipping the Yaml LSP ?

The one thing that we are seeing is duplicate errors and suggestions but other than that we have not observed anything which would either break ours' or yours' functionality.

[gorkem]

@shsuman Our release cycles are not synchronized, you probably are not going to refresh the LSP as often as we do. We end up with getting bug reports for all the YAML issues. Even months after they are fixed.

It is not fair for anybody to expect us to spend time triaging bug reports because somebody have not done the right thing. Therefore, as discussed in this issue, we will warn and even disable functionality in case of conflicts.

[shsuman]

Hey @gorkem!

We appreciate you taking the time to listen to our query. We'd love to provide you with some context on who we are as a team and what we're hoping to achieve with the YAML language server (LS).

We're the Azure Machine Learning (ML) extension team in VS Code. Our extension aims to provide an enhanced developer experience for data scientists and ML engineers targeting the Azure ML service. Many of these users edit YAML configuration files in Code thus we were hoping to be able to provide them with schema validation and completions.

Our plan was to extend your LS to create our own Azure ML language server to provide dynamic completions and preview definitions of cloud resources in addition to the schema-based completions provided by the YAML LS. Currently our completions framework is closely coupled with the LSP validation as we have specific Azure ML tags in our schema which inform when and what completions to surface.

We would love to hear your thoughts regarding the usage and extension of the LSP. Would it be possible for our teams to meet/collaborate to address any concerns you have or talk through alternative approaches? One thing to note is that our customer segment (Azure ML users editing YAML files) is a very small subset of your overall customer base (all VS Code YAML authors) so we don't expect your repository to get flooded with outdated bug reports.

Please let us know if you have any additional questions. We look forward to hearing from you!

Best,

Shantnu Suman on behalf of the Azure ML extension team

[gorkem]

@shsuman
Currently we have APIs to provide schemas dynamically that will allow the YAML LS to provide validation code assist etc. We do support a few JSON schema extensions too. I am not sure if those ways would be enough but of course we are open to introducing more extensibility if there are use cases for it.

Are you looking to extend the Yaml LS or vscode-yaml though?

[shsuman]

Hey @gorkem,

Thank you for your reply.

To answer your question, we are trying to extend the Yaml LS and not vscode-yaml.

Also, my team just had a couple of follow up questions:
1. Do you have any documentation on the APIs that you talked about in your last response ?
2. What did you mean by "We do support a few JSON schema extensions too." ?

Lastly, we would highly appreciate it if you would be willing to hop on a call with us to discuss it in detail ? If yes, let me know your preferred contact method as well as your preferred time.

Regards,
Shantnu Suman on behalf of the Azure ML extension team

[shsuman]

@gorkem Just a friendly follow up on my last message. Let me know if you have any questions or concerns

[gorkem]

@JPinkney Do you have reference to the extension API and JSON schema extensions that we support. I thought it was a package on npm but I could not find it.

[gorkem]

@shsuman pinged @JPinkney he reminded me where we recorded things take a look at https://github.com/redhat-developer/vscode-yaml/wiki/Extension-API

[shsuman]

@gorkem These API won't prove much useful to us as we are trying to contribute to completions as well as diagnostics dynamically i.e. generated through our extension and not the schemas. For this very purpose, we need to be able to directly contribute to the completions as well as diagnostics returned by the LS

[gorkem]

I do not grok why you would need to run your own copy of the LS, if you are trying to have additional diagnostics or completions. You can register multiple providers for a file.

[shsuman]

@gorkem Because we are putting tags in our schema ("arm_type"), we are using those tags from the schema to determine whether to provide completions and also what completions to provide for a given node

[safareli]

@evidolob mentioned that this PR may cause validation errors if JSON Schema doesn’t allow additional properties for "root" Object. here redhat-developer/vscode-yaml#567 (comment)

As a user I do prefer to have this PR merged and be able to use $schema, with the caveat that until the parser changes are made it might throw on noAdditionalProperties.

[carlosedp]

Still no news on merging this?

[bollwyvl]

I didn't see this before doing so, but opened #970 taking a slightly different approach. I'd really rather not consider ignoring "additionalProperties": false, and think the magic comment is just fine in that case. But supporting schema that allow/require $schema seems like a pretty straightforward win.