fix(runtime): resolve static-getter method calls on call-expression heritage (#10210) - #10213
proggeramlug wants to merge 1 commit into
Conversation
…eritage (PerryTS#10210) A static getter on a class whose `extends` clause is a call expression (effect v4 `class Svc extends Context.Service<Svc, Shape>()(id) {}`, i.e. a plain `function KeyClass(){}` whose [[Prototype]] was swapped with `Object.setPrototypeOf`) threw `of is not a function` on `this.of(x)` while the plain read `this.of` returned the function. Two gaps, both on the miss path only: - The dynamic method-call dispatcher's class-object arm resolved nothing but the static-method vtable. It now mirrors the class-ref arm (PerryTS#5437): when the vtable misses, read the property exactly as the read path does (static accessors, the per-evaluation parent class object's statics, the function-valued ancestor's swapped prototype) and call the closure with `this` bound to the receiver. - The class-ref read paths (string and symbol keys) looked for the function-valued parent edge only on the receiver's own class id. They now walk the class chain like `super()` dispatch already did, so a subclass of the class that `extends <function>` inherits those statics too. Unblocks OpenCode v1.18.30's runtime bootstrap (`ConfigService.Service` → `static get layer()` → `tag.of(config)` while AppRuntime builds RuntimeFlags). Claude-Session: https://claude.ai/code/session_01As1fetJAqDFib4n7Wm5Suo
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe runtime now resolves static properties through ancestor class chains and invokes closure-valued static properties with the class receiver. A regression test covers getters, methods, inherited lookups, symbols, and call-expression-derived classes. ChangesStatic getter call-path resolution
Priority: ⬆️ High Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: High Sequence Diagram(s)sequenceDiagram
participant StaticClassReceiver
participant dispatch_primitive
participant js_object_get_field_by_name
participant Closure
StaticClassReceiver->>dispatch_primitive: call static property
dispatch_primitive->>js_object_get_field_by_name: resolve property
js_object_get_field_by_name-->>dispatch_primitive: return closure
dispatch_primitive->>Closure: rebind this and invoke
Closure-->>StaticClassReceiver: return result
Merge Risk: ⚪ Minimal · up to The change includes the required static lookup and receiver-bound invocation behavior with regression coverage for the affected class-chain scenarios. No merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
Fixes #10210 (OpenCode v1.18.30 runtime bootstrap wall, tracker #10107).
What was wrong
Inside a static getter of a class whose
extendsclause is a call expression — effect v4'sContext.Service<..>()(id)returnsfunction KeyClass(){}withObject.setPrototypeOf(KeyClass, ServiceProto)—this.of(x)threwof is not a function, while the plain readthis.ofreturned the function. OpenCode hits it insrc/effect/config-service.ts(static get layer()→tag.of(config)) whileAppRuntimebuildsRuntimeFlags, so every command that needs the runtime (models,run,serve, the TUI) died before the first log line.Two gaps, both on the miss path only (no change for calls that resolved before):
js_native_call_method, class-object receiver (native_call_method/primitive_methods.rs): the arm resolved only the static-method vtable. It now mirrors the class-ref arm (Next.js standalone: deferred-require binding captured by-value (as unresolved thunk) by a class constructor → 'undefined is not a constructor' #5437): on a vtable miss it reads the property exactly as the read path does (static accessors, the per-evaluation parent class object's statics, the function-valued ancestor's swapped prototype) and calls the closure withthisbound to the receiver. A non-callable result keeps falling through to the generic scan and the usual not-a-function error.field_get_set/get_field_by_name.rs,symbol/get.rs): the function-valued parent edge was looked up only on the receiver's own class id (class_parent_closure). They now useparent_closure_in_chain, whichsuper()dispatch already used, so a subclass of the class thatextends <function>inherits those statics too.Verification (perrymaster, Linux x86_64, release build)
The probe matrix from the issue, node vs the patched compiler — identical on every line that is a call or a read:
The pure-TS mirror of OpenCode's
ConfigService.Service+RuntimeFlags(static get layer()→Layer.effect(tag, () => tag.of(...)),static configLayer(input) { return Layer.succeed(this, this.of(input)) }) now prints the node output on all six lines.New test
crates/perry/tests/issue_10210_static_getter_call.rs(10 lines of expected node output, including the inherited read on a grandchild and calling the function a static getter returns on a dynamic receiver) passes; the sibling class/static regression tests (class_static_symbol_inheritance,class_expr_dynamic_parent_field_order,issue_4908_subclass_native_member_base,capture_rereg_renamed_class,instanceof_classexpr_rhs_captured,aliased_native_class_import) still pass.Not covered (documented on #10210)
thisinside the static getter is still the holder object, not the receiver, and a nested capturing class is aClassExprFreshobject while the class name inside its own body lowers to an INT32ClassRef(perry-hir/src/lower_decl/body_stmt.rs:281vs:362) — the identity checks in the issue (this === Sub,Object.getPrototypeOf(Sub) === T) stay false. Dispatch is correct; only identity diverges.KnownTopLevelClass.staticGetter()(calling the function a static getter returns) on a class that extends a call expression still throws: the compile-time static tower routes it tojs_class_static_method_call, whose miss path walksCLASS_DYNAMIC_PROPSand the parent chain but notCLASS_STATIC_ACCESSORS. Same shape as gap 1, one helper over; OpenCode does not use it.https://claude.ai/code/session_01As1fetJAqDFib4n7Wm5Suo
Summary by CodeRabbit
Bug Fixes
Tests