[incubator-kie-drools#5708] [new-parser] function definition causes a… #5793
Conversation
|
Before PR After PR |
| @@ -512,3 +512,37 @@ drlVariableInitializer | |||
| drlArrayInitializer | |||
| : LBRACE (drlVariableInitializer (COMMA drlVariableInitializer)* (COMMA)? )? RBRACE | |||
| ; | |||
|
|
|||
| /* extending JavaParser block */ | |||
| drlBlock | |||
There was a problem hiding this comment.
String literal (e.g. "*") is DRL_STRING_LITERAL, so we need to extend JavaParser rules to accept it (eventually. DRL_STRING_LITERAL is in drlLiteral)
There was a problem hiding this comment.
Quick thought: we keep overriding the JavaParser rules again and again... that adds a lot of duplicate code. Is it possible to remove the original rule from JavaParser every time we override one? Simply remove JavaParser rules that are no longer being used since we use the overriding rules.
We could end up with a pretty minimal JavaParser at the end or perhaps get rid of it completely.
Or is this impossible because we still need to parse plain old Java code in the RHS?
There was a problem hiding this comment.
@yurloc That's a great point. Let me try...
Or is this impossible because we still need to parse plain old Java code in the RHS?
It's okay. RHS is just retrieved as text by the parser. For RHS execution, non-exec-model uses JavaParser.java in drools-compiler, exec-model uses the java code directly with some modification.
For DRL editor, we may need another parser rules specific to RHS, but it would be a later story.
There was a problem hiding this comment.
@yurloc Ahh, I can't. For example, if I remove block in JavaParser.g4, other rules in JavaParser.g4 which depend on block fail to process. Even if I use the same name block in DRLParser.g4, the result is the same because JavaParser.g4doesn't look at DRLParser.g4.
A possible approach would be to fully copy the content of JavaParser.g4 into DRLParser.g4 and modify rules in the single file. It would be worth trying, but it would be another separate issue, WDYT?
There was a problem hiding this comment.
I understand. We could only remove "entry-point" rules that are not depended on by other rules, which is currently not the case.
Let's revisit this idea when we're done fixing the failing tests and we see the final shape of DRLParser and JavaParser.
|
@yurloc @mariofusco @gitgabrio Please review, thanks! |
tkobayas
force-pushed
the
incubator-kie-drools-5708-function
branch
from
3dbdb6d
to
81e97dd
Compare
|
Rebased to resolve conflicts. |
| @@ -193,7 +194,7 @@ public FunctionDescr visitFunctiondef(DRLParser.FunctiondefContext ctx) { | |||
| functionDescr.addParameter(typeTypeContext.getText(), variableDeclaratorIdContext.getText()); | |||
| }); | |||
| } | |||
| functionDescr.setBody(getTextPreservingWhitespace(ctx.block())); | |||
| functionDescr.setBody(stripBracesFromBlock(getTextPreservingWhitespace(ctx.drlBlock()))); | |||
There was a problem hiding this comment.
| functionDescr.setBody(stripBracesFromBlock(getTextPreservingWhitespace(ctx.drlBlock()))); | |
| functionDescr.setBody(getTextPreservingWhitespace(ctx.drlBlock().drlBlockStatement())); |
@tkobayas by using block statements instead of the whole block, we could avoid "manually" trimming away the braces.
Of course, this requires a new whitespace-text-preserving function that works on a list of contexts, something like this:
public static String getTextPreservingWhitespace(List<? extends ParserRuleContext> ctx) {
return ctx.stream().map(Antlr4ParserStringUtils::getTextPreservingWhitespace).collect(Collectors.joining());
}I find this simpler to understand and less error-prone (and maybe also reusable for other use cases) than the low-level string manipulation in stripBracesFromBlock().
Wdyt?
There was a problem hiding this comment.
@yurloc Thank you! Applied the suggestion.
|
Overall fixing progress: There were 396 failed tests. |
apache#5793) * [incubator-kie-drools#5708] [new-parser] function definition causes an error * - enhance getTextPreservingWhitespace to avoid string manipulation
apache#5793) * [incubator-kie-drools#5708] [new-parser] function definition causes an error * - enhance getTextPreservingWhitespace to avoid string manipulation
apache#5793) * [incubator-kie-drools#5708] [new-parser] function definition causes an error * - enhance getTextPreservingWhitespace to avoid string manipulation
#5793) * [incubator-kie-drools#5708] [new-parser] function definition causes an error * - enhance getTextPreservingWhitespace to avoid string manipulation
apache#5793) * [incubator-kie-drools#5708] [new-parser] function definition causes an error * - enhance getTextPreservingWhitespace to avoid string manipulation
…n error
Issue:
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
for a full downstream build
run_fdbfor Jenkins PR check only
Build Nowbutton.