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

[Chore] Fix phpstan issues #5786

Closed
wants to merge 4 commits into from
Closed

Conversation

asispts
Copy link
Contributor

@asispts asispts commented Jun 6, 2023

This PR will fix several PHPStan issues:

These issues are a little bit tricky. Both TranslatableMessageBuilder::withParameters and TranslatableChoiceMessage::__toString are not compatible with TranslatableInterface. Those functions should depend on TranslatableMessage.

 ------ -----------------------------------------------------------------------
 Line   Field/Configurator/ChoiceConfigurator.php
 ------ -----------------------------------------------------------------------
  124    Parameter #1 $message of class
         EasyCorp\Bundle\EasyAdminBundle\Translation\TranslatableChoiceMessage
         constructor expects
         Symfony\Component\Translation\TranslatableMessage,
         Symfony\Contracts\Translation\TranslatableInterface given.
 ------ -----------------------------------------------------------------------

 ------ -----------------------------------------------------------------------
 Line   Translation/TranslatableMessageBuilder.php
 ------ -----------------------------------------------------------------------
  28     Call to an undefined method
         Symfony\Contracts\Translation\TranslatableInterface::getMessage().
  29     Call to an undefined method
         Symfony\Contracts\Translation\TranslatableInterface::getParameters().
  30     Call to an undefined method
         Symfony\Contracts\Translation\TranslatableInterface::getDomain().
 ------ -----------------------------------------------------------------------

tests/Twig/TwigFilterTest.php Show resolved Hide resolved
tests/Twig/TwigFilterTest.php Show resolved Hide resolved
@asispts
Copy link
Contributor Author

asispts commented Jun 6, 2023

Both failed tests and linter issues are unrelated.

@javiereguiluz
Copy link
Collaborator

It's merged now! Thanks!

javiereguiluz added a commit that referenced this pull request Jun 22, 2023
This PR was squashed before being merged into the 4.x branch.

Discussion
----------

[Chore] Fix phpstan issues

This PR will fix several `PHPStan` issues:
  - [x] Fix `Cannot unset offset string on array<int, EasyCorp\Bundle\EasyAdminBundle\Dto\ActionDto>.`
  - [x] Fix `Variable method call on Twig\Extension\RuntimeExtensionInterface.`
      - [x] Refactor `EasyAdminTwigExtension::applyFilterIfExists`
      - [x] Add integration tests for `applyFilterIfExists`. Issues #3762 and #4808 have been taken into consideration.
  - [x] Update parameter type declaration of `Action::new`. We can't use ``@var`` to annotate a parameter. See: https://phpstan.org/r/7695e922-db7f-42b3-ac4f-8ff65a8a37b9
  - [x] Modify incorrect PHPDoc. See: https://phpstan.org/r/df4eb1c2-7286-4aa7-b276-f36ff2c0c82c

These issues are a little bit tricky. Both `TranslatableMessageBuilder::withParameters` and `TranslatableChoiceMessage::__toString` are not compatible with `TranslatableInterface`. Those functions should depend on `TranslatableMessage`.
```txt
 ------ -----------------------------------------------------------------------
 Line   Field/Configurator/ChoiceConfigurator.php
 ------ -----------------------------------------------------------------------
  124    Parameter #1 $message of class
         EasyCorp\Bundle\EasyAdminBundle\Translation\TranslatableChoiceMessage
         constructor expects
         Symfony\Component\Translation\TranslatableMessage,
         Symfony\Contracts\Translation\TranslatableInterface given.
 ------ -----------------------------------------------------------------------

 ------ -----------------------------------------------------------------------
 Line   Translation/TranslatableMessageBuilder.php
 ------ -----------------------------------------------------------------------
  28     Call to an undefined method
         Symfony\Contracts\Translation\TranslatableInterface::getMessage().
  29     Call to an undefined method
         Symfony\Contracts\Translation\TranslatableInterface::getParameters().
  30     Call to an undefined method
         Symfony\Contracts\Translation\TranslatableInterface::getDomain().
 ------ -----------------------------------------------------------------------
```

Commits
-------

053f62c [Chore] Fix phpstan issues
@javiereguiluz
Copy link
Collaborator

I'm closing manually because the tool I use to merge PRs didn't close it. But, this PR is merged (see 2c52fb8)

@asispts asispts deleted the chore/fix-phpstan branch June 22, 2023 18:57
javiereguiluz added a commit that referenced this pull request Jun 23, 2023
This PR was squashed before being merged into the 4.x branch.

Discussion
----------

Throw RuntimeError when unable to load twig filter

Throw a `RuntimeError` as mentioned in #5786 (comment)

Also, simplify function `applyFilterIfExists` and modify test cases.

Commits
-------

5eb41cb Throw RuntimeError when unable to load twig filter
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.

None yet

2 participants