Skip to content

Retain list type via array_push() and array_unshift()#2451

Merged
ondrejmirtes merged 1 commit intophpstan:1.10.xfrom
herndlm:bug-8449-retain-list-in-array-push-unshift
Jun 18, 2023
Merged

Retain list type via array_push() and array_unshift()#2451
ondrejmirtes merged 1 commit intophpstan:1.10.xfrom
herndlm:bug-8449-retain-list-in-array-push-unshift

Conversation

@herndlm
Copy link
Copy Markdown
Contributor

@herndlm herndlm commented Jun 11, 2023 •
edited
Loading

Closes phpstan/phpstan#8449

it has been a while :) I know I have still 2 open PRs, but I didn't feel like finishing them yet. soon™

I was wondering why the array here is even re-created like that. The reason is that this is after the isIterableAtLeastOnce / optional check and the re-creation is just there to remove a NonEmptyArrayType. So if there would be a simpler/better way of removing it (maybe calling ->popArray() but I'm not sure, that feels like a hack), the list would not be removed and this new code would not be needed 🤔

another more explicit way might be to intersect the union of ->getArrays() with a TypeUtils::getAccessoryTypes() where NonEmptyArrayType is filtered away. also feels ugly. I'm not a 100% happy with any of the solutions so far.. like e.g.

$arrayType = TypeCombinator::intersect(
	TypeCombinator::union(...$arrayType->getArrays()),
	...array_filter(
		TypeUtils::getAccessoryTypes($arrayType),
		static fn (AccessoryType $type) => !$type instanceof NonEmptyArrayType,
	),
);

@herndlm herndlm changed the base branch from 1.11.x to 1.10.x June 11, 2023 19:11
@phpstan-bot
Copy link
Copy Markdown
Collaborator

phpstan-bot commented Jun 11, 2023

You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x.

@herndlm herndlm force-pushed the bug-8449-retain-list-in-array-push-unshift branch from dbc40d6 to 53b3eee Compare June 11, 2023 19:11
@herndlm
Copy link
Copy Markdown
Contributor Author

herndlm commented Jun 12, 2023 •
edited
Loading

Ha, I just found that the array_slice return type extension is using toArray() for the same thing essentially! (removing non-empty-array). But IMO toArray() should NOT remove it, that's also a bug I believe. I'd like to fix that one, but we should agree on a way to remove non-empty-array first most likely. maybe even move that to the good old TypeUtils even then hmm

update: this all feels dirty, maybe new type methods for push, unshift and slice or so are needed after all, but that feels like overhead...

let me know what you prefer or if more info is needed please

@herndlm herndlm mentioned this pull request Jun 16, 2023
Merged
@herndlm herndlm force-pushed the bug-8449-retain-list-in-array-push-unshift branch from 53b3eee to d0c4702 Compare June 18, 2023 11:20
@herndlm
Copy link
Copy Markdown
Contributor Author

herndlm commented Jun 18, 2023 •
edited
Loading

rebased with the trick the boss came up with in 9e84671 to make the array non-non-empty again (why did I not think of this..)

now I'm happy with this :)

@ondrejmirtes ondrejmirtes merged commit 1f421d4 into phpstan:1.10.x Jun 18, 2023
@ondrejmirtes
Copy link
Copy Markdown
Member

ondrejmirtes commented Jun 18, 2023

Thank you! :)

@ondrejmirtes ondrejmirtes mentioned this pull request Jun 18, 2023
Closed
@herndlm herndlm deleted the bug-8449-retain-list-in-array-push-unshift branch June 18, 2023 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

list<int> is changed to array when using array_push with variadic parameter

3 participants

@herndlm @phpstan-bot @ondrejmirtes