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

Rename protected and private class members starting with an underscore #3004

Closed
morozov opened this issue Jan 30, 2018 · 4 comments
Closed

Comments

@morozov
Copy link
Member

morozov commented Jan 30, 2018

According to PSR-2, protected and private property and method names should not be prefixed with an underscore. Currently, we have some of them in the code base:

↪ git grep -P '(protected|private) (function |\$)_(?!_)' v2.8.0 | wc -l
284

In order to fix the coding standard violations, they should be renamed which would be backward incompatible. A major release could be used to make such a change.

@morozov
Copy link
Member Author

morozov commented Oct 2, 2018

Partially fixed in #3303 and #3306:

↪ git grep -P '(protected|private) (function |\$)_(?!_)' b45ed5e | wc -l
188

@Majkl578
Copy link
Contributor

Majkl578 commented Oct 3, 2018

For the rest of protected underscored members, imho the best approach would be to make them private and expose protected constructor instead(#3007 (comment)). Should this be too annoying, there could be a __get() polyfill of some sort.
@Ocramius @morozov Thoughts?

@morozov
Copy link
Member Author

morozov commented Oct 3, 2018

For the rest of protected underscored members, imho the best approach would be to make them private and expose protected constructor instead

@Majkl578 you're still talking about breaking changes, right? Some of the currently protected variables like SQLSrvConnection::$conn are private by design, so we just need to declare them private. Also, I agree with #3007 (comment). We should see if we can make eligible classes final and then decide what to do with the rest of protected APIs. The style change will become a byproduct of this cleanup.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants