Skip to content

The preg_match always return with option array key when condition is === 1 #57

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

Open
kayw-geek opened this issue Feb 24, 2025 · 6 comments
Assignees

Comments

@kayw-geek
Copy link

kayw-geek commented Feb 24, 2025

Hi @shish, Thank you for adding more features to Safe.

Currently, I found an issue where the return value of the \Safe\preg_match() method must be assigned to a variable for PHPStan to correctly infer it; otherwise, it assumes that the array key might not exist.

$result = \Safe\preg_match($pattern, $string, $matches);
if ($result === 1){
    \PHPStan\Testing\assertType($type, $matches);
}

if(\Safe\preg_match($pattern, $string, $matches) === 1) {
    \PHPStan\Testing\assertType($type, $matches); // line 24
}
 ------ ------------------------------------------------------------------------------------------------------------------------------------ 
  24     Expected type array{0: string, 1: non-empty-string, 2: 'o', 3?: 'World'}, actual: array{0?: string, 1?: non-empty-string, 2?: 'o',  
         3?: 'World'}                                                                                                                        
 ------ ------------------------------------------------------------------------------------------------------------------------------------ 


                                                                                                                        
 [ERROR] Found 1 error                                                                                                  
                       
@kayw-geek kayw-geek reopened this Feb 24, 2025
@shish
Copy link
Collaborator

shish commented Feb 24, 2025

cc @staabm who has some idea how this part works, I am even more confused by this than the last thread along these lines 😅

@staabm
Copy link
Collaborator

staabm commented Feb 24, 2025

@shish this looks very similar. Was the other fix released already?

@kayw-geek which version of the safe/* packages do you use?

@shish
Copy link
Collaborator

shish commented Feb 24, 2025

@shish this looks very similar. Was the other fix released already?

Yup, checked that a testcase fails with the current phpstan-safe-rule master branch + Safe 3.0.2 (and verified that vendor/thecodingmachine/safe/generated/8.4/pcre.php has the @returns 0|1 annotation just in case) :S

@staabm
Copy link
Collaborator

staabm commented Feb 24, 2025

Ok thanks. Will have a look tomorrow :-)

@kayw-geek
Copy link
Author

@staabm The thecodingmachine/safe version is v3.0.2, and the preg_match function has the @return 0|1 phpdoc in pcre.php

@staabm staabm self-assigned this Feb 26, 2025
@staabm
Copy link
Collaborator

staabm commented Feb 26, 2025

I have tested with this reproducer (as the origin report does not provide a full runnable example):

<?php declare(strict_types = 1);

function doFoo(string $string) {
	$pattern = '/\s/';


	$result = preg_match($pattern, $string, $matches);
	if ($result === 1){
		\PHPStan\dumpType($matches);
	}
	
	if (preg_match($pattern, $string, $matches) === 1) {
		\PHPStan\dumpType( $matches); // line 24
	}
}

this case works with \preg_match() because PHPStan has a built-in hack for it in
https://github.com/phpstan/phpstan-src/blob/de3720dc4e473505002301c247e77a939845be94/src/Analyser/TypeSpecifier.php#L2185-L2198

I have a open todo on phpstan-src which I guess will fix this problem.
I will make sure to test this case after the phpstan-src fix landed. until then there is nothing we can do here.

In case this is a high priority issue for you, please consider sponsoring my open source work.
otherwise I have other priorities atm

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 a pull request may close this issue.

3 participants