[RFC] Improved parser error message

14 years ago by Ferenc Kovacs — view source

unread

Hi all,
As I have proposed previously in an old thread... What about we name all
the
tokens to have an improved parser error message? (i.e. anymore
T_PAAMAYIM_NEKUDOTAYIM, T_DOLLAR_OPEN_CURLY_BRACES in the messages etc)

Some examples:

$ sapi/cli/php -r 'function ""'
Patched:
Parse error: syntax error, unexpected quoted-string, expecting identifier
or
'(' in Command line code on line 1

Current:
Parse error: syntax error, unexpected T_CONSTANT_ENCAPSED_STRING, expecting
T_STRING or '(' in Command line code on line 1

$ sapi/cli/php -r 'echo ::a;'
Patched:
Parse error: syntax error, unexpected :: in Command line code on line 1

Current:
Parse error: syntax error, unexpected T_PAAMAYIM_NEKUDOTAYIM in Command
line
code on line 1

Other examples and patch at:
https://wiki.php.net/rfc/improved-parser-error-message

Any thoughts?

Thanks.

--
Regards,
Felipe Pena

+1

do you think that this could cause some kind of BC?
I don't think that anybody out there parses the tokens as is, but there are
crazy people. :)
the other thing that I can imagine that the developers will be surprised
that they are getting meaningful error messages, but I think they can live
with that. :)

Tyrael

14 years ago by Felipe Pena — view source

unread

Hi,

2011/5/16 Ferenc Kovacs info@tyrael.hu

+1

do you think that this could cause some kind of BC?
I don't think that anybody out there parses the tokens as is, but there are
crazy people. :)
the other thing that I can imagine that the developers will be surprised
that they are getting meaningful error messages, but I think they can live
with that. :)

Tyrael

There is no BC. The changes doesn't affects token_*() functions for example.

$ sapi/cli/php -r 'var_dump(token_name(318));'
string(6) "T_ECHO"

I added such information to the wiki page. Thanks. :)

--
Regards,
Felipe Pena

14 years ago by Etienne Kneuss — view source

unread

Hi Felipe,

Hi all,
As I have proposed previously in an old thread... What about we name all the
tokens to have an improved parser error message? (i.e. anymore
T_PAAMAYIM_NEKUDOTAYIM, T_DOLLAR_OPEN_CURLY_BRACES in the messages etc)

Some examples:

$ sapi/cli/php -r 'function ""'
Patched:
Parse error: syntax error, unexpected quoted-string, expecting identifier or
'(' in Command line code on line 1

Current:
Parse error: syntax error, unexpected T_CONSTANT_ENCAPSED_STRING, expecting
T_STRING or '(' in Command line code on line 1

$ sapi/cli/php -r 'echo ::a;'
Patched:
Parse error: syntax error, unexpected :: in Command line code on line 1

Current:
Parse error: syntax error, unexpected T_PAAMAYIM_NEKUDOTAYIM in Command line
code on line 1

Other examples and patch at:
https://wiki.php.net/rfc/improved-parser-error-message

Any thoughts?

That's great, it will save millions of headaches.

Best,

Thanks.

--
Regards,
Felipe Pena

14 years ago by Pierrick Charron — view source

unread

+1

Regards
Pierrick

Hi all,
As I have proposed previously in an old thread... What about we name all
the
tokens to have an improved parser error message? (i.e. anymore
T_PAAMAYIM_NEKUDOTAYIM, T_DOLLAR_OPEN_CURLY_BRACES in the messages etc)

Some examples:

$ sapi/cli/php -r 'function ""'
Patched:
Parse error: syntax error, unexpected quoted-string, expecting identifier
or
'(' in Command line code on line 1

Current:
Parse error: syntax error, unexpected T_CONSTANT_ENCAPSED_STRING, expecting
T_STRING or '(' in Command line code on line 1

$ sapi/cli/php -r 'echo ::a;'
Patched:
Parse error: syntax error, unexpected :: in Command line code on line 1

Current:
Parse error: syntax error, unexpected T_PAAMAYIM_NEKUDOTAYIM in Command
line
code on line 1

Other examples and patch at:
https://wiki.php.net/rfc/improved-parser-error-message

Any thoughts?

Thanks.

--
Regards,
Felipe Pena

14 years ago by Florian Anderiasch — view source

unread

Other examples and patch at:
https://wiki.php.net/rfc/improved-parser-error-message

Just for completeness, here's the lengthy thread
we had in October 2010 about this:

http://news.php.net/php.internals/49978

FWIW, I like the Easter Egg :)

Greetings,
Florian

14 years ago by Julien Pauli — view source

unread

I remember error messages have been improved since last decade, but
they were engine ones.
Parser ones have never been touched, so to improve them, I +1 your
idea Felipe :)

