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

[question] Using ANTLR to define ISL schema #121

Open
rpreiss07 opened this issue Jun 10, 2019 · 4 comments
Open

[question] Using ANTLR to define ISL schema #121

rpreiss07 opened this issue Jun 10, 2019 · 4 comments
Labels
enhancement New feature or request
Milestone

Comments

@rpreiss07
Copy link

Hi, as ANTLR is used for the schema definition of ion-text, is there a specific reason not to use it for ISL (so that the same parser infrastructure can be used for ion type definitions and value definitions)? Or is there any plan to add a ANTLR schema definition for ISL so that several parsers can be generated out of it (e. g. javascript)? Thx

@pbcornell
Copy link

Hello, in it's current state, the ANTLR Grammar for Ion text is incomplete, is not used to generate Ion parsers, and such a parser would likely fail a number of the tests in ion-tests. There have been some concerns about the performance of a generated parser, as well as whether all aspects of Ion text can be expressed (e.g., calendar rules for timestamp values such as number of days in a month, or the years in which a leap day is valid). I suspect we'd want to shore up the Ion text grammar prior to creating an Ion Schema ANTLR grammar based on it.

That said, your idea is a compelling one as a quick way to provide support in additional languages not only for Ion, but also for Ion Schema. We'd need to address concerns around completeness/correctness and performance.

No plans at this time, but worth further exploration.

@pbcornell pbcornell added the enhancement New feature or request label Jun 10, 2019
@pbcornell pbcornell added this to the backlog milestone Jun 10, 2019
@pbcornell
Copy link

Hmm...perhaps the Ion text grammar is closer to complete than I thought. IonGrammarTestCase already generates a parser from the ANTLR grammar, adds some calendar validation logic, and runs against the ion-tests suite (skipping only about 25 of the test files).

@rpreiss07
Copy link
Author

Thanks for the fast reply. As I would like to parse ISL via javascript, I am thinking about converting the BNF into ANTLR myself and to generate a js parser from it (can be a contribution to this project if it makes sense). But it seems that except for the token of "$ion_schema_1_0" the ISL is already accepted by the ION parser (http://amzn.github.io/ion-docs/), so maybe it would be better to consider ISL just as a kind of 2nd level layer (on top of parser) which translates ION text tokens into ISL items after parsing is done (slightly adapting the ION text parser to accept the schema prefix) - but this would need to be done for each target language.

In the context of what I would like to do, optimizing the performance of the parser wouldn't be so important. I would consider special cases like leap days as something which could be handled by a validation layer on top of the parser to keep the grammer straightforward.

@pbcornell
Copy link

Right...ISL is valid Ion, so an Ion parser can read it, but can't confirm that it is ISL. Parsing ISL via a layer atop an Ion parser is the current approach; if/when we provide a JavaScript implementation of Ion Schema, it is likely to use the ion-js parser as its foundation. Your idea of using ANTLR (and leveraging the existing Ion text grammar) to create an ISL parser in other languages is intriguing, too, and likely a quicker path to get something working. It would be interesting to understand any tradeoffs associated with that approach. Note that the Ion Schema BNF grammar doesn't fully describe ISL correctness either (e.g., that the first value in a range is lower than the second, or that a string given to the regex constraint is a valid regular expression); a validation layer on top of the parser would be wise here, too.

And thank you for noticing that the $ion_schema_1_0 produces an error at http://amzn.github.io/ion-docs/. I believe this is a bug in the ion-js parser, and have opened ion-js#202 to track it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants