Conversation
|
I would expect double quoted string to support the same escaping sequences as PHP. |
|
I initially was going to do so, but didn't for parity with Psalm. I'd be more than happy to mirror PHP's functionality though, if @ondrejmirtes also agrees :) |
|
Yes, parity with PHP would be great, we're in a very edge case territory anyway. And inform the Psalm maintainers afterwards 😊 BTW is this a BC break? Do we need a feature flag for the new behaviour? |
src/Parser/ConstExprParser.php
Outdated
| return new Ast\ConstExpr\ConstExprArrayItemNode($key, $value); | ||
| } | ||
|
|
||
| private function trimString(string $string): string |
There was a problem hiding this comment.
decode or parse string would be better name
5a3df11 to
9e0ab4e
Compare
|
I've basically copied most of the implementation from PHP-Parser, but that probably needs to be mentioned somewhere. Any suggestions? |
|
A github.com permalink in a code comment would be fine I think. |
|
This is gonna need some modification and a bleedingEdge toggle in phpstan-src too, right? |
|
It should only require a feature toggle :) |
|
What about the |
| ); | ||
| } | ||
|
|
||
| private static function codePointToUtf8(int $num): string |
There was a problem hiding this comment.
Would it be possible to use mb_chr() or IntlChar::chr() instead?
There was a problem hiding this comment.
Neither mbstring nor intl are declared as a dependency, so that would need to be conditional. That would only make the code more complicated for little gain.
I would definitely like to get rid of it, especially because it is used with both true/false internally. There isn't a pure BC-friendly approach to removing it I'm afraid, but I really don't think it should be used at all so maybe it's not that bad? |
9e0ab4e to
720fc79
Compare
|
Thank you! |
Fix #142