Julien

Other examples and patch at:
https://wiki.php.net/rfc/improved-parser-error-message

Just for completeness, here's the lengthy thread
we had in October 2010 about this:

http://news.php.net/php.internals/49978

FWIW, I like the Easter Egg :)

Greetings,
Florian

14 years ago by Felipe Pena — view source

unread

2011/5/16 Felipe Pena felipensp@gmail.com

Hi all,
As I have proposed previously in an old thread... What about we name all
the tokens to have an improved parser error message? (i.e. anymore
T_PAAMAYIM_NEKUDOTAYIM, T_DOLLAR_OPEN_CURLY_BRACES in the messages etc)

[...]

Other examples and patch at:

https://wiki.php.net/rfc/improved-parser-error-message

Any thoughts?

Thanks.

So... In case of no objections, I'll commit it in trunk and 5_4 branch, Is
it okay? :)

--
Regards,
Felipe Pena

14 years ago by Andi Gutmans — view source

unread

Hi Felipe,

Read the archives. We discussed T_PAAMAYIM_NEKUDOTAYIM extensively and agreed to keep it as a historical landmark. Let's not have the discussion yet again within the same year. Google it just to see the # of mentions.

What we could do as a compromise between history and improving many of the parse error messages is:
Parse error: syntax error, unexpected :: (T_PAAMAYIM_NEKUDOTAYIM) in Command line code on line 1

Generally speaking I'm OK with the other changes but I don't know if any extensions or scripts depend on them (probably not).
I suggest to give it another couple of days to see if anyone has any valid concerns on breakage this could cause.

Andi

-----Original Message-----
From: Felipe Pena [mailto:felipensp@gmail.com]
Sent: Tuesday, May 17, 2011 7:18 AM
To: internals
Subject: [PHP-DEV] Re: [RFC] Improved parser error message

2011/5/16 Felipe Pena felipensp@gmail.com

Hi all,
As I have proposed previously in an old thread... What about we name
all the tokens to have an improved parser error message? (i.e. anymore
T_PAAMAYIM_NEKUDOTAYIM, T_DOLLAR_OPEN_CURLY_BRACES in the
messages
etc)

[...]

Other examples and patch at:

https://wiki.php.net/rfc/improved-parser-error-message

Any thoughts?

Thanks.

So... In case of no objections, I'll commit it in trunk and 5_4 branch, Is it okay? :)

--
Regards,
Felipe Pena

14 years ago by Gustavo Lopes — view source

unread

Em Tue, 17 May 2011 17:01:26 +0100, Andi Gutmans andi@zend.com escreveu:

Read the archives. We discussed T_PAAMAYIM_NEKUDOTAYIM extensively and
agreed to keep it as a historical landmark. Let's not have the
discussion yet again within the same year. Google it just to see the #
of mentions.

What we could do as a compromise between history and improving many of
the parse error messages is:
Parse error: syntax error, unexpected :: (T_PAAMAYIM_NEKUDOTAYIM) in
Command line code on line 1

This proposal is not about changing the name of the token, only the
message that's shown to the user. The token name will remain as is in
zend_language_parser.y et al. If I recall correctly, there was not
objection raised to changing the message in the last discussion.

Anyway, +1 to the change from you.

--
Gustavo Lopes

14 years ago by Gustavo Lopes — view source

unread

Em Tue, 17 May 2011 17:07:09 +0100, Gustavo Lopes glopes@nebm.ist.utl.pt
escreveu:

[...]
Anyway, +1 to the change from you.

Sorry, it should read "from me".

--
Gustavo Lopes

14 years ago by Ferenc Kovacs — view source

unread

Hi Felipe,

Read the archives. We discussed T_PAAMAYIM_NEKUDOTAYIM extensively and
agreed to keep it as a historical landmark. Let's not have the discussion
yet again within the same year. Google it just to see the # of mentions.

Maybe you don't remember, but Felipe participated in that discussion, to be
exact he proposed this exact feature there:
internals@lists.php.net/msg48173.html" rel="nofollow" target="_blank">http://www.mail-archive.com/internals@lists.php.net/msg48173.html
after Felipe's mail, nobody objected about replacing the constants with
meaningful names, as it was the original idea, but nobody found a clean way
to implement it before.

What we could do as a compromise between history and improving many of the
parse error messages is:
Parse error: syntax error, unexpected :: (T_PAAMAYIM_NEKUDOTAYIM) in
Command line code on line 1

maybe it's a little bit redundant, but it can be a good transition.

Generally speaking I'm OK with the other changes but I don't know if any
extensions or scripts depend on them (probably not).
I suggest to give it another couple of days to see if anyone has any valid
concerns on breakage this could cause.

as Felipe mentioned, his patch doesn't modify the token names(so the token_
functions aren't affected by this change), only the rendering of the error
messages.

