Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHPStan level 6 #487

Closed
wants to merge 2 commits into from
Closed

PHPStan level 6 #487

wants to merge 2 commits into from

Conversation

franmomu
Copy link
Member

@franmomu franmomu commented Apr 23, 2022

Subject

Adds more specific phpdoc types, I've marked as minor because it was more adding missing types, but if you think patch is fine, it can be changed.

There are two commits because I'm not sure about the second one. It's necessary because AuditReader is generic, but I'm not sure if the whole class should be generic or only some of the method 🤔

I am targeting this branch, because these changes are BC.

Changelog

### Fixed
- Specify iterable types

@franmomu franmomu added the minor label Apr 23, 2022
Comment on lines -63 to 66
* @return AuditReader
* @return AuditReader<object>
*/
public function createAuditReader(EntityManager $em)
{
Copy link
Member Author

@franmomu franmomu Apr 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this means that when calling AuditManager::createAuditReader() would be always of type object and calling then for example AuditReader::find(Foo::class) would always return an object of type object instead of a Foo object

Copy link
Member

@VincentLanglet VincentLanglet Apr 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#488 (comment) might be a good way to solve this and still keep the generic

@franmomu
Copy link
Member Author

I've created #488 since raising PHPStan level after this to level 7 shows 131 errors (vs. 23 in #488), example of errors:

------ ------------------------------------------------------
  Line   tests/CoreTest.php
 ------ ------------------------------------------------------
  118    Call to an undefined method object::getId().
  119    Call to an undefined method object::getName().
  126    Call to an undefined method object::getId().
  127    Call to an undefined method object::getName().
  128    Call to an undefined method object::getTailLength().
  135    Call to an undefined method object::getId().
  136    Call to an undefined method object::getName().
  137    Call to an undefined method object::getColor().
  252    Call to an undefined method object::getAuthor().
  274    Call to an undefined method object::getProfile().
 ------ ------------------------------------------------------

 ------ -----------------------------------------------------
  Line   tests/Issue/Issue111Test.php
 ------ -----------------------------------------------------
  47     Call to an undefined method object::getDeletedAt().
 ------ -----------------------------------------------------

 ------ --------------------------------------------------------------
  Line   tests/Issue/Issue196Test.php
 ------ --------------------------------------------------------------
  50     Call to an undefined method object::getSqlConversionField().
 ------ --------------------------------------------------------------

 ------ -------------------------------------------------
  Line   tests/Issue/Issue198Test.php
 ------ -------------------------------------------------
  50     Call to an undefined method object::getOwner().
  53     Call to an undefined method object::getOwner().
 ------ -------------------------------------------------

 ------ -------------------------------------------------------------------------
  Line   tests/Issue/Issue308Test.php
 ------ -------------------------------------------------------------------------
  49     Cannot call method getRev() on SimpleThings\EntityAudit\Revision|false.
  51     Call to an undefined method object::getChildren().
 ------ -------------------------------------------------------------------------

 ------ --------------------------------------------------------
  Line   tests/Issue/Issue87Test.php
 ------ --------------------------------------------------------
  58     Call to an undefined method object::getOrganisation().
  59     Call to an undefined method object::getTitle().
  60     Call to an undefined method object::getSomeProperty().
  63     Call to an undefined method object::getProject().
  69     Call to an undefined method object::getProject().
 ------ --------------------------------------------------------

 ------ ----------------------------------------------------
  Line   tests/Issue/Issue9Test.php
 ------ ----------------------------------------------------
  52     Call to an undefined method object::getCustomer().
 ------ ----------------------------------------------------

 ------ --------------------------------------------------------------
  Line   tests/Issue/IssueConvertToPHPTest.php
 ------ --------------------------------------------------------------
  50     Call to an undefined method object::getSqlConversionField().
 ------ --------------------------------------------------------------

 ------ ------------------------------------------------------------
  Line   tests/RelationTest.php
 ------ ------------------------------------------------------------
  140    Call to an undefined method object::getOwned1().
  141    Call to an undefined method object::getOwned2().
  142    Call to an undefined method object::getOwner().
  183    Call to an undefined method object::getOwned1().
  226    Call to an undefined method object::getTitle().
  ...
 ------ ------------------------------------------------------------

@VincentLanglet
Copy link
Member

Closing in favor of #488

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants