Skip to content

Remember function return type extensions#4644

Merged
ondrejmirtes merged 4 commits intophpstan:2.1.xfrom
staabm:remember-fns
Dec 22, 2025
Merged

Remember function return type extensions#4644
ondrejmirtes merged 4 commits intophpstan:2.1.xfrom
staabm:remember-fns

Conversation

@staabm
Copy link
Contributor

@staabm staabm commented Dec 20, 2025

before this PR:

grafik 
➜  phpstan-src git:(2.1.x) ✗ PHPSTAN_GNSR=1 hyperfine 'php vendor/bin/phpunit tests/PHPStan/Analyser/NodeScopeResolverTest.php' --runs=5
Benchmark 1: php vendor/bin/phpunit tests/PHPStan/Analyser/NodeScopeResolverTest.php
  Time (mean ± σ):     15.219 s ±  0.105 s    [User: 14.850 s, System: 0.359 s]
  Range (min … max):   15.059 s … 15.339 s    5 runs

after this PR:

➜  phpstan-src git:(remember-fns) ✗ PHPSTAN_GNSR=1 hyperfine 'php vendor/bin/phpunit tests/PHPStan/Analyser/NodeScopeResolverTest.php' --runs=5
Benchmark 1: php vendor/bin/phpunit tests/PHPStan/Analyser/NodeScopeResolverTest.php
  Time (mean ± σ):     15.049 s ±  0.230 s    [User: 14.679 s, System: 0.360 s]
  Range (min … max):   14.659 s … 15.264 s    5 runs

@staabm
Copy link
Contributor Author

staabm commented Dec 20, 2025

interessting.. I don't see this errors locally.. will have another look

@staabm staabm marked this pull request as ready for review December 20, 2025 13:40
@phpstan-bot
Copy link
Collaborator

phpstan-bot commented Dec 20, 2025

This pull request has been marked as ready for review.

ondrejmirtes
ondrejmirtes requested changes Dec 22, 2025
View reviewed changes
src/Type/DynamicReturnTypeExtensionRegistry.php Outdated
$supportedFunctions[] = $dynamicFunctionReturnTypeExtension;
}

return $this->dynamicReturnTypeExtensionsByFunction[$functionName] ??= $supportedFunctions;
Copy link
Member

@ondrejmirtes ondrejmirtes Dec 22, 2025

Choose a reason for hiding this comment

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

I'd say this line doesn't have to use ??= because of isset above. It should just be =.

Copy link
Contributor Author

@staabm staabm Dec 22, 2025

Choose a reason for hiding this comment

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

good catch, fixed

@staabm
Copy link
Contributor Author

staabm commented Dec 22, 2025 •
edited
Loading

btw: including this PR and the most recent 10 commits we got considerable faster with the recent changes:

before

➜  hyperfine 'php vendor/bin/phpunit tests/PHPStan/Analyser/NodeScopeResolverTest.php' --runs=5
Benchmark 1: php vendor/bin/phpunit tests/PHPStan/Analyser/NodeScopeResolverTest.php
  Time (mean ± σ):     14.478 s ±  0.134 s    [User: 14.118 s, System: 0.353 s]
  Range (min … max):   14.316 s … 14.669 s    5 runs

now

➜  hyperfine 'php vendor/bin/phpunit tests/PHPStan/Analyser/NodeScopeResolverTest.php' --runs=5
Benchmark 1: php vendor/bin/phpunit tests/PHPStan/Analyser/NodeScopeResolverTest.php
  Time (mean ± σ):     13.764 s ±  0.135 s    [User: 13.423 s, System: 0.334 s]
  Range (min … max):   13.642 s … 13.974 s    5 runs

@ondrejmirtes ondrejmirtes merged commit 48e7d3e into phpstan:2.1.x Dec 22, 2025
631 of 639 checks passed
@ondrejmirtes
Copy link
Member

ondrejmirtes commented Dec 22, 2025

Yeah, great, thank you! A similar thing could be done for methods+static methods.

@staabm staabm deleted the remember-fns branch December 22, 2025 17:42
@staabm
Copy link
Contributor Author

staabm commented Dec 23, 2025

A similar thing could be done for methods+static methods.

I did not see a profile yet were this showed up. I think the reason is, that these are already key-ed by class-name, which means the list we have to work thru is way shorter than with functions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ondrejmirtes ondrejmirtes ondrejmirtes requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@staabm @phpstan-bot @ondrejmirtes