Skip to content

Change Protector class member detector to private #7

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

Closed
Leonede opened this issue Jul 18, 2016 · 5 comments
Closed

Change Protector class member detector to private #7

Leonede opened this issue Jul 18, 2016 · 5 comments

Comments

@Leonede
Copy link

Leonede commented Jul 18, 2016

Hello, I found not logic rule.
https://github.com/magento/marketplace-eqp/blob/develop/MEQP2/Sniffs/PHP/ProtectedClassMemberSniff.php
Comment in line 13 says "Detects possible usage of 'private' scope modifiers.", and whole class detecting 'protected'.
To fast fix: change return [T_PROTECTED]; to return [T_PRIVATE]; in line 32. And protected $warningCode = 'FoundProtected'; to protected $warningCode = 'FoundPrivate';

Same on Russian:
Похоже что произошла ошибка, PHPCS с стилем MEQP2 говорит мне что protected переменные не желательны. А судя по комменту в коде детектора, должно искать private, а не protected.

Fixed file:
ProtectedClassMemberSniff.php.txt

@ikruchynskyi
Copy link
Contributor

@Leonede , thank you for reporting. We will fix it in the next commit.

@lenaorobei
Copy link
Contributor

Fixed typo in comment 810c149.

@Leonede
Copy link
Author

Leonede commented Jul 19, 2016

I expecting it will be another fix..
So, now in Magento 2, protected methods and variables are not consumable? Which one I need to use then?
Is private OK so?

@lenaorobei
Copy link
Contributor

@Leonede, yes, all nop-public methods and properties should be private. For Magento 2 extension development we recommend using object composition over class inheritance. It makes your extension easier to maintain when class changes occur and update when new features need to be implemented. Follow here for the detailed explanation.

lenaorobei added a commit that referenced this issue Oct 10, 2016
* TIGER-1293: Add severity to existing MEQP sniffs

- Added severities to all MEQP sniffs;
- Fixed PHPDoc blocks;
- Merged GetFirstItemSniff and FetchAllSniff into one InefficientMethodsSniff;
- Merged ArrayCountSniff and StrlenSniff into one EmptyCheckSniff.
@devamitbera
Copy link

@Leonede , it means if a protected method is which is existing at extended class,does not override by my own class same rename method?

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

No branches or pull requests

4 participants