Treat whitespace in MacroExpressions as significant

Author: Blacksmoke16

spec/compiler/parser/parser_spec.cr

@@ -40,6 +40,12 @@ private def node_source(string, node)
   source_between(string, node.location, node.end_location)
 end
 
+private def assert_location(node : ASTNode, line_number : Int32, column_number : Int32)
+  location = node.location.should_not be_nil
+  location.line_number.should eq line_number
+  location.column_number.should eq column_number
+end
+
 private def assert_end_location(source, line_number = 1, column_number = source.size, file = __FILE__, line = __LINE__)
   it "gets corrects end location for #{source.inspect}", file, line do
     string = "#{source}; 1"
@@ -1118,7 +1124,7 @@ module Crystal
     it_parses "puts {{**1}}", Call.new(nil, "puts", MacroExpression.new(DoubleSplat.new(1.int32)))
     it_parses "{{a = 1 if 2}}", MacroExpression.new(If.new(2.int32, Assign.new("a".var, 1.int32)))
     it_parses "{% a = 1 %}", MacroExpression.new(Assign.new("a".var, 1.int32), output: false)
-    it_parses "{%\na = 1\n%}", MacroExpression.new(Assign.new("a".var, 1.int32), output: false)
+    it_parses "{%\na = 1\n%}", MacroExpression.new(Assign.new("a".var, 1.int32), output: false, multiline: true)
     it_parses "{% a = 1 if 2 %}", MacroExpression.new(If.new(2.int32, Assign.new("a".var, 1.int32)), output: false)
     it_parses "{% if 1; 2; end %}", MacroExpression.new(If.new(1.int32, 2.int32), output: false)
     it_parses "{%\nif 1; 2; end\n%}", MacroExpression.new(If.new(1.int32, 2.int32), output: false)
@@ -1128,7 +1134,7 @@ module Crystal
     it_parses "{% unless 1; 2; else 3; end %}", MacroExpression.new(Unless.new(1.int32, 2.int32, 3.int32), output: false)
     it_parses "{% unless 1\n  x\nend %}", MacroExpression.new(Unless.new(1.int32, "x".var), output: false)
     it_parses "{% x unless 1 %}", MacroExpression.new(Unless.new(1.int32, "x".var), output: false)
-    it_parses "{%\n1\n2\n3\n%}", MacroExpression.new(Expressions.new([1.int32, 2.int32, 3.int32] of ASTNode), output: false)
+    it_parses "{%\n1\n2\n3\n%}", MacroExpression.new(Expressions.new([1.int32, 2.int32, 3.int32] of ASTNode), output: false, multiline: true)
 
     assert_syntax_error "{% unless 1; 2; elsif 3; 4; end %}"
     assert_syntax_error "{% unless 1 %} 2 {% elsif 3 %} 3 {% end %}"
@@ -2772,6 +2778,79 @@ module Crystal
         else_node_location.line_number.should eq 7
       end
 
+      it "sets the correct location for MacroExpressions in a MacroVerbatim in a finished hook with significant whitespace" do
+        parser = Parser.new(<<-CR)
+        macro finished
+          {% verbatim do %}
+            {%
+              10
+
+              # Foo
+
+              20
+              30
+
+              # Bar
+
+              40
+            %}
+
+            {%
+              50
+              60
+            %}
+          {% end %}
+        end
+        CR
+
+        node = parser.parse.should be_a Macro
+
+        assert_location node, 1, 1
+
+        macro_body = node.body.should be_a Expressions
+        verbatim_node = macro_body[1].should be_a MacroVerbatim
+
+        expressions = verbatim_node.exp.as(Expressions).expressions
+        expressions.size.should eq 5
+
+        expressions[0].should eq MacroLiteral.new("\n    ")
+        expression = expressions[1].should be_a MacroExpression
+
+        macro_expression = expression.exp.as(Expressions).expressions
+        macro_expression.size.should eq 10
+        macro_expression.select(MacroLiteral).size.should eq 6
+
+        num = macro_expression[0].should be_a NumberLiteral
+        num.value.should eq "10"
+        assert_location num, 4, 7
+
+        num = macro_expression[4].should be_a NumberLiteral
+        num.value.should eq "20"
+        assert_location num, 8, 7
+
+        num = macro_expression[5].should be_a NumberLiteral
+        num.value.should eq "30"
+        assert_location num, 9, 7
+
+        num = macro_expression[9].should be_a NumberLiteral
+        num.value.should eq "40"
+        assert_location num, 13, 7
+
+        expression = expressions[3].should be_a MacroExpression
+
+        macro_expression = expression.exp.as(Expressions).expressions
+        macro_expression.size.should eq 2
+        macro_expression.select(MacroLiteral).size.should eq 0
+
+        num = macro_expression[0].should be_a NumberLiteral
+        num.value.should eq "50"
+        assert_location num, 17, 7
+
+        num = macro_expression[1].should be_a NumberLiteral
+        num.value.should eq "60"
+        assert_location num, 18, 7
+      end
+
       it "sets correct location of Begin within another node" do
         parser = Parser.new(<<-CR)
           macro finished

