Skip to content

feat: add tests folder with phpstan level 8 solved all errors #417

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.

Sign up for GitHub

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

Open
shakaran wants to merge 11 commits into
base: master
Choose a base branch
from
Open

feat: add tests folder with phpstan level 8 solved all errors #417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
73826fc
feat: solve phpstan error with null type declared
shakaran Dec 8, 2025
22f988d
feat: resolve phpstan errors in tests
shakaran Dec 9, 2025
65678ec
feat: update phpstan type for tests and errors
shakaran Dec 9, 2025
6e591d5
feat: resolve all errors in phpstan tests
shakaran Dec 9, 2025
f1db5be
fix: avoid assert not null changing MenuTestCase and ItemInterface
shakaran Dec 9, 2025
3f6fef6
fix: use IteratorIterator as suggested by @stof
shakaran Dec 9, 2025
2d5281f
fix: refactor tests for back to use ArrayAccess
shakaran Dec 9, 2025
b70d44a
fix: remove intermediate variables not needed anymore
shakaran Dec 9, 2025
584ec1e
fix: restore cases of ArrayAccess instead getChildren
shakaran Dec 9, 2025
2ccbffb
fix: remove intermediate variables and avoid getChildren()
shakaran Dec 9, 2025
cf50474
feat: add return types only nullables
shakaran Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions phpstan.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ parameters:
treatPhpDocTypesAsCertain: false
paths:
- src
- tests
includes:
- phpstan-baseline.neon
2 changes: 1 addition & 1 deletion src/Knp/Menu/Util/MenuManipulator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ public function toArray(ItemInterface $item, ?int $depth = null): array
*
* @param string|ItemInterface|array<int|string, mixed>|\Traversable<mixed>|null $subItem A string or array to append onto the end of the array
*
* @phpstan-param string|ItemInterface|array<int|string, string|int|float|null|array{label: string, uri: string|null, item: ItemInterface|null}|ItemInterface>|\Traversable<string|int|float|null|array{label: string, uri: string|null, item: ItemInterface|null}|ItemInterface>|null $subItem
* @phpstan-param string|ItemInterface|array<int|string, string|int|float|null|array{label: string, uri: string|null, item: ItemInterface|null}|ItemInterface|object>|\Traversable<string|int|float|null|array{label: string, uri: string|null, item: ItemInterface|null}|ItemInterface|object>|null $subItem
Copy link
Collaborator

@stof stof Dec 9, 2025

Choose a reason for hiding this comment

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

This must be reverted. the place where phpstan is reported an error is inside testBreadcrumbsArrayInvalidData, which is specifically testing the runtime behavior when passing invalid data (as not all users of the library might be using static analysis to catch such usage, and the code has some defense against that in that case).

the proper way to fix the phpstan warning is to use @phpstan-expect-error to make phpstan aware that an error is expected in that line.

