Replace Dot[Variable] with DotVariable #31
Conversation
WalkthroughThis change replaces the use of a generic Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TreeBuilder
participant DotVariable
participant ObjectValue
User->>TreeBuilder: Build tree from expression with property access (e.g., aCar.Brand)
TreeBuilder->>DotVariable: Create DotVariable node with Variable("Brand")
User->>DotVariable: Calculate property access
DotVariable->>ObjectValue: Extract receiver object from value
ObjectValue->>DotVariable: Return property value by name
DotVariable->>User: Return property value
Possibly related PRs
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: can't load config: can't set severity rule option: no default severity defined 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
object.go (1)
53-69: Minor clean-up opportunity: avoid redundant variable & tighten nil-handling
var receiver anyis not necessary – you can assign straight intoreceiver := valand then overwrite it if the object is anObjectValue.- A nil
valwill silently fall through the two-value type assertion; consider handling that explicitly to give a clearer diagnostic (mirroring the check you added for the non-Value receiver).Nitpoke implementation (optional):
-func (dv DotVariable) Calculate(val entry) entry { - var receiver any - - // as this is an object property accessor, we need to get the object first: it is the LHS currently held in val - receiver, ok := val.(Value) +func (dv DotVariable) Calculate(val entry) entry { + if val == nil { + return NewUndefinedWithReasonf("syntax error: object accessor [Variable] called on nil receiver for member '%s'", dv.Name) + } + + receiver, ok := val.(Value)This is purely cosmetic but improves clarity and consistency with the rest of the code-base.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
gal_test.go(1 hunks)object.go(3 hunks)tree.go(2 hunks)tree_builder.go(1 hunks)tree_builder_test.go(1 hunks)tree_config.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
tree_builder_test.go (2)
object.go (1)
DotVariable(51-51)variable.go (1)
NewVariable(7-11)
object.go (3)
variable.go (1)
Variable(3-5)value.go (1)
Value(7-13)value_undefined.go (1)
NewUndefinedWithReasonf(24-28)
tree_builder.go (2)
object.go (1)
DotVariable(51-51)variable.go (1)
NewVariable(7-11)
tree.go (1)
object.go (2)
DotFunction(12-12)DotVariable(51-51)
🔇 Additional comments (5)
tree_config.go (1)
94-94: LGTM: Comment updated correctly to reflect the type name change.The comment has been updated to use "DotVariable" instead of "Dot[Variable]", which aligns with the broader refactoring in the codebase where the generic type was replaced with a concrete type.
tree_builder_test.go (1)
255-259: LGTM: Test correctly updated to use the concrete DotVariable type.The test now properly instantiates the concrete
DotVariabletype instead of the previous genericDot[Variable]type. This matches the implementation inobject.gowhereDotVariabledirectly embedsVariable.tree_builder.go (1)
126-128: LGTM: Object accessor construction updated correctly.The code now instantiates the concrete
DotVariabletype directly with the variable, rather than using the genericDot[Variable]with a namedMemberfield. This simplification is consistent with howDotVariableis defined inobject.goas embeddingVariable.gal_test.go (1)
402-426: LGTM: All dot accessors consistently updated in the test.The test has been thoroughly updated to replace all instances of the generic
Dot[Variable]type with the concreteDotVariabletype. The change is consistent across all dot accessors in the test and matches the implementation pattern used in the other files.tree.go (1)
144-149: Good integration of the new DotVariable implementationThe switch statement now delegates directly to
DotVariable.Calculate, matching the updated signature and removing the previously duplicated helper. The control-flow is clearer and easier to follow.
Summary by CodeRabbit