TypeError when serializing a flattened nullable property

[oprypkhantc]

Detailed description

When using the #[Field(flatten: true)] on a property of type that has a nullable property, it fails with with a TypeError.

Here's a tiny reproduction:

#[ClassSettings(requireValues: true)]
final class BaseSubject
{
    public function __construct(
        public readonly ?string $firstName = null,
    ) {}
}

#[ClassSettings(requireValues: true)]
final class Subject
{
    public function __construct(
        #[Field(flatten: true)]
        public readonly ?BaseSubject $baseSubject = null,
    ) {}
}

// TypeError: Crell\Serde\Formatter\JsonFormatter::serializeString(): Argument #3 ($next) must be of type string, null given, called in /opt/project/vendor/crell/serde/src/PropertyHandler/ScalarExporter.php on line 20
(new SerdeCommon())->serialize(new Subject, 'json');

Context

It does not happen when serializing the nested class directly:

// no errors here
(new SerdeCommon())->serialize(new BaseSubject, 'json');

Possible implementation

Seems like it's caused by a missing check in ObjectExporter. Regular ObjectExporter::flattenValue checks whether given value is null, and if so - doesn't add a new CollectionItem so that field is never serialized. In the case of a flatten value, however, ObjectExporter::reduceObjectProperty() is used instead for some reason, which doesn't have a null check.

If I remove ObjectExporter::reduceObjectProperty() and replace it with direct flattenValue() calls, it works as expected, but I'm not sure why it was implemented that way in the first place.

Your environment

• Version used (e.g. PHP 5.6, HHVM 3): PHP 8.1.20, Serde 0.6.0

[Crell]

Interesting. I get a totally different error when I setup a test case for it. :smile: Possibly because I'm using master and not 0.6.0, and there have been some changes to null handling since. Can you try with the latest master and see what happens?

[oprypkhantc]

Yep. With dev-master it works, awesome :) Moreover, all keys are now included in the serialized JSON, which definitely makes more sense to me. Thank you.

You mentioned you're getting a different error now; feel free to close this issue if it's unrelated.

[Crell]

More null edge cases handled in #30. I'll merge that shortly, which will close this issue. Thanks for the report.