#858 Fixed the COBOL parser so reserved words can be used as field names - #859
Conversation
WalkthroughANTLR is upgraded from 4.8 to 4.9.3. The COBOL identifier grammar accepts additional keyword tokens, generated parser sources are regenerated, and syntax tests cover reserved-word-like field names and ChangesCOBOL parser compatibility update
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant SyntaxErrorsSpec
participant copybookLexer
participant copybookParser
participant ParsedRecordLayout
SyntaxErrorsSpec->>copybookLexer: provide COBOL copybook text
copybookLexer->>copybookParser: provide token stream
copybookParser->>ParsedRecordLayout: parse identifier and field clauses
ParsedRecordLayout-->>SyntaxErrorsSpec: return parsed names and layout
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
JaCoCo code coverage report - 'cobol-parser'
|
JaCoCo code coverage report - 'spark-cobol'
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/antlr/copybookParser.g4 (2)
53-53: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winThe pre-existing empty alternative on the preceding line (
X_S | | N_S) becomes riskier with 60+ new alternatives.It compiles to
case 7: enterOuterAlt(_localctx, 7); { }incopybookParser.java(Lines 634-638) —identifiercan match the empty string. With the alternative set now this large, adaptive prediction is far more likely to select the epsilon branch during error recovery and yield an empty field name instead of a syntax error. Consider removing the stray|while regenerating.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/antlr/copybookParser.g4` at line 53, Remove the empty alternative from the identifier rule by changing the preceding X_S/N_S alternative so it no longer includes a standalone “|”. Regenerate the parser artifacts, including copybookParser.java, and preserve only the valid identifier alternatives.
53-68: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd reserved-word field-name syntax tests for COBOL occurrence/sort/renames clauses.
The
identifierrule already accepts the tokens used as clause separators, and the regenerated parser now resolves ambiguity with adaptive prediction. Add syntax tests for fields named likeON,BY,KEY,IS,TO,TIMES,DEPENDING,INDEXED, orTHRU, coveringOCCURS ... DEPENDING ON,INDEXED BY,ASCENDING/DESCENDING KEY IS, andRENAMES A THRU B.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/antlr/copybookParser.g4` around lines 53 - 68, Add syntax tests for reserved-word field names in the COBOL parser, covering ON, BY, KEY, IS, TO, TIMES, DEPENDING, INDEXED, and THRU within OCCURS ... DEPENDING ON, INDEXED BY, ASCENDING/DESCENDING KEY IS, and RENAMES A THRU B clauses. Ensure the tests assert these declarations parse successfully through the existing identifier rule and regenerated parser.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/antlr/copybookParser.g4`:
- Line 53: Remove the empty alternative from the identifier rule by changing the
preceding X_S/N_S alternative so it no longer includes a standalone “|”.
Regenerate the parser artifacts, including copybookParser.java, and preserve
only the valid identifier alternatives.
- Around line 53-68: Add syntax tests for reserved-word field names in the COBOL
parser, covering ON, BY, KEY, IS, TO, TIMES, DEPENDING, INDEXED, and THRU within
OCCURS ... DEPENDING ON, INDEXED BY, ASCENDING/DESCENDING KEY IS, and RENAMES A
THRU B clauses. Ensure the tests assert these declarations parse successfully
through the existing identifier rule and regenerated parser.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5c9a37f4-7646-43aa-a3a3-be029d298793
📒 Files selected for processing (12)
cobol-parser/pom.xmlcobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/antlr/copybookLexer.javacobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/antlr/copybookParser.g4cobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/antlr/copybookParser.javacobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/antlr/copybookParserBaseVisitor.javacobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/antlr/copybookParserVisitor.javacobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/antlr/jsonBaseVisitor.javacobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/antlr/jsonLexer.javacobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/antlr/jsonParser.javacobol-parser/src/main/scala/za/co/absa/cobrix/cobol/parser/antlr/jsonVisitor.javacobol-parser/src/test/scala/za/co/absa/cobrix/cobol/parser/parse/SyntaxErrorsSpec.scalaproject/Dependencies.scala
1 participant
Closes #858
Summary by CodeRabbit
New Features
Bug Fixes
REDEFINES.Tests