Skip to content

Conversation

VincentLanglet
Copy link
Contributor

This includes the PR #6999 which should be reviewed and merged first.

This show how I'm able to solve every phpstan issue after the merge @javiereguiluz

*/
public function setDefaultSort(array $sortFieldsAndOrder): self
{
$sortFieldsAndOrder = array_map('strtoupper', $sortFieldsAndOrder);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is small phpstan issue with array_map.

PHPStan is loosing some informations about sortFieldsAndOrder ; I fixed it on PHPStan side. But anyway, I feel like it's better to include the strtoupper in the foreach in order to have one loop instead of two.

*/
public function getFieldsInTab(string $tabUniqueId): array
{
/** @phpstan-ignore-next-line return.type */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Class is deprecated, let's avoid this issue.

}

if (\is_callable($label) && $label instanceof \Closure) {
if (!\is_string($label) && !$label instanceof TranslatableInterface && \is_callable($label)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A bug existed for array-callable ; I added a test.

}

if ($theFirstFieldWhichIsATabOrColumn->isFormColumn() && $fieldDto->isFormTab()) {
throw new \InvalidArgumentException(sprintf('When using form columns, you can\'t define tabs inside columns (but you can define columns inside tabs). Move the tab "%s" outside any column.', $fieldDto->getLabel()));
Copy link
Contributor Author

@VincentLanglet VincentLanglet Jul 5, 2025

Choose a reason for hiding this comment

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

Here and in multiple place, getLabel is considered as able to be casted to string, but this is not true.

TranslatableInterface does not extends Stringable. Only TranslatableMessage does.
And it's not possible to implement it for enum for instance.
But if someone use his own implementation, it will crash.

Not sure if this fix is enough, or you want to improve/extract logic somewhere else (like a TranslatableHelper::toString method) @javiereguiluz

$isTabActive = 0 === \count($tabs);
$tabId = sprintf('tab-%s', $fieldDto->getLabel() ? $slugger->slug(strip_tags($fieldDto->getLabel()))->lower()->toString() : ++$tabsWithoutLabelCounter);
$label = $fieldDto->getLabel();
$tabId = sprintf(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same issue with TranslatableInterface

if (null === $entityFqcn && null === $crudControllerFqcn) {
throw new \RuntimeException(sprintf('The CRUD menu item with label "%s" must define either the entity FQCN (using the third constructor argument) or the CRUD Controller FQCN (using the "setController()" method).', $menuItemDto->getLabel()));
$label = $menuItemDto->getLabel();
$labelAsString = (\is_string($label) || $label instanceof Stringable) ? (string) $label : '';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same issue with TranslatableInterface

$currentFormTab = (string) $fieldDto->getLabel();

$label = $fieldDto->getLabel();
$currentFormTab = (\is_string($label) || $label instanceof Stringable) ? (string) $label : '';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same issue with TranslatableInterface


$index = 1;
$pathInfo = pathinfo($filename);
while (file_exists($filename = sprintf('%s/%s_%d.%s', $pathInfo['dirname'], $pathInfo['filename'], $index, $pathInfo['extension']))) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

dirname and extension might theorically not exists.

I handle the case.

include $filePath;

return ob_get_clean();
return (string) ob_get_clean();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can technically be false, but not in our case, let's just ignore this by casting it


if (null !== $idClassType) {
/** @var \ReflectionNamedType|\ReflectionUnionType $idClassType */
if ($idClassType instanceof \ReflectionNamedType) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ReflectionUnionType::getName does not exists

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.

1 participant