Skip to content

Added tests for RenameVariableToMatchNewTypeRector #4898

Merged
TomasVotruba merged 6 commits intomasterfrom
added-tests-for-RenameVariableToMatchNewTypeRector
Dec 18, 2020
Merged

Added tests for RenameVariableToMatchNewTypeRector #4898
TomasVotruba merged 6 commits intomasterfrom
added-tests-for-RenameVariableToMatchNewTypeRector

Conversation

@leoloso
Copy link
Copy Markdown
Contributor

@leoloso leoloso commented Dec 15, 2020

As requested in #4879 (comment)

However, the tests are not failing in the expected manner:

  • Test with_method_call.php.inc should not fail, but it does
  • Test with_double_method_call.php.inc replicates the issue from Rector Rectify. It fails, but it a different way than expected: $flag was not renamed as $bitwiseOr, and $magicGet was left intact

Am I missing something?

@leoloso
Copy link
Copy Markdown
Contributor Author

leoloso commented Dec 15, 2020

Changes to fix, applied by Rector Rectify, can be seen here: 182aef1

@TomasVotruba
Copy link
Copy Markdown
Member

TomasVotruba commented Dec 15, 2020

The test has probably different scope then real file.

Is RenameVariableToMatchNewTypeRector the correct responsible rule?

@leoloso
Copy link
Copy Markdown
Contributor Author

leoloso commented Dec 18, 2020

You were right, I got the wrong rule. The actual one is RenameVariableToMatchMethodCallReturnTypeRectorTest.

There is too much going on. Please spilt it to standlone files per case

I had kept the added test so long, to find out what was different. Now that I found out the problem, I made the test just minimal.

@TomasVotruba
Copy link
Copy Markdown
Member

@leoloso Thank you 👍

@TomasVotruba
Copy link
Copy Markdown
Member

@samsonasik Could you check this one?

It should be either skipped or better renamed with type suffix, like this:

-$magicGet = $this->createClassConstFetch('SomeClass', 'MAGIC_GET');
+$magicGetClassConstFetch = $this->createClassConstFetch('SomeClass', 'MAGIC_GET');

-$magicSet = $this->createClassConstFetch('SomeClass', 'MAGIC_SET');
+$magicSetClassConstFetch = $this->createClassConstFetch('SomeClass', 'MAGIC_SET');

@samsonasik
Copy link
Copy Markdown
Member

I will try.

@samsonasik
Copy link
Copy Markdown
Member

fixed.

@samsonasik samsonasik force-pushed the added-tests-for-RenameVariableToMatchNewTypeRector branch from ef41f59 to af945d0 Compare December 18, 2020 19:21
@samsonasik
Copy link
Copy Markdown
Member

All checks have passed 🎉 @TomasVotruba it is ready for review.

@TomasVotruba TomasVotruba merged commit 20622c2 into master Dec 18, 2020
@TomasVotruba TomasVotruba deleted the added-tests-for-RenameVariableToMatchNewTypeRector branch December 18, 2020 22:18
@TomasVotruba
Copy link
Copy Markdown
Member

Thank you

TomasVotruba added a commit that referenced this pull request Sep 2, 2023
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.

4 participants