[Due for payment 2025-02-13] Add GitHub Action to verify parser updates when parser files change

[sumo-slonik]

Ensure Search Parser Consistency: Add GitHub Action for Verification

Problem

Currently, it is possible to modify the search parser .js files without updating the corresponding .peggy grammar files. This can lead to inconsistencies and discrepancies between the parser implementation and its intended structure. Since we do not have any verification in place to ensure that changes in one are properly reflected in the other, errors may go unnoticed.

Proposal

To prevent this issue, we should add a GitHub Action that verifies whether changes in the .js parser files only occur when there are corresponding modifications in the .peggy files. This check would help maintain consistency and ensure that all updates to the parser are properly reflected in its configuration.

Expected Outcome

• Reduce inconsistencies between parser implementation and configuration.
• Prevent accidental modifications that could lead to incorrect parsing behavior.

@Kicu @blazejkustra

Issue Owner

Current Issue Owner: @slafortune

[melvin-bot]

Triggered auto assignment to @slafortune (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[sumo-slonik]

Here is an example of such an inconsistency between files that once appeared because there is no such check
https://github.com/Expensify/App/pull/53158/files

[Kicu]

CC @luacmartins

We think it's not a lot of work but will be useful to prevent mistakes when editing parser

[sumo-slonik]

I can solve this by adding an action that will be triggered by changes in the folder:
src/libs/SearchParser, which will check if the generated .js files are consistent with what will be generated from the .peggy files. This doesn’t seem like a lot of work, and it will help us ensure consistency.

[luacmartins]

I wonder if creating a script to automatically generate the .js file on build would be a better solution, so we:

1. Don't have to manually do it
2. Wouldn't allow changes to it in the first place

What do you think?

[sumo-slonik]

Currently we already have two such scripts, but it does not protect us from the possibility of changing an already generated file. I have prepared a draft PR in which you can see how such an action could look like:

#55957

Wouldn't allow changes to it in the first place
Here it would be better if @Kicu spoke out

[slafortune]

@sumo-slonik should I assign you to this GH? Does this really need an external label, you aren't really looking for help with this, are you?

[Kicu]

I wonder if creating a script to automatically generate the .js file on build would be a better solution

The only problem to this approach is that it probably is more work. We would need to either run it on commits - which would definitely confuse and irritate some developers (extra few seconds 😅) or most likely do it on the CI.
The script has to be committed, so I guess on the CI we would need to generate extra commit IF there is a mismatch between the files.

At this point I thought that a check that verifies if the files were touched is just simpler and faster.

[sumo-slonik]

@sumo-slonik should I assign you to this GH? Does this really need an external label, you aren't really looking for help with this, are you?

If you can please assign me, PR is ready if only the approach with check on the github actions side will be accepted by you.

[luacmartins]

That sounds good. Let's go with the current solution.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.94-25 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [No QA] Add GitHub Action to verify parser updates when parser files change #55957

If no regressions arise, payment will be issued on 2025-02-13. 🎊

For reference, here are some details about the assignees on this issue:

• @sumo-slonik does not require payment (Contractor)

[melvin-bot]

@luacmartins @slafortune @sumo-slonik The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]