Skip to content

Improve types in stubs#125

Closed
julienfalque wants to merge 1 commit into
phpstan:masterfrom
julienfalque:stubs
Closed

Improve types in stubs#125
julienfalque wants to merge 1 commit into
phpstan:masterfrom
julienfalque:stubs

Conversation

@julienfalque

Copy link
Copy Markdown
Contributor

No description provided.

@ondrejmirtes

Copy link
Copy Markdown
Member

The added errors is the reason why it's not already the case in the current stubs... I want the untyped version to be mixed, not object...

@julienfalque

julienfalque commented May 8, 2020

Copy link
Copy Markdown
Contributor Author

Why? AFAIK those classes are supposed to deal with nothing but objects.

Anyway the reason I made those changes is because the of object part is present in RepositoryFactory leading to inconsistencies between stubs files. It seems to be the cause of the report of the following error in api-platform/core#3525:

Method ApiPlatform\Core\Tests\Bridge\Doctrine\Orm\EntityManager::getRepository() should return Doctrine\Persistence\ObjectRepository<T> but returns Doctrine\Persistence\ObjectRepository<object>.

Adding of object everywhere fixed the error but maybe removing it from RepositoryFactory will do as well.

@ondrejmirtes

Copy link
Copy Markdown
Member

Why?

Because mixed doesn't cause any downstream errors but you cannot call methods and properties on object on PHPStan level 7 and up. So this change would be annoying in practice.

@julienfalque
julienfalque deleted the stubs branch June 10, 2020 08:49
@julienfalque

Copy link
Copy Markdown
Contributor Author

I understand. Does that mean the of object part should be removed from the RepositoryFactory stub?

@ondrejmirtes

Copy link
Copy Markdown
Member

It's fine there because it's only used as part of ObjectRepository<TEntityClass>. Are you seeing any issues because of this stub?

@julienfalque

Copy link
Copy Markdown
Contributor Author

This reason I submitted these changes is this error: https://github.com/api-platform/core/pull/3525/files#diff-0847156bd64b0de0f4d91da2394ec0e8R114.

I updated the stubs locally to add of object everywhere like I did here and the error was not reported anymore. I'll try to remove of object everywhere instead and see if the error disappears as well.

@ondrejmirtes

Copy link
Copy Markdown
Member

Looks like that error is pretty old. I can't find tests/Bridge/Doctrine/Orm/CollectionDataProviderIntegrationTest.php nor ApiPlatform\\Core\\Tests\\Bridge\\Doctrine\\Orm\\EntityManager class.

@julienfalque

Copy link
Copy Markdown
Contributor Author

For now the class is declared in the test file that uses it: https://github.com/api-platform/core/pull/3525/files#diff-7a879088cbd9fce2bb1551e247ef471eR212.

@julienfalque

Copy link
Copy Markdown
Contributor Author

I confirm removing of object also fixes the error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants