incubator-kie-drools icon indicating copy to clipboard operation
incubator-kie-drools copied to clipboard

Experiment: New DRL parser

Open tkobayas opened this issue 1 year ago • 13 comments

Issue Description:

Migrate a new DRL parser based on antlr 4, because the current DRL parser (based on antlr 3) is hard-coded in the generated Java codes that make it hard to maintain.

Acceptance Criteria:

  • Migrate a new DRL parser in the drools code base with a feature branch dev-new-parser.
  • Run unit tests
  • Fix and improve the parser by solving the test failures
    • It could be several separated issues/PRs which we can work on in parallel
  • Improve the parser / implementation design https://github.com/kiegroup/drools-lsp/pull/48#issue-2120552464
  • Possibly redundant token definitions (DRL_STRING_LITERAL in the DRL lexer and STRING_LITERAL in the Java lexer).
  • Inconsistent operator tokens (we now have DRL_MATCHES but not DRL_CONTAINS).
  • What's the benefit of the built-in operator token definitions if we still need to accept pluggable operators?
  • What to do with the overlap between the expression and DRL parsers?

Out of Scope:

  • Merging the new parser to main is out of scope of this issue. When it's ready, we will file a new issue for that
  • Once the parser is migrated to main, drools-lsp will remove its drools-parser and refer the new parser

tkobayas avatar Feb 08 '24 03:02 tkobayas

Migrate a new DRL parser in the drools code base with a feature branch dev-new-parser.

This is done by ~~https://github.com/apache/incubator-kie-drools/pull/5677~~ https://github.com/apache/incubator-kie-drools/pull/5682

tkobayas avatar Feb 08 '24 03:02 tkobayas

  • Fix and improve the parser by solving the test failures
    • It could be several separated issues/PRs which we can work on in parallel

Each issue would be listed in this issue so that this issue (5678) is the parent issue.

tkobayas avatar Feb 08 '24 05:02 tkobayas

Child issue: Test failure : RHS end without preceding white-space https://github.com/apache/incubator-kie-drools/issues/5679

tkobayas avatar Feb 08 '24 07:02 tkobayas

Child issue: Test failure : Unknown language level https://github.com/apache/incubator-kie-drools/issues/5681

tkobayas avatar Feb 09 '24 05:02 tkobayas

Child issue: Better implementation switch https://github.com/apache/incubator-kie-drools/issues/5683

tkobayas avatar Feb 09 '24 09:02 tkobayas

@yurloc Thank you very much for the effort to raise the issue list! I'm going to start with:

  • #5700

tkobayas avatar Feb 21 '24 03:02 tkobayas

As of 2024-07-11, unit tests are all green. Remaining child issues are low priority, so they can be handled after merging to main.

https://github.com/apache/incubator-kie-drools/issues/5799 https://github.com/apache/incubator-kie-drools/issues/5807 https://github.com/apache/incubator-kie-drools/issues/5856 https://github.com/apache/incubator-kie-drools/issues/5874 https://github.com/apache/incubator-kie-drools/issues/5938

The next step is to create a PR to merge this branch to main branch. Notes:

  • Merging has to be done after Drools 10.0 release.
  • Until the Drools 10.0 release, mark the PR [DO-NOT-MERGE] and make it draft.
  • Create 2 PRs. One makes the new parser not default parser. So it shouldn't affect the default behavior. It is the one to be merged. The other makes the new parser default parser (with a system property switch). It is in order to check any regression error. It's not to be merged, so draft PR.
  • After Drools 10 release, discuss the future plan in dev at kie.apache ML.

tkobayas avatar Jun 11 '24 01:06 tkobayas

@tkobayas Hi is there any timeline when we can expect 10.0 with antlr4-runtime updated will be released ?

anand188 avatar Aug 29 '24 08:08 anand188

@anand188 There is no timeline forecast yet. I hope 10.0 (without new parser) release will happen in 1 or 2 months, but not very sure. New parser will be included in the next minor version (probably 10.1).

tkobayas avatar Aug 29 '24 08:08 tkobayas

@tkobayas is that fair statement we can expect 10.1 before December, is this something we can wait for ?

anand188 avatar Aug 29 '24 12:08 anand188

@anand188 Sorry, we cannot provide a timeline forecast at the moment.

Btw, I'm interested in why you wait for the antlr4 based parser. Could you let me know? It's implementation details and doesn't directly affect users.

tkobayas avatar Aug 30 '24 01:08 tkobayas