Property for "dynamic" properties in macroable classes

[jordihelp]

In same cases, the macro functions need properties that do not exist in the macroable class. This forces you to create a property dynamically.

In recent versions of PHP, the dynamic creation of properties is deprecated. Specifically, the dynamic creation of class properties is deprecated as of PHP 8.2 and will result in a fatal error from PHP 9.0.

To solve this, I think it would be enough to add a property that allows you to set/get all these dynamic properties:

trait Macroable
{
    ...

    /**
     * The dynamic properties of macros.
     *
     * @var array
     */
    protected $macroProperties = [];

    /**
     * The dynamic static properties of macros.
     *
     * @var array
     */
    protected static $macroStaticProperties = [];

    ...
}

For example, a macro function with dynamic properties would be:

Route::macro('withDisabled', function ($withDisabled = true) {
    $this->macroProperties['withDisabledBindings'] = $withDisabled;
    
    return $this;
});

Route::macro('allowsDisabledBindings', function () {
    return $this->macroProperties['withDisabledBindings'] ?? false;
});

Discussions on the same topic:
#47337

[macropay-solutions]

Hi. We had this issue when solving this bug labeled "corner case". We solved it with a global property.
Your idea seems nice but it could introduce possible clash with the class where the trait is being used if that class has the same property name. This makes your proposal a breaking change.

[macropay-solutions]

Another breaking change idea with lower impact would be to reuse

    /**
     * The registered string macros.
     *
     * @var array
     */
    protected static $macros = ['macros' => []];

and move all the current logic under 'macros' key but you could mess it up if you reuse the same key for your dynamic properties.

[jordihelp]

It's not a bad idea to avoid a breaking changes. Unfortunately, I've already tried this idea, but managing data was difficult.

For example, we have the macroable class App\Support\MacroableClass.

We can add a new dynamic static property isCallable in the $macros property like this:

/**
 * The registered string macros.
 *
 * @var array
 */
protected static $macros = [
    'App\Support\MacroableClass_isCallable' => true
];

The problem is the non-static properties:

1. Generate a different key for each instance. I try to use the spl_object_id method for get the ID of the object, but the IDs could be reused when the object is destroyed, so it's not safe.
2. Keep the property values ​​when copying an instance. I have not found a way to get the property values ​​to be "copied" to the new instance.
3. Clean up the trash when the instance is destroyed. The property values are not reliably erased, causing a memory leak problem.

I proposed the other idea to avoid these problems, but, as you say, it is a breaking change.

Perhaps it would be better to propose this change for next Laravel version. In that case, it wouldn't matter if it was a breaking change.

[macropay-solutions]

@jordihelp we solved this by forking laravel/lumen and NOT using new but \app(Class::class) for all (possible) classes that have the macroable trait. In that way the developer has control over the class and can register a child in DI that contains the needed property. It will be a miracle if laravel would do this in the core in the future.

[macropay-solutions]

For laravel, you can do the same for the Validator class for example. We did it to fix the last validation vulnerability for laravel 8-9 and 10 (10.48.29 fixes that also in 10 but it was released after we fixed it retroactively). Basically only the service provider for validation has to be changed.

[jordihelp]

It seems that this is currently the only solution.

I still think adding "dynamic" properties would increase the versatility of the macros, but as you say, it's unlikely Laravel will add them.

[shaedrich]

Maybe, this could be solved by introducing a HigherOrderMacroProxy, which would either

• set existing property on the underlying class
• or call a setter to set $macroProperties as suggested on the proxy

This would require

• The proxy to be a singleton that would be kept once initialized
• the underlying class to have a __get fallback to the HigherOrderMacroProxy

[Sophist-UK]

The problem with avoiding all backwards compatibility is that you get stuck in the mud applying band-aid solutions -- and a HigherOrderProxie is essentially a complicated workaround to avoid a very minor potential incompatibility in property names.

IMO breaking changes should be evaluated as to whether they would require major or minor work, and providing that the work needed is reasonable and the change is made in a new major version, then what's the problem?

We all know from experience that properly handled breaking changes are acceptable - just look at how PHP 4 migrated itself to PHP 8 through a lengthy series of deprecated features release on release.

So, choose property names that are complex and so unlikely to be used in existing projects (e.g. rather than $macroProperties/$macroStaticProperties use $dynamicMacroPropertiesExtensibleList/$dynamicMacroPropertiesStaticExtensibleList), and then deliver the change in a major version with a warning that if you happen to use these properties then you need to rename them and create a Rector rule to automatically do such a rename.