Skip to content

Conversation

@herndlm
Copy link
Contributor

@herndlm herndlm commented Apr 18, 2022

Sounds like a bigger general problem, but I figure it is more of an edge case. Found while working on something else. E.g. when building an array from scratch via

$builder = ConstantArrayTypeBuilder::createEmpty();
$builder->setOffsetValueType(null, new BooleanType(), true);
$builder->setOffsetValueType(null, new NullType(), true);
$builder->setOffsetValueType(null, new ConstantIntegerType(17));
$array = $builder->getArray();

I am expecting the result type to be array{0: 17|bool|null, 1?: 17|null, 2?: 17} because the last appended value is not optional any more. But it was still array{0?: 17|bool|null, 1?: 17|null, 2?: 17}. This PR fixes this. That exact example is added as test case.

@herndlm herndlm changed the base branch from 1.6.x to 1.5.x April 18, 2022 12:08
@herndlm herndlm marked this pull request as draft April 18, 2022 12:21
@herndlm
Copy link
Contributor Author

herndlm commented Apr 18, 2022

the fix is not correct and was just working by luck. I'll check later

@herndlm herndlm marked this pull request as ready for review April 18, 2022 13:13
@herndlm
Copy link
Contributor Author

herndlm commented Apr 18, 2022

Should be good now. And apparently somebody fixed CI as well. Good timing.

@ondrejmirtes
Copy link
Member

The CI isn't fixed, it's just that the bugs aren't in 1.5.x :)

@ondrejmirtes
Copy link
Member

Perfect, thank you!

@ondrejmirtes ondrejmirtes merged commit 06a5c26 into phpstan:1.5.x Apr 19, 2022
@herndlm herndlm deleted the fix-constant-array-type-builder-optional-keys branch April 19, 2022 07:11
@herndlm
Copy link
Contributor Author

herndlm commented Apr 19, 2022

Oh no, my stupid second commit message ended up being merged too. I think somebody forgot to squash-merge ;)

@ondrejmirtes
Copy link
Member

No worries 😊 If it was 20 bad commits I'd forcepush but this is fine.

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.

2 participants