Support minProperties and maxProperties on objects #53
Conversation
|
|
| HHVM_VERSION=4.102.2 | ||
| HHVM_NEXT_VERSION=4.102.2 |
There was a problem hiding this comment.
i noticed @sarahhenkens also updated HHVM_VERSION in her PR #52, should we also update HHVM_NEXT_VERSION? i wasn't 100% sure if they should always match
There was a problem hiding this comment.
hmm I think we should probably update this to 4.121?
| $max_properties = $this->typed_schema['maxProperties'] ?? null; | ||
| $min_properties = $this->typed_schema['minProperties'] ?? null; |
There was a problem hiding this comment.
someone asked about whether we should validate if these integers are non-negative. i noticed we don't validate that for StringMinLengthConstraint or ArrayMinItemsConstraint - is there a reason/context for that?
if we were to add that check for objects, i'm trying to figure out where the best place for it would be so that it'd error out for the developer trying to generate a schema with negative min/max numbers. doesn't seem like the getCheckMethod is the right place.
There was a problem hiding this comment.
It wasn't intentional that we left those off, just didn't get around to it. I'd add it anywhere in this codegen path, maybe above where you set the class properties?
There was a problem hiding this comment.
okay, sounds good - i added checks for those above in the build method & tested that it throws an error when trying to generate a file that has invalid min/max values.
| $max_properties = $this->typed_schema['maxProperties'] ?? null; | ||
| $min_properties = $this->typed_schema['minProperties'] ?? null; |
There was a problem hiding this comment.
It wasn't intentional that we left those off, just didn't get around to it. I'd add it anywhere in this codegen path, maybe above where you set the class properties?
Description
Adds support for
minPropertiesandmaxPropertiesfor objects. Partly completes: #5Leveraged a lot of the existing patterns we have for
minItemsandmaxItemsin arrays. So we created two new classes in this PR:ObjectMaxPropertiesConstraintandObjectMinPropertiesConstraintthat are used in theObjectBuilderclass.Testing
Updated unit tests and also manually tested