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

[Ruby Lexer bug] An emoji cannot start a name expression #2009

Open
UlyssesZh opened this issue Nov 5, 2023 · 15 comments
Open

[Ruby Lexer bug] An emoji cannot start a name expression #2009

UlyssesZh opened this issue Nov 5, 2023 · 15 comments
Labels
bugfix-request A request for a bugfix to be developed.

Comments

@UlyssesZh
Copy link

UlyssesZh commented Nov 5, 2023

Name of the lexer
Ruby

Code sample

{🎃:1}

https://rouge.jneen.net/v4.2.0/ruby/e_CfjoM6MX0

Additional context

require 'rouge'
Rouge::Formatters::HTML.new.format Rouge::Lexers::Ruby.lex '{🎃:1}'

Output:

<span class="p">{</span><span class="err">🎃</span><span class="p">:</span><span class="mi">1</span><span class="p">}</span>

The class of the 🎃 character is err, which is not correct (should be n).

@UlyssesZh UlyssesZh added the bugfix-request A request for a bugfix to be developed. label Nov 5, 2023
@jneen
Copy link
Member

jneen commented Nov 5, 2023

Currently symbol names in this syntax have to start with a-z:

rule %r/\b[a-z_]\w*?[?!]?:\s+/, Str::Symbol, :expr_start

Happy to switch this to a unicode property (\p{...}) if you can find documentation of what it's supposed to be.

@jneen
Copy link
Member

jneen commented Nov 5, 2023

The emoji does appear to work in the Ruby console though.

@jneen
Copy link
Member

jneen commented Nov 5, 2023

Also while I'm here this line should probably be non-greedy, even if it's protected by never matching unescaped ':

rule %r/'(\\\\|\\'|[^'])*'/, Str::Single

@UlyssesZh
Copy link
Author

Currently symbol names in this syntax have to start with a-z:

rule %r/\b[a-z_]\w*?[?!]?:\s+/, Str::Symbol, :expr_start

Happy to switch this to a unicode property (\p{...}) if you can find documentation of what it's supposed to be.

It seems upper-case letters also work. My guess is that it is the same as the rules for a variable or a constant.

Does \w match non-ASCII characters? If no, tthe \w*? to match the later characters is probably not right either.

@tancnle
Copy link
Collaborator

tancnle commented Nov 5, 2023

Would this MR help to address this issue #1894?

@UlyssesZh
Copy link
Author

Would this MR help to address this issue #1894?

No. It fixes things like 啊=1 and {啊:1}, but not 🎃=1 or {🎃:1}. Seems like the new regexp rules still do not cover emojis.

@UlyssesZh UlyssesZh changed the title [Ruby Lexer bug] Hash symbol key is sometimes regarded as error [Ruby Lexer bug] An emoji cannot start a name expression Nov 5, 2023
@jneen
Copy link
Member

jneen commented Nov 7, 2023

That is curious, the regexp really should be case-insensitive as well.

@UlyssesZh
Copy link
Author

That is curious, the regexp really should be case-insensitive as well.

/\p{Word}/i =~ '啊' # => 0
/\p{Word}/i =~ '🎃' # => nil

@tancnle
Copy link
Collaborator

tancnle commented Nov 8, 2023

Matching emojis seems to be tricky as they can't be captured in a regex range. I believe this gem https://github.com/ticky/ruby-emoji-regex might have the regex to solve it 🤔

[1] pry(main)> require 'emoji_regex'
[2] pry(main)> EmojiRegex::Regex.match('🎃')
=> #<MatchData "🎃">

@UlyssesZh
Copy link
Author

Matching emojis seems to be tricky as they can't be captured in a regex range. I believe this gem https://github.com/ticky/ruby-emoji-regex might have the regex to solve it 🤔

[1] pry(main)> require 'emoji_regex'
[2] pry(main)> EmojiRegex::Regex.match('🎃')
=> #<MatchData "🎃">

That does not solve the issue either because there are non-word characters other than emojis that can start a name, such as the Chinese period:

/\p{Word}/i =~ '。' # => nil
=1 # no SyntaxError

I think any non-ASCII characters can do it? So what we really need to use is [a-z_|^\x00-\x7F] (for variables) and [a-zA-Z_|^\x00-\x7F] (for Hash symbol keys).

@jneen
Copy link
Member

jneen commented Nov 8, 2023

I'm a little nervous about significantly expanding the name rules without some assurance from Ruby folks about what it is exactly they intend to support. Does ruby have a spec for this?

@UlyssesZh
Copy link
Author

I'm a little nervous about significantly expanding the name rules without some assurance from Ruby folks about what it is exactly they intend to support. Does ruby have a spec for this?

There should have a spec in ISO/IEC 30170:2012, but I didn't buy it. The documentation on ruby-doc.org says any character with the eighth bit set can start a variable, though, so [^\x00-\x7F] should be it.

@jneen
Copy link
Member

jneen commented Nov 8, 2023

image Looks right to me. I didn't want to believe it but it looks like they *actually* implemented it this way:
; ruby -e 'p  :1'
{: =>1}

This is a double-width space, also known as \xe3\x80\x80, and it can be used as an identifier 🤦

As long as there aren't any issues with that regexp leaving glyphs stranded, we should be able to support this somewhat. Basic testing with the Japanese block seems to indicate that [^\x00-\x7F] will work.

@UlyssesZh
Copy link
Author

image

It seems that ISO/IEC 30170:2012 does not talk about non-ASCII characters, so strictly speaking using non-ASCII characters as identifiers is undefined behavior. It just happens to be implemented and documented in CRuby and maybe other Ruby implementations.

I do not know what is the philosophy of Rouge, but I am supporting adopt [^\x00-\x7F]. I have read some of the ISO documentation, and I must say it just gave way too much freedom to implementations of Ruby interpreters. CRuby should be used as the de facto standard instead (and it actually is, when people try to invent new implementations), and any Ruby highlighting program should also use CRuby as the standard.

@jneen
Copy link
Member

jneen commented Nov 8, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix-request A request for a bugfix to be developed.
Projects
None yet
Development

No branches or pull requests

3 participants