diff --git a/src/docblock/tags.rs b/src/docblock/tags.rs index 9f56b17b..9ad5ff58 100644 --- a/src/docblock/tags.rs +++ b/src/docblock/tags.rs @@ -1297,8 +1297,19 @@ pub fn should_override_type_typed(docblock_type: &PhpType, native_type: &PhpType // Unwrap nullable wrappers for further analysis. `?Foo` → `Foo`, // `Foo|null` → `Foo`. For non-nullable types, use as-is. - let doc_inner = docblock_type.unwrap_nullable(); - let native_inner = native_type.unwrap_nullable(); + // + // `non_null_type()` strips nullability from BOTH representations — the + // `?Foo` (`Nullable`) form and the `Foo|null` (`Union` with a `null` + // member) form. Plain `unwrap_nullable()` only handled the former, so a + // nullable-union native such as `object|null` reached the union branch + // below with its `null` member still attached. Since `object` and `null` + // are both "scalar names", that branch then judged the whole type + // unrefinable and discarded a generic docblock return like + // `@psalm-return ?T`, leaving the bare native (`object|null`). + let doc_owned = docblock_type.non_null_type(); + let doc_inner = doc_owned.as_ref().unwrap_or(docblock_type); + let native_owned = native_type.non_null_type(); + let native_inner = native_owned.as_ref().unwrap_or(native_type); // If the docblock type is a bare, unparameterised primitive scalar // (`int`, `string`, `bool`, etc.), there's no value in overriding. diff --git a/tests/phpstan_nsrt/inherited-generic-return-nullable.php b/tests/phpstan_nsrt/inherited-generic-return-nullable.php new file mode 100644 index 00000000..02004fce --- /dev/null +++ b/tests/phpstan_nsrt/inherited-generic-return-nullable.php @@ -0,0 +1,61 @@ +::find(): ?T` shape: before +// the fix, `should_override_type_typed` analysed `object|null` with its `null` +// member attached, judged it unrefinable (both `object` and `null` are "scalar +// names"), and discarded the generic docblock return — so `$repo->find()` +// resolved to `object|null` instead of `Entity|null`. + +namespace InheritedGenericReturnNullable; + +use function PHPStan\Testing\assertType; + +class Entity {} + +/** + * @template T of object + */ +class EntityRepository +{ + /** + * @return object|null + * @psalm-return ?T + */ + public function find(mixed $id): object|null {} +} + +/** + * @template T of object + * @template-extends EntityRepository + */ +class ServiceEntityRepository extends EntityRepository {} + +/** @extends ServiceEntityRepository */ +class EntityRepo extends ServiceEntityRepository {} + +/** @extends EntityRepository */ +class DirectRepo extends EntityRepository {} + +/** @template V */ +class CollectionNonNull +{ + /** @return V */ + public function get(): object {} +} + +/** @extends CollectionNonNull */ +class EntityCollection extends CollectionNonNull {} + +function t(EntityRepo $multi, DirectRepo $single, EntityCollection $coll): void +{ + // Two-level @extends (Doctrine's exact shape): native object|null + @psalm-return ?T + assertType('Entity|null', $multi->find(1)); + // Single-level @extends: native object|null + @psalm-return ?T + assertType('Entity|null', $single->find(1)); + // Control: non-nullable native object + @return V already worked before the fix + assertType('Entity', $coll->get()); +}