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

Resolve refs inside additionalItems #69

Merged
merged 1 commit into from
Aug 31, 2020

Conversation

KapitanOczywisty
Copy link
Contributor

additionalItems was not checked for $refs.

fix microsoft/vscode#69071 fix microsoft/vscode#75394

@aeschli aeschli merged commit a0b585f into microsoft:master Aug 31, 2020
@aeschli
Copy link
Contributor

aeschli commented Aug 31, 2020

Oh wow, cood catch! Thanks @KapitanOczywisty !

@unional
Copy link

unional commented Feb 19, 2021

Just noticed this and want to point out that:

additionalItems doesn’t make sense if you’re doing “list validation” (items is an object), and is ignored in the case.

https://json-schema.org/understanding-json-schema/reference/array.html#id6

@aeschli
Copy link
Contributor

aeschli commented Feb 22, 2021

@unional When looking for $refs to resolve, we always visit all properties, regardless if they are applicable. That might be unnecessary work, but I don't think is a problem. Let me know if you think otherwise.

@unional
Copy link

unional commented Feb 22, 2021

Yeah, I notice that is the design of the code. The only concern is about "soundness", as language-service is used to provide runtime validation, so people writing json schema and rely on IDE to provide feedback will miss this case until they run other validation that catches this.

@KapitanOczywisty
Copy link
Contributor Author

@unional This extension provides intellisense and somewhat accurate validation, but if you want to be sure that it'll run fine on other validation tools, you need to write some tests. Here there are implemented features from different drafts and even non-standard stuff added for vscode use (error messages). I don't think that anyone is willing to bring it closer to drafts, since it's good enough for intellisense and there are very easy to use packages like ajv.

@unional
Copy link

unional commented Feb 22, 2021

Sure, no problem. And regarding drafts, really look forward for 2020-12 support. 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants