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

String interpolation like "hello {$world}" loses literal-string type #5994

Closed
NLthijs48 opened this issue Nov 12, 2021 · 10 comments
Closed

String interpolation like "hello {$world}" loses literal-string type #5994

NLthijs48 opened this issue Nov 12, 2021 · 10 comments

Comments

@NLthijs48
Copy link
Contributor

Bug report

Concatenation of two literal-strings will make the result a literal-string as well, but using string interpolation it does not make the result a literal-string, but instead makes it a non-empty-string.

Code snippet that reproduces the problem

https://phpstan.org/r/c3a9d482-2963-4711-949c-7c387c2aff89

Expected output

I would expect no error from the string interpolation, and allow it to continue as a literal-string. Maybe I'm missing something though and this is intended behavior, let me know what you think.

Did PHPStan help you today? Did it make you happy in any way?

Congratulations with reaching version 1 😄 , looking forward to using all features of it (I already have a couple of them enabled using experimental flags of the previous version)

@staabm
Copy link
Contributor

staabm commented Nov 12, 2021

Thx for the report. Do you plan to contribute and look into it.

Otherwise I will take it

@craigfrancis
Copy link

Hi, I’m not too familiar with the internals of PHPStan, but I might be able to help (considering I asked for the literal-string type to be added).

And just to confirm, the "select {$literal}" should return a literal string - as concatenation of literals (even via interpolation) preserves the literal flag.

@NLthijs48
Copy link
Contributor Author

@staabm I'm not familiar with the PHPStan codebase, so go ahead and work on it. Thanks for taking a look!

@craigfrancis Good to know that it should indeed stay a literal-string. I'm testing out this feature for SQL queries and ran into this case.

@craigfrancis
Copy link

@NLthijs48, That’s the perfect use case for it, I’ve detailed how it can be used on the PHP RFC (my email address is at the top, if you want to discuss anything); and I’ll be trying to get it into PHP again next year.

@ondrejmirtes
Copy link
Member

Hi, if you look at the original implementation phpstan/phpstan-src@a5eb5b5 around the Concat operator in MutatingScope (first thing in the diff), it's easy to figure out how to make it work for these strings - it's represented in the AST by Node\Scalar\Encapsed - just search for this in MutatingScope. Thanks.

@craigfrancis
Copy link

Thanks @ondrejmirtes, I've had a go with PR-768, where there is a second commit to remove the Spread Operator (for PHP 7.3 and below).

There are 5 failing tests for PHP 8.1, but I don't think they relate to this patch.

And "Coding Standard (8.0)" is complaining about "Use early exit to reduce code nesting", where phpcbf is suggesting changing:

if (!$partType->isLiteralString()->yes()) {
    $isLiteralString = false;
}

To use this instead:

if ($partType->isLiteralString()->yes()) {
    continue;
}
$isLiteralString = false;

Personally I find that more complicated to read, so I'm wondering if there is a better way of doing this; or I could use a // phpcs:disable comment?

@ondrejmirtes
Copy link
Member

Fixed: phpstan/phpstan-src#768

@craigfrancis
Copy link

Thanks @ondrejmirtes

@NLthijs48
Copy link
Contributor Author

@craigfrancis and @ondrejmirtes Thanks for updating this!

Meanwhile I already started using literal-string, and soon this can remove a couple of @phpstan-ignore-next-line comments 😄

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants