Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
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
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

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
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

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

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
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

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
14 changes: 9 additions & 5 deletions tests/Knp/Menu/Tests/Iterator/IteratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ final class IteratorTest extends MenuTestCase
{
public function testIterator(): void
{
$pt1 = $this->pt1;
Copy link
Collaborator

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

$count = 0;
foreach ($this->pt1 as $key => $value) {
foreach ($pt1->getChildren() as $key => $value) {
++$count;
$this->assertEquals('Child '.$count, $key);
$this->assertEquals('Child '.$count, $value->getLabel());
Expand All @@ -28,10 +29,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 @@ -40,8 +42,9 @@ public function testRecursiveIterator(): void

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

Expand All @@ -50,8 +53,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
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() => true, true];
yield 'not matching' => [static fn() => false, false];
yield 'skipping' => [static fn() => null, null];
}
}
23 changes: 6 additions & 17 deletions tests/Knp/Menu/Tests/MenuFactoryTest.php
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

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
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