Skip to content

tests(cst): Add regression tests for "not followed by IdentifierStart" rules#642

Merged
Xanewok merged 1 commit intoNomicFoundation:mainfrom
Xanewok:test-trailing-ident-start
Nov 8, 2023
Merged

tests(cst): Add regression tests for "not followed by IdentifierStart" rules#642
Xanewok merged 1 commit intoNomicFoundation:mainfrom
Xanewok:test-trailing-ident-start

Conversation

@Xanewok
Copy link
Copy Markdown
Contributor

@Xanewok Xanewok commented Nov 7, 2023

See #641

@Xanewok Xanewok requested a review from a team as a code owner November 7, 2023 08:44
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Nov 7, 2023

⚠️ No Changeset found

Latest commit: 727b1e4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

# This file is generated automatically by infrastructure scripts. Please don't edit by hand.

Source: >
1 │ "foo"bar │ 0..8
Copy link
Copy Markdown
Contributor

@OmarTawfik OmarTawfik Nov 7, 2023

Choose a reason for hiding this comment

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

perhaps not related to this PR, but I'm not sure I understand why this error is Expected AsciiStringLiteral, based on the grammar in dsl.rs .. the first AsciiStringLiteral token we parsed and created correctly .. the following token is not a valid AsciiStringLiteral (based on the first letter b alone) .. so I would have expected to see a full/correct AsciiStringLiteralsList parent node, some SKIPPED text for bar, and an error like Expected end of input.

Thoughts?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If I recall correctly, the way this currently works is that we enter this parser, accumulate the first result but enter the recovery mode for this parser (such that we can recover from unexpected tokens between the list items). Then, we hit EOF, which is where we wrap up the recovering parser.

We could tweak the ER mechanism such that if only the suffix is skipped, we move the skipped tokens alongside the recovered parse rule (rather than them being a descendent of the rule) but I think that the 'expected' bit is correct/intuitive, as we do in fact expected an AsciiStringLiteral but got some unexpected tokens, instead?

Copy link
Copy Markdown
Contributor

@OmarTawfik OmarTawfik left a comment

Choose a reason for hiding this comment

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

Thank you!

@Xanewok Xanewok added this pull request to the merge queue Nov 8, 2023
Merged via the queue into NomicFoundation:main with commit eb3e2d0 Nov 8, 2023
@Xanewok Xanewok deleted the test-trailing-ident-start branch November 8, 2023 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants