Skip to content

Improved types (PHPStan level 6)#1081

Merged
dantleech merged 1 commit intophpbench:masterfrom
aivchen:phpstan
Jan 25, 2024
Merged

Improved types (PHPStan level 6)#1081
dantleech merged 1 commit intophpbench:masterfrom
aivchen:phpstan

Conversation

@aivchen
Copy link
Copy Markdown
Contributor

@aivchen aivchen commented Jan 16, 2024

I had to disable 'void_return' php-cs-fixer rule. Otherwise it adds return type to \PhpBench\Progress\LoggerInterface after my changes.

@aivchen aivchen force-pushed the phpstan branch 4 times, most recently from c98ae60 to 31808ba Compare January 20, 2024 22:43
@aivchen
Copy link
Copy Markdown
Contributor Author

aivchen commented Jan 20, 2024

Bump

for ($index = 0; $index < $nbIterations; $index++) {
$iteration = $variant->getIteration($index);

foreach ($variant as $index => $iteration) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are we sure that index is a sequential list here? Maybe use array_values if not

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, we are sure.

  • In lib/Model/Variant.php we add values to the private property Variant::$iterations using only $this->iterations[] = XXX.
  • Variant::$iterations has /** @var list<Iteration> */ docblock. It means this array is 0-based sequential list (https://phpstan.org/writing-php-code/phpdoc-types#lists).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes, have some ancient memories about making for loops like that to work around "lists" that had been filtered etc making their array keys non-sequential.

@dantleech dantleech merged commit 7679609 into phpbench:master Jan 25, 2024
private $maxTime,
private $meanTime,
private $meanRelStDev,
private $totalTime
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

😄

@dantleech
Copy link
Copy Markdown
Member

great work again 🙇

@aivchen aivchen deleted the phpstan branch January 25, 2024 23:48
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.

2 participants