*
* @return array<int, array<string, mixed>>
* @phpstan-return list<array{label: string, uri: string|null, item: ItemInterface|null}>
Expand Down
24 changes: 16 additions & 8 deletions tests/Knp/Menu/Tests/Iterator/CurrentItemFilterIteratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,17 @@ final class CurrentItemFilterIteratorTest extends MenuTestCase
{
public function testSimpleFiltering(): void
{
$this->pt1->setCurrent(true);
$this->ch2->setCurrent(true);
$this->gc1->setCurrent(true);
$pt1 = $this->pt1;
$ch2 = $this->ch2;
$gc1 = $this->gc1;
Copy link
Collaborator

@stof stof Dec 9, 2025

Choose a reason for hiding this comment

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

Those changes should be reverted

$menu = $this->menu;
$pt1->setCurrent(true);
$ch2->setCurrent(true);
$gc1->setCurrent(true);

$names = [];
// FilterIterator expects an Iterator implementation explicitly, not an IteratorAggregate.
$iterator = new CurrentItemFilterIterator($this->menu->getIterator(), new Matcher());
$iterator = new CurrentItemFilterIterator(new \IteratorIterator($this->menu), new Matcher());

foreach ($iterator as $value) {
$names[] = $value->getName();
Expand All @@ -28,13 +32,17 @@ public function testSimpleFiltering(): void

public function testFiltering(): void
{
$this->pt1->setCurrent(true);
$this->ch2->setCurrent(true);
$this->gc1->setCurrent(true);
$pt1 = $this->pt1;
$ch2 = $this->ch2;
$gc1 = $this->gc1;
Copy link
Collaborator

@stof stof Dec 9, 2025

Choose a reason for hiding this comment

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

Those changes should be reverted

$menu = $this->menu;
$pt1->setCurrent(true);
$ch2->setCurrent(true);
$gc1->setCurrent(true);

$names = [];
$iterator = new CurrentItemFilterIterator(
new \RecursiveIteratorIterator(new RecursiveItemIterator($this->menu), \RecursiveIteratorIterator::SELF_FIRST),
new \RecursiveIteratorIterator(new RecursiveItemIterator($menu), \RecursiveIteratorIterator::SELF_FIRST),
new Matcher()
);

Expand Down
12 changes: 8 additions & 4 deletions tests/Knp/Menu/Tests/Iterator/DisplayedItemFilterIteratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,17 @@ final class DisplayedItemFilterIteratorTest extends MenuTestCase
{
public function testFiltering(): void
{
$this->ch1->setDisplay(false);
$this->ch2->setDisplay(false);
$this->ch4->setDisplayChildren(false);
$ch1 = $this->ch1;
$ch2 = $this->ch2;
$ch4 = $this->ch4;
Copy link
Collaborator

@stof stof Dec 9, 2025

Choose a reason for hiding this comment

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

Those changes should be reverted

$menu = $this->menu;
$ch1->setDisplay(false);
$ch2->setDisplay(false);
$ch4->setDisplayChildren(false);

$names = [];
$iterator = new \RecursiveIteratorIterator(
new DisplayedItemFilterIterator(new RecursiveItemIterator($this->menu)),
new DisplayedItemFilterIterator(new RecursiveItemIterator(new \ArrayIterator($menu->getChildren()))),
\RecursiveIteratorIterator::SELF_FIRST
);
foreach ($iterator as $value) {
Expand Down
8 changes: 5 additions & 3 deletions tests/Knp/Menu/Tests/Iterator/IteratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@ public function testRecursiveIterator(): void
$child->expects($this->any())
->method('getIterator')
->willReturn(new \EmptyIterator());
$this->menu->addChild($child);
$menu = $this->menu;
$menu->addChild($child);

$names = [];
foreach (new \RecursiveIteratorIterator(new RecursiveItemIterator($this->menu), \RecursiveIteratorIterator::SELF_FIRST) as $value) {
foreach (new \RecursiveIteratorIterator(new RecursiveItemIterator($menu), \RecursiveIteratorIterator::SELF_FIRST) as $value) {
$names[] = $value->getName();
}

Expand All @@ -50,8 +51,9 @@ public function testRecursiveIteratorLeavesOnly(): void

public function testFullTreeIterator(): void
{
$menu = $this->menu;
$fullTreeIterator = new \RecursiveIteratorIterator(
new RecursiveItemIterator(new \ArrayIterator([$this->menu])), // recursive iterator containing the root item
new RecursiveItemIterator(new \ArrayIterator([$menu])), // recursive iterator containing the root item
\RecursiveIteratorIterator::SELF_FIRST
);

Expand Down
8 changes: 4 additions & 4 deletions tests/Knp/Menu/Tests/Matcher/Voter/CallbackVoterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ public function testMatching(callable $callback, ?bool $expected): void
}

/**
* @return iterable<string, array{callable(): ?bool, ?bool}>
* @return iterable<string, array{callable, bool|null}>
*/
public static function provideData(): iterable
{
yield 'matching' => [fn (): ?bool => true, true];
yield 'not matching' => [fn (): ?bool => false, false];
yield 'skipping' => [fn (): ?bool => null, null];
yield 'matching' => [static fn (): bool => true, true];
yield 'not matching' => [static fn (): bool => false, false];
yield 'skipping' => [static fn (): ?bool => null, null];
}
}
23 changes: 6 additions & 17 deletions tests/Knp/Menu/Tests/MenuFactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,22 @@ public function testExtensions(): void
$extension1 = $this->getMockBuilder(ExtensionInterface::class)->getMock();
$extension1->expects($this->once())
->method('buildOptions')
->with(['foo' => 'bar'])
->willReturn(['uri' => 'foobar']);
->with(['uri' => 'foobar'])
->willReturnArgument(0);
Copy link
Collaborator

@stof stof Dec 9, 2025

Choose a reason for hiding this comment

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

This changes the behavior of the test.

$extension1->expects($this->once())
->method('buildItem')
->with($this->isInstanceOf(ItemInterface::class), $this->containsCustom('foobar'));
->with($this->isInstanceOf(ItemInterface::class), $this->containsEqual('foobar'));

$factory->addExtension($extension1);

$extension2 = $this->getMockBuilder(ExtensionInterface::class)->getMock();
$extension2->expects($this->once())
->method('buildOptions')
->with(['foo' => 'baz'])
->willReturn(['foo' => 'bar']);
$extension1->expects($this->once())
->willReturn(['uri' => 'foobar']);
$extension2->expects($this->once())
->method('buildItem')
->with($this->isInstanceOf(ItemInterface::class), $this->containsCustom('foobar'));
->with($this->isInstanceOf(ItemInterface::class), $this->containsEqual('foobar'));

$factory->addExtension($extension2, 10);

Expand All @@ -57,16 +57,5 @@ public function testCreateItem(): void
$this->assertEquals('foo', $item->getLinkAttribute('class'));
}

private function containsCustom(string $value): ?object
{
if (\method_exists($this, 'contains')) {
return $this->contains($value);
}

if (\method_exists($this, 'containsEqual')) {
return $this->containsEqual($value);
}

return null;
}
}
6 changes: 3 additions & 3 deletions tests/Knp/Menu/Tests/MenuItemGetterSetterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ public function testChildren(): void
{
$menu = $this->createMenu();
$child = $this->createMenu('child_menu');
$menu->setChildren([$child]);
$this->assertEquals([$child], $menu->getChildren());
$menu->setChildren(['child' => $child]);
$this->assertEquals(['child' => $child], $menu->getChildren());
}

public function testSetExistingNameThrowsAnException(): void
Expand All @@ -182,7 +182,7 @@ public function testSetExistingNameThrowsAnException(): void
$menu = $this->createMenu();
$menu->addChild('jack');
$menu->addChild('joe');
$menu->getChild('joe')->setName('jack');
$menu->getChildren()['joe']->setName('jack');
}

public function testSetSameName(): void
Expand Down
Loading