-
-
Notifications
You must be signed in to change notification settings - Fork 880
Description
Description
When a property is not promoted in constructor it should be required unless its nullable.
Additional context
I opened an issue in the past (#2469 ) because the generated schemas did not match my class definitions.
There are a multiple factors but constructor signatures affecting schema definition is one of them.
After some research ive found #2222 .
In my opinion this PR should have never been accepted.
A schema (or model) describes the class. Not its constructor and not how its created in any way.
For example:
class TestEntity {
public string $foo;
public function __construct(string $foo = "bar") {
$this->foo = $foo;
}
}
will produce schema:
{
"properties": {
"foo": {
"type": "string",
"default": "bar"
},
},
"type": "object"
}
Which will not mark foo property required.
So for example in TS it will become type:
type TestEntity = {
foo?: string;
}But this is not a valid representation of the model/entity because there will never be a TestEntity without foo set. Its impossible.
The related code is:
| // Fix for https://github.com/nelmio/NelmioApiDocBundle/issues/2222 |
// Fix for https://github.com/nelmio/NelmioApiDocBundle/issues/2222
// Promoted properties with a value initialized by the constructor are not considered to have a default value
// and are therefore not returned by ReflectionClass::getDefaultProperties(); see https://bugs.php.net/bug.php?id=81386
$reflClassConstructor = $reflection->getDeclaringClass()->getConstructor();
$reflClassConstructorParameters = null !== $reflClassConstructor ? $reflClassConstructor->getParameters() : [];
foreach ($reflClassConstructorParameters as $parameter) {
if ($parameter->name !== $serializedName) {
continue;
}
if (!$parameter->isDefaultValueAvailable()) {
continue;
}
if (null === $this->schema) {
continue;
}
if (!Generator::isDefault($property->default)) {
continue;
}
$default = $parameter->getDefaultValue();
if (Generator::UNDEFINED === $property->nullable && null === $default) {
$property->nullable = true;
}
$property->default = $default;
}I would suggest to add another condition and skip parameter if its not promoted:
if (!$parameter->isPromoted()) {
continue;
}This would be the best of both worlds because it still allows those who want to document their constructor properties as not required by using constructor promotion while you can choose not to use it and mark the properties required.
Whats your opinion on this?
While writing this I realized its not as simple as I have thought when I started.