Casting preg_match() return value breaks array shapes for $matches by-ref parameter

Bug report

I just noticed that phpstan/phpstan-src#2589 doesn't seem to play well with strict rules.

This example without strict rules works: https://phpstan.org/r/6187bcca-8fc8-459b-9790-21a82e62b345

But when I enable strict rules, I need to cast the return value of preg_match() to a bool and this seems to block the types from passing through. For example: https://phpstan.org/r/f368fc07-6438-43f1-b80b-93dee22535f4

Same thing happens when I cast to an int: https://phpstan.org/r/0d011355-4446-4bd8-9b8d-0c44907f9a07

Code snippet that reproduces the problem

https://phpstan.org/r/f368fc07-6438-43f1-b80b-93dee22535f4

Expected output

No issue should have been detected

Did PHPStan help you today? Did it make you happy in any way?

No response

related with phpstan/phpstan-src#3175, IMO such hack should not be needed as the return type is always 1|false and thus == 1 is strictly equal to === 1

phpstan-strict-rules doesn't require you to cast to bool. You can do this instead: https://phpstan.org/r/7b3e4faf-28d9-426f-96de-42a65f2a8416 (typo on purpose to verify the inference works)

But otherwise yeah, this is a bug worth fixing.

I think we have the same problem for other functions
https://phpstan.org/r/1984afb8-1492-4c1e-b4d6-33154eb14998

@mvorisek

related with phpstan/phpstan-src#3175, IMO such hack should not be needed as the return type is always 1|false and thus == 1 is strictly equal to === 1

@mvorisek preg_match returns 1|0|false .. I don't think its as easy implemented as you think.

but maybe you want to give it a try and see how it goes

I mean "truthy context" should be implied also by expr == 1 or (bool) expr.

@westonruter

@westonruter After the latest push in 1.12.x, PHPStan now reports different result with your code snippet:

@@ @@
-17: Offset 'background_image' might not exist on array{0?: string, background_image?: string, 1?: string}.
+No errors

@westonruter

@westonruter After the latest push in 1.12.x, PHPStan now reports different result with your code snippet:

@@ @@
-17: Offset 'background_image' might not exist on array{0?: string, background_image?: string, 1?: string}.
+No errors

@ondrejmirtes

@ondrejmirtes After the latest push in 1.12.x, PHPStan now reports different result with your code snippet:

@@ @@
-17: Offset 'background_imagee' does not exist on array{0: string, background_image: string, 1: string}.
+No errors

@westonruter

@westonruter After the latest push in 1.12.x, PHPStan now reports different result with your code snippet:

@@ @@
-17: Offset 'background_image' might not exist on array{0?: string, background_image?: string, 1?: string}.
+17: Offset 'background_image' might not exist on array{0?: string, background_image?: non-empty-string, 1?: non-empty-string}.

Full report

Line	Error
17	`Offset 'background_image' might not exist on array{0?: string, background_image?: non-empty-string, 1?: non-empty-string}.`

@westonruter

@westonruter After the latest push in 1.12.x, PHPStan now reports different result with your code snippet:

@@ @@
-17: Offset 'background_image' might not exist on array{0?: string, background_image?: string, 1?: string}.
+17: Offset 'background_image' might not exist on array{0?: string, background_image?: non-empty-string, 1?: non-empty-string}.

Full report

Line	Error
17	`Offset 'background_image' might not exist on array{0?: string, background_image?: non-empty-string, 1?: non-empty-string}.`

@ondrejmirtes

@ondrejmirtes After the latest push in 1.12.x, PHPStan now reports different result with your code snippet:

@@ @@
-17: Offset 'background_imagee' does not exist on array{0: string, background_image: string, 1: string}.
+17: Offset 'background_imagee' does not exist on array{0: string, background_image: non-empty-string, 1: non-empty-string}.

Full report

Line	Error
17	`Offset 'background_imagee' does not exist on array{0: string, background_image: non-empty-string, 1: non-empty-string}.`

Fixed as of v1.11.8

Are you sure? I can still repro your error from first post, and as far as I know this issue was not addressed

In my PHP code I was needing to add this condition '' !== $matches['background_image'] as in the following if statement:

if (
	is_string( $style )
	&&
	1 === preg_match( '/background(?:-image)?\s*:[^;]*?url\(\s*[\'"]?\s*(?<background_image>.+?)\s*[\'"]?\s*\)/', $style, $matches )
	&&
	'' !== $matches['background_image'] // PHPStan should ideally know that this is a non-empty string based on the `.+?` regular expression. See <https://github.com/phpstan/phpstan/issues/11222>.
	&&
	! $this->is_data_url( $matches['background_image'] )
) {

No longer needed as of the latest version: WordPress/performance#1395

Ah wait, it's working because I changed it to check the return value of preg_match() to be equal to 1. It is still broken if I try (bool) preg_match(...).

@staabm

@staabm After the latest push in 1.12.x, PHPStan now reports different result with your code snippet:

@@ @@
-6: Expected type non-empty-array, actual: array
+No errors

@ondrejmirtes

@ondrejmirtes the repro-snippet of this issue was fixed with the recent cast-narrowing improvements (see phpstan-bot comment above).

I have copied the only left repro from the description into #11293, as it is very similar to the mentioned referenced issue.

this means we can close here IMO.

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Navigation Menu

Repository navigation

Casting preg_match() return value breaks array shapes for $matches by-ref parameter #11262

Bug report

Code snippet that reproduces the problem

Expected output

Did PHPStan help you today? Did it make you happy in any way?

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Casting preg_match() return value breaks array shapes for $matches by-ref parameter #11262

Description

Bug report

Code snippet that reproduces the problem

Expected output

Did PHPStan help you today? Did it make you happy in any way?

Activity

mvorisek commented on Jul 1, 2024

ondrejmirtes commented on Jul 1, 2024

staabm commented on Jul 2, 2024

staabm commented on Jul 2, 2024

mvorisek commented on Jul 2, 2024

phpstan-bot commented on Jul 6, 2024

phpstan-bot commented on Jul 6, 2024

phpstan-bot commented on Jul 6, 2024

phpstan-bot commented on Jul 19, 2024

phpstan-bot commented on Jul 19, 2024

phpstan-bot commented on Jul 19, 2024

westonruter commented on Jul 24, 2024

Seldaek commented on Jul 24, 2024

westonruter commented on Jul 24, 2024

westonruter commented on Jul 24, 2024

phpstan-bot commented on Sep 1, 2024

staabm commented on Sep 3, 2024

github-actions commented on Oct 7, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions