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

Fix trailing garbage syntax bug #7

Merged
merged 3 commits into from
Feb 7, 2025
Merged

Fix trailing garbage syntax bug #7

merged 3 commits into from
Feb 7, 2025

Conversation

jnu
Copy link
Member

@jnu jnu commented Feb 7, 2025

The crocodsl grammar only defined partial expressions, without any rule to parse a complete input in one go. Because of this, calling parser.expr() on any input would simply stop without error when a full expression was parsed even if the input token stream was not fully consumed. This led to the following situation:

Input expression: TimeSince($assigned, 'mo') Gte 6
Parse result:     TimeSince($assigned, 'mo')

The Gte 6 piece was unrecognized by the expr method so it was simply ignored, since the LHS parsed correctly. The supposed function Gte would have caused an error if expr() was called again, but it never was. The function should have been Ge, which would have caused the entire input to be consumed, since it was a valid comparator expression.

The fix is to add a program rule to the grammar which is defined as expr + EOF. This will inspire the parser to consume the entire input, raising one of several different types of errors if it can't. In this case, we would have hit an AttributeError when it tried to look up the function Gte in func.py, since the unrecognized string would be parsed as an infix expression. (We will now catch that exception and re-raise as a SyntaxError.)

@jnu jnu merged commit bd56170 into main Feb 7, 2025
2 checks passed
@jnu jnu deleted the trailing branch February 7, 2025 17:12
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.

1 participant