github-actions[bot] commented on PR #61109:
URL: https://github.com/apache/doris/pull/61109#issuecomment-4010420408
## Code Review Summary: No Issues Found
**PR**: [opt](syntax) fix warning in syntax files and treat warning as error
This PR fixes ANTLR warnings in the grammar files and re-enables
`treatWarningsAsErrors` in the ANTLR Maven plugin configuration. All changes
are correct, minimal, and behavior-preserving.
### Changes Reviewed
1. **`fe/fe-core/pom.xml`**: Re-enables
`<treatWarningsAsErrors>true</treatWarningsAsErrors>` and removes the old
comment about PLParser.g4 (which no longer exists in the codebase). This is a
good hardening measure to prevent future grammar warnings from being silently
introduced.
2. **`DorisLexer.g4` - `GET_FORMAT` token**: Adds explicit `GET_FORMAT:
'GET_FORMAT';` lexer token. This is necessary because the parser grammar uses
`tokenVocab = DorisLexer` (split grammar model), and the parser references
`GET_FORMAT` by name in both the `getFormatFunction` rule and the `nonReserved`
rule. Without the explicit lexer token, ANTLR would warn about an implicit
token.
3. **`DorisLexer.g4` - blank line removal**: Cosmetic removal of blank line
between `GTE` and `PLUS` token rules.
4. **`DorisParser.g4` - `tableAlias?` → `tableAlias`**: The `tableAlias`
rule is already epsilon-capable (`(AS? strictIdentifier identifierList?)?`), so
the `?` at the call site in `warmUpSingleTableRef` was redundant and caused an
ANTLR "rule can already match empty string" warning. Removing it eliminates the
warning without changing parse behavior. Verified: all 8 call sites of
`tableAlias` across the grammar consistently use the bare form without `?`, and
the `withTableAlias()` Java visitor correctly handles the null/epsilon case.
5. **`DorisParser.g4` - formatting**: Alignment fix for `#getFormatFunction`
label and blank line removal in `nonReserved` rule. Cosmetic only.
6. **`SearchLexer.g4` - duplicate comma**: The negated character class
`~[...,+\-!():^[\]"{}\~*?\\/,]` had comma appearing twice. Removing the
duplicate eliminates the ANTLR warning about redundant characters without
changing the character set semantics.
### Critical Checkpoint Conclusions
- **Goal and correctness**: The goal is to fix ANTLR warnings and enforce
warning-as-error. All changes accomplish this correctly.
- **Modification scope**: Minimal and focused — only warning-related fixes
with no behavioral changes.
- **Concurrency**: Not applicable — grammar files only.
- **Parallel code paths**: Verified that no other epsilon-capable rules are
redundantly marked optional at call sites, and no other implicit tokens exist
in the parser grammar.
- **Incompatible changes**: None — the generated parser/lexer behavior is
identical.
- **Test coverage**: PR correctly notes "Previous test can cover this
change" — these are warning fixes with no behavioral impact. Existing
regression and unit tests for `WARM UP SELECT`, `GET_FORMAT()`, and search DSL
nested paths cover all affected syntax paths.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
