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

CDATA used in baseline in a lot more cases than before PHP 8.1 #10632

Closed
come-nc opened this issue Feb 1, 2024 · 4 comments · Fixed by #10633
Closed

CDATA used in baseline in a lot more cases than before PHP 8.1 #10632

come-nc opened this issue Feb 1, 2024 · 4 comments · Fixed by #10633

Comments

@come-nc
Copy link
Contributor

come-nc commented Feb 1, 2024

Not sure if this is a big deal, but because of a change in PHP 8.1, psalm is now using CDATA in a lot more cases.

https://github.com/vimeo/psalm/pull/9184/files#diff-616aac909b305fdec9286d185e99df879c5d48db6886a8ac7eee4e6e58a181ce uses htmlspecialchar to decide if it should use CDATA.

See on https://www.php.net/manual/en/function.htmlspecialchars.php:

8.1.0 flags changed from ENT_COMPAT to ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML401.

So CDATA is now applied as soon as there is a ' or " in the code.

Took me a long time to understand why my baseline changed so much.

Copy link

Hey @come-nc, can you reproduce the issue on https://psalm.dev? These will be used as phpunit tests when implementing the feature or fixing this bug.

@weirdan
Copy link
Collaborator

weirdan commented Feb 1, 2024

To rephrase, Psalm generates different baselines on PHP 8.0 and PHP 8.1, even with the same Psalm version. While it's not a problem for Psalm itself, as either variant is perfectly valid and can be read by Psalm, it may cause diff size increase in projects using Psalm.

I think we should just bite the bullet and wrap all snippets in CDATA. This way it will be just a one-time change.

/cc: @orklah @danog @jack-worman

@jack-worman
Copy link
Contributor

Another option would be setting the flags explicitly, so that they are the same across all versions of PHP.

@weirdan
Copy link
Collaborator

weirdan commented Feb 1, 2024

Yes, that's an option too. Personally, I favour the CDATA-always approach though as it results in a more uniform baseline format.

weirdan added a commit to weirdan/psalm that referenced this issue Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants