Do not flag self, static nor this in an anonymous function declared in class scope

[david-binda]

Related code: https://github.com/Automattic/VIP-Coding-Standards/blob/master/WordPressVIPMinimum/Sniffs/Variables/VariableAnalysisSniff.php#L853

As of PHP 5.4.0, when declared in the context of a class, the current class is automatically bound to it, making $this available inside of the function's scope. If this automatic binding of the current class is not wanted, then static anonymous functions may be used instead.
via http://php.net/manual/en/functions.anonymous.php#example-167

Example code producing false-positives:

<?php

abstract class My_Class {

	protected static $_type = '';

	public static function method() {
		add_action(
			'admin_notices',
			function() {
				self::admin_notice( 'Please pick another $_type for ' . get_called_class() . '. <em><strong>' . static::$_type . '</strong></em> is already registered.' );
			}
		);
	}
}

[GaryJones]

@david-binda This is something covered in PHPCompatibility: PHPCompatibility/PHPCompatibility#669 - can we close this Issue out?

[GaryJones]

I'm going to presumptively close this out. Can be re-opened if necessary.

[rebeccahum]

Re-opening because this is flagging false positives for self:: being used inside a class scope via WordPressVIPMinimum.Variables.VariableAnalysis.StaticInsideClosure.

[GaryJones]

flagging false positives

Can you give some example code?

[rebeccahum]

Sure, the one illustrated above throws a false positive. That should not be flagged.

[GaryJones]

I updated the OP to fix non-related violations in the demo code.

Running it with:

.composer/vendor/bin/phpcs --standard=WordPress-Extra,WordPress-VIP-Go ~/test.php -s

Gave me:

11 | ERROR | Use of static::$_type inside closure. (WordPressVIPMinimum.Variables.VariableAnalysis.StaticOutsideClass)

Which seems correct - there is a static:: in the closure.

I don't see any violation related the self::.

[david-binda]

I believe that this issue should be reported upstream and handled there.

The static, self and $this inside closures in PHP prior to 5.4.0 produced fatal errors. But PHP 5.4.0 automatically bounds current class' scope to closure, making $this, self, and static available inside of the function's scope.

See https://3v4l.org/B43Uj and https://3v4l.org/sdJ6Q with code examples and their processing in different PHP versions (works as expected in PHP 5.4+).

The self part of the sniff was introduced 8 years ago ( 20 Jul 2011
) prior the release of PHP 5.4.0 (which happened on 01 Mar 2012 ).

But really, this is more for upstream, which I'll do shortly.

Let me copy the code examples here, for more posterity:

<?php

class My_Class {

    protected static $output = 'test';

    public static function method() {
        return function() {
            echo static::$output;
        };
    }
}

$object = new My_Class;
$function = $object->method();
$function();

and

<?php

class My_Class {

    protected static $output = 'test';

    public static function method() {
        return function() {
            echo self::$output;
        };
    }
}

$object = new My_Class;
$function = $object->method();
$function();

[david-binda]

sirbrillig/phpcs-variable-analysis#68

[GaryJones]

I believe that this issue should be reported upstream and handled there.

Please note: #236 :-)

[rebeccahum]

Looks like it's been fixed upstream now but our version is out of sync with upstream. Perhaps it's time for syncing with upstream, @GaryJones? If not (since I see that we already done some of our own modifications inside the sniff), another option would be to make the error message clearer like:

Use of `static::$_type` inside closure should not be used outside of class. Manual inspection required.

[GaryJones]

Thank you for demos - I now understand that the code works fine, and the violation no longer needs to be reported (for PHP 5.4 and later at least).

I think there are more benefits to be gained by pulling in @sirbrillig's package, so let's keep this open until that's done, but no separate action should be needed here. The other ticket can focus on where we might already have made changes to our current version.

[GaryJones]

Now that #449 is completed to use VariableAnalysis proper, I've confirmed that the provided examples above don't give an error with the develop version of WordPress-VIP-Go, so this is now fixed for VIPCS 2.2.0.