There is still a bug in some static variables

Hi again,

Regarding to the previous issue #79, now I have found a new bug with static variables, you can reproduce it with this code:

<?php

class MyClass extends MyForms
{
    private static $defaultClass = 'OtherClass';

    final public static function getForm($class = false)
    {
        if ($class !== false) {
            return new $class();
        }

        return new self::$defaultClass();
    }
}

Please check this capture.

[sirbrillig]

Are you sure this is a bug? I think that actually it's a correct warning. See the section titled "When calling static methods, the function call is stronger than the static property operator" in this PHP doc. I actually had to fix this behavior in #27.

In your example, return new self::$defaultClass(); will first evaluate $defaultClass and then look for a static property on self which matches that name. However, $defaultClass is not defined.

Here's a simplified example which you can run to see the failure:

<?php
class Foobar {
	private static $name = 'doSomething';
	public static function doSomething() {
		return 'hi';
	}
	public static function proxy() {
		return self::$name();
	}
}
echo Foobar::proxy();

Or view it here: https://repl.it/repls/DefiantCrowdedTabs

If I'm misunderstanding or I'm wrong (always possible!) please feel free to re-open the issue.

Hi @sirbrillig and thanks for response, I was trying to change the code after your comment and I discovered this...

Why does my previous code throw a warning and this does not?

<?php

class MyClass extends MyForms
{
    private static $defaultClass = 'OtherClass';

    final public static function getForm($class = false)
    {
        $defaultClass = self::$defaultClass;

        if ($class !== false) {
            return new $class();
        }

        return new self::$defaultClass();
    }
}

[sirbrillig]

That example is not an issue, I think, because in that case, $defaultClass is defined by the time it's used. PHP is weird this way.

In the second example, you're creating a local variable called $defaultClass, which you're assigning with the value of self::$defaultClass, which evaluates to the static property and thus has the value of 'OtherClass'. Then you're returning self::$defaultClass(). Because the function call is stronger than the static reference, essentially that means that those parenthesis evaluate first, before the self:: part. So self::$defaultClass() first evaluates $defaultClass(). In the original code, that was causing an error (and also, the sniff was warning you about it) since $defaultClass didn't exist, but in the second example, it does exist, so it will be evaluated into OtherClass(). That function does not exist (it's a class, not a function), so you'll probably get a PHP error, but it's a different error and one that this sniff cannot detect because it only looks to make sure variables exist, not functions.

I understand what you're trying to do: dynamically access the static property and then use that as a class name. This is tricky to do in PHP, though. I'm not sure if it will work, but maybe try this:

<?php

class MyClass extends MyForms
{
    private static $defaultClass = 'OtherClass';

    final public static function getForm($class = false)
    {
        if ($class !== false) {
            return new $class();
        }

        return new (self::$defaultClass)();
    }
}

Thank u so much @sirbrillig, we will review it!