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

Avoid using trim() on null #380

Merged
merged 3 commits into from
Dec 13, 2021
Merged

Conversation

schlessera
Copy link
Contributor

@schlessera schlessera commented Dec 6, 2021

The $delimiters argument for the Tokenizer::scan() method is optional and currently defaults to null (which is not documented as a valid value in the docblock'.

This default value of null is then passed into the trim() function, which causes a deprecation warning starting with PHP 8.1:

Deprecated: trim(): Passing null to parameter #1 ($string) of type string is deprecated in vendor/mustache/mustache/src/Mustache/Tokenizer.php on line 110

This PR changes the default value to be an empty string instead, which makes the deprecation warning go away in most cases without any further changes and is more corrected in terms o the documented argument type.

Furthermore, the PR adds a check to see if the $delimiters are a string before trying to trim them.

Finally, a few tests are added to ensure custom delimiter handling works as expected.

@schlessera
Copy link
Contributor Author

Trying to run the tests produces errors of missing files for me:
Image 2021-12-12 at 9 20 52 AM

Maybe I'm missing something...?

@bobthecow
Copy link
Owner

Awesome, thanks!

Yes, the spec test uses a suuuuper old version of the Symfony YAML parser that doesn't exist in Composer so it's installed via git submodules 😬

Confirmed locally, I'll merge.

@bobthecow bobthecow merged commit 6361644 into bobthecow:main Dec 13, 2021
@schlessera schlessera deleted the fix/trim-php-8.1 branch December 14, 2021 13:41
@schlessera
Copy link
Contributor Author

Great, thanks!

May I ask if there is a release scheduled for PHP 8.1 compatibility?

@bobthecow
Copy link
Owner

https://github.com/bobthecow/mustache.php/releases/tag/v2.14.0

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.

None yet

2 participants