Skip to content

Added PHPStan-generics to preg_replace#304

Closed
tjveldhuizen wants to merge 1 commit intothecodingmachine:masterfrom
tjveldhuizen:preg_replace-return
Closed

Added PHPStan-generics to preg_replace#304
tjveldhuizen wants to merge 1 commit intothecodingmachine:masterfrom
tjveldhuizen:preg_replace-return

Conversation

@tjveldhuizen
Copy link

@tjveldhuizen tjveldhuizen commented Oct 17, 2021

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[]

@tjveldhuizen
Added PHPStan-generics to preg_replace
7eb8d28 
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

ZebulanStanphill commented Oct 20, 2021

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

tjveldhuizen commented Oct 20, 2021

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

@ZebulanStanphill
Copy link

ZebulanStanphill commented Nov 2, 2021

@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

Kharhamel commented Mar 22, 2022

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

@Kharhamel Kharhamel mentioned this pull request Mar 25, 2022
Merged
@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

Reviewers

@dbrekelmans dbrekelmans dbrekelmans approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tjveldhuizen @ZebulanStanphill @Kharhamel @dbrekelmans