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

Literal string concat with scalars #774

Closed
wants to merge 3 commits into from
Closed

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Nov 18, 2021

simliar to #768 this PR improves the literal-string-type for string-concat cases with other scalar types

closes phpstan/phpstan#6033

@staabm staabm marked this pull request as ready for review November 18, 2021 11:07
@staabm staabm changed the title Literal string concat with other scalars Literal string concat with scalars Nov 18, 2021
@craigfrancis
Copy link
Contributor

Sorry @staabm, I'd rather not include this, so we can keep it in line with the is_literal() implementation. Hopefully my notes on issue #6033 explains why (and yes, in the ideal world PHP would be able to track all developer defined values, including integers, but that's very unlikely to change).

I'd also note, there was also a long discussion about treating all integers as trusted (resulting in the suggestion of a function called is_trusted()), this caused a lot of confusion, and was probably the main reason why the original RFC failed (I intend to try again next year).

Feel free to email me off-list if you want to discuss anything on this subject (I'm fairly sure Ondřej doesen't want to receive loads of discussion emails).

@ondrejmirtes
Copy link
Member

@craigfrancis has some good points here, so I'm not gonna merge this, thanks for understanding.

@staabm staabm deleted the literals branch November 18, 2021 16:25
@craigfrancis
Copy link
Contributor

Thank you Ondřej... and Markus, please get in touch via email if you want to discuss further, I want to make sure we have the best solution, and we have everything covered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants