Skip to content

Adds support for interpolated strings to HEREDOC#121

Merged
aryx merged 4 commits intotree-sitter:masterfrom
cfroystad:heredoc
Jun 22, 2022
Merged

Adds support for interpolated strings to HEREDOC#121
aryx merged 4 commits intotree-sitter:masterfrom
cfroystad:heredoc

Conversation

@cfroystad
Copy link
Collaborator

@cfroystad cfroystad commented Jan 31, 2022 •
edited
Loading

Adds support for:

  • interpolated strings to heredoc with a structure similar to encapsed_string
  • access to the heredoc and nowdoc tags
  • string_value nodes to allow getting the actual string value without the surrounding '|"

Issues

In order to not have text_interpolation kick in whenever ?> or <? is seen in heredocs (could be across heredocs),
I've had to add these as tokens in _interpolated_string_body_heredoc to be aliased as string_value. This feels ugly and
should be unneccessary, but I've yet to find a way around it with what limited time I'm able to devote at the moment. Any
suggestions would be most welcome!

Checklist:

  • All tests pass in CI
  • There are sufficient tests for the new fix/feature
  • Grammar rules have not been renamed unless absolutely necessary (0 rules renamed)
  • The conflicts section hasn't grown too much (1 new conflict)
  • The parser size hasn't grown too much (master: 2352, PR: 2543)

@cfroystad cfroystad mentioned this pull request Jan 31, 2022
Closed
32 tasks
This was linked to issues Jan 31, 2022
Heredoc incorrectly terminated causes parse error #101
Closed
Quoted heredoc delimiter gives parse error #104
Closed
@cfroystad cfroystad mentioned this pull request Jan 31, 2022
Closed
@cfroystad

This comment was marked as outdated.

Sign in to view
@cfroystad cfroystad marked this pull request as draft February 3, 2022 07:52
@cfroystad

This comment was marked as outdated.

Sign in to view
@cfroystad cfroystad removed the request for review from maxbrunsfeld February 3, 2022 12:16
@cfroystad cfroystad force-pushed the heredoc branch 2 times, most recently from 169fc16 to 42ecf1a Compare May 6, 2022 10:06
@cfroystad cfroystad marked this pull request as ready for review May 6, 2022 14:04
@cfroystad cfroystad requested a review from maxbrunsfeld May 9, 2022 16:03
@cfroystad cfroystad linked an issue May 10, 2022 that may be closed by this pull request
Strange behavior with strings? #122
Closed
@cfroystad cfroystad mentioned this pull request May 19, 2022
Closed
@Sjord
Copy link
Contributor

Sjord commented May 31, 2022

Empty heredocs don't seem to work:

<?php
$a = <<< EOF
EOF;

$ tree-sitter parse test.php
(program [0, 0] - [3, 0]
  (php_tag [0, 0] - [0, 5])
  (expression_statement [1, 0] - [2, 4]
    (assignment_expression [1, 0] - [2, 3]
      left: (variable_name [1, 0] - [1, 2]
        (name [1, 1] - [1, 2]))
      (ERROR [1, 5] - [2, 0]
        (heredoc_start [1, 8] - [1, 12]))
      right: (name [2, 0] - [2, 3]))))
test.php	0 ms	(ERROR [1, 5] - [2, 0])

@Sjord
Copy link
Contributor

Sjord commented May 31, 2022

Also, this fails to parse:

<?php
$a = <<<EOT
 \$
EOT;

$ tree-sitter parse test.php
(program [0, 0] - [4, 0]
  (php_tag [0, 0] - [0, 5])
  (expression_statement [1, 0] - [3, 4]
    (assignment_expression [1, 0] - [3, 3]
      left: (variable_name [1, 0] - [1, 2]
        (name [1, 1] - [1, 2]))
      right: (heredoc [1, 5] - [3, 3]
        identifier: (heredoc_start [1, 8] - [1, 11])
        value: (heredoc_body [1, 11] - [2, 3]
          (ERROR [2, 1] - [2, 2])
          (string_value [2, 2] - [2, 3]))
        end_tag: (heredoc_end [3, 0] - [3, 3])))))
test.php	0 ms	(ERROR [2, 1] - [2, 2])

@Sjord
Copy link
Contributor

Sjord commented May 31, 2022

Also, lines with only whitespace fail to parse:

<?php

echo <<<EOT
hello
 
world
EOT;

The line between "hello" and "world" contains a single space.

$ tree-sitter parse test.php
(program [0, 0] - [7, 0]
  (php_tag [0, 0] - [0, 5])
  (echo_statement [2, 0] - [6, 4]
    (ERROR [2, 5] - [5, 5]
      (heredoc_start [2, 8] - [2, 11])
      (string_value [3, 0] - [3, 5])
      (name [5, 0] - [5, 5]))
    (name [6, 0] - [6, 3])))
test.php	0 ms	(ERROR [2, 5] - [5, 5])

@cfroystad
Copy link
Collaborator Author

cfroystad commented Jun 9, 2022

Thanks for testing, @Sjord

The issues has been fixed and your excellent edge cases has been added to the test suite.

@Sjord
Copy link
Contributor

Sjord commented Jun 9, 2022

Thanks. I have tested some more and came up with some more edge cases:

  • It currently fails to parse when there is a null-byte in the heredoc content.
  • It fails to parse carriage return followed by some letter.
  • It fails to parse when a line starts with a partial end tag:
<?php
echo <<< EOF
E
EOF;

<?php
echo <<< EOF
EO
EOF;

@cfroystad
Copy link
Collaborator Author

cfroystad commented Jun 9, 2022

Nice finds! Testing is something I'm working on becoming better at, so thank you for the "lesson" 🙂

I'm unable to add a test for the carriage return version since git seems to be converting my line endings 🙁 However, all bugs has been fixed. I've added CR as a choice in_new_line since that's how Notepad++ interprets it and how Mac OS handled line endings until 2002.

@Sjord
Copy link
Contributor

Sjord commented Jun 10, 2022

Thanks. I have found some more nowdocs that fail to parse. In particular, a nowdoc with an empty line:

<?php
echo <<< 'EOF'

EOF;

Also, with \x0b, \x0c or \x0d on a line.

@cfroystad
Copy link
Collaborator Author

cfroystad commented Jun 21, 2022

Thanks! Given that I've understood the issues correctly, they're all fixed in the latest commit.

@Sjord
Copy link
Contributor

Sjord commented Jun 21, 2022

Confirmed, everything works.

I see I was a little bit unclear with the hex values in my last message. With \x0b I meant the byte which hexadecimal representation is 0b, not literally \x0b. But I tested it and it works as expected.

@cfroystad cfroystad requested a review from aryx June 22, 2022 08:32
aryx
aryx approved these changes Jun 22, 2022
View reviewed changes
Copy link
Contributor

@aryx aryx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@aryx aryx merged commit 6ff54a9 into tree-sitter:master Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maxbrunsfeld maxbrunsfeld Awaiting requested review from maxbrunsfeld

1 more reviewer

@aryx aryx aryx approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Strange behavior with strings? Quoted heredoc delimiter gives parse error Heredoc incorrectly terminated causes parse error

3 participants

@cfroystad @Sjord @aryx