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

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

Merged
merged 21 commits into from May 4, 2022

Conversation

TomasVotruba
Copy link
Contributor

@TomasVotruba TomasVotruba commented Feb 4, 2022

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
@TomasVotruba TomasVotruba force-pushed the tv-json-decode-narrow-type branch 3 times, most recently from 5547fb8 to 55cc2d4 Compare February 6, 2022 12:40
@TomasVotruba
Copy link
Contributor Author

I'm working on the static errors...

@ondrejmirtes
Copy link
Member

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

@TomasVotruba
Copy link
Contributor Author

It's ready from my side 👍

I'm actually waiting for feedback last 2 months:

image

@ondrejmirtes
Copy link
Member

It is not - there were conflicts and build failures.

@ondrejmirtes ondrejmirtes reopened this Mar 24, 2022
Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

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.

composer.json Outdated Show resolved Hide resolved
@TomasVotruba
Copy link
Contributor Author

I'm on it 👍

@TomasVotruba TomasVotruba force-pushed the tv-json-decode-narrow-type branch 3 times, most recently from 2e46788 to aafd2a3 Compare March 24, 2022 14:14
@TomasVotruba
Copy link
Contributor Author

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 TomasVotruba force-pushed the tv-json-decode-narrow-type branch 5 times, most recently from 1cac3b6 to 834eff1 Compare March 24, 2022 17:39
@TomasVotruba
Copy link
Contributor Author

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

@herndlm herndlm force-pushed the tv-json-decode-narrow-type branch from b8357ff to 961f9c2 Compare May 2, 2022 08:43
@ondrejmirtes ondrejmirtes merged commit 231990a into phpstan:1.6.x May 4, 2022
@ondrejmirtes
Copy link
Member

Thank you!

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

👍

@herndlm Thank you for finishing 👏

@herndlm
Copy link
Contributor

herndlm commented May 4, 2022

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

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