src/compiler/crystal/syntax/ast.cr

@@ -2192,19 +2192,20 @@ module Crystal
   class MacroExpression < ASTNode
     property exp : ASTNode
     property? output : Bool
+    property? multiline : Bool
 
-    def initialize(@exp : ASTNode, @output = true)
+    def initialize(@exp : ASTNode, @output = true, @multiline = false)
     end
 
     def accept_children(visitor)
       @exp.accept visitor
     end
 
     def clone_without_location
-      MacroExpression.new(@exp.clone, @output)
+      MacroExpression.new(@exp.clone, @output, @multiline)
     end
 
-    def_equals_and_hash exp, output?
+    def_equals_and_hash exp, output?, multiline?
   end
 
   # Free text that is part of a macro

src/compiler/crystal/syntax/lexer.cr

@@ -7,6 +7,7 @@ module Crystal
   class Lexer
     property? doc_enabled : Bool
     property? comments_enabled : Bool
+    property? comments_as_newlines : Bool
     property? count_whitespace : Bool
     property? wants_raw : Bool
     property? slash_is_regex : Bool
@@ -69,6 +70,7 @@ module Crystal
       @doc_enabled = false
       @comment_is_doc = true
       @comments_enabled = false
+      @comments_as_newlines = false
       @count_whitespace = false
       @slash_is_regex = true
       @wants_raw = false
@@ -125,6 +127,12 @@ module Crystal
             return consume_comment(start)
           else
             skip_comment
+
+            if @comments_as_newlines
+              @token.type = :newline
+              @token.value = "\n"
+              return @token
+            end
           end
         end
       end

src/compiler/crystal/syntax/parser.cr

@@ -110,26 +110,43 @@ module Crystal
       end
 
       exp = parse_multi_assign
+      exps = [] of ASTNode
+      newlines = [] of ASTNode
+      exps.push exp
 
       slash_is_regex!
-      skip_statement_end
+      collect_significant_newlines.tap do |newlines|
+        exps.concat newlines if newlines.size > 1
+      end
 
       if end_token?
         return exp
       end
 
-      exps = [] of ASTNode
-      exps.push exp
-
       loop do
         exps << parse_multi_assign
-        skip_statement_end
+        collect_significant_newlines.tap do |newlines|
+          exps.concat newlines if newlines.size > 1
+        end
+
         break if end_token?
       end
 
       Expressions.from(exps)
     end
 
+    # Replicates what `#skip_statement_end` does, but collects the newlines if within a macro expression
+    private def collect_significant_newlines : Array(ASTNode)
+      newlines = [] of ASTNode
+
+      while (@token.type.space? || @token.type.newline? || @token.type.op_semicolon?)
+        newlines << MacroLiteral.new("") if @token.type.newline? && @in_macro_expression
+        next_token
+      end
+
+      newlines
+    end
+
     def parse_multi_assign
       location = @token.location
 
@@ -3353,7 +3370,13 @@ module Crystal
 
     def parse_macro_control(start_location, macro_state = Token::MacroState.default)
       location = @token.location
-      next_token_skip_space_or_newline
+      next_token_skip_space
+      multiline = false
+
+      if @token.type.newline?
+        multiline = true
+        next_token_skip_space_or_newline
+      end
 
       case @token.value
       when Keyword::FOR
@@ -3431,16 +3454,18 @@ module Crystal
         next_token_skip_space
         check :OP_PERCENT_RCURLY
 
-        return MacroVerbatim.new(body).at_end(token_end_location)
+        return MacroVerbatim.new(body).at(location).at_end(token_end_location)
       else
         # will be parsed as a normal expression
       end
 
       @in_macro_expression = true
+      @comments_as_newlines = true
       exps = parse_expressions
       @in_macro_expression = false
+      @comments_as_newlines = false
 
-      MacroExpression.new(exps, output: false).at(location).at_end(token_end_location)
+      MacroExpression.new(exps, output: false, multiline: multiline).at(location).at_end(token_end_location)
     end
 
     def parse_macro_if(start_location, macro_state, check_end = true, is_unless = false)

src/compiler/crystal/syntax/to_s.cr

@@ -730,6 +730,7 @@ module Crystal
     def visit(node : MacroExpression)
       @str << (node.output? ? "{{" : "{% ")
       @str << ' ' if node.output?
+      newline if node.multiline?
       outside_macro do
         node.exp.accept self
       end