Merge pull request #2592 from crystal-lang/feature/external_args_names

Add external/internal names of method/macro arguments
crystal-lang · May 16, 2016 · 8027c8d · 8027c8d
2 parents 380d4ea + bb0be61
commit 8027c8d
Showing 18 changed files with 258 additions and 54 deletions.
diff --git a/spec/compiler/formatter/formatter_spec.cr b/spec/compiler/formatter/formatter_spec.cr
@@ -896,4 +896,8 @@ describe Crystal::Formatter do
   assert_format "def foo(**z, &block)\nend"
   assert_format "def foo(x, **z)\nend"
   assert_format "def foo(x, **z, &block)\nend"
+
+  assert_format "def foo(x y)\nend"
+  assert_format "def foo(x @y)\nend"
+  assert_format "def foo(x @@y)\nend"
 end
diff --git a/spec/compiler/normalize/def_spec.cr b/spec/compiler/normalize/def_spec.cr
@@ -183,4 +183,30 @@ describe "Normalize: def" do
     other_def = a_def.expand_default_arguments(Program.new, 1, ["y"])
     other_def.to_s.should eq("def foo:y(x, y)\n  x + y\nend")
   end
+
+  it "expands a def with external names (1)" do
+    a_def = parse("def foo(x y); y; end").as(Def)
+    actual = a_def.expand_default_arguments(Program.new, 0, ["x"])
+    actual.should be(a_def)
+  end
+
+  it "expands a def with external names (2)" do
+    a_def = parse("def foo(x x1, y y1); x1 + y1; end").as(Def)
+    other_def = a_def.expand_default_arguments(Program.new, 0, ["y", "x"])
+    other_def.to_s.should eq("def foo:y:x(y y1, x x1)\n  foo(x1, y1)\nend")
+  end
+
+  it "expands a def on request with default arguments (external names)" do
+    a_def = parse("def foo(x x1, y y1 = 1, z z1 = 2); x1 + y1 + z1; end").as(Def)
+    actual = a_def.expand_default_arguments(Program.new, 1)
+    expected = parse("def foo(x x1); y1 = 1; z1 = 2; foo(x1, y1, z1); end")
+    actual.should eq(expected)
+  end
+
+  it "expands a def on request with default arguments that yields (external names)" do
+    a_def = parse("def foo(x x1, y y1 = 1, z z1 = 2); yield x1 + y1 + z1; end").as(Def)
+    actual = a_def.expand_default_arguments(Program.new, 1)
+    expected = parse("def foo(x x1); y1 = 1; z1 = 2; yield x1 + y1 + z1; end")
+    actual.should eq(expected)
+  end
 end
diff --git a/spec/compiler/parser/parser_spec.cr b/spec/compiler/parser/parser_spec.cr
@@ -212,6 +212,19 @@ describe "Parser" do
 
   assert_syntax_error "def foo(**args, **args2)"
 
+  it_parses "def foo(x y); y; end", Def.new("foo", args: [Arg.new("y", external_name: "x")], body: "y".var)
+  it_parses "def foo(x @var); end", Def.new("foo", [Arg.new("var", external_name: "x")], [Assign.new("@var".instance_var, "var".var)] of ASTNode)
+  it_parses "def foo(x @@var); end", Def.new("foo", [Arg.new("var", external_name: "x")], [Assign.new("@@var".class_var, "var".var)] of ASTNode)
+  assert_syntax_error "def foo(_ y); y; end"
+
+  assert_syntax_error "def foo(x x); 1; end", "when specified, external name must be different than internal name"
+  assert_syntax_error "def foo(x @x); 1; end", "when specified, external name must be different than internal name"
+  assert_syntax_error "def foo(x @@x); 1; end", "when specified, external name must be different than internal name"
+
+  assert_syntax_error "def foo(*a foo); end"
+  assert_syntax_error "def foo(**a foo); end"
+  assert_syntax_error "def foo(&a foo); end"
+
   it_parses "macro foo(**args)\n1\nend", Macro.new("foo", body: MacroLiteral.new("1\n"), double_splat: "args")
 
   assert_syntax_error "macro foo(x, *); 1; end", "named arguments must follow bare *"
