Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor parser #23

Merged
merged 12 commits into from Mar 21, 2024
Merged

Refactor parser #23

merged 12 commits into from Mar 21, 2024

Conversation

quephird
Copy link
Owner

@quephird quephird commented Mar 17, 2024

There are two main objectives in the PR:

  • Centralize the logic associated with any one helper. For example, the detection of a "class" token should actually be part of the helper for parsing a class declaration. That necessitated that said helpers needed to return a nilable Statement or Expression in the event the detection of the first token wasn't made, which meant that call sites needed to have an if let block to capture and return a non-nil value. This not only improved clarity, but also improved the reusability of such helpers.
  • Introduce new helpers, and move out logic for parsing various statements and expressions into them. For example, parsePostfix() had logic for handling any of three expression types, making the method harder to read and understand. Moving each bundle of logic into its own method, namely parseCall(), parseGet(), and parseSubscript(), and giving each a name makes intent much clearer.
  • Introduced consumeToken() to reduce boilerplate and the number of places the low-level call to advanceCursor() were made.
  • Ensured that the last statement in every helper returned a value instead of throwing an error. In those cases, I introduced a guard block to throw that error, and if control fell through, then the value would be returned at the end of the function block. This also (hopefully) improves clarity.

@quephird quephird merged commit f186e05 into main Mar 21, 2024
@quephird quephird deleted the refactor_parser branch March 21, 2024 20:16
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.

None yet

1 participant