Phpinspectionsea: False-positive of static closures when called via bindTo

Created on 30 Jul 2019  Â·  5Comments  Â·  Source: kalessil/phpinspectionsea


| Subject | Details |
| :------------- | :---------------------------------------------------------------------------- |
| Plugin | Php Inspections (EA Extended), 3.0.14.1 |
| Language level | PHP 7.2 |

Current behaviour

As mentioned in https://github.com/kalessil/phpinspectionsea/issues/1043#issuecomment-469839795, here's a false-positive:

When writing routes using Laravel Lumen and inlining the action using a closure, the plugin gives a false-positive when it wants to add "static" to the closure.

Simply writing the following in the routes file already triggers the expected exception:

$router->get('foo', static function (Request $request) {
    return $request->all();
});

As seen in the source, this is due to RoutesRequests using Closure::bindTo behind the scenes:
https://github.com/laravel/lumen-framework/blob/f03478eb595bbe920d0ca213e75441e89291e380/src/Concerns/RoutesRequests.php#L282-L284

Expected behaviour

I'd at least like to be able to mark the line as a false-positive but the "suppress inspection for the line" does not seem to have an effect…

Ideally, this would figure out the bindTo call, but I expect this to be quite hard to do behind the scenes…

Environment details

PhpStorm 2019.2
Build #PS-192.5728.108, built on July 24, 2019
Licensed to XXXXX
Subscription is active until XXXXXX
Runtime version: 11.0.3+12-b304.10 x86_64
VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o
macOS 10.14.2
GC: ParNew, ConcurrentMarkSweep
Memory: 1979M
Cores: 12
Registry: run.processes.with.pty=TRUE
Non-Bundled Plugins: ColorIde, NEON support, com.kalessil.phpStorm.phpInspectionsEA, org.sylfra.idea.plugins.linessorter, ru.adelf.idea.dotenv

bug / false-positive

Most helpful comment

@jdreesen, @spaceemotion: I put some thoughts together and deiced to introduce new setting "Skip in methods calls". Will be enabled by default.

We also fixed #1564, but your cases here are not possible to detect.

// cc @schlessera - FYI

All 5 comments

This is a problem with Prophecy, too.

You can write $prophecy->method()->will(function () {...}); there, and the closure will automatically be bound to the $prophecy object.

See https://github.com/phpspec/prophecy#method-prophecies-idempotency for an example.

The binding happens here and here.

Checked the Lumen and Prophecy resources, no chance to discover this automatically. I can only add a white-list settings for methods which are not accepting static closures. That max what i can do on my end.

Will this work for both of you (the setting)?

PS: suppression are handled by IDE, I also think with settings it;ll be no suppression needed.

I already thought that this can not be detected automatically.

A whitelist would be okay for me (I hope that it works with the Prophecy magic, though).

@jdreesen, @spaceemotion: I put some thoughts together and deiced to introduce new setting "Skip in methods calls". Will be enabled by default.

We also fixed #1564, but your cases here are not possible to detect.

// cc @schlessera - FYI

So, at the end of day I had to rewrite the inspection from the scratch. When the next release is out, please share feedback if it reports only valid cases (we'll report only cases where static 100% can be applied).

Was this page helpful?
0 / 5 - 0 ratings