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

"$comment" keyword #197

Closed
handrews opened this issue Dec 15, 2016 · 63 comments
Closed

"$comment" keyword #197

handrews opened this issue Dec 15, 2016 · 63 comments

Comments

@handrews
Copy link
Contributor

handrews commented Dec 15, 2016

[EDIT: I changed it from "comment" to "$comment" as explained in a later... um... comment]

JSON lacks comments. Comments in complex schemas are good. The "description" keyword, as a general annotation feature that is noted as being usable in interface display is not suitable for schema author-to-schema author comments.

Let's reserve "$comment" as a keyword.

Implementations MUST NOT use "$comment" in any sort of end user data presentation.

Implementations MAY choose to use the value of "$comment" in debugging messages. Similar, schema editing tools MAY make use of or encourage the use of the comment keyword.

Implementations MAY opt to not store the comment field and its value in memory as an optimization.

There is no restriction on the comment's value. While a string is the most obvious sort of comment, schema authors may decide to format comments in any way they wish. Implementations that use comments for debugging or other purposes SHOULD support strings. Schema authors inventing more complex comment forms MUST NOT assume that peer implementations will be able to understand those forms.

"$comment" would also be legal within a "$ref" object, and an implementation would still be able to use it for debugging. All other keywords would still be in the "MUST be ignored" category should they appear in a "$ref" object.

[EDIT: originally this suggested the validation spec but as noted below it really belongs in core if we have it at all]

As an obvious use case, the reviews around "jsonReference" and "notJsonReference" in the meta-schema would have been much easier if a comment field is allowed.

@erosb
Copy link

erosb commented Dec 15, 2016

I'm not sure if it is a necessary feature. I can just hardly imagine any usecase where "description" is unsuitable for debugging and I need an other programmer-friendly "comment" too.

@handrews
Copy link
Contributor Author

Explaining complex usage of allOf/anyOf/oneOf/not/dependencies is very useful for schema maintainers, but should not be used as a description of anything user-visible.

@erosb
Copy link

erosb commented Dec 15, 2016

Ah-ha, now it is more understandable.

Well it an optional feature and not too complex to implement, so I don't mind it.

@handrews
Copy link
Contributor Author

Well it an optional feature and not too complex to implement, so I don't mind it.

Yeah, the idea here is really just to reserve the keyword so that it is not co-opted by extensions.

@handrews
Copy link
Contributor Author

Should probably actually be "$comment" in the core spec, just as comments are part of the core syntax of programming languages rather than some sort of library or extension. If a media type supports comments, that should be in the media type definition.

That will also make clear that it has nothing to do with validation or hypermedia.

@handrews
Copy link
Contributor Author

Here is an example of how comment could be used, direct from all of the confusion that these changes for "$ref" in the meta-schema have caused in #168, #188, #194, and json-schema-org/json-schema-org.github.io#57:

{
    "$schema": "http://json-schema.org/draft/schema#",
    "id": "http://json-schema.org/draft/schema#",
    "title": "Core schema meta-schema",
    "definitions": {
        "jsonReference": {
            "type": "object",
            "properties": {
                "$ref": {
                    "type": "string",
                    "format": "uriref"
                }
            },
            "required": [ "$ref" ],
            "$comment": "We do not set '\"additionalProperties\": false' because the spec says that other properties MUST be ignored.  It does not forbid their existence."
        },
        "notJsonReference": {
            "not": {
                "required": [ "$ref" ]
            },
            "$comment": "This gets used to ensure that it is never ambiguous whether an object is or is not a JSON Reference."
        },
       "schema": {...}
    },
    "$comment": "This oneOf ensures that everything is either considered a JSON Reference, or a schema, but never both.  This is necessary because $ref requires that all other keywords are ignored, but does not forbid their existence.",
    "oneOf": [
        {
            "$comment": "Including the notJsonReference definition here ensures that if $ref is present, it will validate as a JSON Reference (the other branch of the enclosing oneOf) or it will not validate at all.  This is what we want, as only schemas withouth $ref can be treated as regular schema objects.",
            "allOf": [
                { "$ref": "#/definitions/schema" },
                { "$ref": "#/definitions/notJsonReference" }
            ]
        },
        { "$ref": "#/definitions/jsonReference" }
    ],
    "default": {}
}

@handrews handrews changed the title "comment" keyword "$comment" keyword Dec 27, 2016
@epoberezkin
Copy link
Member

@handrews At the moment only keywords that affect validation start with $. description and title don't have it. Comment also doesn't affect validation - why does it need $ in front of it?

@handrews
Copy link
Contributor Author

@handrews At the moment only keywords that affect validation start with $. description and title don't have it. Comment also doesn't affect validation - why does it need $ in front of it?