Tyrael

14 years ago by Derick Rethans — view source

unread

2011/5/16 Felipe Pena felipensp@gmail.com

Hi all,
As I have proposed previously in an old thread... What about we name all
the tokens to have an improved parser error message? (i.e. anymore
T_PAAMAYIM_NEKUDOTAYIM, T_DOLLAR_OPEN_CURLY_BRACES in the messages etc)

[...]

Other examples and patch at:

https://wiki.php.net/rfc/improved-parser-error-message

Any thoughts?

Thanks.

So... In case of no objections, I'll commit it in trunk and 5_4 branch, Is
it okay? :)

Go for it.

Derick

14 years ago by Zeev Suraski — view source

unread

+1 with Andi's improvement.

Gustavo - I realize it's not about changing the token's name, but nobody (=very few) looks at the token names. The point is to keep the tradition of also exposing this specific token name to the user, but still making it clear that what was expected was :: - without the need to head to Google Translate...

Zeev

-----Original Message-----
From: Felipe Pena [mailto:felipensp@gmail.com]
Sent: Tuesday, May 17, 2011 5:18 PM
To: internals
Subject: [PHP-DEV] Re: [RFC] Improved parser error message

2011/5/16 Felipe Pena felipensp@gmail.com

Hi all,
As I have proposed previously in an old thread... What about we name
all the tokens to have an improved parser error message? (i.e. anymore
T_PAAMAYIM_NEKUDOTAYIM, T_DOLLAR_OPEN_CURLY_BRACES in the
messages
etc)

[...]

Other examples and patch at:

https://wiki.php.net/rfc/improved-parser-error-message

Any thoughts?

Thanks.

So... In case of no objections, I'll commit it in trunk and 5_4 branch, Is it okay?
:)

--
Regards,
Felipe Pena

14 years ago by Stas Malyshev — view source

unread

Hi!

I think we need to keep token name in the message, since it makes it
easier to understand what parser expected if you need to debug the
parser (as opposed to your code). So I think we need to have both the
human-readable name and the token name, as Andi suggested.

Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

14 years ago by Ferenc Kovacs — view source

unread

On Tue, May 17, 2011 at 6:49 PM, Stas Malyshev smalyshev@sugarcrm.comwrote:

Hi!

I think we need to keep token name in the message, since it makes it easier
to understand what parser expected if you need to debug the parser (as
opposed to your code). So I think we need to have both the human-readable
name and the token name, as Andi suggested.

on the sidenote, I don't think that many people out there would start
debuging the php src if they are presented with an error by the engine.
but I'm fine with Andi's suggestion.

Tyrael

14 years ago by Rasmus Lerdorf — view source

unread

On Tue, May 17, 2011 at 6:49 PM, Stas Malyshevsmalyshev@sugarcrm.comwrote:

Hi!

I think we need to keep token name in the message, since it makes it easier
to understand what parser expected if you need to debug the parser (as
opposed to your code). So I think we need to have both the human-readable
name and the token name, as Andi suggested.

on the sidenote, I don't think that many people out there would start
debuging the php src if they are presented with an error by the engine.
but I'm fine with Andi's suggestion.

While that is true, I did run across some code a while back that used
the tokenizer extension in conjunction with the error messages. If we
decouple the error messages completely from the tokens there would be no
way to write that same code. An edge case, but I don't think it hurts to
keep the token name there.

-Rasmus

14 years ago by Gustavo Lopes — view source

unread

Em Tue, 17 May 2011 17:49:53 +0100, Stas Malyshev smalyshev@sugarcrm.com
escreveu:

I think we need to keep token name in the message, since it makes it
easier to understand what parser expected if you need to debug the
parser (as opposed to your code). So I think we need to have both the
human-readable name and the token name, as Andi suggested.

I think this alternative has very little value for debugging the parser.
There are no repeated labels for the tokens, so even if you don't know the
token name by heart, you can look it up in 10 seconds.

Keeping the token name will only perpetuate the confusion it has caused. I
think it ought to be dropped, but if no consensus can be reached, this
would be better than the status quo.

--
Gustavo Lopes

14 years ago by Felipe Pena — view source

unread

2011/5/18 Gustavo Lopes glopes@nebm.ist.utl.pt:

Em Tue, 17 May 2011 17:49:53 +0100, Stas Malyshev smalyshev@sugarcrm.com
escreveu:

I think we need to keep token name in the message, since it makes it
easier to understand what parser expected if you need to debug the parser
(as opposed to your code). So I think we need to have both the
human-readable name and the token name, as Andi suggested.

I think this alternative has very little value for debugging the parser.
There are no repeated labels for the tokens, so even if you don't know the
token name by heart, you can look it up in 10 seconds.

