-
-
Notifications
You must be signed in to change notification settings - Fork 344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed undefined index referring #818
Conversation
Thank you @a-menshchikov did you find when/why (with which analyzed code) |
@kylekatarnls error rising on the following code:
Full error output (with xdebug enabled):
Maybe it will be useful: |
@a-menshchikov thanks for the PR (and the issue) do you have the possibility to add a test to the PR? With that we have a verification that it is fixed and we don't destroy it in another release. I assign this PR to you, but feel free to say if you need help creating the tests. |
@tvbeek I think I need help with tests because at the moment I have no idea how to create it`s. |
No problem. I will use the example you provided to add it. |
I just tried the same code chunk but get no error. I also tried with PDepend 2.7.1. I think we should merge this and #821, release the patch 2.9.1 to fix the error then take the time to understand why and how it happen to fix it properly. |
I could reproduce the bug. I'll add the tests in a second PR. |
For the record: the error happen if there is more arguments in call than in definition: function foo($a, $b) {}
foo($v1, $v2, $v3); Or if the definition of the function/method couldn't not be found. I both situation, it would be fine IMO to consider by default the variable is not passed by reference. |
Erratum: the actual trigger is the method name "flush". PHPMD tried to match the arguments with the So it could lead to argument wrongly detected as passed by reference. |
Funny fact: PHPStorm seems to do the same mistake as it underlines |
@kylekatarnls
|
#818 Fix passing-by-reference detection
Type: bugfix
Issue: Resolves #816
Breaking change: no