From e6c90504ac451bebddedf333e01d4690d4be3bc6 Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Fri, 20 Dec 2024 23:29:33 -0500 Subject: [PATCH 01/10] Handle properly stringifying multiline macro expressions --- spec/compiler/parser/parser_spec.cr | 5 +++-- spec/compiler/parser/to_s_spec.cr | 6 ++++++ src/compiler/crystal/syntax/ast.cr | 18 +++++++++++++----- src/compiler/crystal/syntax/parser.cr | 10 ++++++++-- src/compiler/crystal/syntax/to_s.cr | 24 ++++++++++++++++++++---- 5 files changed, 50 insertions(+), 13 deletions(-) diff --git a/spec/compiler/parser/parser_spec.cr b/spec/compiler/parser/parser_spec.cr index 897e5bf7060c..a6aa70a37395 100644 --- a/spec/compiler/parser/parser_spec.cr +++ b/spec/compiler/parser/parser_spec.cr @@ -1118,7 +1118,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 "{%\n a = 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 +1128,8 @@ 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 "{%\n x unless 1\n%}", MacroExpression.new(Unless.new(1.int32, "x".var), output: false, multiline: true) + it_parses "{%\n 1\n 2\n 3\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 %}" diff --git a/spec/compiler/parser/to_s_spec.cr b/spec/compiler/parser/to_s_spec.cr index 86464e197267..bfcce21aee2a 100644 --- a/spec/compiler/parser/to_s_spec.cr +++ b/spec/compiler/parser/to_s_spec.cr @@ -248,6 +248,12 @@ describe "ASTNode#to_s" do expect_to_s "1.//(2, &block)" expect_to_s %({% verbatim do %}\n 1{{ 2 }}\n 3{{ 4 }}\n{% end %}) expect_to_s %({% for foo in bar %}\n {{ if true\n foo\n bar\nend }}\n{% end %}) + expect_to_s "{% a = 1 %}" + expect_to_s "{{ a = 1 }}" + expect_to_s "{%\n 1\n 2\n 3\n%}" + expect_to_s "{%\n 1\n%}" + expect_to_s "{%\n 2 + 2\n%}" + expect_to_s %(asm("nop" ::::)) expect_to_s %(asm("nop" : "a"(1), "b"(2) : "c"(3), "d"(4) : "e", "f" : "volatile", "alignstack", "intel")) expect_to_s %(asm("nop" :: "c"(3), "d"(4) ::)) diff --git a/src/compiler/crystal/syntax/ast.cr b/src/compiler/crystal/syntax/ast.cr index 9ccd8dda1f69..66d18c308845 100644 --- a/src/compiler/crystal/syntax/ast.cr +++ b/src/compiler/crystal/syntax/ast.cr @@ -2187,13 +2187,21 @@ module Crystal end # A macro expression, - # surrounded by {{ ... }} (output = true) - # or by {% ... %} (output = false) + # surrounded by {{ ... }}` (output = true, multiline = false) + # or by `{% ... %}` (output = false, multiline = false) + # + # ``` + # # (output = false, multiline = true) + # {% + # ... + # %} + # ``` 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 : Bool = false) end def accept_children(visitor) @@ -2201,10 +2209,10 @@ module Crystal 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 diff --git a/src/compiler/crystal/syntax/parser.cr b/src/compiler/crystal/syntax/parser.cr index 569bbd4d9409..3cf31410b41b 100644 --- a/src/compiler/crystal/syntax/parser.cr +++ b/src/compiler/crystal/syntax/parser.cr @@ -3353,7 +3353,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 @@ -3440,7 +3446,7 @@ module Crystal exps = parse_expressions @in_macro_expression = 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) diff --git a/src/compiler/crystal/syntax/to_s.cr b/src/compiler/crystal/syntax/to_s.cr index 4ce9ca7efc43..a6537027ce5e 100644 --- a/src/compiler/crystal/syntax/to_s.cr +++ b/src/compiler/crystal/syntax/to_s.cr @@ -728,13 +728,29 @@ module Crystal end def visit(node : MacroExpression) - @str << (node.output? ? "{{" : "{% ") - @str << ' ' if node.output? + @str << (node.output? ? "{{ " : node.multiline? ? "{%" : "{% ") + + if node.multiline? + newline + @indent += 1 + end + outside_macro do + # If the MacroExpression consists of a single node we need to manually handle appending indent and trailing newline if #multiline? + # Otherwise, the Expressions logic handles that for us + if !node.exp.is_a? Expressions + append_indent + end + node.exp.accept self end - @str << ' ' if node.output? - @str << (node.output? ? "}}" : " %}") + + if node.multiline? + @indent -= 1 + newline if !node.exp.is_a? Expressions + end + + @str << (node.output? ? " }}" : node.multiline? ? "%}" : " %}") false end From 3408a66c08d8b86b366c4a6c11ad2b37c0dd37e5 Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Sat, 21 Dec 2024 09:31:56 -0500 Subject: [PATCH 02/10] Add specs with a newline at only one end --- spec/compiler/parser/to_s_spec.cr | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/compiler/parser/to_s_spec.cr b/spec/compiler/parser/to_s_spec.cr index bfcce21aee2a..a339b120d328 100644 --- a/spec/compiler/parser/to_s_spec.cr +++ b/spec/compiler/parser/to_s_spec.cr @@ -253,6 +253,8 @@ describe "ASTNode#to_s" do expect_to_s "{%\n 1\n 2\n 3\n%}" expect_to_s "{%\n 1\n%}" expect_to_s "{%\n 2 + 2\n%}" + expect_to_s "{%\n a = 1 %}", "{%\n a = 1\n%}" + expect_to_s "{% a = 1\n%}", "{% a = 1 %}" expect_to_s %(asm("nop" ::::)) expect_to_s %(asm("nop" : "a"(1), "b"(2) : "c"(3), "d"(4) : "e", "f" : "volatile", "alignstack", "intel")) From 3f32cf996d2da1e9c63258ef772f36d646097547 Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Sun, 22 Dec 2024 10:31:33 -0500 Subject: [PATCH 03/10] Properly increment indentation within some macro nodes --- spec/compiler/parser/to_s_spec.cr | 66 ++++++++++++++++++++++++++++- src/compiler/crystal/syntax/to_s.cr | 13 +++++- 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/spec/compiler/parser/to_s_spec.cr b/spec/compiler/parser/to_s_spec.cr index a339b120d328..276e0380b9fb 100644 --- a/spec/compiler/parser/to_s_spec.cr +++ b/spec/compiler/parser/to_s_spec.cr @@ -246,7 +246,13 @@ describe "ASTNode#to_s" do expect_to_s "1.+(&block)" expect_to_s "1.//(2, a: 3)" expect_to_s "1.//(2, &block)" - expect_to_s %({% verbatim do %}\n 1{{ 2 }}\n 3{{ 4 }}\n{% end %}) + expect_to_s <<-'CR' + {% verbatim do %} + 1{{ 2 }} + 3{{ 4 }} + {% end %} + CR + expect_to_s %({% for foo in bar %}\n {{ if true\n foo\n bar\nend }}\n{% end %}) expect_to_s "{% a = 1 %}" expect_to_s "{{ a = 1 }}" @@ -256,6 +262,64 @@ describe "ASTNode#to_s" do expect_to_s "{%\n a = 1 %}", "{%\n a = 1\n%}" expect_to_s "{% a = 1\n%}", "{% a = 1 %}" + # FIXME: Should respect significant whitespace + expect_to_s <<-'CR', <<-'CR' + macro finished + {% verbatim do %} + {% + 10 + + # Foo + + 20 + %} + {% end %} + end + CR + macro finished + {% verbatim do %} + {% + 10 + 20 + %} + {% end %} + end + CR + + # FIXME: Should respect significant whitespace + expect_to_s <<-'CR', <<-'CR' + macro finished + {% verbatim do %} + {% + 10 + + # Foo + 20 + %} + {% end %} + end + CR + macro finished + {% verbatim do %} + {% + 10 + 20 + %} + {% end %} + end + CR + + expect_to_s <<-'CR' # , focus: true + macro finished + {% verbatim do %} + {% + 10 + 20 + %} + {% end %} + end + CR + expect_to_s %(asm("nop" ::::)) expect_to_s %(asm("nop" : "a"(1), "b"(2) : "c"(3), "d"(4) : "e", "f" : "volatile", "alignstack", "intel")) expect_to_s %(asm("nop" :: "c"(3), "d"(4) ::)) diff --git a/src/compiler/crystal/syntax/to_s.cr b/src/compiler/crystal/syntax/to_s.cr index a6537027ce5e..8206f92053d2 100644 --- a/src/compiler/crystal/syntax/to_s.cr +++ b/src/compiler/crystal/syntax/to_s.cr @@ -223,7 +223,9 @@ module Crystal else node.expressions.each_with_index do |exp, i| unless exp.nop? - append_indent unless node.keyword.paren? && i == 0 + if !exp.is_a?(MacroLiteral) || !exp.value.blank? + append_indent unless node.keyword.paren? && i == 0 + end exp.accept self newline unless node.keyword.paren? && i == node.expressions.size - 1 end @@ -717,9 +719,11 @@ module Crystal end newline + @indent += 1 inside_macro do accept node.body end + @indent -= 1 # newline append_indent @@ -738,7 +742,7 @@ module Crystal outside_macro do # If the MacroExpression consists of a single node we need to manually handle appending indent and trailing newline if #multiline? # Otherwise, the Expressions logic handles that for us - if !node.exp.is_a? Expressions + if node.multiline? && !node.exp.is_a?(Expressions) append_indent end @@ -747,6 +751,7 @@ module Crystal if node.multiline? @indent -= 1 + append_indent newline if !node.exp.is_a? Expressions end @@ -806,9 +811,13 @@ module Crystal def visit(node : MacroVerbatim) @str << "{% verbatim do %}" + + @indent += 1 inside_macro do node.exp.accept self end + @indent -= 1 + @str << "{% end %}" false end From 18a8195e4a020fafc10e444331307a1a2bb4c36e Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Sun, 22 Dec 2024 10:32:12 -0500 Subject: [PATCH 04/10] Remove debug reference --- spec/compiler/parser/to_s_spec.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/compiler/parser/to_s_spec.cr b/spec/compiler/parser/to_s_spec.cr index 276e0380b9fb..ed28ecd55dd1 100644 --- a/spec/compiler/parser/to_s_spec.cr +++ b/spec/compiler/parser/to_s_spec.cr @@ -309,7 +309,7 @@ describe "ASTNode#to_s" do end CR - expect_to_s <<-'CR' # , focus: true + expect_to_s <<-'CR' macro finished {% verbatim do %} {% From 1264859017b75be86697955154e34c14cd897c7e Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Sun, 22 Dec 2024 10:39:19 -0500 Subject: [PATCH 05/10] This was erroneously included --- src/compiler/crystal/syntax/to_s.cr | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/compiler/crystal/syntax/to_s.cr b/src/compiler/crystal/syntax/to_s.cr index 8206f92053d2..3da5b72fab4a 100644 --- a/src/compiler/crystal/syntax/to_s.cr +++ b/src/compiler/crystal/syntax/to_s.cr @@ -223,9 +223,7 @@ module Crystal else node.expressions.each_with_index do |exp, i| unless exp.nop? - if !exp.is_a?(MacroLiteral) || !exp.value.blank? - append_indent unless node.keyword.paren? && i == 0 - end + append_indent unless node.keyword.paren? && i == 0 exp.accept self newline unless node.keyword.paren? && i == node.expressions.size - 1 end From 12b061be342006133e77014d62161484d1948a7a Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Sun, 22 Dec 2024 18:44:11 -0500 Subject: [PATCH 06/10] Refactor to only touch `ToSVisitor` --- spec/compiler/parser/parser_spec.cr | 6 ++--- src/compiler/crystal/syntax/ast.cr | 18 +++++---------- src/compiler/crystal/syntax/parser.cr | 10 ++------- src/compiler/crystal/syntax/to_s.cr | 32 +++++++++++++++------------ 4 files changed, 28 insertions(+), 38 deletions(-) diff --git a/spec/compiler/parser/parser_spec.cr b/spec/compiler/parser/parser_spec.cr index a6aa70a37395..53fbedbcd7c1 100644 --- a/spec/compiler/parser/parser_spec.cr +++ b/spec/compiler/parser/parser_spec.cr @@ -1118,7 +1118,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 "{%\n a = 1\n%}", MacroExpression.new(Assign.new("a".var, 1.int32), output: false, multiline: true) + it_parses "{%\n a = 1\n%}", MacroExpression.new(Assign.new("a".var, 1.int32), output: false) 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,8 +1128,8 @@ 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 "{%\n x unless 1\n%}", MacroExpression.new(Unless.new(1.int32, "x".var), output: false, multiline: true) - it_parses "{%\n 1\n 2\n 3\n%}", MacroExpression.new(Expressions.new([1.int32, 2.int32, 3.int32] of ASTNode), output: false, multiline: true) + it_parses "{%\n x unless 1\n%}", MacroExpression.new(Unless.new(1.int32, "x".var), output: false) + it_parses "{%\n 1\n 2\n 3\n%}", MacroExpression.new(Expressions.new([1.int32, 2.int32, 3.int32] of ASTNode), output: false) assert_syntax_error "{% unless 1; 2; elsif 3; 4; end %}" assert_syntax_error "{% unless 1 %} 2 {% elsif 3 %} 3 {% end %}" diff --git a/src/compiler/crystal/syntax/ast.cr b/src/compiler/crystal/syntax/ast.cr index 66d18c308845..9ccd8dda1f69 100644 --- a/src/compiler/crystal/syntax/ast.cr +++ b/src/compiler/crystal/syntax/ast.cr @@ -2187,21 +2187,13 @@ module Crystal end # A macro expression, - # surrounded by {{ ... }}` (output = true, multiline = false) - # or by `{% ... %}` (output = false, multiline = false) - # - # ``` - # # (output = false, multiline = true) - # {% - # ... - # %} - # ``` + # surrounded by {{ ... }} (output = true) + # or by {% ... %} (output = false) class MacroExpression < ASTNode property exp : ASTNode property? output : Bool - property? multiline : Bool - def initialize(@exp : ASTNode, @output = true, @multiline : Bool = false) + def initialize(@exp : ASTNode, @output = true) end def accept_children(visitor) @@ -2209,10 +2201,10 @@ module Crystal end def clone_without_location - MacroExpression.new(@exp.clone, @output, @multiline) + MacroExpression.new(@exp.clone, @output) end - def_equals_and_hash exp, output?, multiline? + def_equals_and_hash exp, output? end # Free text that is part of a macro diff --git a/src/compiler/crystal/syntax/parser.cr b/src/compiler/crystal/syntax/parser.cr index 3cf31410b41b..569bbd4d9409 100644 --- a/src/compiler/crystal/syntax/parser.cr +++ b/src/compiler/crystal/syntax/parser.cr @@ -3353,13 +3353,7 @@ module Crystal def parse_macro_control(start_location, macro_state = Token::MacroState.default) location = @token.location - next_token_skip_space - multiline = false - - if @token.type.newline? - multiline = true - next_token_skip_space_or_newline - end + next_token_skip_space_or_newline case @token.value when Keyword::FOR @@ -3446,7 +3440,7 @@ module Crystal exps = parse_expressions @in_macro_expression = false - MacroExpression.new(exps, output: false, multiline: multiline).at(location).at_end(token_end_location) + MacroExpression.new(exps, output: false).at(location).at_end(token_end_location) end def parse_macro_if(start_location, macro_state, check_end = true, is_unless = false) diff --git a/src/compiler/crystal/syntax/to_s.cr b/src/compiler/crystal/syntax/to_s.cr index 3da5b72fab4a..d30e2dd52d7f 100644 --- a/src/compiler/crystal/syntax/to_s.cr +++ b/src/compiler/crystal/syntax/to_s.cr @@ -717,11 +717,11 @@ module Crystal end newline - @indent += 1 - inside_macro do - accept node.body + with_indent do + inside_macro do + accept node.body + end end - @indent -= 1 # newline append_indent @@ -730,30 +730,34 @@ module Crystal end def visit(node : MacroExpression) - @str << (node.output? ? "{{ " : node.multiline? ? "{%" : "{% ") + # The node is considered multiline if its starting location is on a different line than its expression. + is_multiline = (start_loc = node.location) && (end_loc = node.exp.location) && end_loc.line_number > start_loc.line_number + + @str << (node.output? ? "{{ " : is_multiline ? "{%" : "{% ") - if node.multiline? + if is_multiline newline @indent += 1 end outside_macro do - # If the MacroExpression consists of a single node we need to manually handle appending indent and trailing newline if #multiline? + # If the MacroExpression consists of a single node we need to manually handle appending indent and trailing newline if *is_multiline* # Otherwise, the Expressions logic handles that for us - if node.multiline? && !node.exp.is_a?(Expressions) + if is_multiline && !node.exp.is_a?(Expressions) append_indent end node.exp.accept self end - if node.multiline? + # If the opening tag has a newline after it, force trailing tag to have one as well + if is_multiline @indent -= 1 append_indent newline if !node.exp.is_a? Expressions end - @str << (node.output? ? " }}" : node.multiline? ? "%}" : " %}") + @str << (node.output? ? " }}" : is_multiline ? "%}" : " %}") false end @@ -810,11 +814,11 @@ module Crystal def visit(node : MacroVerbatim) @str << "{% verbatim do %}" - @indent += 1 - inside_macro do - node.exp.accept self + with_indent do + inside_macro do + node.exp.accept self + end end - @indent -= 1 @str << "{% end %}" false From ad601d0bb02ecc7c03cea9443bb4d1f5b2185bf9 Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Sun, 22 Dec 2024 18:45:27 -0500 Subject: [PATCH 07/10] Finish reverting changes to `parser_spec` --- spec/compiler/parser/parser_spec.cr | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/compiler/parser/parser_spec.cr b/spec/compiler/parser/parser_spec.cr index 53fbedbcd7c1..897e5bf7060c 100644 --- a/spec/compiler/parser/parser_spec.cr +++ b/spec/compiler/parser/parser_spec.cr @@ -1118,7 +1118,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 "{%\n a = 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) 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,8 +1128,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 "{%\n x unless 1\n%}", MacroExpression.new(Unless.new(1.int32, "x".var), output: false) - it_parses "{%\n 1\n 2\n 3\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) assert_syntax_error "{% unless 1; 2; elsif 3; 4; end %}" assert_syntax_error "{% unless 1 %} 2 {% elsif 3 %} 3 {% end %}" From 79641c7ac6291ab709470ff0fdb22364b534b807 Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Mon, 23 Dec 2024 10:37:19 -0500 Subject: [PATCH 08/10] Handle emitting significant whitespace as well --- spec/compiler/parser/to_s_spec.cr | 15 +++++++++------ src/compiler/crystal/syntax/to_s.cr | 27 ++++++++++++++++++++++++--- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/spec/compiler/parser/to_s_spec.cr b/spec/compiler/parser/to_s_spec.cr index ed28ecd55dd1..3ea28be4e46e 100644 --- a/spec/compiler/parser/to_s_spec.cr +++ b/spec/compiler/parser/to_s_spec.cr @@ -1,7 +1,7 @@ require "../../support/syntax" -private def expect_to_s(original, expected = original, emit_doc = false, file = __FILE__, line = __LINE__) - it "does to_s of #{original.inspect}", file, line do +private def expect_to_s(original, expected = original, emit_doc = false, file = __FILE__, line = __LINE__, focus = false) + it "does to_s of #{original.inspect}", file, line, focus: focus do str = IO::Memory.new expected.bytesize source = original @@ -262,8 +262,7 @@ describe "ASTNode#to_s" do expect_to_s "{%\n a = 1 %}", "{%\n a = 1\n%}" expect_to_s "{% a = 1\n%}", "{% a = 1 %}" - # FIXME: Should respect significant whitespace - expect_to_s <<-'CR', <<-'CR' + expect_to_s <<-'CR', <<-CR macro finished {% verbatim do %} {% @@ -280,14 +279,16 @@ describe "ASTNode#to_s" do {% verbatim do %} {% 10 + + + 20 %} {% end %} end CR - # FIXME: Should respect significant whitespace - expect_to_s <<-'CR', <<-'CR' + expect_to_s <<-'CR', <<-CR macro finished {% verbatim do %} {% @@ -303,6 +304,8 @@ describe "ASTNode#to_s" do {% verbatim do %} {% 10 + + 20 %} {% end %} diff --git a/src/compiler/crystal/syntax/to_s.cr b/src/compiler/crystal/syntax/to_s.cr index d30e2dd52d7f..5d8d28820c68 100644 --- a/src/compiler/crystal/syntax/to_s.cr +++ b/src/compiler/crystal/syntax/to_s.cr @@ -55,6 +55,21 @@ module Crystal true end + private def add_whitespace_around(first_node_location : Location?, second_node_location : Location?, &) : Nil + # If any location information is missing, don't add any extra newlines. + if !first_node_location || !second_node_location + yield + return + end + + # Only emit extra newlines. I.e. those more than the first handled via the Expressions visitor. + ((second_node_location.line_number - 1) - first_node_location.line_number).times do + newline + end + + yield + end + def visit(node : Nop) false end @@ -221,11 +236,17 @@ module Crystal if @inside_macro > 0 node.expressions.each &.accept self else + last_node = node.expressions.first + node.expressions.each_with_index do |exp, i| unless exp.nop? - append_indent unless node.keyword.paren? && i == 0 - exp.accept self - newline unless node.keyword.paren? && i == node.expressions.size - 1 + self.add_whitespace_around last_node.end_location, exp.location do + append_indent unless node.keyword.paren? && i == 0 + exp.accept self + newline unless node.keyword.paren? && i == node.expressions.size - 1 + end + + last_node = exp end end end From 86e895986b23c2f387e61a6c7de0f9da52a3d32a Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Mon, 23 Dec 2024 12:59:00 -0500 Subject: [PATCH 09/10] Handle single node MacroExpressions some minor cleanup --- spec/compiler/parser/to_s_spec.cr | 58 ++++++++++++++++++++++++++++- src/compiler/crystal/syntax/to_s.cr | 8 ++-- 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/spec/compiler/parser/to_s_spec.cr b/spec/compiler/parser/to_s_spec.cr index 3ea28be4e46e..d6d2ca79780d 100644 --- a/spec/compiler/parser/to_s_spec.cr +++ b/spec/compiler/parser/to_s_spec.cr @@ -262,7 +262,7 @@ describe "ASTNode#to_s" do expect_to_s "{%\n a = 1 %}", "{%\n a = 1\n%}" expect_to_s "{% a = 1\n%}", "{% a = 1 %}" - expect_to_s <<-'CR', <<-CR + expect_to_s <<-'CR', <<-'CR' macro finished {% verbatim do %} {% @@ -288,7 +288,7 @@ describe "ASTNode#to_s" do end CR - expect_to_s <<-'CR', <<-CR + expect_to_s <<-'CR', <<-'CR' macro finished {% verbatim do %} {% @@ -312,6 +312,50 @@ describe "ASTNode#to_s" do end CR + expect_to_s <<-'CR', <<-'CR' + macro finished + {% verbatim do %} + {% + 10 + + # Foo + + 20 + 30 + + # Bar + + 40 + %} + {% + 50 + 60 + %} + {% end %} + end + CR + macro finished + {% verbatim do %} + {% + 10 + + + + 20 + 30 + + + + 40 + %} + {% + 50 + 60 + %} + {% end %} + end + CR + expect_to_s <<-'CR' macro finished {% verbatim do %} @@ -323,6 +367,16 @@ describe "ASTNode#to_s" do end CR + expect_to_s <<-'CR' + macro finished + {% verbatim do %} + {% + 10 + %} + {% end %} + end + CR + expect_to_s %(asm("nop" ::::)) expect_to_s %(asm("nop" : "a"(1), "b"(2) : "c"(3), "d"(4) : "e", "f" : "volatile", "alignstack", "intel")) expect_to_s %(asm("nop" :: "c"(3), "d"(4) ::)) diff --git a/src/compiler/crystal/syntax/to_s.cr b/src/compiler/crystal/syntax/to_s.cr index 5d8d28820c68..36c34cb045bb 100644 --- a/src/compiler/crystal/syntax/to_s.cr +++ b/src/compiler/crystal/syntax/to_s.cr @@ -55,14 +55,14 @@ module Crystal true end - private def add_whitespace_around(first_node_location : Location?, second_node_location : Location?, &) : Nil + private def write_extra_newlines(first_node_location : Location?, second_node_location : Location?, &) : Nil # If any location information is missing, don't add any extra newlines. if !first_node_location || !second_node_location yield return end - # Only emit extra newlines. I.e. those more than the first handled via the Expressions visitor. + # Only write the "extra" newlines. I.e. If there are more than one. The first newline is handled directly via the Expressions visitor. ((second_node_location.line_number - 1) - first_node_location.line_number).times do newline end @@ -240,7 +240,7 @@ module Crystal node.expressions.each_with_index do |exp, i| unless exp.nop? - self.add_whitespace_around last_node.end_location, exp.location do + self.write_extra_newlines last_node.end_location, exp.location do append_indent unless node.keyword.paren? && i == 0 exp.accept self newline unless node.keyword.paren? && i == node.expressions.size - 1 @@ -774,8 +774,8 @@ module Crystal # If the opening tag has a newline after it, force trailing tag to have one as well if is_multiline @indent -= 1 - append_indent newline if !node.exp.is_a? Expressions + append_indent end @str << (node.output? ? " }}" : is_multiline ? "%}" : " %}") From 0f32ebd0ef40a7ebd4d51f093570f5af65d75489 Mon Sep 17 00:00:00 2001 From: George Dietrich Date: Mon, 23 Dec 2024 14:11:08 -0500 Subject: [PATCH 10/10] Fix bug related to empty expressions Update other specs to use new format --- spec/compiler/crystal/tools/expand_spec.cr | 11 ++++++----- spec/compiler/semantic/restrictions_augmenter_spec.cr | 6 ++++++ src/compiler/crystal/syntax/to_s.cr | 4 ++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/spec/compiler/crystal/tools/expand_spec.cr b/spec/compiler/crystal/tools/expand_spec.cr index e8f9b770f3ec..4ebdfff6b836 100644 --- a/spec/compiler/crystal/tools/expand_spec.cr +++ b/spec/compiler/crystal/tools/expand_spec.cr @@ -157,7 +157,7 @@ describe "expand" do {% end %} CRYSTAL - assert_expand_simple code, "1\n2\n3\n" + assert_expand_simple code, "1\n\n2\n\n3\n" end it "expands macro control {% for %} with cursor inside it" do @@ -167,7 +167,7 @@ describe "expand" do {% end %} CRYSTAL - assert_expand_simple code, "1\n2\n3\n" + assert_expand_simple code, "1\n\n2\n\n3\n" end it "expands macro control {% for %} with cursor at end of it" do @@ -177,7 +177,7 @@ describe "expand" do ‸{% end %} CRYSTAL - assert_expand_simple code, "1\n2\n3\n" + assert_expand_simple code, "1\n\n2\n\n3\n" end it "expands macro control {% for %} with indent" do @@ -195,7 +195,7 @@ describe "expand" do {% end %} CRYSTAL - assert_expand_simple code, original: original, expanded: "1\n2\n3\n" + assert_expand_simple code, original: original, expanded: "1\n\n2\n\n3\n" end it "expands simple macro" do @@ -258,7 +258,7 @@ describe "expand" do ‸foo CRYSTAL - assert_expand_simple code, original: "foo", expanded: %("if true"\n"1"\n"2"\n"3"\n) + assert_expand_simple code, original: "foo", expanded: %("if true"\n\n\n"1"\n\n"2"\n\n"3"\n) end it "expands macros with 2 level" do @@ -615,6 +615,7 @@ describe "expand" do def hello_str "hello" end + # symbol of hello def hello_sym :hello diff --git a/spec/compiler/semantic/restrictions_augmenter_spec.cr b/spec/compiler/semantic/restrictions_augmenter_spec.cr index 2b7250658693..d77b61327f82 100644 --- a/spec/compiler/semantic/restrictions_augmenter_spec.cr +++ b/spec/compiler/semantic/restrictions_augmenter_spec.cr @@ -78,7 +78,9 @@ describe "Semantic: restrictions augmenter" do class Baz end end + @x : Bar::Baz + def initialize(value : ::Foo::Bar::Baz) @x = value end @@ -110,7 +112,9 @@ describe "Semantic: restrictions augmenter" do class Baz end end + @x : Bar::Baz + def initialize(value : Bar::Baz) @x = value end @@ -400,8 +404,10 @@ describe "Semantic: restrictions augmenter" do macro foo {{ yield }} end + class Foo end + class Bar @x : Foo def initialize(value : ::Foo) diff --git a/src/compiler/crystal/syntax/to_s.cr b/src/compiler/crystal/syntax/to_s.cr index 36c34cb045bb..ae3576051a87 100644 --- a/src/compiler/crystal/syntax/to_s.cr +++ b/src/compiler/crystal/syntax/to_s.cr @@ -236,11 +236,11 @@ module Crystal if @inside_macro > 0 node.expressions.each &.accept self else - last_node = node.expressions.first + last_node = nil node.expressions.each_with_index do |exp, i| unless exp.nop? - self.write_extra_newlines last_node.end_location, exp.location do + self.write_extra_newlines((last_node || exp).end_location, exp.location) do append_indent unless node.keyword.paren? && i == 0 exp.accept self newline unless node.keyword.paren? && i == node.expressions.size - 1