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.

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

Added PHPStan-generics to preg_replace #304

Closed
wants to merge 1 commit into from
Closed

Added PHPStan-generics to preg_replace #304

wants to merge 1 commit into from

Conversation

tjveldhuizen
Copy link

The subject in preg_replace can be a string or an arrays of strings. The return value is the same type as the provided subject. By adding the generics PHPStan 'knows' the correct return type in a situation like this:

use function Safe\preg_replace;

$foo = 'some string 123';
$bar = preg_replace('/[0-9]/', '', $foo);
$baz = trim($bar); // PHPStan complains that trim does not accept string|string[]

The subject in preg_replace can be a string or an arrays of strings. The return value is the same type as the provided subject. By adding the generics PHPStan 'knows' the correct return type.
@ZebulanStanphill
Copy link

Support for array as an @template type bound was only very recently added to PHPStan in phpstan/phpstan-src#673, and the current PHPStan release (0.12.99), was made a few days before that PR was merged.

So the changes in this PR will work with the next PHPStan release... but until then, this PR will have to wait.

@tjveldhuizen
Copy link
Author

Ah, so that's the reason... should we leave the PR open, or close it for now?

@ZebulanStanphill
Copy link

@tjveldhuizen PHPStan 1.0 just came out, and it includes support for @template T of array, so once this project updates its PHPStan requirement, this PR can be rebased and should start working as intended.

@Kharhamel
Copy link
Collaborator

Sorry I completly missed this PR. I will update the phpstan recs this week

@Kharhamel Kharhamel mentioned this pull request Mar 25, 2022
@tjveldhuizen tjveldhuizen closed this by deleting the head repository Apr 14, 2023
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.

4 participants