Fix issue with modified backtrace #250
Conversation
php allows the backtrace arguments to be modified via the backtrace. By using array_map we make sure that we do not overwrite references.
| @@ -15,4 +15,8 @@ | |||
| <rule ref="SlevomatCodingStandard.Classes.SuperfluousAbstractClassNaming.SuperfluousPrefix"> | |||
| <exclude-pattern>*/src/*/Abstract*.php</exclude-pattern> | |||
| </rule> | |||
|
|
|||
| <rule ref="SlevomatCodingStandard.Classes.UnusedPrivateElements.UnusedMethod"> | |||
There was a problem hiding this comment.
this could be moved to the source file, but it depends on your code policy.
There was a problem hiding this comment.
My proposed change would avoid this error
| static function (array $call) use ($flatten) : array { | ||
| array_walk_recursive($call['args'], $flatten); | ||
| function (array $call) : array { | ||
| $call['args'] = array_map([$this, 'flattenArguments'], $call['args']); |
There was a problem hiding this comment.
Give the php version requirements I would recommend (not tested):
https://github.com/phpDocumentor/ReflectionDocBlock/blob/master/composer.json#L17
| $call['args'] = array_map([$this, 'flattenArguments'], $call['args']); | |
| $call['args'] = array_map(function($value) { return $this->flattenArguments($value); }, $call['args']); |
There was a problem hiding this comment.
👎 for changing this. The array-callable syntax is much shorter and clearer?
There was a problem hiding this comment.
Nope, for me this syntax is "compilation" compatible and explicit for any code searcher or analyser :)
Don't you agree?
There was a problem hiding this comment.
And that would remove the need of ref="SlevomatCodingStandard.Classes.UnusedPrivateElements.UnusedMethod">
There was a problem hiding this comment.
Both can be formally detected. I guess it just hasn't been implemented by that code style package. psalm itself certainly recognises uses like that, in it's core stuff.
|
@williamdes thanks for the review, are you able to test this with doctum? |
Not yet, it will take me some time to test it because I can not reproduce it locally.. |
|
If you can provide a phar I can reproduce the issue in docker😁 |
Well if you succeed that would help me because I could not replicate this on my workstation or docker instances Follow nikic/PHP-Parser#687 (comment) to replicate the issue, @nikic seems to have reproduced it |
|
That's what I already did, but can you provide me a phar with the fix included? |
Sure, here it is: https://doctum.long-term.support/releases/5.0.2-dev/doctum.phar You can see on my manifest diff that I applied your patch code-lts/doctum@d2695c0#diff-d313403d9a8ddaf7d325ee4001bd3fb6 |
|
The patch works fine on TravisCI ! 🚀 |
php allows the backtrace arguments to be modified via the
backtrace. By using array_map we make sure that we do not overwrite
references.
fixes #249