Skip to content

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Aug 4, 2024

support literals within classes for constant type inference.

I do intentionally not support constant types for classes which define "from-to" ranges like a-z or 0-9, as the semantics can get complex easily and the list of constant chars explodes pretty fast.

@staabm staabm marked this pull request as ready for review August 4, 2024 11:28
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@staabm
Copy link
Contributor Author

staabm commented Aug 4, 2024

btw: I think the issue-bot is confused, as the result seem to be reporting wrong assertion errors

@ondrejmirtes
Copy link
Member

That's because the issue-bot already posted comments from 1.12.x where the preg array shapes do not require bleeding edge. Here we're still running on 1.11.x.

@ondrejmirtes ondrejmirtes merged commit 18aa502 into phpstan:1.11.x Aug 4, 2024
454 of 462 checks passed
@ondrejmirtes
Copy link
Member

Thank you!

@staabm staabm deleted the classes branch August 4, 2024 12:17

function (string $s): void {
if (preg_match('/Price: (ba[rz])/i', $s, $matches)) {
assertType("array{string, 'bar'|'baz'}", $matches);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i modifier must be honored, ie. 'baR' must be present in the expected type

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At that point I'd fall back to not have literal strings at all. Too many combinations -> performance problems

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, had the same thing in mind. Will fix.

Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix in #3288

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.

4 participants