search

Prezent / doctrine-translatable

Translatable behaviour extension for Doctrine2
MIT License
26 stars 17 forks source link

Nullable return types: don't omit return statements #35

Closed barthy-koeln closed 4 years ago

barthy-koeln commented 4 years ago

Dear Prezent Team,

I just stumbled across a bug after upgrading the prezent/doctrine-translatable-bundle to v1.0.7, which updated this bundle to v2.0.0.

Error message:

Return value of Prezent\Doctrine\Translatable\Mapping\Driver\AnnotationDriver::loadMetadataForClass() must be an instance of Metadata\ClassMetadata or null, none returned

My research has led me to this specific change in AnnotationDriver.php on line 47 of PR #33:

+ public function loadMetadataForClass(\ReflectionClass $class): ?ClassMetadata

Since the method now has a nullable, but typed return value, it cannot omit the return statement any more, despite the following statement:

If the return is omitted the value NULL will be returned.

(source: php.net/manual)

The 'Nullable Types' segment in ''PHP 7.1 New Features' has the following comment:

Note that declaring nullable return type does not mean that you can skip return statement at all.

(source: php.net/manual)

Now if none of the two conditions in the aforementioned method is true, the function will terminate without returning the required value of null or ClassMetadata.

I suggest adding return null; at the end of the method. I can submit a pull request if desired.

sandermarechal commented 4 years ago

Thank you. Fixed in 2.0.1.