Skip to content

More concrete return type for opcache_get_configuration#5424

Merged
staabm merged 1 commit intophpstan:2.1.xfrom
devnix:opcache_get_configuration
Apr 29, 2026
Merged

More concrete return type for opcache_get_configuration#5424
staabm merged 1 commit intophpstan:2.1.xfrom
devnix:opcache_get_configuration

Conversation

@devnix
Copy link
Copy Markdown
Contributor

@devnix devnix commented Apr 7, 2026

@VincentLanglet
Copy link
Copy Markdown
Contributor

Not sure such precision is needed ; wasn't https://github.com/phpstan/phpstan-src/pull/5422/changes enough ?
Dunno what you think about it @staabm ; this would make a huge constant array (which are often simplified)

@staabm
Copy link
Copy Markdown
Contributor

staabm commented Apr 7, 2026

what the real world use-case/motivation for this change?

@devnix
Copy link
Copy Markdown
Contributor Author

devnix commented Apr 7, 2026

Expecting a shape, avoiding unnecessary checks of array keys, reporting typos in array keys...

@staabm
Copy link
Copy Markdown
Contributor

staabm commented Apr 8, 2026

I think we can land the change after we added a separate NodeScopeResolverTest in nsrt/.

usually we don't enforce nsrt/ tests for changes in resources/functionMap.php but I think in this case it would be useful, because we add a very long line full of escaping etc. and its pretty easy to get something wrong in this case.

does the suggested return-type array-shape also work for PHP7?
is it something we need to make php version specific?

@devnix
Copy link
Copy Markdown
Contributor Author

devnix commented Apr 9, 2026

@staabm I'm going to carefully review the shape between versions.

Just one quick question: if there is any difference, which version should be in functionMap.php, a 7.3 <= version and then the different version in the deltas?

@staabm
Copy link
Copy Markdown
Contributor

staabm commented Apr 10, 2026

Just one quick question: if there is any difference, which version should be in functionMap.php, a 7.3 <= version and then the different version in the deltas?

yes. we only need it in delta-files when there are differences.
since we are adding a more precise return type, functionMap.php is fine.
if we would e.g. add a more precise parameter type (which is a BC break), we would likely used functionMap_bleedingEdge.php instead.

if php version differences exist, we need multiple nsrt/ tests

@devnix devnix force-pushed the opcache_get_configuration branch from cc18d31 to 710f5a3 Compare April 29, 2026 13:46
@devnix
Copy link
Copy Markdown
Contributor Author

devnix commented Apr 29, 2026

I think I've got it @staabm, sorry for the delay!

@devnix devnix force-pushed the opcache_get_configuration branch 2 times, most recently from f2abaae to 736033d Compare April 29, 2026 14:36
Comment thread tests/PHPStan/Analyser/nsrt/opcache-get-configuration.php Outdated
@devnix devnix force-pushed the opcache_get_configuration branch from 736033d to 54940e6 Compare April 29, 2026 19:23
@devnix devnix requested a review from VincentLanglet April 29, 2026 19:42
Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

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

My only worry is for how long the type from PHP8.3 will stay the same...

@VincentLanglet VincentLanglet requested a review from staabm April 29, 2026 20:19
@staabm staabm merged commit 6a76584 into phpstan:2.1.x Apr 29, 2026
655 of 658 checks passed
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.

3 participants