NewClosures sniff: add checking for new features added in PHP 5.4

[jrfnl]

Closures were introduced in PHP 5.3.
However, until PHP 5.4, they could not be declared as static, nor could they use the $this variable when declared within a class context.
Ref: http://php.net/manual/en/functions.anonymous.php

This will now be sniffed for.

Additionally this PR contains two extra checks for incorrect/unsupported usage of the $this variable in closures for PHP 5.4 and higher:

• usage of $this in static closures is not supported by PHP
• usage of $this in closures outside of a class context is not supported by PHP

Includes unit tests.

Related #24

[coveralls]

Coverage increased (+0.02%) to 95.869% when pulling c19191b on jrfnl:feature/additional-closure-checks into 043b66b on wimg:master.

[SvenRtbg]

Sorry that I do comment very late:

usage of $this in closures outside of a class context is not supported by PHP

In fact, it is. You can define a closure outside of a class, and bind that closure to a class later, but before execution. This is used e.g. when defining the routes for Slim, which may access the Di container object.

Example (the second line is reported as having an error):

$app->post('/authorizations/authCode', function ($request, $response) {
    $resource = $this->{\WebResource\Authorizations::class};
    return $resource->requestActivationCode($request, $response);
});

This line runs perfectly fine because Slim will bind the DI container to the closure before execution. I'd agree that this isn't the best choice because it is not that obvious, but technically it works, and so it's wrong to report an error here. At least downgrade to a warning.

[jrfnl]

@SvenRtbg Have you got a more complete code sample ? We may be able to detect it more specifically if necessary.

The current check is based on the manual (emphasis is mine):

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.

Ref: http://php.net/manual/en/functions.anonymous.php

In the mean time, you can always downgrade it to a warning in your custom ruleset or even exclude that specific error message completely:

<rule ref="PHPCompatibility">
   <exclude name="PHPCompatibility.PHP.NewClosure.ThisFoundOutsideClass"/>
</rule>

<!-- or -->
<rule ref="PHPCompatibility.PHP.NewClosure.ThisFoundOutsideClass">
    <type>warning</type>
</rule>

[SvenRtbg]

I've created a basic example here: https://3v4l.org/VMmcp

$closure = function () {
    return $this->getMessage();
};

class Foo {
    public function getMessage() {
        return "Hello World";
    }
}

$foo = new Foo();
$closure = $closure->bindTo($foo);
echo $closure();

I haven't checked that the scanner will complain about the $this in the second line, but it looks and feels just like the one in my projects. I do understand that tracing the usage of the closure deep into the code until a call to bindTo is made is a real challenge.

As a side note, the Slim part that does the binding is located here: https://github.com/slimphp/Slim/blob/3.8.1/Slim/App.php#L235-L237 - basically you can call App::map on an instance of the object with the route parameters and the closure to be executed OUTSIDE of the class context, but still can access the DI container via $this inside the closure.

[jrfnl]

@SvenRtbg Thanks for that. I'm going to open an issue for this (or you can do so yourself) as, as this is a merged PR/closed issue, it will get lost otherwise.

[jrfnl]

Thanks for opening the issue @SvenRtbg !

[SvenRtbg]

Issue created. Sorry about complaining, let me add some praise that this scanner really is helpful for migrating a large codebase to newer versions of PHP.