Skip to content

Filter out falsy nodes in blocks #85

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

Merged
merged 2 commits into from
Aug 22, 2017
Merged

Conversation

motiz88
Copy link
Contributor

@motiz88 motiz88 commented Aug 17, 2017

Fixes #84 and adds a regression test. This is perhaps a crude fix, as I chose not to hunt down the specific null-producing cases in e.g. parser/if.js, but rather to remove them after the fact in the Block constructor.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.401% when pulling 1e18f48 on motiz88:fix-84 into a99ae87 on glayzzle:master.

@ichiriac
Copy link
Member

Hi @motiz88,

Thanks for the fix. I've digged into the issue, it's caused by this code : https://github.com/glayzzle/php-parser/blob/master/src/parser/statement.js#L317

It can also be reproduced by this code :

<?php if (true): ; ; ?>
<?php endif; ?>

The actual fix works on every case, but may slow down a bit the AST building process. I'm looking at a more early filtering solution.

On the other hand, the bracket { ... } form does not have the problem because the falsy values are filtered here : https://github.com/glayzzle/php-parser/blob/master/src/parser/statement.js#L83

I've also found a helper here https://github.com/glayzzle/php-parser/blob/master/src/parser/utils.js#L15 used on loops (could be fixed and used with multiple tokens)

Do you want to digg more ?

If you don't have the time I can merge the fix and continue to correct accordingly.

@ichiriac ichiriac merged commit 6c4249e into glayzzle:master Aug 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Null node found in short-form if body
3 participants