Issue 223

[sinri]

Made a fix.

Fixes: #223

[williamdes]

LGTM
Thank you @sinri !
Can a regression test be added ?

[sinri]

LGTM
Thank you @sinri !
Can a regression test be added ?

Can you provide me any instructions about the regression test? So I could try to add then.

[sinri]

Tried PHPUnit,

There was 1 failure:

1) PhpMyAdmin\SqlParser\Tests\Parser\CallStatementTest::testCall with data set #2 ('parser/parseCall3')
Failed asserting that two objects are equal.
--- Expected
+++ Actual
@@ @@
             'call' => PhpMyAdmin\SqlParser\Components\FunctionCall Object (...)
             'options' => null
             'first' => 0
-            'last' => 3
+            'last' => 2
         )
     )
     'brackets' => 0

/Users/sinri/code/phpstorm/sql-parser/tests/TestCase.php:107
/Users/sinri/code/phpstorm/sql-parser/tests/Parser/CallStatementTest.php:16

The test case IN file contains CALL foo;.

After checking, I think here last should be updated to 2.
It might need some discuss.

---

BTW, the operator instanceof may throw a lot of Error: Class name must be a valid object or a string, need confirming that this had been there with the existed codes...

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[ibennetch]

This looks good to me, thank you for your contribution!

[ibennetch]

I'm trying to find a way to say this a bit differently since I'm a little not clear about what the "one" is here. Would this phrasing work for you?

// Issue 223: If the previous token is a delimiter, go back to the previous token

[staabm]

I guess this line is missing a $class !== null check beforehand, as we use it a few lines above.

I tried to PR a suggestion, but it seems this lines are not contained in the master branch?

[ibennetch]

@staabm This should be part of master now, can you make sure you're pulling the latest change from this repository?

[ibennetch]

This breaks quite a few things in tests, there are 11 errors relating to

Error: Class name must be a valid object or a string

that weren't there before merging this pull request.

If you can fix the test failures, I can merge that fix, otherwise I may have to roll back this change. I should have noticed the errors before merging but was focused on the test failure in parseCall3 instead.

[williamdes]

Hi @sinri
I am re-working this PR to fix #372

After checking, I think here last should be updated to 2.
It might need some discuss.

I am not really sure you where so certain about that, do you remember ?
Edit: nevermind, I found out that your fix could be done some other way. And found why you did it
See: #442