@@ -1124,9 +1137,8 @@ describe "Parser" do
 
   assert_syntax_error %<{"x": [] of Int32,\n}\n1.foo(>, "unterminated call", 3, 6
 
-  assert_syntax_error "def foo(x y); end", "unexpected token: y (expected ',' or ')')"
   assert_syntax_error "def foo x y; end", "parentheses are mandatory for def arguments"
-  assert_syntax_error "macro foo(x y); end", "unexpected token: y (expected ',' or ')')"
+  assert_syntax_error "macro foo(x y z); end"
   assert_syntax_error "macro foo x y; end", "parentheses are mandatory for macro arguments"
   assert_syntax_error "macro foo *y;end", "parentheses are mandatory for macro arguments"
   assert_syntax_error %(macro foo x; 1 + 2; end), "parentheses are mandatory for macro arguments"

diff --git a/spec/compiler/parser/to_s_spec.cr b/spec/compiler/parser/to_s_spec.cr
@@ -65,4 +65,5 @@ describe "ASTNode#to_s" do
   expect_to_s "def foo(x, **args, &block)\nend"
   expect_to_s "macro foo(**args)\nend"
   expect_to_s "macro foo(x, **args)\nend"
+  expect_to_s "def foo(x y)\nend"
 end
diff --git a/spec/compiler/type_inference/external_internal_spec.cr b/spec/compiler/type_inference/external_internal_spec.cr
@@ -0,0 +1,65 @@
+require "../../spec_helper"
+
+describe "Type inference: external/internal" do
+  it "can call with external name and use with internal" do
+    assert_type(%(
+      def foo(x y)
+        y
+      end
+
+      foo x: 10
+      )) { int32 }
+  end
+
+  it "can call positionally" do
+    assert_type(%(
+      def foo(x y)
+        y
+      end
+
+      foo 10
+      )) { int32 }
+  end
+
+  it "can call with external name and use with internal, after splat" do
+    assert_type(%(
+      def foo(*, x y)
+        y
+      end
+
+      foo x: 10
+      )) { int32 }
+  end
+
+  context "macros" do
+    it "can call with external name and use with internal" do
+      assert_type(%(
+        macro foo(x y)
+          {{y}}
+        end
+
+        foo x: 10
+        )) { int32 }
+    end
+
+    it "can call positionally" do
+      assert_type(%(
+        macro foo(x y)
+          {{y}}
+        end
+
+        foo 10
+        )) { int32 }
+    end
+
+    it "can call with external name and use with internal, after splat" do
+      assert_type(%(
+        macro foo(*, x y)
+          {{y}}
+        end
+
+        foo x: 10
+        )) { int32 }
+    end
+  end
+end
diff --git a/src/compiler/crystal/macros/macros.cr b/src/compiler/crystal/macros/macros.cr
@@ -128,7 +128,7 @@ module Crystal
           if named_args = call.named_args
             named_args.each do |named_arg|
               # Skip an argument that's already there as a positional argument
-              next if a_macro.args.any? &.name.==(named_arg.name)
+              next if a_macro.args.any? &.external_name.==(named_arg.name)
 
               named_tuple_elems << NamedTupleLiteral::Entry.new(named_arg.name, named_arg.value)
             end
@@ -150,7 +150,9 @@ module Crystal
 
         # The named arguments
         call.named_args.try &.each do |named_arg|
-          vars[named_arg.name] = named_arg.value
+          arg = a_macro.args.find { |arg| arg.external_name == named_arg.name }
+          arg_name = arg.try(&.name) || named_arg.name
+          vars[arg_name] = named_arg.value
         end
 
         # The block arg

diff --git a/src/compiler/crystal/semantic/ast.cr b/src/compiler/crystal/semantic/ast.cr
@@ -300,7 +300,8 @@ module Crystal
   end
 
   class Arg
-    def initialize(@name, @default_value : ASTNode? = nil, @restriction : ASTNode? = nil, @type : Type? = nil)
+    def initialize(@name : String, @default_value : ASTNode? = nil, @restriction : ASTNode? = nil, external_name : String? = nil, @type : Type? = nil)
+      @external_name = external_name || @name
     end
 
     def clone_without_location

diff --git a/src/compiler/crystal/semantic/call.cr b/src/compiler/crystal/semantic/call.cr
@@ -1045,11 +1045,13 @@ class Crystal::Call
     end
 
     named_args_types.try &.each do |named_arg|
+      arg = typed_def.args.find { |arg| arg.external_name == named_arg.name }.not_nil!
+
       type = named_arg.type
-      var = MetaVar.new(named_arg.name, type)
+      var = MetaVar.new(arg.name, type)
       var.bind_to(var)
-      args[named_arg.name] = var
-      arg = typed_def.args.find { |arg| arg.name == named_arg.name }.not_nil!
+
+      args[arg.name] = var
       arg.type = type
     end
 

diff --git a/src/compiler/crystal/semantic/call_error.cr b/src/compiler/crystal/semantic/call_error.cr
@@ -304,7 +304,7 @@ class Crystal::Call
     end
 
     named_args.try &.each do |named_arg|
-      found_index = a_def.args.index { |arg| arg.name == named_arg.name }
+      found_index = a_def.args.index { |arg| arg.external_name == named_arg.name }
       if found_index
         mandatory_args[found_index] = true
       end
@@ -316,9 +316,9 @@ class Crystal::Call
 
       arg = a_def.args[index]
       next if arg.default_value
-      next if arg.name.empty?
+      next if arg.external_name.empty?
 
-      missing_args << arg.name
+      missing_args << arg.external_name
     end
 
     case missing_args.size
@@ -393,7 +393,14 @@ class Crystal::Call
     a_def.args.each_with_index do |arg, i|
       str << ", " if printed
       str << '*' if a_def.splat_index == i
+
+      if arg.external_name != arg.name
+        str << (arg.external_name.empty? ? "_" : arg.external_name)
+        str << " "
+      end
+
       str << arg.name
+
       if arg_default = arg.default_value
         str << " = "
         str << arg.default_value

diff --git a/src/compiler/crystal/semantic/default_arguments.cr b/src/compiler/crystal/semantic/default_arguments.cr
@@ -9,7 +9,7 @@ class Crystal::Def
       all_match = true
       named_args.each_with_index do |named_arg, i|
         arg = args[args_size + i]
-        unless arg.name == named_arg
+        unless arg.external_name == named_arg
           all_match = false
           break
         end
@@ -53,7 +53,7 @@ class Crystal::Def
     end
 
     # Splat arg
-    if splat_index && !args[splat_index].name.empty?
+    if splat_index && !args[splat_index].external_name.empty?
       splat_names = [] of String
 
       splat_size = args_size - splat_index
@@ -73,7 +73,14 @@ class Crystal::Def
         named_args.each do |named_arg|
           str << ':'
           str << named_arg
-          new_args << Arg.new(named_arg)
+
+          # If a named argument matches an argument's external name, use the internal name
+          matching_arg = args.find { |arg| arg.external_name == named_arg }
+          if matching_arg
+            new_args << Arg.new(matching_arg.name, external_name: named_arg)
+          else
+            new_args << Arg.new(named_arg)
+          end
         end
       end
     else
@@ -104,12 +111,8 @@ class Crystal::Def
         # Skip if this is the splat index argument
         next if index == splat_index
 
-        # pp arg, index, splat_index, args_size
-        # pp index, args_size, splat_index
-        # next if index <= splat_index || index < args_size
-
         # But first check if we already have it in the named arguments
-        unless named_args.try &.index(arg.name)
+        unless named_args.try &.index(arg.external_name)
           default_value = arg.default_value.not_nil!
 
           # If the default value is a magic constant we add it to the expanded
@@ -172,7 +175,7 @@ class Crystal::Def
         arg = args[index]
 
         # But first check if we already have it in the named arguments
-        if named_args.try &.index(arg.name)
+        if named_args.try &.index(arg.external_name)
           new_args.push Var.new(arg.name)
         else
           default_value = arg.default_value.not_nil!
@@ -196,8 +199,6 @@ class Crystal::Def
       expansion.body = Expressions.new(body)
     end
 
-    # pp expansion
-
     expansion
   end
 

diff --git a/src/compiler/crystal/semantic/method_lookup.cr b/src/compiler/crystal/semantic/method_lookup.cr
@@ -172,7 +172,7 @@ module Crystal
       if named_args
         min_index = signature.arg_types.size
         named_args.each do |named_arg|
-          found_index = a_def.args.index { |arg| arg.name == named_arg.name }
+          found_index = a_def.args.index { |arg| arg.external_name == named_arg.name }
           if found_index
             # A named arg can't target the splat index
             if found_index == splat_index

diff --git a/src/compiler/crystal/semantic/to_s.cr b/src/compiler/crystal/semantic/to_s.cr
@@ -3,6 +3,10 @@ require "../syntax/to_s"
 module Crystal
   class ToSVisitor
     def visit(node : Arg)
+      if node.external_name != node.name
+        @str << (node.external_name.empty? ? "_" : node.external_name)
+        @str << " "
+      end
       if node.name
         @str << decorate_arg(node, node.name)
       else

diff --git a/src/compiler/crystal/syntax/ast.cr b/src/compiler/crystal/syntax/ast.cr
@@ -835,12 +835,15 @@ module Crystal
   class Arg < ASTNode
     include SpecialVar
 
+    # The internal name
     property name : String
+    property external_name : String
     property default_value : ASTNode?
     property restriction : ASTNode?
     property doc : String?
 
-    def initialize(@name, @default_value : ASTNode? = nil, @restriction : ASTNode? = nil)
+    def initialize(@name : String, @default_value : ASTNode? = nil, @restriction : ASTNode? = nil, external_name : String? = nil)
+      @external_name = external_name || @name
     end
 
     def accept_children(visitor)
@@ -853,10 +856,10 @@ module Crystal
     end
 
     def clone_without_location
-      Arg.new @name, @default_value.clone, @restriction.clone
+      Arg.new @name, @default_value.clone, @restriction.clone, @external_name.clone
     end
 
-    def_equals_and_hash name, default_value, restriction
+    def_equals_and_hash name, default_value, restriction, external_name
   end
 
   class Fun < ASTNode
@@ -1006,7 +1009,7 @@ module Crystal
       splat_index = self.splat_index
 
       if splat_index
-        if args[splat_index].name.empty?
+        if args[splat_index].external_name.empty?
           min_args_size = max_args_size = splat_index
         else
           min_args_size -= 1
@@ -1043,7 +1046,7 @@ module Crystal
 
       # Check named args
       named_args.try &.each do |named_arg|
-        found_index = args.index { |arg| arg.name == named_arg.name }
+        found_index = args.index { |arg| arg.external_name == named_arg.name }
         if found_index
           # A named arg can't target the splat index
           if found_index == splat_index