Technical guidelines > Abstract classes MUST NOT be marked as public @api

[lfluvisotto]

Technical guidelines > Abstract classes MUST NOT be marked as public @api

Description (*)

According to Technical guidelines > Abstract classes MUST NOT be marked as public @api.

https://devdocs.magento.com/guides/v2.3/coding-standards/technical-guidelines.html

Having the tag @api in abstract classes generates every time warning when using php code sniffer + coding standard magento.

As discussed in the comments of this pull request with @buskamuza, we cannot simply remove the @api tag from source code for compatibility with third party modules (extension developers).

However we cannot keep this warning if we want to have all source code covered and validated with php code sniffer + magento standard coding.

The solution is to add the line "phpcs: disable Magento2.Classes.AbstractApi" to all abstract classes that have the @api tag as has already been done in some abstract classes.

The purpose of this pull request is to increase and intensify coverage and validations with php code sniffer + magento standard coding.

Fixed Issues (if relevant)

N/A

Manual testing scenarios (*)

N/A

Questions or comments

N/A

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @lfluvisotto. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

[buskamuza]

We can't remove existing @api from abstract classes. If a class is marked with @api, it is considered necessary for extension developers. But for the new code, this should not happen.
In order to remove @api from existing classes, we should provide other ways of the same extensibility first, and allow people to migrate.

[lfluvisotto]

@buskamuza

So the "Technical guidelines" > "2.8. Abstract classes MUST NOT be marked as public @api" is incorrect ? Should we remove this item from devdocs ? Or the "MUST NOT" in the item 2.8 needs to be more detailed, for example, only for third party source code (third party modules) and not for magento source code ?

The php code sniffer with standard magento coding is already validating this rule and impacting us, every moment generates warning.

Do you think we'd better add "phpcs: disable Magento2.Classes.AbstractApi" in all abstract magento source code classes where there "@api" for php code sniffer to stop generating warnings?

[buskamuza]

Yes, I'd recommend to suppress validation for specific files.

Regarding the rule itself, no, I'd not remove it. New code should comply with it. But old code needs time and effort to be refactored, so that it can comply with Tech Guidelines. Having the rule helps to evolve code in the desired direction.

[ihor-sviziev]

This class already contains it. this line above, please fix duplicate

[ihor-sviziev]

Hi @lfluvisotto,
Your changes looks good. Could you fix static test?
Also note that static test failures related to not meaningful phpdoc blocks could be just removed.
Related PR in coding standard: magento/magento-coding-standard#171

[ihor-sviziev]

@lfluvisotto I am closing this PR now due to inactivity.
Please reopen and update if you wish to continue.
Thank you for the collaboration!

[m2-assistant]

Hi @lfluvisotto, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.