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

Cicero parse can't handle missing conditional at end of contract and beginning of line. #554

Closed
lambdafu opened this issue Jun 14, 2020 · 4 comments
Labels
Type: Bug 🐛 Something isn't working

Comments

@lambdafu
Copy link

lambdafu commented Jun 14, 2020

Describe the bug
If the template grammar has a conditional at the very end of the contract, in its own line, the cicero parse command fails to parse the contract if the optional part is missing.

To Reproduce
Grammar:

Hello
{{#if flag}}Bye{{/if}}

Model: o Boolean flag, sample.md: Hello

Output of cicero parse is:

error: Parsing clause text returned a null AST. This may mean the text is valid, but not complete.

Expected behavior
A data.json that has { "flag": false }.

It works if I put another static text after the conditional, so I think the parser gets stuck because it does not terminate the pending conditional at the EOF marker.

Comment

I like Markdown for this application, but this reminds me of very bad experiences I had parsing an extended Markdown in a product I worked on. The above bug is something that a fuzzer could find very quickly, so maybe consider that. Good luck, you'll need it! 🦸

@dselman
Copy link
Contributor

dselman commented Jun 14, 2020

Thanks for the report. @jeromesimeon has a major project underway reimplementing the parsing stack — so we will retest that there.

@dselman dselman added the Type: Bug 🐛 Something isn't working label Jun 14, 2020
@jeromesimeon
Copy link
Member

jeromesimeon commented Jul 2, 2020

Thanks for the bug report @lambdafu. With the old parser the whitespace handling is a bit ad hoc, so I'm not sure how I would go at fixing it.

With the new parser I think there is a clear answer to your question: the issue is that this template:

Hello
{{#if flag}}Bye{{/if}}

has always a soft break after Hello.

While the corresponding sample when flag = false is:

Hello

which will never have a soft break.

I think the clean way to do this is to place that soft break inside the conditional block like this:

Hello{{#if flag}}
Bye{{/if}}

Which now cleanly parses for flag either true or false.

Here is a run on the current development branch:

bash-3.2$ more test/data/testIfEnd/grammar.tem.md
Hello{{#if flag}}
Bye{{/if}}
bash-3.2$ more test/data/testIfEnd/sample.md 
Hello
bash-3.2$ ../markdown-cli/index.js transform --from markdown --to data --grammar test/data/testIfEnd/grammar.tem.md --ctoFiles test/data/testIfEnd/model.cto --input test/data/testIfEnd/sample.md
10:23:34 AM - info:
{
  "$class": "org.test.MyClause",
  "flag": false,
  "clauseId": "a7eeaa1d-cd3a-4e71-a8bb-ba25e04d7ff2"
}
bash-3.2$ more test/data/testIfEnd/sample-true.md
Hello
Bye
bash-3.2$ ../markdown-cli/index.js transform --from markdown --to data --grammar test/data/testIfEnd/grammar.tem.md --ctoFiles test/data/testIfEnd/model.cto --input test/data/testIfEnd/sample-true.md
10:23:44 AM - info:
{
  "$class": "org.test.MyClause",
  "flag": true,
  "clauseId": "df006bff-a066-47ac-b33a-461ce8bdd734"
}

Does that make sense?

@jeromesimeon
Copy link
Member

The corresponding fixes supporting the examples above is now in Cicero version 0.21. @lambdafu can we close this?

@lambdafu
Copy link
Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug 🐛 Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants