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

Fix pipeline by updating PHPStan #43

Closed
wants to merge 16 commits into from

Conversation

StephenBeirlaen
Copy link
Contributor

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets n/a
Related issues/PRs #42
License MIT
Documentation PR n/a

What's in this PR?

I was interested in adding support for PHP8(.2). In preparation of doing that, phpstan can be updated.

I also noticed in the current pipeline that there were 9 errors on PHP 7.4. I first fixed all but one of these (one which I couldn't fix). Then I performed the update of phpstan and all related packages. After the update, a number of extra errors appeared (56).

I tried to fix the relatively low-hanging fruit, but was still left with 30 remaining errors that were put in the baseline.

Many of the errors put in the baseline are related to phpstan-doctrine not being able to infer the resulting type of the query, which causes Method Sulu\Bundle\CommentBundle\Entity\CommentRepository::findComments() should return array<Sulu\Bundle\CommentBundle\Entity\CommentInterface> but returns mixed.. The main sulu/sulu also has this for Sulu\Bundle\CategoryBundle\Entity\CategoryRepository::findCategories(), as an example.

Another cause is some inconsistencies about fields being nullable or not:

  • Entity/Comment.php::$message is a non-nullable property, but the getter does a ?? check. Not a check that feels safe to just remove.
  • Entity/Thread.php::$title, $changer, $creator, they are not nullable in code, while they are in the Doctrine mapping.

I would suggest that these can be fixed later?

Why?

To add support for PHP 8+

Stephen Beirlaen added 15 commits December 12, 2022 14:03
Return typehint of method Sulu\Bundle\CommentBundle\Entity\CommentRepository::createNew() has invalid type Sulu\Component\Persistence\Repository\T
Parameter #1 $url of class Symfony\Component\HttpFoundation\RedirectResponse constructor expects string, float|int|string|true given.
Parameter #1 $message of method Sulu\Bundle\CommentBundle\Entity\CommentInterface::setMessage() expects string, bool|float|int|string|null given.
Parameter #1 $title of method Sulu\Bundle\CommentBundle\Entity\ThreadInterface::setTitle() expects string, bool|float|int|string|null given.
Parameter #2 $str of function explode expects string, float|int|string|true given.
Parameter excludes_analyse has been deprecated so use excludePaths only from now on
Ignored error #Symfony\\Contracts\\EventDispatcher\\EventDispatcherInterface::dispatch()# has an unescaped '()' which leads to ignoring all errors. Use '\(\)' instead.
Class Sulu\Bundle\CommentBundle\Entity\ThreadRepository extends generic class Doctrine\ORM\EntityRepository but does not specify its types: TEntityClass
Interface Sulu\Bundle\CommentBundle\Entity\CommentRepositoryInterface extends generic interface Sulu\Component\Persistence\Repository\RepositoryInterface but does not specify its types: T
PHPDoc tag @var for variable $commentFormToolbarActions has no value type specified in iterable type array.
@StephenBeirlaen StephenBeirlaen changed the title Bugfix/phpstan errors Fix pipeline by updating PHPStan Dec 12, 2022
PHPUnit\Framework\Exception: This test uses TestCase::prophesize(), but phpspec/prophecy is not installed. Please run "composer require --dev phpspec/prophecy".
@StephenBeirlaen StephenBeirlaen marked this pull request as ready for review December 12, 2022 17:50
@alexander-schranz
Copy link
Member

Thx for your PR, we did lot of changes and tackled also the different phpdocs and so we not longer have any PHPStan issues open. If you still stumble over something let us know.

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