feat: populate schema required from its properties#2034
Open
alleknalle wants to merge 1 commit into
Open
Conversation
Adds an AugmentRequired processor: explicit `required: true`/`false` always applies, and nullability inference is opt-in via `augmentRequired.enabled`. Closes zircote#2017
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds an AugmentRequired processor: explicit
required: true/falsealways applies, and nullability inference is opt-in viaaugmentRequired.enabled.Closes #2017
Summary
Today
Schema::required(the array of required property names) has to be maintained separately from the properties it refers to. This adds a newAugmentRequiredprocessor that derives it from the properties themselves, in two layers:Explicit per-property flag (always on). A property can declare
required: trueorrequired: false.trueadds its name to the parentSchema::required,falseremoves it. The boolean is consumed during processing so it never serialises on the property, whererequiredmust be an array.Nullability inference (opt-in). With the
augmentRequired.enabledconfig flag (off by default, same shape ascleanUnusedComponents.enabled), a property backed by a PHP member becomes required when it has a known, non-nullable type:Design notes
required/nullablevalue always takes precedence over inference, and an explicitly declaredrequiredlist on a schema is never modified by inference.!isNullable()rule used for parametersand request bodies, property scanning also hits untyped/mixed/unresolvable properties; those have no determinable nullability, so they are left optional rather than assumed required.API change (non-breaking)
The
Propertyattribute'srequiredparameter widens from?arraytobool|array|null. Arrays andnullbehave exactly as before; only the boolean form is new.Type of Change
Related Issues
Closes #2017
Test Plan
Unit / processor tests —
AugmentRequiredTest(16 cases) plus fixtures cover: the opt-in flag, non-nullable typed properties (incl.$ref), nullable/untyped/mixedleft optional, explicitnullableoverride, promoted-parameter andmethod-backed properties, the reflector gate, explicit
true/falseadding to andremoving from both empty and declared lists, the boolean being consumed (named and unnamed properties), and an arrayrequiredon an object property left untouched. TheInvalidPropertyAttributetest now passes a string to stillassert the constructor's type error.
Integration test — a
ScratchTestfixture (RequiredFromProperties) generates a full spec for a schema with explicitrequired: true/falseproperties referenced from aGETendpoint, and compares the serialized YAML across OpenAPI 3.0/3.1/3.2 and both type resolvers. This verifies end-to-end thatrequired: trueemitsrequired: [id]on the component and that norequired: trueleaks onto a property in the output.Ran locally: full suite (1081 tests), PHPStan, php-cs-fixer, and
rector --dry-runall pass.Checklist