Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add support for DenormalizerInterface::denormalize #54

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

Merged
lookyman merged 2 commits into phpstan:master from tyx:feature/denormalizer-interface
Oct 6, 2019

Conversation

@tyx
Copy link

@tyx tyx commented Jul 8, 2019

Hi people,

As SerializerInterface, DenormalizerInterface has the same trouble.

So I just copy the code and adapt it to work with DenormalizerInterface.

jvasseur, alsciende, and NavyCoat reacted with thumbs up emoji
Copy link
Member

Hi, if the code is the same it shouldn't be done with copy-pasting source code but with reusing the same class as a different DI service.

@tyx tyx force-pushed the feature/denormalizer-interface branch from 028dbc8 to d96eaa0 Compare July 8, 2019 12:21
Copy link
Author

tyx commented Jul 8, 2019

Yes only getClass and isMethodSupported methods change.

But I'm not sure to know what you mean behind

reusing the same class as a different DI service.

You want to inject through the constructor the variable stuff ? or by inheritance and override concerned methods ?

@tyx tyx force-pushed the feature/denormalizer-interface branch from d96eaa0 to 4a4f888 Compare July 8, 2019 12:28
Copy link
Member

Injecting in constructor. Register it in extension.neon multiple times with different arguments.

tyx and NavyCoat reacted with thumbs up emoji

@tyx tyx force-pushed the feature/denormalizer-interface branch 2 times, most recently from 98c6d26 to d369fe8 Compare July 8, 2019 13:31
@tyx tyx force-pushed the feature/denormalizer-interface branch from d369fe8 to 4073030 Compare July 8, 2019 14:23
Copy link
Author

tyx commented Jul 8, 2019

Ok, here it is !

But I'm not sure to know why tests with highest dependencies fail. If anyone can help on this, please let me know.

PHPStan\Type\Symfony\EnvelopeReturnTypeExtensionTest::testAll with data set #0 ('$test1', 'array<Symfony\Component\Messe...Stamp>')
ArgumentCountError: Too few arguments to function PHPStan\Analyser\NodeScopeResolver::__construct(), 9 passed in /home/travis/build/phpstan/phpstan-symfony/tests/Type/Symfony/ExtensionTestCase.php on line 55 and exactly 10 expected
/home/travis/build/phpstan/phpstan-symfony/vendor/phpstan/phpstan/src/Analyser/NodeScopeResolver.php:140
/home/travis/build/phpstan/phpstan-symfony/tests/Type/Symfony/ExtensionTestCase.php:55
/home/travis/build/phpstan/phpstan-symfony/tests/Type/Symfony/EnvelopeReturnTypeExtensionTest.php:21

Copy link
Member

ondrejmirtes commented Jul 8, 2019 via email

That’s an unrelated error from newer version of PHPStan.
On Mon, 8 Jul 2019 at 16:31, Timothée Barray ***@***.***> wrote: Ok, here it is ! But I'm not sure to know why tests with highest dependencies fail. If anyone can help on this, please let me know. PHPStan\Type\Symfony\EnvelopeReturnTypeExtensionTest::testAll with data set #0 ('$test1', 'array<Symfony\Component\Messe...Stamp>') ArgumentCountError: Too few arguments to function PHPStan\Analyser\NodeScopeResolver::__construct(), 9 passed in /home/travis/build/phpstan/phpstan-symfony/tests/Type/Symfony/ExtensionTestCase.php on line 55 and exactly 10 expected /home/travis/build/phpstan/phpstan-symfony/vendor/phpstan/phpstan/src/Analyser/NodeScopeResolver.php:140 /home/travis/build/phpstan/phpstan-symfony/tests/Type/Symfony/ExtensionTestCase.php:55 /home/travis/build/phpstan/phpstan-symfony/tests/Type/Symfony/EnvelopeReturnTypeExtensionTest.php:21 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#54?email_source=notifications&email_token=AAAZTOHFPSFWLK5QETWZNRLP6NFV3A5CNFSM4H63BJY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZNITLY#issuecomment-509249967>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAZTOGVDWE24ZMYG4FIQNTP6NFV3ANCNFSM4H63BJYQ> .
-- Ondřej Mirtes

Copy link
Author

tyx commented Jul 8, 2019
edited
Loading

Indeed here is the commit that breaks test highest deps :
phpstan/phpstan@22f904a

I updated to change ExtensionTestCase by adding true to the NodeScopeResolver constructor.

edit: unfortunately this change breaks the lowest deps.

Please let me know what is the best solution for you.

Copy link
Collaborator

lookyman commented Jul 8, 2019

I'll take a look tonight. We might have to do some fancy schmancy magic to accomodate for both old and new PHPStan versions..

tyx reacted with thumbs up emoji

Copy link
Member

You should open 0.12-dev before merging this, that will make your job easier.

Copy link

Any hope this will be merged soon?

Copy link

NavyCoat commented Oct 1, 2019

@tyx @lookyman what's need to be done here? Can I help?

Copy link
Author

tyx commented Oct 2, 2019

We still need to find a way to make both world (lowest and highest deps) green. I should admit I am using my fork for now as I failed to make it green ^^

Copy link
Collaborator

lookyman commented Oct 2, 2019

Guys, sorry, I completely forgot about this. I will fix and merge it this weekend during a hackathon. Thank you for your patience.

tyx reacted with thumbs up emoji

@lookyman lookyman merged commit 7d34a13 into phpstan:master Oct 6, 2019
Copy link
Collaborator

lookyman commented Oct 6, 2019

Thanks!

tyx reacted with hooray emoji

@tyx tyx deleted the feature/denormalizer-interface branch October 7, 2019 07:29
Copy link
Author

tyx commented Oct 7, 2019

Great! Thanks for your help!

Copy link
Author

tyx commented Oct 7, 2019

I guess we are close to publish a new release ?
0.11.6...master

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.

AltStyle によって変換されたページ (->オリジナル) /