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.

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

Update to PHPStan level 3; various fixes #281

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+116 −89
Open

Update to PHPStan level 3; various fixes #281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion phpstan.dist.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
parameters:
level: 1
level: 3
paths:
- src
- tests
2 changes: 1 addition & 1 deletion src/Behat/Gherkin/Loader/AbstractFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ protected function findRelativePath($path)
*
* @param string $path Relative path
*
* @return string
* @return false|string
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't see any better way to solve this - downstream users with PHPStan that extended this class will now get warnings, unless they fix it in a similar way to (for example), the DirectoryLoader changes in this PR.

*/
protected function findAbsolutePath($path)
{
Expand Down
6 changes: 5 additions & 1 deletion src/Behat/Gherkin/Loader/DirectoryLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ public function __construct(Gherkin $gherkin)
public function supports($resource)
{
return is_string($resource)
&& is_dir($this->findAbsolutePath($resource));
&& ($path = $this->findAbsolutePath($resource)) !== false
&& is_dir($path);
}

/**
Expand All @@ -57,6 +58,9 @@ public function supports($resource)
public function load($resource)
{
$path = $this->findAbsolutePath($resource);
if ($path === false) {
throw new \LogicException("Unable to locate absolute path of supported resource: $resource");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hope that what is happening here is clear - it should not be possible for this exception to be thrown unless something happens between calling supports() and load().
In any case, feel free to come up with a better message.. I'm a bit unsure with the current one.

}

$iterator = new RecursiveIteratorIterator(
new RecursiveDirectoryIterator($path, RecursiveDirectoryIterator::SKIP_DOTS)
Expand Down
8 changes: 6 additions & 2 deletions src/Behat/Gherkin/Loader/GherkinFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,9 @@ public function setCache(CacheInterface $cache)
public function supports($resource)
{
return is_string($resource)
&& is_file($absolute = $this->findAbsolutePath($resource))
&& pathinfo($absolute, PATHINFO_EXTENSION) === 'feature';
&& ($path = $this->findAbsolutePath($resource)) !== false
&& is_file($path)
&& pathinfo($path, PATHINFO_EXTENSION) === 'feature';
}

/**
Expand All @@ -70,6 +71,9 @@ public function supports($resource)
public function load($resource)
{
$path = $this->findAbsolutePath($resource);
if ($path === false) {
throw new \LogicException("Unable to locate absolute path of supported resource: $resource");
}

if ($this->cache) {
if ($this->cache->isFresh($path, filemtime($path))) {
Expand Down
8 changes: 6 additions & 2 deletions src/Behat/Gherkin/Loader/YamlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ public function __construct()
public function supports($resource)
{
return is_string($resource)
&& is_file($absolute = $this->findAbsolutePath($resource))
&& pathinfo($absolute, PATHINFO_EXTENSION) === 'yml';
&& ($path = $this->findAbsolutePath($resource)) !== false
&& is_file($path)
&& pathinfo($path, PATHINFO_EXTENSION) === 'yml';
}

/**
Expand All @@ -51,6 +52,9 @@ public function supports($resource)
public function load($resource)
{
$path = $this->findAbsolutePath($resource);
if ($path === false) {
throw new \LogicException("Unable to locate absolute path of supported resource: $resource");
}
$hash = Yaml::parse(file_get_contents($path));

$features = $this->loader->load($hash);
Expand Down
12 changes: 4 additions & 8 deletions src/Behat/Gherkin/Node/OutlineNode.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class OutlineNode implements ScenarioInterface
*/
private $steps;
/**
* @var ExampleTableNode|ExampleTableNode[]
* @var array<array-key, ExampleTableNode>
Copy link
Contributor Author

@uuf6429 uuf6429 Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

According to the (original) constructor code, this property could never contain a singular value.

*/
private $tables;
/**
Expand All @@ -52,7 +52,7 @@ class OutlineNode implements ScenarioInterface
* @param string|null $title
* @param string[] $tags
* @param StepNode[] $steps
* @param ExampleTableNode|ExampleTableNode[] $tables
* @param ExampleTableNode|array<array-key, ExampleTableNode> $tables
* @param string $keyword
* @param int $line
*/
Expand All @@ -69,11 +69,7 @@ public function __construct(
$this->steps = $steps;
$this->keyword = $keyword;
$this->line = $line;
if (!is_array($tables)) {
$this->tables = [$tables];
} else {
$this->tables = $tables;
}
$this->tables = is_array($tables) ? $tables : [$tables];
}

/**
Expand Down Expand Up @@ -197,7 +193,7 @@ public function getExamples()
/**
* Returns examples tables array for the outline.
*
* @return ExampleTableNode[]
* @return array<array-key, ExampleTableNode>
*/
public function getExampleTables()
{
Expand Down
10 changes: 4 additions & 6 deletions src/Behat/Gherkin/Node/TableNode.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,12 @@
*/
class TableNode implements ArgumentInterface, IteratorAggregate
{
private array $table;

/**
* @var array
*/
private $table;
/**
* @var int
* @var array<array-key, int>
*/
private $maxLineLength = [];
private array $maxLineLength = [];

/**
* Initializes table.
Expand Down
13 changes: 8 additions & 5 deletions tests/Behat/Gherkin/Filter/LineFilterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

class LineFilterTest extends FilterTestCase
{
public function testIsFeatureMatchFilter()
public function testIsFeatureMatchFilter(): void
{
$feature = new FeatureNode(null, null, [], null, [], null, null, null, 1);

Expand All @@ -32,7 +32,7 @@ public function testIsFeatureMatchFilter()
$this->assertFalse($filter->isFeatureMatch($feature));
}

public function testIsScenarioMatchFilter()
public function testIsScenarioMatchFilter(): void
{
$scenario = new ScenarioNode(null, [], [], null, 2);

Expand All @@ -54,7 +54,7 @@ public function testIsScenarioMatchFilter()
$this->assertTrue($filter->isScenarioMatch($outline));
}

public function testFilterFeatureScenario()
public function testFilterFeatureScenario(): void
{
$filter = new LineFilter(2);
$feature = $filter->filterFeature($this->getParsedFeature());
Expand All @@ -71,19 +71,20 @@ public function testFilterFeatureScenario()
$this->assertCount(0, $scenarios = $feature->getScenarios());
}

public function testFilterFeatureOutline()
public function testFilterFeatureOutline(): void
{
$filter = new LineFilter(13);
$feature = $filter->filterFeature($this->getParsedFeature());
/* @var OutlineNode[] $scenarios */
$this->assertCount(1, $scenarios = $feature->getScenarios());
$this->assertSame('Scenario#3', $scenarios[0]->getTitle());
$this->assertInstanceOf(OutlineNode::class, $scenarios[0]);
Copy link
Contributor Author

@uuf6429 uuf6429 Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

These assertInstanceOf() statements are a convenient way to (phpunit-)test the code as well as to tell phpstan that the ScenarioInterface object is in fact an OutlineNode (otherwise it complains that e.g. the getExampleTable() below, doesn't exist).

$this->assertCount(4, $scenarios[0]->getExampleTable()->getRows());

$filter = new LineFilter(20);
$feature = $filter->filterFeature($this->getParsedFeature());
$this->assertCount(1, $scenarios = $feature->getScenarios());
$this->assertSame('Scenario#3', $scenarios[0]->getTitle());
$this->assertInstanceOf(OutlineNode::class, $scenarios[0]);
$exampleTableNodes = $scenarios[0]->getExampleTables();
$this->assertCount(1, $exampleTableNodes);
$this->assertCount(2, $exampleTableNodes[0]->getRows());
Expand All @@ -97,6 +98,7 @@ public function testFilterFeatureOutline()
$feature = $filter->filterFeature($this->getParsedFeature());
$this->assertCount(1, $scenarios = $feature->getScenarios());
$this->assertSame('Scenario#3', $scenarios[0]->getTitle());
$this->assertInstanceOf(OutlineNode::class, $scenarios[0]);
$exampleTableNodes = $scenarios[0]->getExampleTables();
$this->assertCount(1, $exampleTableNodes);
$this->assertCount(2, $exampleTableNodes[0]->getRows());
Expand All @@ -110,6 +112,7 @@ public function testFilterFeatureOutline()
$feature = $filter->filterFeature($this->getParsedFeature());
$this->assertCount(1, $scenarios = $feature->getScenarios());
$this->assertSame('Scenario#3', $scenarios[0]->getTitle());
$this->assertInstanceOf(OutlineNode::class, $scenarios[0]);
$this->assertCount(1, $scenarios[0]->getExampleTable()->getRows());
$this->assertSame([['action', 'outcome']], $scenarios[0]->getExampleTable()->getRows());
}
Expand Down
28 changes: 14 additions & 14 deletions tests/Behat/Gherkin/Filter/LineRangeFilterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@
use Behat\Gherkin\Node\FeatureNode;
use Behat\Gherkin\Node\OutlineNode;
use Behat\Gherkin\Node\ScenarioNode;
use PHPUnit\Framework\Attributes\DataProvider;

class LineRangeFilterTest extends FilterTestCase
{
public function featureLineRangeProvider()
public static function featureLineRangeProvider(): iterable
{
return [
['1', '1', true],
Expand All @@ -29,18 +30,16 @@ public function featureLineRangeProvider()
];
}

/**
* @dataProvider featureLineRangeProvider
*/
public function testIsFeatureMatchFilter($filterMinLine, $filterMaxLine, $expected)
#[DataProvider('featureLineRangeProvider')]
public function testIsFeatureMatchFilter(string $filterMinLine, string $filterMaxLine, bool $expected): void
{
$feature = new FeatureNode(null, null, [], null, [], null, null, null, 1);

$filter = new LineRangeFilter($filterMinLine, $filterMaxLine);
$this->assertSame($expected, $filter->isFeatureMatch($feature));
}

public function scenarioLineRangeProvider()
public static function scenarioLineRangeProvider(): iterable
{
return [
['1', '2', 1],
Expand All @@ -55,22 +54,20 @@ public function scenarioLineRangeProvider()
];
}

/**
* @dataProvider scenarioLineRangeProvider
*/
public function testIsScenarioMatchFilter($filterMinLine, $filterMaxLine, $expectedNumberOfMatches)
#[DataProvider('scenarioLineRangeProvider')]
public function testIsScenarioMatchFilter(string $filterMinLine, string $filterMaxLine, int $expectedNumberOfMatches): void
{
$scenario = new ScenarioNode(null, [], [], null, 2);
$outline = new OutlineNode(null, [], [], [new ExampleTableNode([], null)], null, 3);

$filter = new LineRangeFilter($filterMinLine, $filterMaxLine);
$this->assertEquals(
$expectedNumberOfMatches,
intval($filter->isScenarioMatch($scenario)) + intval($filter->isScenarioMatch($outline))
(int) $filter->isScenarioMatch($scenario) + (int) $filter->isScenarioMatch($outline)
);
}

public function testFilterFeatureScenario()
public function testFilterFeatureScenario(): void
{
$filter = new LineRangeFilter(1, 3);
$feature = $filter->filterFeature($this->getParsedFeature());
Expand All @@ -87,19 +84,20 @@ public function testFilterFeatureScenario()
$this->assertCount(0, $scenarios = $feature->getScenarios());
}

public function testFilterFeatureOutline()
public function testFilterFeatureOutline(): void
{
$filter = new LineRangeFilter(12, 14);
$feature = $filter->filterFeature($this->getParsedFeature());
/* @var OutlineNode[] $scenarios */
$this->assertCount(1, $scenarios = $feature->getScenarios());
$this->assertSame('Scenario#3', $scenarios[0]->getTitle());
$this->assertInstanceOf(OutlineNode::class, $scenarios[0]);
$this->assertFalse($scenarios[0]->hasExamples());

$filter = new LineRangeFilter(16, 21);
$feature = $filter->filterFeature($this->getParsedFeature());
$this->assertCount(1, $scenarios = $feature->getScenarios());
$this->assertSame('Scenario#3', $scenarios[0]->getTitle());
$this->assertInstanceOf(OutlineNode::class, $scenarios[0]);
$exampleTableNodes = $scenarios[0]->getExampleTables();
$this->assertCount(1, $exampleTableNodes);
$this->assertCount(3, $exampleTableNodes[0]->getRows());
Expand All @@ -114,6 +112,7 @@ public function testFilterFeatureOutline()
$feature = $filter->filterFeature($this->getParsedFeature());
$this->assertCount(1, $scenarios = $feature->getScenarios());
$this->assertSame('Scenario#3', $scenarios[0]->getTitle());
$this->assertInstanceOf(OutlineNode::class, $scenarios[0]);
$exampleTableNodes = $scenarios[0]->getExampleTables();
$this->assertCount(2, $exampleTableNodes);

Expand All @@ -137,6 +136,7 @@ public function testFilterFeatureOutline()
$feature = $filter->filterFeature($this->getParsedFeature());
$this->assertCount(1, $scenarios = $feature->getScenarios());
$this->assertSame('Scenario#3', $scenarios[0]->getTitle());
$this->assertInstanceOf(OutlineNode::class, $scenarios[0]);
$exampleTableNodes = $scenarios[0]->getExampleTables();
$this->assertCount(1, $exampleTableNodes);
$this->assertCount(2, $exampleTableNodes[0]->getRows());
Expand Down
Loading