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.

Sign up for GitHub

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

Jump to bottom

[Transform] Move ReservedFnFunctionRector from Php74 to Transform namespace #1423

Merged
merged 1 commit into from
Dec 9, 2021
Merged

[Transform] Move ReservedFnFunctionRector from Php74 to Transform namespace #1423

merged 1 commit into from
Dec 9, 2021

Conversation

samsonasik
Copy link
Member

ReservedFnFunctionRector need to be configured to run, while registered in Php74 config set is not configured.

I think it should be moved to Transform namespace.

@samsonasik
Copy link
Member Author

Another solution is to unregister it from php74.php only, like AnnotationToAttributeRector for php8 which not registered in php80.php config set.

@samsonasik samsonasik mentioned this pull request Dec 8, 2021
Closed
@samsonasik
Copy link
Member Author

I created alternative PR for only unregister ReservedFnFunctionRector from php74 config set #1424

@samsonasik
Copy link
Member Author

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

@TomasVotruba
Copy link
Member

ReservedFnFunctionRector need to be configured to run, while registered in Php74 config set is not configured.

Agreed. The rule name is misleading. It should be probably renamed to ReservedKeywordRector. Then configured for php74.php set with "fn" keyword

@samsonasik
Copy link
Member Author

what replacement of fn if it is configured in php74.php config set?

@TomasVotruba
Copy link
Member

Not sure. I would check the RFC, how they handled it.

@TomasVotruba
Copy link
Member

TomasVotruba commented Dec 9, 2021 •
edited
Loading

Mergin to move this forward.

I've checked the RFC and the fn was renamed to f there: /~https://github.com/php/php-src/pull/3941/files#diff-2bc3b423036f43533a55430d03d811a9998fb0e52571d238ea84d4a50b007c7bR6

In next PR, this rule should be renamed and registered to php74.php with configuration: fn => f

@TomasVotruba TomasVotruba merged commit 7e13d0c into main Dec 9, 2021
@TomasVotruba TomasVotruba deleted the move-reserved-fn-transform branch December 9, 2021 14:24
@samsonasik
Copy link
Member Author

samsonasik commented Dec 9, 2021 •
edited
Loading

@TomasVotruba thank you, if function named f exists, change to f1, f2, etc ?

@TomasVotruba
Copy link
Member

Yes, that would be ideal 👍

mallardduck added a commit to mallardduck/rector-src that referenced this pull request Dec 9, 2021
@mallardduck
Merge branch 'main' into php82-remove-allow-dynamic-attribute
ee69839 
* main: (50 commits)
  Add SimpleStaticType for return type completion (rectorphp#1441)
  [Transform] Move ReservedFnFunctionRector from Php74 to Transform namespace (rectorphp#1423)
  [Php80] Add AddParamBasedOnParentClassMethodRector (rectorphp#1431)
  [DowngradePhp80] Add DowngradeRecursiveDirectoryIteratorHasChildrenRector (rectorphp#1440)
  [Scoped] Skip symfony console Event and EventListener and exclude in parallel-lint (rectorphp#1439)
  [Scoped] Fix exclude symfony/console/Tester (rectorphp#1438)
  [Scoped] Require symfony-eventdispatcher on build (rectorphp#1437)
  [Scoped] Fix downgrade error on symfony/console/Tester (rectorphp#1436)
  [Scoped] Fix downgrade error on symfony/Event* (rectorphp#1435)
  [TypeDeclaration][Php 8.1] Add Closure support to ReturnNeverTypeRector (rectorphp#1434)
  [composer] require own rector packages in dev-main, to avoid endless tagging (rectorphp#1433)
  [TypeDeclaration][Php 8.1] Do not change parent function to never on ReturnNeverTypeRector (rectorphp#1432)
  [DX] Improve configuration objects + allow Symfony 6 (rectorphp#1430)
  Require Symplify 10.0.2, remove stringy deprecated package from installed dependencies 🎉 (rectorphp#1429)
  Bump to Symplify 10.0.1 stable (rectorphp#1428)
  [DX] Decouple AssignVariableNameResolver (rectorphp#1427)
  [DowngradePhp80] Fix DowngradeThrowExprRector on property fetch to use not isset (rectorphp#1426)
  [DX] Replace deprecated danielstjules/stringy with Symfony\String (rectorphp#1425)
  [Alternative][Validation] Add AllowEmptyConfigurableRectorInterface interface for alternative to not show warning on allowed rector rule to have default fallback config (rectorphp#1422)
  [DX] Less addNodeAfterNode() methods (rectorphp#1421)
  ...
@samsonasik samsonasik mentioned this pull request Dec 12, 2021
Merged
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.
2 participants
@samsonasik @TomasVotruba