Keeping the token name will only perpetuate the confusion it has caused. I
think it ought to be dropped, but if no consensus can be reached, this would
be better than the status quo.

--
Gustavo Lopes

--

So, following some suggestion I made a patch wich displays such messages:

$ sapi/cli/php -r 'class '
Current:
Parse error: syntax error, unexpected $end, expecting T_STRING

Patched:
Parse error: syntax error, unexpected end of file, expecting
'identifier' (T_STRING)

$ sapi/cli/php -r 'class abc foo'
Current:
Parse error: syntax error, unexpected T_STRING, expecting '{' in
Command line code on line 1

Patched:
Parse error: syntax error, unexpected 'foo' (T_STRING), expecting '{'
in Command line code on line 1

As can be noticed, I added the actual scanned string in the
"unexpected" part. This might be useful for finding really which makes
the parser error. (It was a bit tricky though :D)

Any thoughts?
https://wiki.php.net/rfc/improved-parser-error-message

--
Regards,
Felipe Pena

14 years ago by Jordi Boggiano — view source

unread

Thanks for the great work. More should be done on the front of helping
newcomers solve trivial issues imo.

$ sapi/cli/php -r 'class '
Current:
Parse error: syntax error, unexpected $end, expecting T_STRING

Patched:
Parse error: syntax error, unexpected end of file, expecting
'identifier' (T_STRING)

identifier should not be quoted. The quotes are fine for "expecting '{'"
and "unexpected 'foo'" because those are text literals, but this implies
you were supposed to actually type "class identifier", which is
obviously not the intended message.

$ sapi/cli/php -r 'class abc foo'
Current:
Parse error: syntax error, unexpected T_STRING, expecting '{' in
Command line code on line 1

Patched:
Parse error: syntax error, unexpected 'foo' (T_STRING), expecting '{'
in Command line code on line 1

As can be noticed, I added the actual scanned string in the
"unexpected" part. This might be useful for finding really which makes
the parser error. (It was a bit tricky though :D)

Good stuff, maybe "unexpected string 'foo' (T_STRING)" or "unexpected
string (T_STRING) 'foo'" would be more clear, but no big deal really.

Cheers

--
Jordi Boggiano
@seldaek - http://nelm.io/jordi

14 years ago by Paul Dragoonis — view source

unread

Thanks for the great work. More should be done on the front of helping
newcomers solve trivial issues imo.

$ sapi/cli/php -r 'class '
Current:
Parse error: syntax error, unexpected $end, expecting T_STRING

Patched:
Parse error: syntax error, unexpected end of file, expecting
'identifier' (T_STRING)

identifier should not be quoted. The quotes are fine for "expecting '{'"
and "unexpected 'foo'" because those are text literals, but this implies
you were supposed to actually type "class identifier", which is
obviously not the intended message.

$ sapi/cli/php -r 'class abc foo'
Current:
Parse error: syntax error, unexpected T_STRING, expecting '{' in
Command line code on line 1

Patched:
Parse error: syntax error, unexpected 'foo' (T_STRING), expecting '{'
in Command line code on line 1

As can be noticed, I added the actual scanned string in the
"unexpected" part. This might be useful for finding really which makes
the parser error. (It was a bit tricky though :D)

Good stuff, maybe "unexpected string 'foo' (T_STRING)" or "unexpected
string (T_STRING) 'foo'" would be more clear, but no big deal really.

Cheers

--
Jordi Boggiano
@seldaek - http://nelm.io/jordi

--

Good work Felipe! the $end was very ambiguous :-)

14 years ago by Stas Malyshev — view source

unread

Hi!

As can be noticed, I added the actual scanned string in the
"unexpected" part. This might be useful for finding really which makes
the parser error. (It was a bit tricky though :D)

Any thoughts?
https://wiki.php.net/rfc/improved-parser-error-message

Looks cool. Any tests we'd need to fix for that? Except for that, I
think it's all good.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/
(408)454-6900 ext. 227

14 years ago by Sebastian Bergmann — view source

unread

Patched:
Parse error: syntax error, unexpected end of file, expecting
'identifier' (T_STRING)

At least for developers who do not know how a compiler works, I think
that we should think about removing the information duplication of
"parse error" and "syntax error". Just "Syntax error: ..." should be
enough.

--
Sebastian Bergmann Co-Founder and Principal Consultant
http://sebastian-bergmann.de/ http://thePHP.cc/

[RFC] Improved parser error message

I think we need to keep token name in the message, since it makes it easier to understand what parser expected if you need to debug the parser (as opposed to your code). So I think we need to have both the human-readable name and the token name, as Andi suggested.

I think we need to keep token name in the message, since it makes it
easier to understand what parser expected if you need to debug the
parser (as opposed to your code). So I think we need to have both the
human-readable name and the token name, as Andi suggested.