fix: move string_value to external scanner

[calebdw]

Checklist

• All tests pass in CI
• There are enough 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 (0 new conflicts)
• The parser size hasn't grown too much (no change)

Hello!

This moves the string_value rule to the external scanner.

Closes #234

Thanks!

[amaanq]

does it have to be in the external scanner? this seems like an easy enough case to handle with an alternation I think?

[calebdw]

Some context on the issue:

The $string_value should match everything between the '...' except for the two possible escape sequences, \\ and \'.

A simplified version of the string rule is:

      string: $ => seq(
        '\'',
        repeat(choice(
          alias(token(choice('\\\\', '\\\'')), $.escape_sequence),
          token(prec(1, repeat1(/\\?[^'\\]/))), // $string_value
        )),
        '\'',
      ),

Note that the prec was added to $string_value because there were issues with parsing a string that started with a comment char (e.g., '#foo').

The above rule correctly parses single quoted strings with escape sequences and strings with comment chars. The problem arises when trying to parse a string with both an escape sequence and a comment char (e.g., '#^(\\)#').

I tried a lot of things to fix the issue but wasn't getting anywhere---this does not occur with double quoted strings because it uses the scanner for the body of the string. So I eventually just gave up and decided to use the scanner for the $string_value as well. This also solved the comment precedence issue so kinda killed two birds with one stone.

Granted, I'm not a tree-sitter (or regex) expert so there may be a better way to solve this---I'm not sure what you mean by alternation, could you show an example?

[calebdw]

Any thoughts @amaanq? I'm open to ideas :)

[amaanq]

yeah you can just use + in the regex instead of repeat1, sorry for taking a while got super busy updating every other grammar and just forgot about this, I'll open a PR with a fix soon, if it doesn't look right let me know, but I think it should be good.

[calebdw]

No worries, I understand!

[calebdw]

@amaanq,

I've tried using:

      string_value: _ => token(prec(1, /(\\?[^'\\])+/)),

instead of:

      string_value: _ => token(prec(1, repeat1(/\\?[^'\\]/))),

but no luck. Am I missing something?

[amaanq]

Sorry I've been kinda blocked on this because of #3305 upstream, I was debugging that with php because even rewriting the scanner to use the array/alloc headers didn't fix it, took a bit of time + had other stuff to do so I'll try and update php asap now, sorry

FYI that PR should fix the very sporadic failures when editing and undoing edits with php where the syntax trees are not consistent