schmittjoh / serializer

Library for (de-)serializing data of any complexity (supports JSON, and XML)
http://jmsyst.com/libs/serializer
MIT License
2.32k stars 589 forks source link

fix ignored null type #1454

Closed simPod closed 1 year ago

simPod commented 1 year ago
Q A
Bug fix? yes
Tests pass? yes/no
License MIT
simPod commented 1 year ago

v3.19 is broken since there's count() called on null value in DocBlockTypeResolver::getPropertyDocblockTypeHint()

goetas commented 1 year ago

@dgafka can you please have a look?

dgafka commented 1 year ago

@simPod can you add test case for this, to show the error?

simPod commented 1 year ago

I'd not add tests to test types but keep it for phpstan https://github.com/schmittjoh/serializer/pull/1446

simPod commented 1 year ago

@dgafka yup, that will be covered by phpstan. Such tests, that can be replaced by SA, are basically a mess that is hard to get rid of then SA is enabled.

smoench commented 1 year ago

I think https://github.com/schmittjoh/serializer/blob/master/tests/Fixtures/ObjectWithPhpDocProperty.php needs to be extended with a property without a docblock and in https://github.com/schmittjoh/serializer/blob/master/tests/Metadata/Driver/DocBlockTypeResolverTest.php following test needs to be added:

public function testGetPropertyDocblockTypeHintDoesNotCrashOnAPropertyWithoutADocBlock(): void
    {
        $resolver = new DocBlockTypeResolver();
        self::assertNull(
            $resolver->getPropertyDocblockTypeHint(
                new ReflectionProperty(ObjectWithPhpDocProperty::class, 'noneDocBlock')
            )
        );
    }
simPod commented 1 year ago

Maybe but that's not relevant to this PR. I just fixed a wrong type in a private method.

dgafka commented 1 year ago

ping @goetas

goetas commented 1 year ago

thank you!

dgafka commented 1 year ago

@goetas can you release? :)

goetas commented 1 year ago

https://github.com/schmittjoh/serializer/releases/tag/3.21.0