refactor: replace ANTLR parser with omni recursive descent parser#26
refactor: replace ANTLR parser with omni recursive descent parser#26h3n4l wants to merge 3 commits intobytebase:mainfrom
Conversation
Replace the ANTLR-based parser (bytebase/parser/mongodb) with the hand-written recursive descent parser (bytebase/omni/mongo). This eliminates the ANTLR runtime dependency and simplifies the translator from ~3400 lines to ~1600 lines by replacing the visitor pattern with direct AST type switches. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors MongoDB shell parsing/translation by replacing the previous ANTLR-based parser/visitor implementation with the github.com/bytebase/omni/mongo recursive-descent parser and a new translation layer built on omni AST node type switches.
Changes:
- Replace ANTLR parse-tree visitor workflow with omni parsing (
mongo.Parse) and direct AST translation (translateNode). - Rework value/helper decoding to consume omni AST nodes for BSON conversion (documents, arrays, literals, helper calls).
- Update module dependencies to drop ANTLR/runtime and introduce
github.com/bytebase/omni.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/translator/visitor.go | Removes the ANTLR visitor-based operation extraction implementation. |
| internal/translator/translator.go | Switches parsing entrypoint to mongo.Parse and routes to omni AST translation. |
| internal/translator/translate.go | Adds omni AST → Operation translation via type switches and method dispatch. |
| internal/translator/helpers.go | Replaces parse-context value conversion with omni AST node conversion helpers. |
| internal/translator/database.go | Ports DB-level argument extraction to omni AST args. |
| internal/translator/collection.go | Ports collection/cursor method arg parsing and option extraction to omni AST args. |
| internal/translator/bson_helpers.go | Ports helper function decoding to omni AST helper calls. |
| go.mod | Removes ANTLR/parser deps; adds github.com/bytebase/omni and bumps/adjusts related deps. |
| go.sum | Updates checksums to reflect dependency graph changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| stmts, err := mongo.Parse(statement) | ||
| if err != nil { | ||
| if pe, ok := err.(*parser.ParseError); ok { | ||
| return nil, &ParseError{ | ||
| Line: pe.Line, | ||
| Column: pe.Column, | ||
| Message: pe.Message, | ||
| } | ||
| } | ||
| return nil, err |
There was a problem hiding this comment.
Parse() extracts parser.ParseError using a direct type assertion to *parser.ParseError. If the omni parser returns a non-pointer ParseError or wraps it (e.g., via %w), this mapping will be skipped and callers won’t get a translator.ParseError. Consider switching to errors.As to robustly detect/unwrap parse errors before returning.
| @@ -290,6 +235,5 @@ func convertTimestampDoc(ctx *mongodb.TimestampDocHelperContext) (bson.Timestamp | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| return bson.Timestamp{T: t, I: i}, nil | |||
| } | |||
There was a problem hiding this comment.
convertTimestampDoc() silently returns {T:0, I:0} when the document is missing required fields (t/i) or when those fields are present but not numeric int32/int64 (e.g., string/float). This can produce an incorrect timestamp without any error. Consider validating that both fields exist and are valid integers, and return an error otherwise.
internal/translator/collection.go
Outdated
| return | ||
| } | ||
| specs = append(specs, doc) | ||
| op.DropTarget = &boolNode.Value |
There was a problem hiding this comment.
extractRenameCollectionArgs() stores op.DropTarget as &boolNode.Value (a pointer into the AST node). This differs from other option parsing in the translator (which copies into a local variable before taking the address) and keeps the AST object alive unnecessarily. Prefer copying the bool into a local variable and taking its address for consistency and to avoid retaining the parse tree.
| op.DropTarget = &boolNode.Value | |
| dropTarget := boolNode.Value | |
| op.DropTarget = &dropTarget |
The omni dependency requires Go 1.25.5, which is incompatible with golangci-lint v1.64.8 (built with Go 1.24). Upgrade both the Go version and linter in CI workflows. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use errors.As for robust ParseError unwrapping in translator.go - Validate that Timestamp document contains both 't' and 'i' fields with integer values in bson_helpers.go - Copy bool value before taking address in extractRenameCollectionArgs to avoid retaining AST node Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
bytebase/parser/mongodb(ANTLR-based) withbytebase/omni/mongo(hand-written recursive descent parser)antlr4-go/antlr/v4runtime dependency and itsreplacedirectiveTest plan
go build ./...passesgolangci-lint runpasses with 0 issues🤖 Generated with Claude Code