@epoberezkin that's not correct. Only core keywords have $. Validation does not. This is not a meta-data keyword that is part of a vocabulary that is for consumers of schemas. We already have that with description. This is for notes by and for schema maintainers about technical issues with the schema itself. You would not put that stuff about "oneOf" in a description, as description is supposed to be part of the data that an implementation uses (as help text, for instance).

So $comment is like /* .. */ in C-derived languages, or '#' in Perl, Python, etc. It's something that a debugger can use, but a production compiler just strips it out.

@handrews
Copy link
Contributor Author

A comment feature is only useful if it is independent of all vocabularies, hence putting it in core, hence making it $comment with the $ prefix.

@epoberezkin
Copy link
Member

@handrews I agree only core keywords have $. But all core keywords affect validation now. And you suggest introducing another core keyword that has no effect on validation. I'd rather comment be in the category where there are other keywords not affecting validation...

@handrews
Copy link
Contributor Author

@epoberezkin how do core keywords directly affect validation?

$schema sets the meta-schema, e.g. the vocabulary. That vocabulary may not even be a validation vocabulary.

id/$id identifies and changes the base URI. No impact on validation, it's purely about how to resolve URIs, whatever those URIs do.

$ref allows a schema located elsewhere to be re-used. Depending on how you look at it, it either logically includes the referenced schema, which then behaves however it would have behaved if written in line (so validates if it is a validation schema, but if it's not a validation schema it has no validation effect). Or it simply has the result of the schema to which it refers. Which again, is a validation result if that is a validation schema, but if it is a schema using a non-validation vocabulary, $ref doesn't suddenly add validation to that non-validation vocabulary.

$ref is either an inclusion or a proxy (as we've discussed ad nauseum elsewhere), but it doesn't care what it is including or proxying.

@epoberezkin
Copy link
Member

as we've discussed ad nauseum

lol

@epoberezkin
Copy link
Member

Yes, $schema does not indeed, but it at least defines what is a valid schema... $id and $ref affect validation, changing them may either break schema or change validation result. comment does absolutely nothing. And yet you want it to be a core keyword.

@handrews
Copy link
Contributor Author

comment does absolutely nothing. And yet you want it to be a core keyword.

Do you like having comments in programming languages?

@epoberezkin
Copy link
Member

No. Never use them

@epoberezkin
Copy link
Member

Well, almost never. Naming things and making them self-evident is much better, comments usually go out of sync with code and there is no way of checking it.

@handrews
Copy link
Contributor Author

No. Never use them

Then I'm pretty sure you're not the audience for this keyword.

@epoberezkin
Copy link
Member

That's why I don't see why it should be part of the core. It's not just me, it's a very common approach to eliminate or at least minimise comments. It encourages making things more complex than they should be, so instead of simplifying people start writing comments (exactly the use case for metaschema in#194).
Why "comment" is not good enough? I may use it once or twice. Why make it so prominent?

@handrews
Copy link
Contributor Author

Why is "$" or putting it in core prominent? The only relevant question is "is this specific to a vocabulary, or is it universal". Universal things go in core. It makes no sense at all in validation or hyper-schema, it's not specific to either.

@awwright
Copy link
Member

I anticipate that most vocabularies (if we get more than besides hyper-schema) will pull in JSON Schema Validation, so I don't really see a problem with making it a JSON Schema Validation keyword, myself.

@handrews
Copy link
Contributor Author

@awwright I strongly dislike polluting validation with non-validation keywords, and making assumptions on behalf of unknown future users that this validation vocabulary will always be fundamental. If it's going to be impossible to use JSON Schema without validation, why do we have a core spec at all?

@awwright
Copy link
Member

That's a question that a lot of people including myself have asked...

I moved forward with the approach on the idea it's useful to separate the media type definition from the idea of a "vocabulary", even if it is one that's going to be subsumed into every single other vocabulary.

@handrews
Copy link
Contributor Author

I moved forward with the approach on the idea it's useful to separate the media type definition from the idea of a "vocabulary", even if it is one that's going to be subsumed into every single other vocabulary.

I'm in favor of that (the media type definition approach). While I don't want to bend over backwards on account of hypothetical non-validation vocabularies, I do want to make sure we don't make it difficult to use application/schema+json for something else entirely. Or for a completely different validation approach- it wouldn't be standard but people should be able to make and use non-standard vocabularies.

@handrews
Copy link
Contributor Author

As far as $comment, if enough people like the idea and agree that it is analogous to a programming language comment syntax, then it should go in the media type so it can be used automatically with any vocabulary.

If more people just don't see the need for comments I'm not going to be too upset to drop the idea. I just really don't want it to be a validation keyword. I'd rather drop the idea than further clutter validation.

@epoberezkin
Copy link
Member

Why don't we park it until draft 6 is out?

@handrews
Copy link
Contributor Author

Why don't we park it until draft 6 is out?

It's not a draft 6 proposal, so if you don't want to talk about it now just don't talk about it. It will be here when we get to the next draft.

@awwright
Copy link
Member

I haven't seen anyone clamoring for this ("description" is usually good enough), so it might be prudent to sit on it for a tiny bit longer.

Should probably actually be "$comment" in the core spec, just as comments are part of the core syntax of programming languages rather than some sort of library or extension.

Also, this is a very compelling argument

@handrews
Copy link
Contributor Author

(as a note to myself- there is probably a bit to put in "Security Considerations" that schema authors should not put sensitive information in the comments as anyone who views the raw file can see them, and a malicious/erroneous implementation could display the strings to users in violation of the spec.)

@ruifortes
Copy link

Don't you think this should be a case for json preprocessing(/extension) like json5

@handrews
Copy link
Contributor Author

@ruifortes only if it reaches RFC. I don't want to staple JSON Schema to another under-development project. For now, it needs to work within RFC 7159.

There is, of course, nothing stopping you from handing comments some other way, such as by writing in YAML, JavaScript, or some other thing and pre-processing it into JSON.

I have to admit, I'm baffled at the level of resistance to this. No one is forced to use it, the only impact this has at all is to reserve "$comment" and ensure that no one implements conflicting functionality. Implementations don't even have to do anything with it. The specification already indicates that unknown keywords should be ignored. They way I've phrased this proposal, simply treating "$comment" as unknown and ignored is a compliant implementation.

@seagreen
Copy link
Collaborator

seagreen commented Mar 21, 2017

@ruifortes: Changing JSON Schema to not actually be JSON would be a huge change. (EDIT: I do think json5 is the best of the JSON-with-comments specs though)

@handrews: I think people are just touchy about this because people keep advocating a backward-compatibility breaking change to add comments to JSON itself, which has made them irrationally touchy about any kind of issue involving comments. I think that would describe my reaction pretty well. Obviously you won me over though!

@handrews
Copy link
Contributor Author

@seagreen yeah that's actually pretty understandable. If I thought one of those comments-in-JSON things was actually on the verge of broad adoption/RFC, I wouldn't be bothering with this :-P

@ruifortes
Copy link

Json5 is sweet though.
Just require('json5/lib/require') in node entry point or use json5-loader in webpack . :)

@Relequestual
Copy link
Member

@ruifortes Sure, very nice. As @handrews has said...

There is, of course, nothing stopping you from handing comments some other way, such as by writing in YAML, JavaScript, or some other thing and pre-processing it into JSON.

I can't see myself being convinced that JSON Schema should use json5 exclusivly. That would be JSON5 Schema, which this spec is not.

@Anthropic
Copy link
Collaborator

Anthropic commented Apr 19, 2017

@handrews
I saw this a while back on reddit somewhere:
{ "//comment": "I am a comment :)" }
Perhaps any property prefixed with // could considered the same way? It kind of is already as nothing knows what to do with it, just not officially, I kind of do that when testing anyway, if you have a property and want to disable it quickly you can just add a slash prefix making it a comment. Then a comment with comment slashes could be used for debug output etc.. as described as the only semi-exception to being ignored.

@handrews
Copy link
Contributor Author

@Anthropic interesting idea! I do like the reference comments to JavaScript (among other languages).

I went for the dollar sign because it is a prefix we are already using for core keywords.

Since the only point of having a comment keyword is to outright forbid conforming implementations from doing anything else with it, I'm reluctant to block out an entire prefixed namespace. What if someone has a JSON file where the keys are some sort of filesystem path? Or for that matter, what if keys are protocol-relative URIs, which start with '//'?

I'm open to further argument, but I think for the specification, we really want to block out the minimal space possible to ensure interoperability. That would mean a single keyword, and staying with a prefix that is already associated with core keywords.

Does that make sense?

If so, I'm going to submit a PR for this. But I am open to more discussion first.

@handrews handrews added this to the draft-07 (wright-*-02) milestone Aug 21, 2017
@handrews
Copy link
Contributor Author

I have since seen a set of schemas where, depending on who wrote a given file, any one of three different commenting extension keywords may be in use: two custom keywords, plus "$comment" from this proposal.

To me, that's a pretty compelling reason to add this. People are going to do it anyway, and the issue of multiple keywords doing the same thing will only get more likely if schemas are shared across completely separate organizations.

handrews added a commit to handrews/json-schema-spec that referenced this issue Aug 21, 2017
This addresses issue json-schema-org#197.  The primary purpose is to reserve the
keyword and ensure that implementations do NOT do anything based
on its presence or contents.  This improves interoperability by
ensuring that comments are safe for use with any conforming system.

Without this keyword, two implementations may implement it with
differing semantics, potentially producing undesirable or
insecure behavior.

Effort has been made to ensure that existing implementations
that properly ignore unknown keywords will not need to make
any changes at all to be in conformance for this keyword.
@Relequestual
Copy link
Member

Are we waiting for 14 days since PR opened to merge? Any objections?

@handrews
Copy link
Contributor Author

handrews commented Sep 1, 2017

@Relequestual yes, so I'll merge it on Monday or whenever the 14 days is up.

@handrews
Copy link
Contributor Author

handrews commented Sep 5, 2017

PR merged.

@Sasino97
Copy link

Why can't we just have C-style /* */ comments?

@karenetheridge
Copy link
Member

Because that's outside the JSON document model.

@jdesrosiers
Copy link
Member

@Sasino97 I have good news for you. You don't have the write JSON Schemas in JSON. JSON Schema works against the JSON data model, not the JSON text itself. That means you can write your schemas using any format you want as long as it's compatible with what is expressible by JSON. Some options that support comments are JSONC, JSON5, and YAML. You can even write schemas directly in the language you write code in. We see people write schemas in JavaScript or Python all the time. I hope that helps.

@xtergo
Copy link

xtergo commented Apr 4, 2022

I think the $comment isn't optimal. It helps for some scenarios but not for all.

I'd like that the Json standard support comments using something like /* */ even if it doesn't fit the Json standard. This would be more powerful. With this syntax other kinds of metadata could be added on any "node" which could be very useful for utils/tools that would like to extend Json usage.

The Json implementation would just need to strip this of this first and then it would conform to standard format. This leaves room for other tools to work on the Json schema with the comments.

@awwright
Copy link
Member

awwright commented Apr 4, 2022

I'd like that the Json standard support comments using something like /* */ even if it doesn't fit the Json standard.

JSON is pretty fixed in stone at this point. I think JSON should add support trailing commas, and they won't even do that.

The only way we could support this would be with a new media type, like a domain-specific language for expressing a JSON Schema. I think that would be too much scope creep (it's not strictly necessary for our goal), but it could be defined as a separate specification.

There's a few separate projects in this area by others:

@karenetheridge
Copy link
Member

I'd like that the Json standard support comments using something like /* */ even if it doesn't fit the Json standard

I think this is what you're looking for: http://www.relaxedjson.org/

JSON Schema follows the JSON document model, but you can use any format you like that produces the same data, e.g. yaml or even (for some data structures) csv. You could certainly create a new media type that used relaxed json as its data format -- comments will be stripped though and not visible to json schema.

@xtergo
Copy link

xtergo commented Apr 6, 2022

JSON is pretty fixed in stone at this point. I think JSON should add support trailing commas, and they won't even do that.

The only way we could support this would be with a new media type, like a domain-specific language for expressing a JSON Schema. I think that would be too much scope creep (it's not strictly necessary for our goal), but it could be defined as a separate specification.

There's a few separate projects in this area by others:

I understand there are other variants of Json that can solve this. But the problem is that if this doesn't get into the Json core standard many, including me, can't rely on that all tools/IDE:s can handle this. And then we will loose the potential to include cool new features, instead it will be solved with a Json variant that only works for some specific usage.
It has to be part of the core standard in order to work with the 3pp eco-system.

The comments enable other 3pp:s to place metadata at any place in the Json structure. This could be GUI, model, matching information that otherwise would not be possible. A new Json tool would only need to strip away this info and then it would comply to current standard.

Over the year we have seen so many requests about this that it would be unfortunate to just state that Json is "fixed".
People might think that comments is some minor thing, I think it's a very big thing and if we don't get this in place we limit the potential of Json more than one might think.

@gregsdennis
Copy link
Member

gregsdennis commented Apr 6, 2022

if this doesn't get into the Json core standard...

I think you're in the wrong place to request this. We don't control the JSON standard here, just JSON Schema. We use JSON, and for maximum compatibility, it doesn't make sense for us to use any one variant.

I think your options are down to asking a particular implementation to support a variant that you're interested in using.

@awwright
Copy link
Member

awwright commented Apr 6, 2022

I think your options are down to asking a particular implementation to support a variant that you're interested in using.

@xtergo This is your best option. For better or worse, JSON is set in stone. It's designed for encoding bulk data, it was never really intended for hand-authored data. You're going to have more luck advocating for adoption of an alternative format that's better suited for your purpose.

@json-schema-org json-schema-org locked as off-topic and limited conversation to collaborators Apr 7, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

16 participants