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

Fixed the third argument in VarDumperTestTrait. #2070

Merged

Conversation

adrozdek
Copy link
Contributor

@adrozdek adrozdek commented Oct 1, 2019

In VarDumperTestTrait's asserts the third argument is called $filter and by default it should be equal to 0.
Right now it's called $context or $format and is set to null. Also $data argument is called $format sometimes.

public function assertDumpEquals($expected, $data, $filter = 0, $message = '')
public function assertDumpMatchesFormat($expected, $data, $filter = 0, $message = '') -
https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/VarDumper/Test/VarDumperTestTrait.php

@TomasVotruba
Copy link
Member

TomasVotruba commented Oct 2, 2019

This rule is in Symfony 4.0 set. You address Symfony 4.4

When was this changed? Link to UPGRADE.md or PR in Symfony is needed. Maybe we need new rules to Symfony 4.4, we cannot change old ones

@adrozdek
Copy link
Contributor Author

adrozdek commented Oct 3, 2019

Sorry for giving 4.4 version but this parameter is the same for 4.0: https://github.com/symfony/symfony/blob/4.0/src/Symfony/Component/VarDumper/Test/VarDumperTestTrait.php
The UPGRADE-4.0.md for VarDumper is wrong. I created PR for changing that: symfony/symfony#33821

The description is wrong but the example taken from code is correct:
Zrzut ekranu z 2019-10-03 20-40-57

@TomasVotruba
Copy link
Member

I see, thanks for clarification

@TomasVotruba TomasVotruba merged commit 7226e7e into rectorphp:master Oct 4, 2019
TomasVotruba added a commit that referenced this pull request Apr 14, 2022
rectorphp/rector-src@d30a863 [Feature] Add configurable InlineSimplePropertyAnnotationRector for inlining of simple annotations (#2070)
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