-
Notifications
You must be signed in to change notification settings - Fork 154
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
[New Rule] Controllers Should Implement HTTP Method Marker Interface #34
Comments
Hello @sprankhub! Thanks for creating this proposal. Would you mind to join public Magento architecture meeting to discuss this rule? If you are interested please add this topic as a comment to the corresponding GitHub issue. |
I will try, but I will probably not be able to join the meeting. Could you add it to the list of new rules in your topic suggestion nevertheless @lenaorobei? Thanks! |
@sprankhub, sure. I'll add this topic. |
I've created PHPMD rule when I introduced HTTP method marker interfaces, isn't that enough? |
@AlexMaxHorkun good to know, thanks. |
This rule cannot be implemented using PHP CodeSniffer without false-positive findings. As per PHP Stan Proposal this rule can be covered by PHPStan. |
@lenaorobei could you elaborate why this is not possible without false positives? If this can/should be covered with PHPStan, should we add this proposal somewhere else, so that we do not forget about it? |
Rule
Controllers SHOULD implement one of the HTTP method marker interfaces like
HttpPostActionInterface
(see https://github.com/magento/magento2/tree/2.3/lib/internal/Magento/Framework/App/Action).Reason
Source: Magento DevDocs About Routing
Implementation
Check for subclasses of
\Magento\Framework\App\Action\AbstractAction
or classes, which implement\Magento\Framework\App\ActionInterface
and check if these classes also implement one of the marker interfaces.Important Details
This sniff should be applicable to Magento >=2.3, because older versions of Magento do not have HTTP method marker interfaces.
The text was updated successfully, but these errors were encountered: