Skip to content

[ReturnTypeExtension] add test case for narrow typed json_decode - #993

Merged
ondrejmirtes merged 21 commits into
phpstan:1.6.xfrom
TomasVotruba:tv-json-decode-narrow-type
May 4, 2022
Merged

[ReturnTypeExtension] add test case for narrow typed json_decode#993
ondrejmirtes merged 21 commits into
phpstan:1.6.xfrom
TomasVotruba:tv-json-decode-narrow-type

Conversation

@TomasVotruba

@TomasVotrubaTomasVotruba commented Feb 4, 2022

Copy link
Copy Markdown
Contributor

Port of https://github.com/phpstan/phpstan-nette/pull/89/files for json_decode()

Any feedback appreciated 👍

How to test?

vendor/bin/phpunit tests/PHPStan/Analyser/NodeScopeResolverTest.php --filter "json"

@TomasVotruba
TomasVotruba marked this pull request as ready for review February 4, 2022 14:15
Comment threadsrc/Type/Php/JsonThrowOnErrorDynamicReturnTypeExtension.php Outdated
@TomasVotruba
TomasVotrubaforce-pushed the tv-json-decode-narrow-type branch from 96ca0b7 to f682afaCompareFebruary 4, 2022 16:29
Comment threadsrc/Type/Php/JsonThrowOnErrorDynamicReturnTypeExtension.php Outdated
Comment threadtests/PHPStan/Analyser/data/json-decode/narrow_type_with_force_array.php Outdated
Comment threadtests/PHPStan/Analyser/data/json-decode/narrow_type_with_force_array.php Outdated
@TomasVotruba
TomasVotrubaforce-pushed the tv-json-decode-narrow-type branch 3 times, most recently from 5547fb8 to 55cc2d4CompareFebruary 6, 2022 12:40
@TomasVotruba

Copy link
Copy Markdown
ContributorAuthor

I'm working on the static errors...

@ondrejmirtes
ondrejmirtesforce-pushed the 1.5.x branch 3 times, most recently from ddd20b4 to 95d480bCompareMarch 18, 2022 19:53
@ondrejmirtes

Copy link
Copy Markdown
Member

Feel free to open a new PR if you ever decide to work on this again 👍 Thanks.

@TomasVotruba

Copy link
Copy Markdown
ContributorAuthor

It's ready from my side 👍

I'm actually waiting for feedback last 2 months:

image

@ondrejmirtes

Copy link
Copy Markdown
Member

It is not - there were conflicts and build failures.

@ondrejmirtesondrejmirtes left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

and please fix the conflict by cleanly rebasing the branch on top of 1.5.x. I'll review it once the build is green. Thanks.

Comment threadcomposer.json Outdated
@TomasVotruba

Copy link
Copy Markdown
ContributorAuthor

I'm on it 👍

@TomasVotruba
TomasVotrubaforce-pushed the tv-json-decode-narrow-type branch 3 times, most recently from 2e46788 to aafd2a3CompareMarch 24, 2022 14:14
@TomasVotruba

Copy link
Copy Markdown
ContributorAuthor

I've rebased. Tests, PHPStan and coding standards are passing localy.

Seems GitHub Actions are crashing now because cache is stuck: https://github.com/phpstan/phpstan-src/runs/5677735280?check_suite_focus=true

I'll check it today/tomorrow

@TomasVotruba
TomasVotrubaforce-pushed the tv-json-decode-narrow-type branch 5 times, most recently from 1cac3b6 to 834eff1CompareMarch 24, 2022 17:39
@ondrejmirtes
ondrejmirtesforce-pushed the tv-json-decode-narrow-type branch from 834eff1 to 2aa201aCompareMarch 25, 2022 06:31
@TomasVotruba

Copy link
Copy Markdown
ContributorAuthor

CI is passing 👍
What is now missing from my side?

@herndlm
herndlmforce-pushed the tv-json-decode-narrow-type branch from b8357ff to 961f9c2CompareMay 2, 2022 08:43
@ondrejmirtes
ondrejmirtesforce-pushed the tv-json-decode-narrow-type branch from 961f9c2 to 7d306fbCompareMay 3, 2022 15:56
@ondrejmirtes
ondrejmirtes merged commit 231990a into phpstan:1.6.xMay 4, 2022
@ondrejmirtes

Copy link
Copy Markdown
Member

Thank you!

@TomasVotruba
TomasVotruba deleted the tv-json-decode-narrow-type branch May 4, 2022 06:49
@TomasVotruba

Copy link
Copy Markdown
ContributorAuthor

👍

@herndlm Thank you for finishing 👏

@herndlm

Copy link
Copy Markdown
Contributor

Nice, thank you too. I was waiting for this getting merged, have a tiny follow-up idea :)

Sign up for freeto 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

@TomasVotruba@ondrejmirtes@herndlm@staabm