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

[TypeDeclaration] Add AddParamTypeFromCallersRector #5782

Merged
merged 7 commits into from
Mar 6, 2021

Conversation

TomasVotruba
Copy link
Member

No description provided.

@TomasVotruba TomasVotruba force-pushed the types-only-type branch 3 times, most recently from b067afb to 13f6191 Compare March 6, 2021 20:41
@TomasVotruba TomasVotruba enabled auto-merge (squash) March 6, 2021 20:41
@TomasVotruba TomasVotruba merged commit 031deda into master Mar 6, 2021
@TomasVotruba TomasVotruba deleted the types-only-type branch March 6, 2021 21:16
@staabm
Copy link
Contributor

staabm commented Mar 7, 2021

Wouldnt it make sense to reduce this rector (optional?) to only process private methods?

that way the public api of the classes won‘t get touched and therefore the transformation can be done easily

@TomasVotruba
Copy link
Member Author

For private projects it does not make sense.
But for opened source, it would. There is option PROJECT_TYPE could active it: https://github.com/symplify/symplify/blob/0f796431bc04adad1db35f0af212da72aa6e5635/rector.php#L67

@staabm
Copy link
Contributor

staabm commented Mar 7, 2021

My comment was more regarding private vs public visibility of methods, rather then projects

@TomasVotruba
Copy link
Member Author

Understood, I was reacting to that. Public and private methods are the same in scope of private project. Only current code base is using them, so all passed types are known.

In matter of open-source project, where input data are missing, it's valid point.

TomasVotruba added a commit that referenced this pull request Mar 31, 2024
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