Precise error location in UNRECOGNIZED by attaching trivia to the expected nonterminal #1172
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 72d4858 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Likely this deserves a changeset, since it's changing the way an UNRECOGNIZED node is return when it's in the top-most position. |
OmarTawfik
reviewed
Dec 6, 2024
crates/codegen/runtime/cargo/crate/src/runtime/parser/parser_support/parser_function.rs
Show resolved
Hide resolved
OmarTawfik
reviewed
Dec 6, 2024
...snapshots/cst_output/ContractDefinition/unicode_in_doc_comments/generated/0.4.11-failure.yml
Show resolved
Hide resolved
OmarTawfik
reviewed
Dec 6, 2024
...snapshots/cst_output/ContractDefinition/unicode_in_doc_comments/generated/0.4.11-failure.yml
Show resolved
Hide resolved
OmarTawfik
reviewed
Dec 6, 2024
OmarTawfik
reviewed
Dec 10, 2024
crates/codegen/runtime/cargo/crate/src/runtime/parser/parser_support/parser_function.rs
Show resolved
Hide resolved
OmarTawfik
approved these changes
Dec 11, 2024
This was referenced Dec 11, 2024
Merged
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jan 13, 2025
…1187) Solves #1184 To go on top of #1172 This has a big impact in the failing examples, which now return the expected `NonTerminal`. This was acknowledgedly coded by playing the whack-a-mole with the type system. I'm not so happy with the bunch of `Rc::clone()` that leaked out; I will consider an alternative once we settle this is what we need and there are no more pressing issues. Currently doing a full CI run locally to find what more to fix. In the meantime, I'll mark this as draft.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jan 24, 2025
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and publish to npm yourself or [setup this action to publish automatically](https://github.com/changesets/action#with-publishing). If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @nomicfoundation/[email protected] ### Minor Changes - [#1156](#1156) [`3a82f06`](3a82f06) Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - add `node.descendants()` and `cursor.descendants()` APIs to allow iterating over all descendants of the current node in pre-order traversal. - [#1156](#1156) [`3a82f06`](3a82f06) Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - fix `node.children()` and `parseOutput.errors()` return types - [#1194](#1194) [`7a25d63`](7a25d63) Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - split `parser/Parser.supportedVersions()` into a new `utils/LanguageFacts` API, with `allVersions()`, `earliestVersion()`, and `latestVersion()` methods. - [#1194](#1194) [`7a25d63`](7a25d63) Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - expose the `BingingGraph` API to allow querying definitions/references between source files. - [#1156](#1156) [`3a82f06`](3a82f06) Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - add `cursor.ancestors()` API to allow iterating over all ancestors of the current node, starting with the immediate parent, and moving upwards, ending with the root node. - [#1156](#1156) [`3a82f06`](3a82f06) Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - add `cursor.remainingNodes()` API to allow iterating over all the remaining nodes in the current tree, moving in pre-order traversal, until the tree is completed. - [#1223](#1223) [`3e85a14`](3e85a14) Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - split `Parser.parse()` API into `parse_file_contents()` and `parse_nonterminal()`. - [#1194](#1194) [`7a25d63`](7a25d63) Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - add a `CompilationBuilder` API to incrementally load and resolve source files and their imports. - [#1223](#1223) [`3e85a14`](3e85a14) Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - rename `Query.parse()` to `Query.create()`, and provide exact `TextRange` for any errors it returns. - [#1172](#1172) [`6102886`](6102886) Thanks [@beta-ziliani](https://github.com/beta-ziliani)! - Improved error recovery, where leading trivia are always parsed and included before an erroneous terminal. - [#1223](#1223) [`3e85a14`](3e85a14) Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - add `TerminalKindExtensions.is_identifier()` API to distinguish terminals like Solidity's `Identifier` and Yul's `YulIdentifier`. ### Patch Changes - [#1134](#1134) [`cfc62f2`](cfc62f2) Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - remove `YulPathComponent` and just use `YulIdentifier` instead. - [#1138](#1138) [`44a706f`](44a706f) Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - add `ThisKeyword` and `SuperKeyword` to the grammar, instead of parsing them as identifiers. - [#1134](#1134) [`cfc62f2`](cfc62f2) Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - unreserve `AddressKeyword`, and let it be used for `MemberAccessExpression`, `StructMember`, etc... - [#1154](#1154) [`7b9b478`](7b9b478) Thanks [@beta-ziliani](https://github.com/beta-ziliani)! - Adding support for deprecated keywords `jump` and `jumpi` Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #841
In examples like the following:
The parsing function was returning a terminal with
UNRECOGNIZEDkind and the entire text inside. Since a terminal cannot have trivia attached, we have to change the returning value to be a nonterminal. But which one? For a moment I thought of a new specificUNRECOGNIZEDkind, but after discussing with Omar, it was clear that the better option was to return the expected one: if we are parsing aStatement, then return aStatementwith the trivia and theUNRECOGNIZEDterminal at the end. This is realized in theNoMatchstructure with a new field for the optional expected nonterminal kind, which is essentially attached in theParserResult::with_kindmethod.The commit history contains a previous attempt in which the trivia was cached within the
NoMatchstruct. This added unnecessary complexity, so I decided against it.Note1: In a couple of places the code will use a
ParserResult::no_match(<default values>). Noting there was aParseResult::default()function, I decided to use it instead, but I'm not sure if this is the expected use of the default function.Note2: I see that the error is one off at the end, probably accounting EOF? But was already the case before, so there's no real change there.