From dcd3806dca9033ccbd161b40c53c92f7b62b32c6 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Fri, 25 Aug 2023 16:04:55 -0400 Subject: [PATCH 1/7] Improve how we declare ripper exceptions in parse_test.rb Specific files are named earlier in the block, and we now have the ability to skip just the lex matching, or skip ripper entirely (for files that don't parse). --- test/yarp/parse_test.rb | 44 ++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/test/yarp/parse_test.rb b/test/yarp/parse_test.rb index b9f852abf93..70f061242ba 100644 --- a/test/yarp/parse_test.rb +++ b/test/yarp/parse_test.rb @@ -46,7 +46,7 @@ def test_parse_lex_file # To accurately compare against Ripper, we need to make sure that we're # running on Ruby 3.2+. - check_ripper = RUBY_VERSION >= "3.2.0" + ripper_enabled = RUBY_VERSION >= "3.2.0" # The FOCUS environment variable allows you to specify one particular fixture # to test, instead of all of them. @@ -63,6 +63,22 @@ def test_parse_lex_file directory = File.dirname(snapshot) FileUtils.mkdir_p(directory) unless File.directory?(directory) + ripper_should_parse = ripper_should_match = ripper_enabled + + # This file has changed behavior in Ripper in Ruby 3.3, so we skip it if + # we're on an earlier version. + ripper_should_match = false if relative == "seattlerb/pct_w_heredoc_interp_nested.txt" && RUBY_VERSION < "3.3.0" + + # It seems like there are some oddities with nested heredocs and ripper. + # Waiting for feedback on https://bugs.ruby-lang.org/issues/19838. + ripper_should_match = false if relative == "seattlerb/heredoc_nested.txt" + + # Ripper seems to have a bug that the regex portions before and after the heredoc are combined + # into a single token. See https://bugs.ruby-lang.org/issues/19838. + # + # Additionally, Ripper cannot parse the %w[] fixture in this file, so set ripper_should_parse to false. + ripper_should_match = false if relative == "wrapping_heredoc.txt" + define_method "test_filepath_#{relative}" do # First, read the source from the filepath. Use binmode to avoid converting CRLF on Windows, # and explicitly set the external encoding to UTF-8 to override the binmode default. @@ -70,7 +86,7 @@ def test_parse_lex_file # Make sure that it can be correctly parsed by Ripper. If it can't, then we have a fixture # that is invalid Ruby. - refute_nil Ripper.sexp_raw(source) if check_ripper + refute_nil(Ripper.sexp_raw(source), "Ripper failed to parse") if ripper_should_parse # Next, assert that there were no errors during parsing. result = YARP.parse(source, relative) @@ -118,25 +134,13 @@ def test_parse_lex_file assert_equal expected_newlines, YARP.const_get(:Debug).newlines(source) - # This file has changed behavior in Ripper in Ruby 3.3, so we skip it if - # we're on an earlier version. - return if relative == "seattlerb/pct_w_heredoc_interp_nested.txt" && RUBY_VERSION < "3.3.0" - - # It seems like there are some oddities with nested heredocs and ripper. - # Waiting for feedback on https://bugs.ruby-lang.org/issues/19838. - return if relative == "seattlerb/heredoc_nested.txt" - - # Ripper seems to have a bug that the regex portions before and after the heredoc are combined - # into a single token. - return if relative == "wrapping_heredoc.txt" - - # Finally, assert that we can lex the source and get the same tokens as - # Ripper. - lex_result = YARP.lex_compat(source) - assert_equal [], lex_result.errors - tokens = lex_result.value + if ripper_should_parse && ripper_should_match + # Finally, assert that we can lex the source and get the same tokens as + # Ripper. + lex_result = YARP.lex_compat(source) + assert_equal [], lex_result.errors + tokens = lex_result.value - if check_ripper begin YARP.lex_ripper(source).zip(tokens).each do |(ripper, yarp)| assert_equal ripper, yarp From 87f348889f360e6ea77f5e78320d442e1a1107b3 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sun, 27 Aug 2023 15:54:45 -0400 Subject: [PATCH 2/7] fix: yp_interpolated_symbol_node_append Made this function's behavior match the interpolated_string implementation. Previously, the start location was not set and left as 0. --- src/yarp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/yarp.c b/src/yarp.c index ba8722a9250..129a176decb 100644 --- a/src/yarp.c +++ b/src/yarp.c @@ -2816,10 +2816,11 @@ yp_interpolated_symbol_node_create(yp_parser_t *parser, const yp_token_t *openin static inline void yp_interpolated_symbol_node_append(yp_interpolated_symbol_node_t *node, yp_node_t *part) { - yp_node_list_append(&node->parts, part); - if (!node->base.location.start) { + if (node->parts.size == 0 && node->opening_loc.start == NULL) { node->base.location.start = part->location.start; } + + yp_node_list_append(&node->parts, part); node->base.location.end = part->location.end; } From c148d955fda7b44899d0fd74a75226b6ce3978f0 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sun, 27 Aug 2023 16:19:12 -0400 Subject: [PATCH 3/7] test: backfill tests for %q/%Q spanning a heredoc Also rename the fixture file --- test/yarp/fixtures/spanning_heredoc.txt | 25 +++++ test/yarp/fixtures/wrapping_heredoc.txt | 13 --- test/yarp/parse_test.rb | 2 +- test/yarp/snapshots/spanning_heredoc.txt | 115 +++++++++++++++++++++++ test/yarp/snapshots/wrapping_heredoc.txt | 80 ---------------- 5 files changed, 141 insertions(+), 94 deletions(-) create mode 100644 test/yarp/fixtures/spanning_heredoc.txt delete mode 100644 test/yarp/fixtures/wrapping_heredoc.txt create mode 100644 test/yarp/snapshots/spanning_heredoc.txt delete mode 100644 test/yarp/snapshots/wrapping_heredoc.txt diff --git a/test/yarp/fixtures/spanning_heredoc.txt b/test/yarp/fixtures/spanning_heredoc.txt new file mode 100644 index 00000000000..5040840e765 --- /dev/null +++ b/test/yarp/fixtures/spanning_heredoc.txt @@ -0,0 +1,25 @@ +# test regex, string, and lists that span a heredoc thanks to an escaped newline + +# ripper incorrectly creates a "b\nb" token instead of two separate string tokens +pp <<-A.gsub(/b\ +a +A +b/, "") + +# ripper incorrectly creates a "d\nd" token instead of two separate string tokens +pp <<-A, "d\ +c +A +d" + +# ripper gets this right +pp <<-A, %q[f\ +e +A +f] + +# ripper incorrectly creates a "h\nh" token instead of two separate string tokens +pp <<-A, %Q[h\ +g +A +h] diff --git a/test/yarp/fixtures/wrapping_heredoc.txt b/test/yarp/fixtures/wrapping_heredoc.txt deleted file mode 100644 index d5fc7101780..00000000000 --- a/test/yarp/fixtures/wrapping_heredoc.txt +++ /dev/null @@ -1,13 +0,0 @@ -# test regex, string, and lists that wrap a heredoc thanks to an escaped newline - -# ripper incorrectly creates a "b\nc" string instead of two separate string tokens -pp <<-A.gsub(/b\ -a -A -c/, "") - -# ripper incorrectly creates a "e\nf" string instead of two separate string tokens -pp <<-A + "e\ -d -A -f" diff --git a/test/yarp/parse_test.rb b/test/yarp/parse_test.rb index 70f061242ba..1eb0033208a 100644 --- a/test/yarp/parse_test.rb +++ b/test/yarp/parse_test.rb @@ -77,7 +77,7 @@ def test_parse_lex_file # into a single token. See https://bugs.ruby-lang.org/issues/19838. # # Additionally, Ripper cannot parse the %w[] fixture in this file, so set ripper_should_parse to false. - ripper_should_match = false if relative == "wrapping_heredoc.txt" + ripper_should_match = false if relative == "spanning_heredoc.txt" define_method "test_filepath_#{relative}" do # First, read the source from the filepath. Use binmode to avoid converting CRLF on Windows, diff --git a/test/yarp/snapshots/spanning_heredoc.txt b/test/yarp/snapshots/spanning_heredoc.txt new file mode 100644 index 00000000000..244ced125b5 --- /dev/null +++ b/test/yarp/snapshots/spanning_heredoc.txt @@ -0,0 +1,115 @@ +ProgramNode(164...448)( + [], + StatementsNode(164...448)( + [CallNode(164...192)( + nil, + nil, + (164...166), + nil, + ArgumentsNode(167...192)( + [CallNode(167...192)( + InterpolatedStringNode(167...171)( + (167...171), + [StringNode(181...183)(nil, (181...183), nil, "a\n")], + (183...185) + ), + (171...172), + (172...176), + (176...177), + ArgumentsNode(177...191)( + [InterpolatedRegularExpressionNode(177...187)( + (177...178), + [StringNode(178...181)(nil, (178...181), nil, "b"), + StringNode(185...186)(nil, (185...186), nil, "b")], + (186...187), + 0 + ), + StringNode(189...191)( + (189...190), + (190...190), + (190...191), + "" + )] + ), + (191...192), + nil, + 0, + "gsub" + )] + ), + nil, + nil, + 0, + "pp" + ), + CallNode(276...295)( + nil, + nil, + (276...278), + nil, + ArgumentsNode(279...295)( + [InterpolatedStringNode(279...283)( + (279...283), + [StringNode(289...291)(nil, (289...291), nil, "c\n")], + (291...293) + ), + InterpolatedStringNode(285...295)( + (285...286), + [StringNode(286...289)(nil, (286...289), nil, "d"), + StringNode(293...294)(nil, (293...294), nil, "d")], + (294...295) + )] + ), + nil, + nil, + 0, + "pp" + ), + CallNode(322...343)( + nil, + nil, + (322...324), + nil, + ArgumentsNode(325...343)( + [InterpolatedStringNode(325...329)( + (325...329), + [StringNode(337...339)(nil, (337...339), nil, "e\n")], + (339...341) + ), + InterpolatedStringNode(331...343)( + (331...334), + [StringNode(334...337)(nil, (334...337), nil, "f\\\n"), + StringNode(341...342)(nil, (341...342), nil, "f")], + (342...343) + )] + ), + nil, + nil, + 0, + "pp" + ), + CallNode(427...448)( + nil, + nil, + (427...429), + nil, + ArgumentsNode(430...448)( + [InterpolatedStringNode(430...434)( + (430...434), + [StringNode(442...444)(nil, (442...444), nil, "g\n")], + (444...446) + ), + InterpolatedStringNode(436...448)( + (436...439), + [StringNode(439...442)(nil, (439...442), nil, "h"), + StringNode(446...447)(nil, (446...447), nil, "h")], + (447...448) + )] + ), + nil, + nil, + 0, + "pp" + )] + ) +) diff --git a/test/yarp/snapshots/wrapping_heredoc.txt b/test/yarp/snapshots/wrapping_heredoc.txt deleted file mode 100644 index 674db56ed12..00000000000 --- a/test/yarp/snapshots/wrapping_heredoc.txt +++ /dev/null @@ -1,80 +0,0 @@ -ProgramNode(165...298)( - [], - StatementsNode(165...298)( - [CallNode(165...193)( - nil, - nil, - (165...167), - nil, - ArgumentsNode(168...193)( - [CallNode(168...193)( - InterpolatedStringNode(168...172)( - (168...172), - [StringNode(182...184)(nil, (182...184), nil, "a\n")], - (184...186) - ), - (172...173), - (173...177), - (177...178), - ArgumentsNode(178...192)( - [InterpolatedRegularExpressionNode(178...188)( - (178...179), - [StringNode(179...182)(nil, (179...182), nil, "b"), - StringNode(186...187)(nil, (186...187), nil, "c")], - (187...188), - 0 - ), - StringNode(190...192)( - (190...191), - (191...191), - (191...192), - "" - )] - ), - (192...193), - nil, - 0, - "gsub" - )] - ), - nil, - nil, - 0, - "pp" - ), - CallNode(278...298)( - nil, - nil, - (278...280), - nil, - ArgumentsNode(281...298)( - [CallNode(281...298)( - InterpolatedStringNode(281...285)( - (281...285), - [StringNode(292...294)(nil, (292...294), nil, "d\n")], - (294...296) - ), - nil, - (286...287), - nil, - ArgumentsNode(288...298)( - [InterpolatedStringNode(288...298)( - (288...289), - [StringNode(289...292)(nil, (289...292), nil, "e"), - StringNode(296...297)(nil, (296...297), nil, "f")], - (297...298) - )] - ), - nil, - nil, - 0, - "+" - )] - ), - nil, - nil, - 0, - "pp" - )] - ) -) From 4e707937cb41870db89f3ea069a04f88cb13b5cf Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sun, 27 Aug 2023 16:32:19 -0400 Subject: [PATCH 4/7] fix: %w list spanning a heredoc Two fixes were necessary: - ensure we are handling newlines correctly - accept two consecutive string tokens without a separator Co-authored-by: Kevin Newton --- src/yarp.c | 25 ++++++++++++++-------- test/yarp/fixtures/spanning_heredoc.txt | 6 ++++++ test/yarp/parse_test.rb | 2 +- test/yarp/snapshots/spanning_heredoc.txt | 27 ++++++++++++++++++++++-- 4 files changed, 48 insertions(+), 12 deletions(-) diff --git a/src/yarp.c b/src/yarp.c index 129a176decb..a4b1c9be3ca 100644 --- a/src/yarp.c +++ b/src/yarp.c @@ -6942,9 +6942,19 @@ parser_lex(yp_parser_t *parser) { yp_unescape_type_t unescape_type = lex_mode->as.list.interpolation ? YP_UNESCAPE_ALL : YP_UNESCAPE_MINIMAL; size_t difference = yp_unescape_calculate_difference(parser, breakpoint, unescape_type, false); - // If the result is an escaped newline, then we need to - // track that newline. - yp_newline_list_check_append(&parser->newline_list, breakpoint + difference - 1); + // If the result is an escaped newline ... + if (*(breakpoint + difference - 1) == '\n') { + if (parser->heredoc_end) { + // ... if we are on the same line as a heredoc, flush the heredoc and + // continue parsing after heredoc_end. + parser->current.end = breakpoint + difference; + parser_flush_heredoc_end(parser); + LEX(YP_TOKEN_STRING_CONTENT); + } else { + // ... else track the newline. + yp_newline_list_append(&parser->newline_list, breakpoint + difference - 1); + } + } breakpoint = yp_strpbrk(parser, breakpoint + difference, breakpoints, parser->end - (breakpoint + difference)); continue; @@ -12098,12 +12108,9 @@ parse_expression_prefix(yp_parser_t *parser, yp_binding_power_t binding_power) { accept(parser, YP_TOKEN_WORDS_SEP); while (!match_any_type_p(parser, 2, YP_TOKEN_STRING_END, YP_TOKEN_EOF)) { - if (yp_array_node_size(array) == 0) { - accept(parser, YP_TOKEN_WORDS_SEP); - } else { - expect(parser, YP_TOKEN_WORDS_SEP, "Expected a separator for the strings in a `%w` list."); - if (match_type_p(parser, YP_TOKEN_STRING_END)) break; - } + accept(parser, YP_TOKEN_WORDS_SEP); + if (match_type_p(parser, YP_TOKEN_STRING_END)) break; + expect(parser, YP_TOKEN_STRING_CONTENT, "Expected a string in a `%w` list."); yp_token_t opening = not_provided(parser); diff --git a/test/yarp/fixtures/spanning_heredoc.txt b/test/yarp/fixtures/spanning_heredoc.txt index 5040840e765..1b17edac5d7 100644 --- a/test/yarp/fixtures/spanning_heredoc.txt +++ b/test/yarp/fixtures/spanning_heredoc.txt @@ -23,3 +23,9 @@ pp <<-A, %Q[h\ g A h] + +# ripper can't parse this successfully, though ruby runs it correctly +pp <<-A, %w[j\ +i +A +j] diff --git a/test/yarp/parse_test.rb b/test/yarp/parse_test.rb index 1eb0033208a..b288d597b23 100644 --- a/test/yarp/parse_test.rb +++ b/test/yarp/parse_test.rb @@ -77,7 +77,7 @@ def test_parse_lex_file # into a single token. See https://bugs.ruby-lang.org/issues/19838. # # Additionally, Ripper cannot parse the %w[] fixture in this file, so set ripper_should_parse to false. - ripper_should_match = false if relative == "spanning_heredoc.txt" + ripper_should_parse = false if relative == "spanning_heredoc.txt" define_method "test_filepath_#{relative}" do # First, read the source from the filepath. Use binmode to avoid converting CRLF on Windows, diff --git a/test/yarp/snapshots/spanning_heredoc.txt b/test/yarp/snapshots/spanning_heredoc.txt index 244ced125b5..f948e1e9d3d 100644 --- a/test/yarp/snapshots/spanning_heredoc.txt +++ b/test/yarp/snapshots/spanning_heredoc.txt @@ -1,6 +1,6 @@ -ProgramNode(164...448)( +ProgramNode(164...541)( [], - StatementsNode(164...448)( + StatementsNode(164...541)( [CallNode(164...192)( nil, nil, @@ -110,6 +110,29 @@ ProgramNode(164...448)( nil, 0, "pp" + ), + CallNode(520...541)( + nil, + nil, + (520...522), + nil, + ArgumentsNode(523...541)( + [InterpolatedStringNode(523...527)( + (523...527), + [StringNode(535...537)(nil, (535...537), nil, "i\n")], + (537...539) + ), + ArrayNode(529...541)( + [StringNode(532...535)(nil, (532...535), nil, "j\\\n"), + StringNode(539...540)(nil, (539...540), nil, "j")], + (529...532), + (540...541) + )] + ), + nil, + nil, + 0, + "pp" )] ) ) From 6df729fe72f77e34ef78054024da150da562d94c Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sun, 27 Aug 2023 16:34:53 -0400 Subject: [PATCH 5/7] fix: %W list spanning a heredoc Primarily this fix is to accept a string node and concatenate it onto an interpolated string. --- src/yarp.c | 13 ++++++++++ test/yarp/fixtures/spanning_heredoc.txt | 7 ++++++ test/yarp/snapshots/spanning_heredoc.txt | 31 ++++++++++++++++++++++-- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/yarp.c b/src/yarp.c index a4b1c9be3ca..7306d9e8e5a 100644 --- a/src/yarp.c +++ b/src/yarp.c @@ -12160,6 +12160,19 @@ parse_expression_prefix(yp_parser_t *parser, yp_binding_power_t binding_power) { // to the list of child nodes. yp_node_t *part = parse_string_part(parser); yp_interpolated_string_node_append((yp_interpolated_string_node_t *) current, part); + } else if (YP_NODE_TYPE_P(current, YP_NODE_STRING_NODE)) { + // If we hit string content and the current node is a string node, + // then we need to convert the current node into an interpolated + // string and add the string content to the list of child nodes. + yp_token_t opening = not_provided(parser); + yp_token_t closing = not_provided(parser); + yp_interpolated_string_node_t *interpolated = + yp_interpolated_string_node_create(parser, &opening, NULL, &closing); + yp_interpolated_string_node_append(interpolated, current); + + yp_node_t *part = parse_string_part(parser); + yp_interpolated_string_node_append(interpolated, part); + current = (yp_node_t *) interpolated; } else { assert(false && "unreachable"); } diff --git a/test/yarp/fixtures/spanning_heredoc.txt b/test/yarp/fixtures/spanning_heredoc.txt index 1b17edac5d7..a81731a1815 100644 --- a/test/yarp/fixtures/spanning_heredoc.txt +++ b/test/yarp/fixtures/spanning_heredoc.txt @@ -29,3 +29,10 @@ pp <<-A, %w[j\ i A j] + +# ripper can't parse this successfully, though ruby runs it correctly +# TODO: yarp does not include the "\n" in "l\nl" in the AST like ruby does +pp <<-A, %W[l\ +k +A +l] diff --git a/test/yarp/snapshots/spanning_heredoc.txt b/test/yarp/snapshots/spanning_heredoc.txt index f948e1e9d3d..a0f58a97bfb 100644 --- a/test/yarp/snapshots/spanning_heredoc.txt +++ b/test/yarp/snapshots/spanning_heredoc.txt @@ -1,6 +1,6 @@ -ProgramNode(164...541)( +ProgramNode(164...709)( [], - StatementsNode(164...541)( + StatementsNode(164...709)( [CallNode(164...192)( nil, nil, @@ -133,6 +133,33 @@ ProgramNode(164...541)( nil, 0, "pp" + ), + CallNode(688...709)( + nil, + nil, + (688...690), + nil, + ArgumentsNode(691...709)( + [InterpolatedStringNode(691...695)( + (691...695), + [StringNode(703...705)(nil, (703...705), nil, "k\n")], + (705...707) + ), + ArrayNode(697...709)( + [InterpolatedStringNode(700...708)( + nil, + [StringNode(700...703)(nil, (700...703), nil, "l"), + StringNode(707...708)(nil, (707...708), nil, "l")], + nil + )], + (697...700), + (708...709) + )] + ), + nil, + nil, + 0, + "pp" )] ) ) From f869fbdbe54f5fb8f03c206a947a5b663654b101 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sun, 27 Aug 2023 16:40:18 -0400 Subject: [PATCH 6/7] fix: %i list spanning a heredoc The fix here is similar to what we did in a previous commit for %w, to accept two consecutive string tokens without a separator. --- src/yarp.c | 9 ++------ test/yarp/fixtures/spanning_heredoc.txt | 6 ++++++ test/yarp/snapshots/spanning_heredoc.txt | 27 ++++++++++++++++++++++-- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/src/yarp.c b/src/yarp.c index 7306d9e8e5a..1f707696bb2 100644 --- a/src/yarp.c +++ b/src/yarp.c @@ -11944,14 +11944,9 @@ parse_expression_prefix(yp_parser_t *parser, yp_binding_power_t binding_power) { yp_array_node_t *array = yp_array_node_create(parser, &parser->previous); while (!match_any_type_p(parser, 2, YP_TOKEN_STRING_END, YP_TOKEN_EOF)) { - if (yp_array_node_size(array) == 0) { - accept(parser, YP_TOKEN_WORDS_SEP); - } else { - expect(parser, YP_TOKEN_WORDS_SEP, "Expected a separator for the symbols in a `%i` list."); - if (match_type_p(parser, YP_TOKEN_STRING_END)) break; - } - + accept(parser, YP_TOKEN_WORDS_SEP); if (match_type_p(parser, YP_TOKEN_STRING_END)) break; + expect(parser, YP_TOKEN_STRING_CONTENT, "Expected a symbol in a `%i` list."); yp_token_t opening = not_provided(parser); diff --git a/test/yarp/fixtures/spanning_heredoc.txt b/test/yarp/fixtures/spanning_heredoc.txt index a81731a1815..c1852e377a5 100644 --- a/test/yarp/fixtures/spanning_heredoc.txt +++ b/test/yarp/fixtures/spanning_heredoc.txt @@ -36,3 +36,9 @@ pp <<-A, %W[l\ k A l] + +# ripper can't parse this successfully, though ruby runs it correctly +pp <<-A, %i[n\ +m +A +n] diff --git a/test/yarp/snapshots/spanning_heredoc.txt b/test/yarp/snapshots/spanning_heredoc.txt index a0f58a97bfb..3d223e5e4d3 100644 --- a/test/yarp/snapshots/spanning_heredoc.txt +++ b/test/yarp/snapshots/spanning_heredoc.txt @@ -1,6 +1,6 @@ -ProgramNode(164...709)( +ProgramNode(164...802)( [], - StatementsNode(164...709)( + StatementsNode(164...802)( [CallNode(164...192)( nil, nil, @@ -160,6 +160,29 @@ ProgramNode(164...709)( nil, 0, "pp" + ), + CallNode(781...802)( + nil, + nil, + (781...783), + nil, + ArgumentsNode(784...802)( + [InterpolatedStringNode(784...788)( + (784...788), + [StringNode(796...798)(nil, (796...798), nil, "m\n")], + (798...800) + ), + ArrayNode(790...802)( + [SymbolNode(793...796)(nil, (793...796), nil, "n\\\n"), + SymbolNode(800...801)(nil, (800...801), nil, "n")], + (790...793), + (801...802) + )] + ), + nil, + nil, + 0, + "pp" )] ) ) From 6b5911b95eec150d3a1d1e145938ba6934bdaf36 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sun, 27 Aug 2023 16:43:27 -0400 Subject: [PATCH 7/7] fix: %I list spanning a heredoc Similar to the previous %W fix, we accept a symbol node and concatenate it onto an interpolated symbol. --- src/yarp.c | 13 ++++++++++ test/yarp/fixtures/spanning_heredoc.txt | 7 ++++++ test/yarp/snapshots/spanning_heredoc.txt | 31 ++++++++++++++++++++++-- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/yarp.c b/src/yarp.c index 1f707696bb2..6f66affd78b 100644 --- a/src/yarp.c +++ b/src/yarp.c @@ -12001,6 +12001,19 @@ parse_expression_prefix(yp_parser_t *parser, yp_binding_power_t binding_power) { // to the list of child nodes. yp_node_t *part = parse_string_part(parser); yp_interpolated_symbol_node_append((yp_interpolated_symbol_node_t *) current, part); + } else if (YP_NODE_TYPE_P(current, YP_NODE_SYMBOL_NODE)) { + // If we hit string content and the current node is a string node, + // then we need to convert the current node into an interpolated + // string and add the string content to the list of child nodes. + yp_token_t opening = not_provided(parser); + yp_token_t closing = not_provided(parser); + yp_interpolated_symbol_node_t *interpolated = + yp_interpolated_symbol_node_create(parser, &opening, NULL, &closing); + yp_interpolated_symbol_node_append(interpolated, current); + + yp_node_t *part = parse_string_part(parser); + yp_interpolated_symbol_node_append(interpolated, part); + current = (yp_node_t *) interpolated; } else { assert(false && "unreachable"); } diff --git a/test/yarp/fixtures/spanning_heredoc.txt b/test/yarp/fixtures/spanning_heredoc.txt index c1852e377a5..a52a4c3c27b 100644 --- a/test/yarp/fixtures/spanning_heredoc.txt +++ b/test/yarp/fixtures/spanning_heredoc.txt @@ -42,3 +42,10 @@ pp <<-A, %i[n\ m A n] + +# ripper gets this one wrong in the same way that YARP does ... +# TODO: yarp does not include the "\n" in "p\np" in the AST like ruby does +pp <<-A, %I[p\ +o +A +p] diff --git a/test/yarp/snapshots/spanning_heredoc.txt b/test/yarp/snapshots/spanning_heredoc.txt index 3d223e5e4d3..301c70adf2a 100644 --- a/test/yarp/snapshots/spanning_heredoc.txt +++ b/test/yarp/snapshots/spanning_heredoc.txt @@ -1,6 +1,6 @@ -ProgramNode(164...802)( +ProgramNode(164...964)( [], - StatementsNode(164...802)( + StatementsNode(164...964)( [CallNode(164...192)( nil, nil, @@ -183,6 +183,33 @@ ProgramNode(164...802)( nil, 0, "pp" + ), + CallNode(943...964)( + nil, + nil, + (943...945), + nil, + ArgumentsNode(946...964)( + [InterpolatedStringNode(946...950)( + (946...950), + [StringNode(958...960)(nil, (958...960), nil, "o\n")], + (960...962) + ), + ArrayNode(952...964)( + [InterpolatedSymbolNode(955...963)( + nil, + [SymbolNode(955...958)(nil, (955...958), nil, "p"), + StringNode(962...963)(nil, (962...963), nil, "p")], + nil + )], + (952...955), + (963...964) + )] + ), + nil, + nil, + 0, + "pp" )] ) )