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

Fix Comparison::CONTAINS to check is_string before str_compare #548

Merged

Conversation

cage-is
Copy link
Contributor

@cage-is cage-is commented Jan 9, 2025

In Comparison there is a line that says

self::Contains => str_contains($subject, $reference), /* @phpstan-ignore-line */

Since this is checking $subject with str_contains it should check first if it is a string. This seems to have been handled in other various comparisons already (e.g. self::Regexp => is_string($subject) && 1 === preg_match($reference, $subject), /* @phpstan-ignore-line */)

TypeError: str_contains(): Argument #1 ($haystack) must be of type string, null given
/var/www/src/Query/Constraint/Comparison.php:102
/var/www/src/Query/Constraint/Column.php:78
/var/www/src/Query/Constraint/Criteria.php:47
/var/www/src/Query/Constraint/Criteria.php:108
/var/www/src/ResultSet.php:395
/var/www/src/StatementTest.php:260

@nyamsprod
Copy link
Member

thanks for the bugfix and the nice catch! I will merge your contribution 👍🏾

@nyamsprod nyamsprod merged commit 888a37e into thephpleague:master Jan 9, 2025
7 of 9 checks passed
@nyamsprod
Copy link
Member

I have updated your test case the assertSame was not mandatory an assertCount is enough as the ResultSet implements the Countable interface which means that iteration is done which highlights your fix 😉

@nyamsprod nyamsprod self-assigned this Jan 9, 2025
@nyamsprod nyamsprod added the bug label Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants