whateverthing.com

☰

Dynamic Properties on classes have been possible in PHP for quite some time. They have been an ill-advised practice for equally as long.

When you have a class, you can assign values to non-existent properties and the system will automagically create properties so that your assignment doesn't fail.

For example:

<?php

class SampleClass {
    public function doTheThing(): void
    {
        $this->theThing = 'done';
    }

    public function isTheThingDone(): bool
    {
        return (bool)$this->theThing
            ?? false;
    }
}

This code would work fine in PHP 8, PHP 7, PHP 5 ... but it pains me even to read it. (Sorry, melodrama alert.) That class is missing a declared property called theThing. PHP creates it in the background.

You might say that that's helpful, or that it allows you to do lots of neat things. And you're not exactly wrong.

However, it is also a hindrance. It leads to maintenance problems. What happens if you add a property called theThing later, for a different purpose? Suddenly you have two entirely different codepaths using the same property for different purposes.

And what happens if you want to reference theThing directly, but you forget its exact name and use something like swampThing instead? The code would seem to work fine, but your typo might go unnoticed until a bug report is filed.

So these reasons and more have lead to the RFC to deprecate Dynamic Properties in PHP and eventually remove them at the language level.

Unfortunately, some projects - applications or libraries - may rely on this functionality to deliver their value. Those projects could require a lot of effort to find ways of mitigating the deprecation and removal of this functionality. And in this day and age of late-stage capitalism combined with an exhausting pandemic and the threat of climate catastrophe around the corner, people don't have time for this sort of thing. We've collectively run out of spoons.

There is a global spoon shortage.

It would be good for the language, to remove this ability. This sort of implicit magic makes it challenging to predict things about code written using patterns like this.

On the other hand, the human expense, the burden of the spoons, may be a valid argument for reassessing the deprecation and removal plans. Maybe there are other strategies that could move the language to a purer and more optimized implementation, while still allowing legacy projects to access this functionality?

Regardless, maintainers of projects need some way of quickly finding out if they might be impacted by this issue.

This is where Static Analyzers enter the picture. They have the ability to scan an entire codebase for all kinds of gasp-inducing sins, dynamic properties included.

Let's spin up an example project containing the above class, and see which static analysis tooling can help point out the problem.

I've used composer init to create a project called whateverthing/sample-dynamic-properties. I've taken the above SampleClass code and placed it inside the src/ folder, updating its namespace so it matches the autoloader that Composer helpfully defined.

Then I've created a script to exercise the above code, called go.php. It's pretty short and unpretentious:

<?php

require __DIR__ . '/vendor/autoload.php';

$obj = new Whateverthing\SampleDynamicProperties\SampleClass;

echo "Has the thing been done? "
    . ($obj->isTheThingDone() ? 'yes' : 'no')
    . "\n";
echo "Doing the thing ..." . "\n";
$obj->doTheThing() . "\n";
echo "Has the thing been done? "
    . ($obj->isTheThingDone() ? 'yes' : 'no')
    . "\n";

Now, when I run it under PHP 8.0, I see the following output:

# php go.php
PHP Warning:  Undefined property: Whateverthing\SampleDynamicProperties\SampleClass::$theThing in /home/sample/projects/whateverthing-sample-dynamic-properties/src/SampleClass.php on line 11
PHP Stack trace:
PHP   1. {main}() /home/sample/projects/whateverthing-sample-dynamic-properties/go.php:0
PHP   2. Whateverthing\SampleDynamicProperties\SampleClass->isTheThingDone() /home/sample/projects/whateverthing-sample-dynamic-properties/go.php:7

Warning: Undefined property: Whateverthing\SampleDynamicProperties\SampleClass::$theThing in /home/sample/projects/whateverthing-sample-dynamic-properties/src/SampleClass.php on line 11

Call Stack:
    0.0002     395400   1. {main}() /home/sample/projects/whateverthing-sample-dynamic-properties/go.php:0
    0.0012     477400   2. Whateverthing\SampleDynamicProperties\SampleClass->isTheThingDone() /home/sample/projects/whateverthing-sample-dynamic-properties/go.php:7

Has the thing been done? no
Doing the thing ...
Has the thing been done? yes

Whoa, what's all this? That's a lot of warning text. Seems like anyone who has been using dynamic properties up until now might have been ignoring some pretty important messaging. Either that or I've misunderstood the common usage pattern for dynamic properties. Who knows? But I was able to confirm that this warning goes back to at least PHP 7.2. You ARE auditing your code to monitor and remove warnings, right? (I'm pretty sure most companies and projects try to minimize warnings, but they're likely not OCD about it)

Anyway. Back to the task at hand. I'm going to start by checking out PHPStan's ability to detect this issue, so I install it with composer require --dev phpstan/phpstan.

Sure enough, running vendor/bin/phpstan analyze src/SampleClass.php returns some useful output:

