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

Fix edge cases for symbol quoting rules #10389

Merged
merged 9 commits into from
Feb 18, 2021

Conversation

HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented Feb 13, 2021

There are three edge cases addressed here:

  • :&** shouldn't require quotes when inspected, because it is a valid operator name.
  • The empty symbol (:"") should require quotes when inspected, but currently produces just :, which is syntatically invalid. This affects SymbolLiteral#to_s and indirectly methods such as NamedTuple#inspect:
    # the following ultimately expands to `self[:].inspect(io)`
    p({"": 1}) # Error: expecting token ']', not ':'
  • Although the operator symbols and the underscore symbol (:_) don't require quotes in the symbol notation, they do need them when used as names (e.g. named argument keys, external parameter names). For example:
    {+: 1, -: -1}           # Error: unexpected token: :
    {"+": 1, "-": -1}       # Okay
    {{ {"+": 1, "-": -1} }} # Error: unexpected token: :
    
    NamedTuple(_: String)         # Error: expecting token ')', not ':'
    NamedTuple("_": String)       # Okay
    {{ NamedTuple("_": String) }} # Error: expecting token ')', not ':'
    
    macro foo(call)
      {{ call }}
      {% debug %} # => bar(*: 1)
    end
    
    foo bar("*": 1) # Error: unterminated call

@HertzDevil HertzDevil changed the title Fix edge cases for inspecting symbols Fix edge cases for symbol quoting rules Feb 13, 2021
src/symbol.cr Outdated Show resolved Hide resolved
@bcardiff bcardiff added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler labels Feb 18, 2021
@bcardiff bcardiff added this to the 1.0.0 milestone Feb 18, 2021
@bcardiff
Copy link
Member

I merged master to resolve the conflicts due to recent PRs.

@bcardiff bcardiff merged commit ad2249b into crystal-lang:master Feb 18, 2021
@HertzDevil HertzDevil deleted the bug/symbol-needs-quotes branch February 19, 2021 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants