Skip to content

[Downgrade] Apply trailing commas rule also on func/method/static call#4878

Merged
TomasVotruba merged 10 commits intomasterfrom
downgrade-trailing-commas-in-constructor-issue
Dec 14, 2020
Merged

[Downgrade] Apply trailing commas rule also on func/method/static call#4878
TomasVotruba merged 10 commits intomasterfrom
downgrade-trailing-commas-in-constructor-issue

Conversation

@leoloso
Copy link
Copy Markdown
Contributor

@leoloso leoloso commented Dec 14, 2020

DowngradeTrailingCommasInParamUseRector was downgrading code on the definition of the function only, but not when calling the function:

var_dump('foo',);

This PR fixes it:

-var_dump('foo',);
+var_dump('foo');

@leoloso leoloso marked this pull request as draft December 14, 2020 08:57
@leoloso
Copy link
Copy Markdown
Contributor Author

leoloso commented Dec 14, 2020

Must be fixed also for constructor calls:

$object = new ConfiguredCodeSample(
    $codeSampleFrom,
-    $codeSampleTo,
+    $codeSampleTo
);

Working on it now

@leoloso leoloso marked this pull request as ready for review December 14, 2020 09:11
@simivar
Copy link
Copy Markdown
Contributor

simivar commented Dec 14, 2020

Are your PRs based on each other? I see the same changes here as in #4877 and #4879

@leoloso
Copy link
Copy Markdown
Contributor Author

leoloso commented Dec 14, 2020

Nops, they are all based on master. All those same changes are being added by the Rector Rectify bot

@TomasVotruba
Copy link
Copy Markdown
Member

Thank you 👍

Could you make a failing test fixture for the class const fetch rename?

@TomasVotruba TomasVotruba merged commit 78c619b into master Dec 14, 2020
@TomasVotruba TomasVotruba deleted the downgrade-trailing-commas-in-constructor-issue branch December 14, 2020 11:40
@leoloso
Copy link
Copy Markdown
Contributor Author

leoloso commented Dec 15, 2020

Could you make a failing test fixture for the class const fetch rename?

Sorry for asking, but I'm a bit confused. You mean to test downgrading new RenameClassConstant('SomeClass', 'OLD_CONSTANT', 'NEW_CONSTANT', )? (it won't fail). Or to test the ClassConstFetch node? (It doesn't receive params). Or if not, what?

@TomasVotruba
Copy link
Copy Markdown
Member

For these #4878 (comment)

TomasVotruba added a commit that referenced this pull request Aug 29, 2023
rectorphp/rector-src@6424bae [TypeDeclaration] Using ClassMethodReturnTypeOverrideGuard on ReturnTypeFromStrictParamRector (#4878)
TomasVotruba added a commit that referenced this pull request Aug 29, 2023
rectorphp/rector-src@6424bae [TypeDeclaration] Using ClassMethodReturnTypeOverrideGuard on ReturnTypeFromStrictParamRector (#4878)
TomasVotruba added a commit that referenced this pull request Aug 29, 2023
rectorphp/rector-src@6424bae [TypeDeclaration] Using ClassMethodReturnTypeOverrideGuard on ReturnTypeFromStrictParamRector (#4878)
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Apr 26, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants