Restrict Union and Intersection types in public methods

[barbazul]

Rule

These examples should be marked as an error:

interface FooInterface {
    /**
     * Union return type
     */
    public function method1(): Bar|Baz;

    /**
     * Intersection return type
     */
    public function method1(): Bar&Baz;
}

Reason

Existing code generation in Magento does not support either Union Types or Intersection Types

https://github.com/magento/magento2/issues/34264

Using these in public methods has unpredictable consequences that range from broken code to Interceptor classes silently not being generated.

Implementation

I am not familiar with writing Sniffs, but I guess the use of the tokens T_TYPE_UNION or T_TYPE_INTERSECTION should be forbidden as return types in public methods.

[m2-assistant[bot]]

Hi @barbazul. Thank you for your report. To speed up processing of this issue, make sure that you provided sufficient information.

Add a comment to assign the issue: @magento I am working on this

---

• Join Magento Community Engineering Slack and ask your questions in #github channel.

[ihor-sviziev]

Hi @barbazul, We should not add such a check to the coding standard. Instead, we should fix the issue in the Magento core.

[barbazul]

I could not agree more.

However it doesn't seem to be a priority as you can see by the tags in the linked issue ("Feature Request", "Priority P2").

So, in order to not leave this bear trap open for unsuspecting PHP 8 developers to step on, I propose we add a rule to catch the issue sooner rather than later.

[ihor-sviziev]

@barbazul i think instead of developing a rule that will highlight the potential issue, it's better to invest this time to fixing at least one of these issues in magento itself. That's why I think we should close this issue

[ihor-sviziev]

@barbazul Since no new arguments have been added since last week, I assume we agreed that we shouldn't implement such a rule, so I'm closing this issue.

[barbazul]

We have not come to such agreement and I strongly disagree with your proposal of not adjusting the Coding Standards to match the features currently supported by Magento as I already stated in prior comments.

However I cannot provide any further arguments to support my point without repeating myself.

[fredden]

It doesn't seem efficient to create a coding standard rule to try to avoid every bug, and then remember to remove the rule when the bug is fixed.

In the case where a bug is detected, a rule is created, then the bug is fixed and the rule is removed: anyone who updates the coding standard but not the module where the bug existed (perhaps because a fixed version of the module hasn't yet been released) won't benefit from the temporary rule anyway. As Magento versions are not released often (and uptake of new versions isn't always quick) this makes the problem with temporary rules even more acute.

[barbazul]

At the moment, this is not considered a bug in the platform repository, but a requested feature.

[fredden]

You're welcome to replace "bug" with "lack of feature" in my comment regarding temporary rules if that makes it easier for you. The sentiment still holds true.

Let's go get these GitHub issues resolved, whatever their labels.