Syntax: check Expressions#keyword in to_s and add If#ternary? (#…

…5226)
crystal-lang · Nov 2, 2017 · bf3dd80 · bf3dd80
1 parent ef117af
commit bf3dd80
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 52 deletions.
diff --git a/spec/compiler/normalize/expressions_spec.cr b/spec/compiler/normalize/expressions_spec.cr
@@ -1,15 +1,15 @@
 require "../../spec_helper"
 
 describe "Normalize: expressions" do
-  it "normalizes an empty expression" do
-    assert_normalize "begin\nend", ""
+  it "normalizes an empty expression with begin/end" do
+    assert_normalize "begin\nend", "begin\nend"
   end
 
   it "normalizes an expression" do
     assert_normalize "(1 < 2).as(Bool)", "(1 < 2).as(Bool)"
   end
 
-  it "normalizes expressions" do
-    assert_normalize "begin\n  1\n  2\nend", "1\n2"
+  it "normalizes expressions with begin/end" do
+    assert_normalize "begin\n  1\n  2\nend", "begin\n  1\n  2\nend"
   end
 end
diff --git a/spec/compiler/parser/to_s_spec.cr b/spec/compiler/parser/to_s_spec.cr
@@ -104,9 +104,16 @@ describe "ASTNode#to_s" do
   expect_to_s %(enum Foo\n  A = 0\n  B\nend)
   expect_to_s %(alias Foo = Void)
   expect_to_s %(type(Foo = Void))
-  expect_to_s %(return true ? 1 : 2), %(return begin\n  if true\n    1\n  else\n    2\n  end\nend)
+  expect_to_s %(return true ? 1 : 2)
   expect_to_s %(1 <= 2 <= 3)
   expect_to_s %((1 <= 2) <= 3)
   expect_to_s %(1 <= (2 <= 3))
   expect_to_s %(case 1; when .foo?; 2; end), %(case 1\nwhen .foo?\n  2\nend)
+  expect_to_s %({(1 + 2)})
+  expect_to_s %({foo: (1 + 2)})
+  expect_to_s %q("#{(1 + 2)}")
+  expect_to_s %({(1 + 2) => (3 + 4)})
+  expect_to_s %([(1 + 2)] of Int32)
+  expect_to_s %(foo(1, (2 + 3), bar: (4 + 5)))
+  expect_to_s %(if (1 + 2\n3)\n  4\nend)
 end
diff --git a/src/compiler/crystal/syntax/ast.cr b/src/compiler/crystal/syntax/ast.cr
@@ -587,8 +587,9 @@ module Crystal
     property cond : ASTNode
     property then : ASTNode
     property else : ASTNode
+    property? ternary : Bool
 
-    def initialize(@cond, a_then = nil, a_else = nil)
+    def initialize(@cond, a_then = nil, a_else = nil, @ternary = false)
       @then = Expressions.from a_then
       @else = Expressions.from a_else
     end
@@ -600,7 +601,7 @@ module Crystal
     end
 
     def clone_without_location
-      If.new(@cond.clone, @then.clone, @else.clone)
+      If.new(@cond.clone, @then.clone, @else.clone, @ternary)
     end
 
     def_equals_and_hash @cond, @then, @else

diff --git a/src/compiler/crystal/syntax/parser.cr b/src/compiler/crystal/syntax/parser.cr
@@ -429,7 +429,7 @@ module Crystal
         false_val = parse_question_colon
         @no_type_declaration -= 1
 
-        cond = If.new(cond, true_val, false_val).at(cond).at_end(false_val)
+        cond = If.new(cond, true_val, false_val, ternary: true).at(cond).at_end(false_val)
       end
 
       cond

diff --git a/src/compiler/crystal/syntax/to_s.cr b/src/compiler/crystal/syntax/to_s.cr
@@ -198,21 +198,52 @@ module Crystal
     end
 
     def visit(node : Expressions)
+      parens = node.keyword == :"("
+      begin_end = node.keyword == :begin
+
+      case
+      when parens
+        @str << "("
+      when begin_end
+        @str << "begin"
+        @indent += 1
+        newline
+      end
+
       if @inside_macro > 0
         node.expressions.each &.accept self
       else
-        node.expressions.each do |exp|
+        node.expressions.each_with_index do |exp, i|
           unless exp.nop?
             append_indent
             exp.accept self
-            newline
+            newline unless parens && i == node.expressions.size - 1
           end
         end
       end
+
+      case
+      when parens
+        @str << ")"
+      when begin_end
+        @indent -= 1
+        append_indent
+        @str << "end"
+      end
+
       false
     end
 
     def visit(node : If)
+      if node.ternary?
+        node.cond.accept self
+        @str << " ? "
+        node.then.accept self
+        @str << " : "
+        node.else.accept self
+        return false
+      end
+
       visit_if_or_unless "if", node
     end
 
@@ -526,14 +557,14 @@ module Crystal
     def visit(node : Assign)
       node.target.accept self
       @str << " = "
-      accept_with_maybe_begin_end node.value
+      node.value.accept self
       false
     end
 
     def visit(node : OpAssign)
       node.target.accept self
       @str << " " << node.op << "=" << " "
-      accept_with_maybe_begin_end node.value
+      node.value.accept self
       false
     end
 
@@ -963,7 +994,7 @@ module Crystal
       @str << keyword(keyword)
       if exp = node.exp
         @str << " "
-        accept_with_maybe_begin_end exp
+        exp.accept self
       end
       false
     end
@@ -1519,31 +1550,6 @@ module Crystal
       newline
     end
 
-    def accept_with_maybe_begin_end(node)
-      case node
-      when Expressions
-        if node.expressions.size == 1
-          @str << "("
-          node.expressions.first.accept self
-          @str << ")"
-        else
-          @str << keyword("begin")
-          newline
-          accept_with_indent(node)
-          append_indent
-          @str << keyword("end")
-        end
-      when If, Unless, While, Until
-        @str << keyword("begin")
-        newline
-        accept_with_indent(node)
-        append_indent
-        @str << keyword("end")
-      else
-        node.accept self
-      end
-    end
-
     def inside_macro
       @inside_macro += 1
       yield

diff --git a/src/compiler/crystal/tools/formatter.cr b/src/compiler/crystal/tools/formatter.cr
@@ -1175,6 +1175,19 @@ module Crystal
     end
 
     def visit(node : If)
+      if node.ternary?
+        accept node.cond
+        skip_space_or_newline
+        write_token " ", :"?", " "
+        skip_space_or_newline
+        accept node.then
+        skip_space_or_newline
+        write_token " ", :":", " "
+        skip_space_or_newline
+        accept node.else
+        return false
+      end
+
       visit_if_or_unless node, :if
     end
 
@@ -1193,20 +1206,6 @@ module Crystal
         return false
       end
 
-      # This is the case of `cond ? exp1 : exp2`
-      if keyword == :if && !@token.keyword?(:if)
-        accept node.cond
-        skip_space_or_newline
-        write_token " ", :"?", " "
-        skip_space_or_newline
-        accept node.then
-        skip_space_or_newline
-        write_token " ", :":", " "
-        skip_space_or_newline
-        accept node.else
-        return false
-      end
-
       write_keyword keyword, " "
       format_if_at_cond node, keyword