[JENKINS-66613] Correct Antlr grammar for LabelExpression #5722
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See JENKINS-66613.
The Antlr grammar used for LabelExpress (LabelExpr.g) contained flaws since more than 10 years. The first one is a nondeterminism between ATOM and IMPLIES as both could start with a
-
. Second point is about the ATOM being greedy by default, eating the next IMPLIES if there is no space inbetween (a->b
expression is parsed as a single ATOM, while it should be a IMPLIES with two ATOMs).Some tests are added to cover the first point and others (related to lack of spaces) to cover the second point.
💡 It should also cover JENKINS-16728 and JENKINS-22975 (and also second part of JENKINS-16734, thanks Tim).
FTR I tried different approaches while "learning" ANTLR, like Nongreedy Subrules, ending up with Explicit Lookahead as the working solution.
Bonus:
Before this PR during the build
After this PR during the build
Proposed changelog entries
Proposed upgrade guidelines
N/A
Submitter checklist
Proposed changelog entries
section only if there are breaking changes or other changes which may require extra steps from users during the upgradeDesired reviewers
@daniel-beck (I know you will be really happy with this one :D)
Maintainer checklist
Before the changes are marked as
ready-for-merge
:Proposed changelog entries
are correctupgrade-guide-needed
label is set and there is aProposed upgrade guidelines
section in the PR title. (example)lts-candidate
to be considered (see query).