Skip to content

Commit

Permalink
bug #5786 [Chore] Fix phpstan issues (asispts)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
javiereguiluz committed Jun 22, 2023
2 parents efd99dc + 053f62c commit 2c52fb8
Show file tree
Hide file tree
Showing 10 changed files with 168 additions and 29 deletions.
17 changes: 10 additions & 7 deletions src/Config/Action.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ public function __toString()
}

/**
* @param mixed $label Use FALSE to hide the label; use NULL to autogenerate it
* @param string|null $icon The full CSS classes of the FontAwesome icon to render (see https://fontawesome.com/v6/search?m=free)
* @param TranslatableInterface|string|false|null $label Use FALSE to hide the label; use NULL to autogenerate it
* @param string|null $icon The full CSS classes of the FontAwesome icon to render (see https://fontawesome.com/v6/search?m=free)
*/
public static function new(string $name, /** @var TranslatableInterface|string|false|null */ $label = null, ?string $icon = null): self
public static function new(string $name, $label = null, ?string $icon = null): self
{
if (!\is_string($label)
&& !$label instanceof TranslatableInterface
Expand All @@ -57,7 +57,7 @@ public static function new(string $name, /** @var TranslatableInterface|string|f
'Argument "%s" for "%s" must be one of these types: %s. Passing type "%s" will cause an error in 5.0.0.',
'$label',
__METHOD__,
'"string", "false" or "null"',
sprintf('"%s", "string", "false" or "null"', TranslatableInterface::class),
\gettype($label)
);
}
Expand Down Expand Up @@ -89,9 +89,9 @@ public function createAsBatchAction(): self
}

/**
* @param mixed $label Use FALSE to hide the label; use NULL to autogenerate it
* @param TranslatableInterface|string|false|null $label Use FALSE to hide the label; use NULL to autogenerate it
*/
public function setLabel(/* @var TranslatableInterface|string|false|null */ $label): self
public function setLabel($label): self
{
if (!\is_string($label)
&& !$label instanceof TranslatableInterface
Expand Down Expand Up @@ -193,7 +193,10 @@ public function linkToRoute(string $routeName, array|callable $routeParameters =
return $this;
}

public function linkToUrl(/* @var string|callable */ $url): self
/**
* @param string|callable $url
*/
public function linkToUrl($url): self
{
if (!\is_string($url) && !\is_callable($url)) {
trigger_deprecation(
Expand Down
6 changes: 3 additions & 3 deletions src/Config/Crud.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public static function new(): self
*
* @psalm-param mixed $label
*/
public function setEntityLabelInSingular(/* @var TranslatableInterface|string|callable */ $label): self
public function setEntityLabelInSingular($label): self
{
if (!\is_string($label) && !$label instanceof TranslatableInterface && !\is_callable($label)) {
trigger_deprecation(
Expand All @@ -71,7 +71,7 @@ public function setEntityLabelInSingular(/* @var TranslatableInterface|string|ca
*
* @psalm-param mixed $label
*/
public function setEntityLabelInPlural(/* @var TranslatableInterface|string|callable */ $label): self
public function setEntityLabelInPlural($label): self
{
if (!\is_string($label) && !$label instanceof TranslatableInterface && !\is_callable($label)) {
trigger_deprecation(
Expand All @@ -95,7 +95,7 @@ public function setEntityLabelInPlural(/* @var TranslatableInterface|string|call
*
* @psalm-param mixed $title
*/
public function setPageTitle(string $pageName, /* @var TranslatableInterface|string|callable */ $title): self
public function setPageTitle(string $pageName, $title): self
{
if (!\is_string($title) && !$title instanceof TranslatableInterface && !\is_callable($title)) {
trigger_deprecation(
Expand Down
4 changes: 3 additions & 1 deletion src/Dto/ActionConfigDto.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
final class ActionConfigDto
{
private ?string $pageName = null;
/** @var array<string, array<int, ActionDto>> */
/**
* @var array<string,array<string,ActionDto>>
*/
private array $actions = [
Crud::PAGE_DETAIL => [],
Crud::PAGE_EDIT => [],
Expand Down
12 changes: 9 additions & 3 deletions src/Dto/ActionDto.php
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,10 @@ public function getRouteParameters()/* : array|callable */
return $this->routeParameters;
}

public function setRouteParameters(/* @var array|callable */ $routeParameters): void
/**
* @param array|callable $routeParameters
*/
public function setRouteParameters($routeParameters): void
{
if (!\is_array($routeParameters) && !\is_callable($routeParameters)) {
trigger_deprecation(
Expand All @@ -190,12 +193,15 @@ public function setRouteParameters(/* @var array|callable */ $routeParameters):
/**
* @return string|callable|null
*/
public function getUrl()/* : string|callable */
public function getUrl()
{
return $this->url;
}

public function setUrl(/* @var string|callable */ $url): void
/**
* @param string|callable $url
*/
public function setUrl($url): void
{
if (!\is_string($url) && !\is_callable($url)) {
trigger_deprecation(
Expand Down
5 changes: 4 additions & 1 deletion src/Dto/CrudDto.php
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,10 @@ public function getCustomPageTitle(?string $pageName = null, $entityInstance = n
return t($title, $translationParameters);
}

public function setCustomPageTitle(string $pageName, /* @var TranslatableInterface|string|callable */ $pageTitle): void
/**
* @param TranslatableInterface|string|callable $pageTitle
*/
public function setCustomPageTitle(string $pageName, $pageTitle): void
{
if (!\is_string($pageTitle) && !$pageTitle instanceof TranslatableInterface && !\is_callable($pageTitle)) {
trigger_deprecation(
Expand Down
7 changes: 5 additions & 2 deletions src/Dto/FieldDto.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,15 @@ public function setFormatValueCallable(?callable $callable): void
/**
* @return TranslatableInterface|string|false|null
*/
public function getLabel()/* : TranslatableInterface|string|false|null */
public function getLabel()
{
return $this->label;
}

public function setLabel(/* @var TranslatableInterface|string|false|null */ $label): void
/**
* @param TranslatableInterface|string|false|null $label
*/
public function setLabel($label): void
{
if (!\is_string($label) && !$label instanceof TranslatableInterface && false !== $label && null !== $label) {
trigger_deprecation(
Expand Down
5 changes: 4 additions & 1 deletion src/Dto/FilterConfigDto.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ public function __construct()
$this->filters = KeyValueStore::new();
}

public function addFilter(/* @var FilterInterface|string */ $filterNameOrConfig): void
/**
* @param FilterInterface|string $filterNameOrConfig
*/
public function addFilter($filterNameOrConfig): void
{
if (!\is_string($filterNameOrConfig) && !$filterNameOrConfig instanceof FilterInterface) {
trigger_deprecation(
Expand Down
7 changes: 5 additions & 2 deletions src/Dto/FilterDto.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,15 @@ public function setProperty(string $propertyName): void
/**
* @return TranslatableInterface|string|false|null
*/
public function getLabel()/* : TranslatableInterface|string|false|null */
public function getLabel()
{
return $this->label;
}

public function setLabel(/* @var TranslatableInterface|string|false|null */ $label): void
/**
* @param TranslatableInterface|string|false|null $label
*/
public function setLabel($label): void
{
if (!\is_string($label) && !$label instanceof TranslatableInterface && false !== $label && null !== $label) {
trigger_deprecation(
Expand Down
25 changes: 16 additions & 9 deletions src/Twig/EasyAdminTwigExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@
use Symfony\Component\DependencyInjection\ServiceLocator;
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
use Twig\Environment;
use Twig\Error\RuntimeError;
use Twig\Extension\AbstractExtension;
use Twig\Extension\ExtensionInterface;
use Twig\Extension\GlobalsInterface;
use Twig\Extension\RuntimeExtensionInterface;
use Twig\TwigFilter;
use Twig\TwigFunction;

Expand Down Expand Up @@ -94,21 +93,29 @@ public function fileSize(int $bytes): string
return (int) ($bytes / (1024 ** $factor)).@$size[$factor];
}

// Code adapted from https://stackoverflow.com/a/48606773/2804294 (License: CC BY-SA 3.0)
/**
* Code adapted from https://stackoverflow.com/a/48606773/2804294 (License: CC BY-SA 3.0).
*
* @throws RuntimeError When twig runtime can't find `$class`
*/
public function applyFilterIfExists(Environment $environment, $value, string $filterName, ...$filterArguments)
{
if (null === $filter = $environment->getFilter($filterName)) {
return $value;
}

[$class, $method] = $filter->getCallable();
if ($class instanceof ExtensionInterface) {
return $filter->getCallable()($value, ...$filterArguments);
$callback = $filter->getCallable();
if (\is_callable($callback)) {
return \call_user_func($callback, $value, ...$filterArguments);
}

if (!\is_array($callback)) {
return null;
}

$object = $environment->getRuntime($class);
if ($object instanceof RuntimeExtensionInterface && method_exists($object, $method)) {
return $object->$method($value, ...$filterArguments);
$callback[0] = $environment->getRuntime($callback[0]);
if (\is_callable($callback)) {
return \call_user_func($callback, $value, ...$filterArguments);
}

return null;
Expand Down
109 changes: 109 additions & 0 deletions tests/Twig/TwigFilterTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
<?php

namespace EasyCorp\Bundle\EasyAdminBundle\Tests\Twig;

use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
use Twig\Environment;
use Twig\Error\RuntimeError;
use Twig\RuntimeLoader\FactoryRuntimeLoader;
use Twig\TwigFilter;

final class TwigFilterTest extends KernelTestCase
{
private Environment $twig;

protected function setUp(): void
{
self::bootKernel();

$this->twig = $this->getContainer()->get(Environment::class);
}

public function myFilter($number, int $decimals, string $decPoint, string $thousandsSep)
{
return number_format($number, $decimals, $decPoint, $thousandsSep);
}

public function testNonLazyLoadedFilter(): void
{
$this->twig->addFilter(new TwigFilter('my_filter', [$this, 'myFilter']));

$view = "{{number | ea_apply_filter_if_exists('my_filter', 2, ',', '.')}}";
$context = ['number' => 123_456.789];
$result = $this->twig->createTemplate($view)->render($context);
$this->assertSame('123.456,79', $result);
}

public function testLazyLoadedFilter(): void
{
$loader = new FactoryRuntimeLoader([
'EATests\MyLazyFilterRuntime' => fn () => new class() {
public function myFilter($number, int $decimals, string $decPoint, string $thousandsSep)
{
return number_format($number, $decimals, $decPoint, $thousandsSep);
}
},
]);

$this->twig->addRuntimeLoader($loader);
$this->twig->addFilter(new TwigFilter('my_filter', ['EATests\MyLazyFilterRuntime', 'myFilter']));

$view = "{{number | ea_apply_filter_if_exists('my_filter', 2, ',', '.')}}";
$context = ['number' => 123_456.789];
$result = $this->twig->createTemplate($view)->render($context);
$this->assertSame('123.456,79', $result);
}

public function testBuiltinFilter(): void
{
$view = "{{number | ea_apply_filter_if_exists('abs')}}";
$context = ['number' => -10];
$result = $this->twig->createTemplate($view)->render($context);
$this->assertSame('10', $result);
}

public function testNotFoundFilter(): void
{
$view = "{{number | ea_apply_filter_if_exists('imagine_filter')}}";
$context = ['number' => 3.14];
$result = $this->twig->createTemplate($view)->render($context);
$this->assertSame('3.14', $result);
}

public function testNotFoundClass(): void
{
$this->expectException(RuntimeError::class);
$this->expectExceptionMessage('a');

$this->twig->addFilter(new TwigFilter('my_filter', ['EATests\NotFoundClass', 'myFilter']));

$view = "{{number | ea_apply_filter_if_exists('my_filter')}}";
$context = ['number' => 123_456.789];
$this->twig->createTemplate($view)->render($context);
}

public function testInvalidCallableString(): void
{
$this->twig->addFilter(new TwigFilter('my_filter', 'not-callable'));

$view = "{{number | ea_apply_filter_if_exists('my_filter')}}";
$context = ['number' => 123_456.789];
$result = $this->twig->createTemplate($view)->render($context);
$this->assertSame('', $result);
}

public function testInvalidCallableArray(): void
{
$loader = new FactoryRuntimeLoader([
'EATests\MyLazyFilterRuntime' => fn () => new class() {},
]);

$this->twig->addRuntimeLoader($loader);
$this->twig->addFilter(new TwigFilter('my_filter', ['EATests\MyLazyFilterRuntime', 'unknownMethod']));

$view = "{{number | ea_apply_filter_if_exists('my_filter')}}";
$context = ['number' => 123_456.789];
$result = $this->twig->createTemplate($view)->render($context);
$this->assertSame('', $result);
}
}

0 comments on commit 2c52fb8

Please sign in to comment.