1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ -------
  Line   SampleClass.php
 ------ -------
  7      Access to an undefined property Whateverthing\SampleDynamicProperties\SampleClass::$theThing.
  11     Access to an undefined property Whateverthing\SampleDynamicProperties\SampleClass::$theThing.

 [ERROR] Found 2 errors

Good, it found what we wanted it to find.

Now let's try Phan, another tool for static analysis. This one required a bit more configuration, and an extra flag to use an AST polyfill for scanning the abstract syntax tree, because I didn't have the php-ast extension installed.

# vendor/bin/phan --init
# vendor/bin/phan --allow-polyfill-parser

[info] Disabling Xdebug: Phan is around five times as slow when Xdebug is enabled (Xdebug only makes sense when debugging Phan itself)
[info] To run Phan with Xdebug, set the environment variable PHAN_ALLOW_XDEBUG to 1.
[info] To disable this warning, set the environment variable PHAN_DISABLE_XDEBUG_WARN to 1.
[info] To include function signatures of Xdebug, see .phan/internal_stubs/xdebug.phan_php
[debug] Checking PHAN_ALLOW_XDEBUG
[debug] Because xdebug was installed, Phan will restart.
[debug] The Xdebug extension is loaded (3.0.4) mode=develop
[debug] Process restarting (PHAN_ALLOW_XDEBUG=internal|3.0.4|1|*|*)
[debug] Running /opt/local/bin/php80 -n -c /private/var/folders/jv/j8t0hwbd7_qbl89wjkymf7380000gn/T/Kpot39 vendor/bin/phan --allow-polyfill-parser
   analyze ███████████████████ 100.0% 359MB/363MB
src/SampleClass.php:7 PhanUndeclaredProperty Reference to undeclared property \Whateverthing\SampleDynamicProperties\SampleClass->theThing
src/SampleClass.php:11 PhanUndeclaredProperty Reference to undeclared property \Whateverthing\SampleDynamicProperties\SampleClass->theThing
[debug] Restarted process exited 1

This one found what we wanted, too. And both of them are doing this with an out-of-the-box implementation.

Seems like it should be pretty easy for maintainers to find out if their project has this issue. But how easy is it to fix?

Well, that depends on why dynamic properties are being used - and how extensively. The Dynamic Properties RFC has a couple of recommendations - the easiest being to declare the properties, the second-easiest being to declare a map property and override __get/__set magic methods to interact with it. There's also a tricksy way of doing it that involves declaring the property, unsetting it in the constructor, and then setting it in the __get when it is requested.

My personal opinion on this RFC is that I like the idea, but the mounting complaints over the speed of PHP's development cycle is concerning. It's good for a language to grow and mature, don't get me wrong, but changing too much too quickly has a tendency to leave people on shaky ground about their own knowledge and investment in the language.

If the goal is to make the language more approachable, this seems like it would be low on the list of places where newcomers might stub their toes.

I would suggest either delaying, or looking into a different approach entirely. Maybe an INI setting combined with an extension or a userland package that restores the original functionality. The RFC talks about an Attribute that can enable the old functionality; maybe an INI setting could cause it to be applied to all objects (albeit at a performance cost)?

Barring that, rolling forward with the plan as-is is fine as well. It just might bring some blowback and unanticipated consequences with it... kind of like using Dynamic Properties themselves.

UPDATE (2021-11-15):

Benjamin Eberlei points out a flaw in the Static Analysis approach, which is when variable-variables are used to update dynamic properties, e.g., populating a data object from a database result.

Static Analyzers cannot detect one important case of setting dynamic properties such as code setting columns from database on objects. $object->$rowName = $value;

The static analyzer argument has no merrit as an argument against Deprecating Dynamic Properties.

— Benjamin Eberlei (@beberlei) November 15, 2021

I was able to 100% confirm this. I created a DbSampleClass.php:

<?php

namespace Whateverthing\SampleDynamicProperties;

class DbSampleClass {
    public function __construct(array $data)
    {
        foreach ($data as $key => $value) {
            $this->$key = $value;
        }
    }
}

It uses the key as a variable-variable to set the dynamic property. Technically, since it's not using the $$variable definition, it might not be a true variable-variable - but the underlying operation is very similar. Perhaps it could be considered a property-variable?

Anyway - next, I created db-go.php to load the class:

<?php

require __DIR__ . '/vendor/autoload.php';

$obj = new Whateverthing\SampleDynamicProperties\DbSampleClass(
    [
        'Row1' => 'value1',
        'Row2' => 'value2',
    ]
);

echo "What is Row1? " . $obj->Row1 . "\n";
echo "What is Row2? " . $obj->Row2 . "\n";

I ran php db-go.php and got this output:

# php db-go.php
What is Row1? value1
What is Row2? value2

No warnings.

Then I ran both phpstan and phan against the file.

Both of them gave it a 100% passing grade. No issues detected.

So that's worrying, and points to the issue being even less cut-and-dry than some are suggesting.