Sessions to be used only in controllers and blocks

[AlexMaxHorkun]

It is unsafe to use sessions anywhere but controllers and blocks due to multiple web APIs in Magento

[buskamuza]

6.2.2. Request, Response, Session, Store Manager and Cookie objects MUST be used only in the Presentation layer.

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

So, yes, this is correct and is in guidelines. And it would be great to cover it with a test.

[buskamuza]

1. Also a plugin for a block/controller should be OK to use sessions.
2. Would be good to expand the rule to cover "6.2.2. Request, Response, Session, Store Manager and Cookie objects MUST be used only in the Presentation layer.". Can be implemented as separate rules.
3. Also, data providers for UI components are part of presentation layer, so they can depend on session.

[AlexMaxHorkun]

1. makes sense, but I don't know how a block plugin can be identified in a PHPMD rule. Controller plugin can be sort of identified by having before/after/aroundExecute/Dispatch but I'm not sure if that would be correct. I think it would be OK if devs would just have to put SuppressWarnings for those cases
2. Ok, I'll update the proposal
3. Sure, instances of Magento\Framework\View\Element\UiComponent\DataProvider\Document and Magento\Framework\View\Element\UiComponent\DataProvider\DataProviderInterface will be ignored by the rule then

[buskamuza]

1. The plugin has $subject which can go through the same checks on whether it's a block or controller.

[AlexMaxHorkun]

I don't think a warning should be issued when using store manager - it can be used outside of presentation layer.
My goal with this proposal is to prevent unexpected behavior when using the same model-layer classes for both HTML and web API, and issuing warnings for request/response objects goes outside of my intent

[buskamuza]

Approved from my side.
Though the proposed rules don't cover all the rules from the Tech Guidelines, they bring a good improvement and allow simpler implementation w/o dealing with exceptional cases. So it makes sense to implement it as described for now.

[Vinai]

In general I agree, but I do have to say that \Magento\Customer\Model\Authorization\CustomerSessionUserContext makes REST API usage on the store front very convenient.

I think It should stick around for at least a few years still, until the GraphQL integration is mature enough and has effectively replaces the frontend REST API uses in extensions.

[AlexMaxHorkun]

@Vinai CustomerSessionUserContext is an exception case when we can't identify automatically whether a session object is used properly - CustomerSessionUserContext is fine and will stay