[bug] incorrect start location in many nodes

[alexander-akait]

Input:

while (true) // Comment
{
};

Currently while node starts with test node location, but should be start with while keyword. Same for other control structures. It would be great if you show how to fix such things (send a PR with fix) and i will start fix it self and send new PRs. This would speed up problem solving.

[ichiriac]

Hi,

Any PR is welcome, actually I have a ton of work and I finish late, also a personal project at home, so it's hard to find time to work. A lot of issues can be fixed with simple changes and should not take much time but even 1 hour it's hard to find for me.

Here my email : [email protected], email me and we can continue to chat with skype or whatever.

For your bug it's simple, here the parsing of the node : https://github.com/glayzzle/php-parser/blob/master/src/parser/loops.js#L17

The const result = this.node("while"); node function get track of the position. The parser eats a token and go to the next with the next function, meaning the while token was eaten before creating the AST node and so excluding it from locations.

Take a look at : https://github.com/glayzzle/php-parser/blob/fd8d0dc6e80f74f4dce7df7d07eb21334a2d4915/src/parser/statement.js#L213 it's here from where the function is called, and you can see that before calling the function, an extra next consumes the token (bug is here).

To fix this bug you have to remove the next call from statement.js, and to consume it into the function read_while juste after the this.node statement. For example here : https://github.com/glayzzle/php-parser/blob/cf6dcf8d65d39cfc4c619e2d5c29ac909a90b8b0/src/parser/loops.js#L22

Note the next returns this, so you can write something as : this.next().expect(...

If you pay attention into statement.js, you will see that other keywords may have the bug :

      case this.tok.T_FOR:
        return this.next().read_for();

      case this.tok.T_FOREACH:
        return this.next().read_foreach();

      case this.tok.T_WHILE:
        return this.next().read_while();

      case this.tok.T_DO:
        return this.next().read_do();

We can keep this thread for any question around the code

[alexander-akait]

@ichiriac looks i find how fix it, can you look my PR https://github.com/glayzzle/php-parser/pull/209 and comment?

[alexander-akait]

Here my email : [email protected], email me and we can continue to chat with skype or whatever.

My English is very bad :smile: It's better for me to type than talk, sorry.

[ichiriac]

I'll try to explain how you can fix bugs when it's simple. Your help is much appreciated !

[ichiriac]

this bug was fixed & released

[alexander-akait]

@ichiriac let's left it open because not all nodes was fixed

[alexander-akait]

/cc @ichiriac find problem with if, input:

if ($bool0
    || $bool1 // if condition 1
    || $bool2 // if condition 2
  ) {
    $ok = true;
}

The test property has invalid loc (https://astexplorer.net/), issue https://github.com/prettier/plugin-php/issues/1092

[ctf0]

any updates ?, 4 years and still no fix 😢