[new-parser] Allow package to be used as an identifier

[yurloc]

Fixes

• [new-parser] Parsing fails if a Java keyword appears in a qualified name #5818

DRAFT

Let's discuss whether this is a good approach.

How to replicate CI configuration locally?

Build Chain tool does "simple" maven build(s), the builds are just Maven commands, but because the repositories relates and depends on each other and any change in API or class method could affect several of those repositories there is a need to use build-chain tool to handle cross repository builds and be sure that we always use latest version of the code for each repository.

build-chain tool is a build tool which can be used on command line locally or in Github Actions workflow(s), in case you need to change multiple repositories and send multiple dependent pull requests related with a change you can easily reproduce the same build by executing it on Github hosted environment or locally in your development environment. See local execution details to get more information about it.

How to retest this PR or trigger a specific build:

• for pull request and downstream checks

  • Push a new commit to the PR. An empty commit would be enough.

• for a full downstream build

  • for github actions job: add the label run_fdb

• for Jenkins PR check only

  • If you are an ASF committer for KIE podling, login to Jenkins (https://ci-builds.apache.org/job/KIE/job/drools/), go to the specific PR job, and click on Build Now button.

[yurloc]

@tkobayas @mariofusco @gitgabrio Together with THIS on the line above, this is just the tip of the iceberg. If we want to keep the new parser 100% backward compatible, we need to A) also add all the other Java keywords, for example, long, break, for, etc. or B) revert closer to the old approach and stop using JavaLexer that defines all these keyword tokens.

B) means using just a few tokens for operators, parentheses, and literals of various types, and IDENTIFIER for everything else on the Lexer level. The parser then needs to use semantic predicates (e.g. helper.validateIdentifierKey(DroolsSoftKeywords.PACKAGE) to see whether the next IDENTIFIER token actually contains package).

A) means listing all JavaLexer tokens for various Java keywords here and also replacing nearly all usages of IDENTIFIER with drlIdentifier.

[tkobayas]

I think the original test failure #5818 org.kie.declarativetypes.JavaBeansEventRoleTest#testImportBean is a very exceptional case.

        String drl = "package some.other.package;\n" +
                "import org.drools.compiler.Bean;\n" +
                "declare Bean\n" +
                "  @role(event)\n" +
                "end";

The test doesn't fail with the old parser, because the package some.other.package doesn't have anything to compile actually. If a user adds a rule to the package, it would fail on the java compilation phase even with the old parser.

DRL is more or less based on Java syntax, so I think, theoretically, it's fine to fail such a java keyword usage (which normally fails with java compiler) at the parse phase with the new parser (As written in #5818, no-error-report is another issue, though). In other words, fix the test case not to use package in package.

C) Keep excluding java keywords from drlIdentifier

It's against the policy "100% backward compatible with the old parser", but sounds acceptable. WDYT?

(We may face a situation that we will need to choose option A or B in the future, though)

[gitgabrio]

Hi @yurloc @tkobayas @mariofusco
sorry I could provide only very general observations:

1. I would stick with 100% backward compatibility; without that, something that was working previously, simply stop to work, and it is not clear who/how would fix the issues that will popup, nor how much time this could take
2. I wonder (again, just asking) if there is not a programmatic way to automatically add all the java keywords from JavaLexer to drlIdentifier : drlKeywords - and then keep going with option A (IIUC)

Wdyt ? Am I missing something ?

[tkobayas]

1. I would stick with 100% backward compatibility; without that, something that was working previously, simply stop to work, and it is not clear who/how would fix the issues that will popup, nor how much time this could take

Okay, I'm not opinionated on this. We can go with Option A and leave Option C as a future enhancement possibility.

2. I wonder (again, just asking) if there is not a programmatic way to automatically add all the java keywords from JavaLexer to drlIdentifier : drlKeywords - and then keep going with option A (IIUC)

There is no programmatic way because those Java keywords are individually declared in JavaLexer.g4. I think explicitly listing them in drlIdentifier is readable enough.

Thanks,

[gitgabrio]

ok @tkobayas
Anyway, just remember that copying/duplicating stuff creates problem of maintanability, not readability.
Anyway, fine for me 👍

[yurloc]

Superseded by #5958.