Fix edge cases for symbol quoting rules (#10389)

* Fix rules for symbol quoting

* Operator symbols must still use quotes in key form

* Additional specs

* names -> named_argument

* Missing cases

* Print named generic type arguments in docs

* oops

* Revert "Print named generic type arguments in docs"

This reverts commit e5c63d2.

Co-authored-by: Brian J. Cardiff <bcardiff@gmail.com>

spec/compiler/codegen/named_tuple_spec.cr

@@ -334,6 +334,14 @@ describe "Code gen: named tuple" do
       ").to_i.should eq(2)
   end
 
+  it "does to_s for NamedTuple class" do
+    run(%(
+      require "prelude"
+
+      NamedTuple(a: Int32, "b c": String, "+": Char).to_s
+      )).to_string.should eq(%(NamedTuple(a: Int32, "b c": String, "+": Char)))
+  end
+
   it "doesn't error if NamedTuple includes a non-generic module (#10380)" do
     codegen(%(
       module Foo

spec/compiler/crystal/types_spec.cr

@@ -39,6 +39,10 @@ describe "types to_s of" do
     assert_type_to_s "(Int32 | String)" { union_of(string, int32) }
   end
 
+  it "named tuple" do
+    assert_type_to_s %(NamedTuple(a: Int32, "b c": String, "+": Char)) { named_tuple_of({"a" => int32, "b c" => string, "+" => char}) }
+  end
+
   it "nilable reference type" do
     assert_type_to_s "(String | Nil)" { nilable string }
   end

spec/compiler/macro/macro_methods_spec.cr

@@ -1011,15 +1011,15 @@ module Crystal
       end
 
       it "executes double splat" do
-        assert_macro "", %({{**{a: 1, "foo bar": 2}}}), [] of ASTNode, %(a: 1, "foo bar": 2)
+        assert_macro "", %({{**{a: 1, "foo bar": 2, "+": 3}}}), [] of ASTNode, %(a: 1, "foo bar": 2, "+": 3)
       end
 
       it "executes double splat" do
-        assert_macro "", %({{{a: 1, "foo bar": 2}.double_splat}}), [] of ASTNode, %(a: 1, "foo bar": 2)
+        assert_macro "", %({{{a: 1, "foo bar": 2, "+": 3}.double_splat}}), [] of ASTNode, %(a: 1, "foo bar": 2, "+": 3)
       end
 
       it "executes double splat with arg" do
-        assert_macro "", %({{{a: 1, "foo bar": 2}.double_splat(", ")}}), [] of ASTNode, %(a: 1, "foo bar": 2, )
+        assert_macro "", %({{{a: 1, "foo bar": 2, "+": 3}.double_splat(", ")}}), [] of ASTNode, %(a: 1, "foo bar": 2, "+": 3, )
       end
 
       describe "#each" do

spec/compiler/parser/parser_spec.cr

@@ -71,7 +71,7 @@ module Crystal
     it_parses %(%q{hello \#{foo} world}), "hello \#{foo} world".string
 
     [":foo", ":foo!", ":foo?", ":\"foo\"", ":かたな", ":+", ":-", ":*", ":/", ":==", ":<", ":<=", ":>",
-     ":>=", ":!", ":!=", ":=~", ":!~", ":&", ":|", ":^", ":~", ":**", ":>>", ":<<", ":%", ":[]", ":[]?",
+     ":>=", ":!", ":!=", ":=~", ":!~", ":&", ":|", ":^", ":~", ":**", ":&**", ":>>", ":<<", ":%", ":[]", ":[]?",
      ":[]=", ":<=>", ":==="].each do |symbol|
       value = symbol[1, symbol.size - 1]
       value = value[1, value.size - 2] if value.starts_with?('"')
@@ -85,6 +85,7 @@ module Crystal
     it_parses %(:"\\"foo\\""), "\"foo\"".symbol
     it_parses %(:"\\a\\b\\n\\r\\t\\v\\f\\e"), "\a\b\n\r\t\v\f\e".symbol
     it_parses %(:"\\u{61}"), "a".symbol
+    it_parses %(:""), "".symbol
 
     it_parses "[1, 2]", ([1.int32, 2.int32] of ASTNode).array
     it_parses "[\n1, 2]", ([1.int32, 2.int32] of ASTNode).array

spec/std/symbol_spec.cr

@@ -19,11 +19,15 @@ describe Symbol do
   end
 
   it "displays symbols that don't need quotes without quotes" do
-    a = %i(+ - * / == < <= > >= ! != =~ !~ & | ^ ~ ** >> << % [] <=> === []? []=)
-    b = "[:+, :-, :*, :/, :==, :<, :<=, :>, :>=, :!, :!=, :=~, :!~, :&, :|, :^, :~, :**, :>>, :<<, :%, :[], :<=>, :===, :[]?, :[]=]"
+    a = %i(+ - * / == < <= > >= ! != =~ !~ & | ^ ~ ** &** >> << % [] <=> === []? []=)
+    b = "[:+, :-, :*, :/, :==, :<, :<=, :>, :>=, :!, :!=, :=~, :!~, :&, :|, :^, :~, :**, :&**, :>>, :<<, :%, :[], :<=>, :===, :[]?, :[]=]"
     a.inspect.should eq(b)
   end
 
+  it "displays the empty symbol with quotes" do
+    :"".inspect.should eq(%(:""))
+  end
+
   describe "clone" do
     it { :foo.clone.should eq(:foo) }
   end

src/compiler/crystal/macros/methods.cr

@@ -1062,7 +1062,7 @@ module Crystal
 
     private def to_double_splat(trailing_string = "")
       MacroId.new(entries.join(", ") do |entry|
-        if Symbol.needs_quotes?(entry.key)
+        if Symbol.needs_quotes_for_named_argument?(entry.key)
           "#{entry.key.inspect}: #{entry.value}"
         else
           "#{entry.key}: #{entry.value}"

src/compiler/crystal/syntax/to_s.cr

@@ -922,7 +922,7 @@ module Crystal
     end
 
     def visit_named_arg_name(name)
-      if Symbol.needs_quotes?(name)
+      if Symbol.needs_quotes_for_named_argument?(name)
         name.inspect(@str)
       else
         @str << name
@@ -1159,7 +1159,7 @@ module Crystal
       else
         @str << node.name
         @str << " = "
-        if Symbol.needs_quotes?(node.real_name)
+        if Symbol.needs_quotes_for_named_argument?(node.real_name)
           node.real_name.inspect(@str)
         else
           @str << node.real_name

src/compiler/crystal/tools/doc/macro.cr

@@ -98,11 +98,14 @@ class Crystal::Doc::Macro
 
   def arg_to_s(arg : Arg, io : IO) : Nil
     if arg.external_name != arg.name
-      name = arg.external_name.presence || "_"
-      if Symbol.needs_quotes? name
-        HTML.escape name.inspect, io
+      if name = arg.external_name.presence
+        if Symbol.needs_quotes_for_named_argument? name
+          HTML.escape name.inspect, io
+        else
+          io << name
+        end
       else
-        io << name
+        io << "_"
       end
       io << ' '
     end

src/compiler/crystal/tools/doc/method.cr

@@ -270,11 +270,14 @@ class Crystal::Doc::Method
 
   def arg_to_html(arg : Arg, io, links = true)
     if arg.external_name != arg.name
-      name = arg.external_name.presence || "_"
-      if Symbol.needs_quotes? name
-        HTML.escape name.inspect, io
+      if name = arg.external_name.presence
+        if Symbol.needs_quotes_for_named_argument? name
+          HTML.escape name.inspect, io
+        else
+          io << name
+        end
       else
-        io << name
+        io << "_"
       end
       io << ' '
     end

src/compiler/crystal/tools/doc/type.cr

@@ -605,7 +605,7 @@ class Crystal::Doc::Type
   def type_to_html(type : Crystal::NamedTupleInstanceType, io, text = nil, links = true)
     io << '{'
     type.entries.join(io, ", ") do |entry|
-      if Symbol.needs_quotes?(entry.name)
+      if Symbol.needs_quotes_for_named_argument?(entry.name)
         entry.name.inspect(io)
       else
         io << entry.name

src/compiler/crystal/types.cr

@@ -2530,7 +2530,7 @@ module Crystal
     def to_s_with_options(io : IO, skip_union_parens : Bool = false, generic_args : Bool = true, codegen : Bool = false) : Nil
       io << "NamedTuple("
       @entries.join(io, ", ") do |entry|
-        if Symbol.needs_quotes?(entry.name)
+        if Symbol.needs_quotes_for_named_argument?(entry.name)
           entry.name.inspect(io)
         else
           io << entry.name

src/named_tuple.cr

@@ -351,7 +351,7 @@ struct NamedTuple
         io << ", "
       {% end %}
       key = {{key.stringify}}
-      if Symbol.needs_quotes?(key)
+      if Symbol.needs_quotes_for_named_argument?(key)
         key.inspect(io)
       else
         io << key
@@ -370,7 +370,7 @@ struct NamedTuple
         {% end %}
         pp.group do
           key = {{key.stringify}}
-          if Symbol.needs_quotes?(key)
+          if Symbol.needs_quotes_for_named_argument?(key)
             pp.text key.inspect
           else
             pp.text key

src/symbol.cr

@@ -54,7 +54,7 @@ struct Symbol
     io << to_s
   end
 
-  # Determines if a string needs to be quoted to be used for a symbol.
+  # Determines if a string needs to be quoted to be used for a symbol literal.
   #
   # ```
   # Symbol.needs_quotes? "string"      # => false
@@ -63,9 +63,23 @@ struct Symbol
   def self.needs_quotes?(string) : Bool
     case string
     when "+", "-", "*", "&+", "&-", "&*", "/", "//", "==", "<", "<=", ">", ">=", "!", "!=", "=~", "!~"
-      # Nothing
-    when "&", "|", "^", "~", "**", ">>", "<<", "%", "[]", "<=>", "===", "[]?", "[]="
-      # Nothing
+      false
+    when "&", "|", "^", "~", "**", "&**", ">>", "<<", "%", "[]", "<=>", "===", "[]?", "[]="
+      false
+    when "_"
+      false
+    else
+      needs_quotes_for_named_argument?(string)
+    end
+  end
+
+  # :nodoc:
+  # Determines if a string needs to be quoted to be used for an external
+  # parameter name or a named argument's key.
+  def self.needs_quotes_for_named_argument?(string)
+    case string
+    when "", "_"
+      true
     else
       string.each_char_with_index do |char, i|
         if i == 0 && char.ascii_number?
@@ -79,8 +93,8 @@ struct Symbol
           return true
         end
       end
+      false
     end
-    false
   end
 
   def clone