Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Rename required to present #186

Closed
notEthan opened this issue Jun 16, 2022 · 8 comments
Closed

Rename required to present #186

notEthan opened this issue Jun 16, 2022 · 8 comments

Comments

@notEthan
Copy link

I will recognize at the outset this is change would be a significant backwards incompatibility. I want to at least bring it for discussion.

I think the keyword "required" has a poor name for its purpose. All of the validation keywords express requirements of one sort or another. required should be named for what it is actually requiring - the presence of the indicated properties. I think "present" is the best keyword to express this. some variant of "properties present" might be an option too.

The current name is okay for positively validating - saying a property is required does imply that its presence is the requirement. That case is natural enough to read and make intuitive sense. But we have all seen the confusion that results when using it in conditional or negated schemas.

if:
  required:
    - foo
  then:
    ...

"If property foo is required, then ..." is how that reads, but what it means is not very intuitive. "If property foo is present" is much more applicable, and reads more naturally from if: {present: ["foo"]}.

Likewise, not: {required: ["foo"]} does not mean "property foo is not required". Multiple properties are worse: not: {required: ["foo", "bar"]} reads like "neither foo nor bar is required". Much improved, not: {present: ["foo", "bar"]} does assert "properties foo and bar are not present" or "must not be present".

I think required made more sense back when it was a boolean keyword of a property schema, but would have been better renamed when it was put in its right place as a sibling of properties. I think the name present is a significant improvement - I'm interested to hear people's opinions.

Of course, there are countless schemas which use required. This would be a widely impactful change, though not the biggest (e.g. smaller than id / $id, I think). There are tools to convert schemas between spec versions, and this would be a trivial conversion to implement. Maybe required could live on for a while as a deprecated alias with the same function as present (though I don't think the spec has ever done that before and that may be a whole other issue to address).

I think in the long run, though this change would have some pain in its transition, it is worth the benefit of more clearly expressing the intent and meaning of the keyword in all the contexts where it is used.

@notEthan
Copy link
Author

this would impact dependentRequired as well, and some open proposals regarding required/present property names, including: json-schema-org/json-schema-spec#1144, json-schema-org/json-schema-spec#1112, json-schema-org/json-schema-spec#846

@awwright
Copy link
Member

The naming is one of a series of quirks that I don't think can be perfect... we have a few other properties where the name is a bit ambiguous, for example "minimum"... well, minimum what? (scalar value, item count, string length, sort order, ...?)

Sometimes this could have been better thought through: When we were changing how "required" worked, that may have been a good opportunity to name it "requiredProperties" instead. But even in this case it might not matter (it only makes sense to think of properties as being required or not).

However I don't think "present" would have been a better name. The naming scheme for validation keywords follows assertive language. A schema "requires" a property, it doesn't "present" a property.

Likewise, there's other properties that could be confusing: "if minLength: 5 then ..." well hold up, this will evaluate true for all numbers and other non-string types! A better way to explain its behavior might be like this: "If the condition that the property 'foo' is required is valid, then ..." or "If the condition minLength: 5 validates, then ..."

@handrews
Copy link

I'd be extremely hesitant to start renaming keywords, since (as @awwright noted) there are several that could probably be better. Most of the keywords were chosen by people who left the project nearly a decade ago or longer, and most of the newer keywords were chosen to align with those.

We have done a few renames:

  • $ prefixes to clarify the core vocabn (first $id, then the rest) — we took advantage of this to rename definitions to $defs instead of $definitions, but we would not have done that if we weren't already renaming it
  • items and additionalItems got renamed to prefixItems and items, but that was primarily done to eliminate the dual syntax of the old items, and eliminate the confusing duplicate functionality between items and additionalItems. So it wasn't s straight-up rename
  • The Hyper-Schema media keyword got broken up into content* when it was moved over to the validation spec, but there were a lot of reasons why that was relatively low-impact: Hyper-Schema was never as broadly implemented, media was not, as far as anyone could tell, widely used in the first place, and it had previously been content* in earlier drafts. That's very different from renaming one of the most commonly used keywords.

Perhaps at some point if we need to do some sort of large-scale breaking change, a holistic rename could be consider. But more likely, it's better to push for $vocabulary support and if you really can't stand the standard names, you can make a vocabulary that just aliases the keywords you don't like.

@notEthan
Copy link
Author

A schema "requires" a property, it doesn't "present" a property.

I would describe the real meaning behind this to be that a schema requires the presence of a property. It looks like you're using the verb form, 'to present a property' rather than an adjective, which isn't how I mean it (though that potential ambiguity may be a downside of the name).

The naming scheme for validation keywords follows assertive language

I agree with this; I see the assertion being made to be that the property is present. Asserting a condition and requiring that condition seem synonymous to me.

For minimum it requires the number to be at least the given value; for type it requires the value be of the given type. They all assert requirements. The required keyword doesn't say what requirement it is asserting, and it's only intuitive what that assertion is when positively validating. Asserting that the property is present, using the present keyword to say this, seems clearer to me in all contexts, but especially when schemas are conditional or negated.

@karenetheridge
Copy link
Member

karenetheridge commented Jun 16, 2022

I object to the choice of "present" because it's such an overloaded word in english:

  • noun the period of time now occurring
  • adjective in a particular place
  • adjective fully focused on or involved in what one is doing or experiencing
  • verb give, or show, or offer...
  • verb to introduce someone
  • etc etc etc...

If we are in the mood to change the name of this keyword (and personally I am not), I think "mustExist" or "exists" would be clearer than "present".

@notEthan
Copy link
Author

extremely hesitant to start renaming keywords [@handrews ]

I do agree. I also agree with all of the naming changes that have been made so far, the ones you've listed, the dependencies split - I don't think there has been a keyword change that has not been an improvement. I think this is an improvement too, and maybe enough of an improvement to implement, if there is positive feedback on this issue.

It may be the case that other keywords could be named better too. And it is always the case that I can take any idea I like and do my own vocabulary that nobody but me will use. That doesn't help improve the clarity of the core validation's required - I think if this is viewed as an improvement (by more people than myself - looking tenuous so far) it shouldn't be discounted because other keywords could be improved, too.

@notEthan
Copy link
Author

@karenetheridge I do think exists or some flavor of "these properties exist" may be an improvement at least as good as present.

@handrews
Copy link

@notEthan

I do agree. I also agree with all of the naming changes that have been made so far, the ones you've listed, the dependencies split - I don't think there has been a keyword change that has not been an improvement. I think this is an improvement too, and maybe enough of an improvement to implement, if there is positive feedback on this issue.

The critically important difference here is that none of them were renamed just to rename them. The closest would be adding $ to core, but that was to create a general design principle, which happened to involve some renaming. The renaming was the result of a deeper change, not an end in itself.

That is extremely important. We have never renamed a keyword just because we didn't like the name.

No one will ever agree on what names are best. We all have keyword names that we think are unclear or just not ideal.
But changes have their own detractors. @karenetheridge doesn't like present, and I completely agree with her reasoning.

I would strongly prefer not to debate different names because the question that needs to be answered first is "should we be renaming at all?", and I think the answer to that is an emphatic "no."

@Relequestual Relequestual transferred this issue from json-schema-org/json-schema-spec Jun 17, 2022
@json-schema-org json-schema-org locked and limited conversation to collaborators Jun 17, 2022
@Relequestual Relequestual converted this issue into discussion #187 Jun 17, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants