Skip to content
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

Variable declaration via list inside foreach not recognized #33

Closed
gmazzap opened this issue Feb 12, 2018 · 5 comments · Fixed by #36
Closed

Variable declaration via list inside foreach not recognized #33

gmazzap opened this issue Feb 12, 2018 · 5 comments · Fixed by #36
Labels

Comments

@gmazzap
Copy link

gmazzap commented Feb 12, 2018

Following is perfectly valid PHP 5.5+ code:

$data = [
    ['foo', 'Foo'],
    ['bar', 'Bar'],
];

foreach ($data as list($name, $label)) {
    printf('<div id="%s">%s</div>', $name, $label);
}

However, it raises the following warnings:

------------------------------------------
WARNING | Variable $name is undefined.
WARNING | Variable $label is undefined.
------------------------------------------
@sirbrillig
Copy link
Owner

Thanks for the report!

@gmazzap
Copy link
Author

gmazzap commented Feb 13, 2018

I think the fix actually broke used variables check inside foreach.

In my code I have:

foreach ($callbacks as $callback) {
  $callback();
 }

Which causes a:

WARNING | Unused variable $callback.

But I could notify that there are a lot of things that now raise unused variables false positive for variable declared by foreach.

@gmazzap
Copy link
Author

gmazzap commented Feb 13, 2018

Not sure if I should I open another issue, or maybe you could re-open this.

@sirbrillig sirbrillig reopened this Feb 13, 2018
@sirbrillig
Copy link
Owner

D'oh! I guess these unit tests are not good enough to prevent regressions. 😭 Well, we'll make them better.

@sirbrillig
Copy link
Owner

Sorry about that. There's still one bug I'm tracking in #37 but 2.0.7 should fix the major bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants