Use static:: to support late static bindings in Invoice and Creditmemo

[jokeputs]

Description

When using self:: to call static properties and static methods late static binding (see http://php.net/manual/en/language.oop5.late-static-bindings.php) will not be supported. Instead static:: should be used.
When a class inherits from Magento\Sales\Model\Order\Invoice or Magento\Sales\Model\Order\Creditmemo and has its own implementation of the static method (or property) this implementation might not be called.

This is not a bug in the Magento code base but it would improve the code for customisation.

Example

I have class ExtendedInvoice which inherits from Invoice. It has its own implementation of getStates() because I want to add an extra state.

<?php
class ExtendedInvoice extends \Magento\Sales\Model\Order\Invoice
{
    const STATE_EXTRA = 4;

    /**
     * This method will not be used when getStateName() is called
     */
    public static function getStates()
    {
        if (null === self::$_states) {
            self::$_states = [
                self::STATE_OPEN => __('Pending'),
                self::STATE_PAID => __('Paid'),
                self::STATE_CANCELED => __('Canceled'),
                self::STATE_EXTRA => __('Extra State'),
            ];
        }
        return self::$_states;
    }
}

Now when getStateName() is called on an instance of ExtendedInvoice, the getStates() method of the parent class will be used instead of my implementation and thus my extra state will be unknown.

When changing self:: to static:: as in my commit the correct implementation of getStates() will be called.

[magento-cicd2]

All committers have signed the CLA.

[ihor-sviziev]

Instead of replacing self::$_states you could use plugins (interceptors) for your needs.

[korostii]

@ihor-sviziev, have you even read the doc you've just linked? Seems not.

Limitations
Plugins cannot be used with any of the following:
...
Static methods
...

Let's not be too presumptuous about plugins: there are still valid cases when overriding the class via di.xml is a better (or sometimes, the only) solution. Hard-coded list of statuses here is a perfect example.

[ihor-sviziev]

@korostii you could create after plugin for method getStates and add your own state to result. You don't need to use static variables in these classes.

[korostii]

Except... getStates() is a static method and the doc you've referenced mentions that those cannot be intercepted (never tested myself though).

[ihor-sviziev]

@korostii ah, it's my mistake, sorry for that.
As for me your fix looks like workaround, and it doesn't fix your issue fully, as example you can't use plugins for this method.
I think in order to fix your issue we need to mark this static function and get list of states as deprecated, create new class that will manage states for credit memo with setting states in constructor + configuration in di.xml.

@vrann what do you think about this issue?

[orlangur]

Extending core classes is not a recommended way of customization nowadays.

Why getStates is static in the first place? Can we make it non-static or it's a BC break?

[ishakhsuvarov]

@orlangur I suspect it is a BC break at this point.

[orlangur]

@ishakhsuvarov ah, yeah, in such case we would force class instantiation. So, the only way we can go in long-term is to encapsulate states in separate new class and deprecate this method (but still use within this new class until BC break allowed).