Add ToObject coercion for primitives across all Object.* static methods#271
Merged
Add ToObject coercion for primitives across all Object.* static methods#271
Conversation
…ds (fixes #263) Per ES2026 §7.1.18, Object static methods must call ToObject on their first argument, wrapping primitives in their respective wrapper objects and only throwing TypeError for null and undefined. Key changes: - Add a standalone ToObject() helper in GlobalObject.pas that uses ThrowTypeError (not a generic callback) so errors are properly catchable as TypeError - Update Object.keys/values/entries, getOwnPropertyNames, getOwnPropertyDescriptor/s, getOwnPropertySymbols, getPrototypeOf, hasOwn, and assign to use ToObject coercion - Fix Object.setPrototypeOf to use RequireObjectCoercible semantics: throw for null/undefined, but silently return the primitive for other non-objects - Add character-index enumeration to TGocciaStringObjectValue so that boxed strings expose their character indices as own enumerable properties: Object.keys('str') → ['0','1','2'], Object.getOwnPropertyNames('str') → ['0','1','2','length'] - Add GetOwnPropertyDescriptor and HasOwnProperty overrides so string index access via Object.getOwnPropertyDescriptor works correctly - Fix the existing Object.getOwnPropertyDescriptors test that incorrectly expected TypeError for all primitive arguments - Cover all changed methods with null/undefined throw-TypeError tests and primitive coercion tests Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughImplements ES ToObject coercion across many Object.* built-ins, updates prototype/setPrototype handling and proxy routing, adds StringExoticObject own-property semantics, and expands tests to cover primitive coercion, canonical string index behavior, symbol semantics, and null/undefined TypeError cases. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
coderabbitai
Bot
added
the
spec compliance
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/built-ins/Object/getPrototypeOf.js (1)
29-38: Tighten the primitive-prototype assertions.
proto !== undefinedstill passes if boxing regresses tonull, so these tests won't catch a missing wrapper prototype. Assert the concrete wrapper prototype if that's stable here, or at least assert the result is neitherundefinednornull.✅ Minimal strengthening without depending on prototype identity
test("with number coerces to wrapper — returns a value", () => { // Primitives are coerced via ToObject; boxed number has the number prototype const proto = Object.getPrototypeOf(42); - expect(proto !== undefined).toBe(true); + expect(proto === undefined).toBe(false); + expect(proto === null).toBe(false); }); @@ test("with boolean coerces to wrapper — returns a value", () => { const proto = Object.getPrototypeOf(true); - expect(proto !== undefined).toBe(true); + expect(proto === undefined).toBe(false); + expect(proto === null).toBe(false); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/built-ins/Object/getPrototypeOf.js` around lines 29 - 38, The tests named "with number coerces to wrapper — returns a value" and "with boolean coerces to wrapper — returns a value" currently only assert proto !== undefined; tighten these assertions to ensure the boxed wrapper prototype isn't null by asserting the result of Object.getPrototypeOf(42) and Object.getPrototypeOf(true) is neither undefined nor null (e.g., check proto !== undefined && proto !== null or use two expectations) so regressions that return null will fail.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@units/Goccia.Builtins.GlobalObject.pas`:
- Around line 757-772: The current check for invalid proto uses ThrowError which
produces a generic error; replace that call with ThrowTypeError so invalid
prototype types (as in Object.setPrototypeOf({}, 1)) produce a JavaScript
TypeError. Locate the ProtoArg validation in
units/Goccia.Builtins.GlobalObject.pas (the block referencing ProtoArg and the
check "not (ProtoArg is TGocciaObjectValue) and not (ProtoArg is
TGocciaNullLiteralValue)"), and change the ThrowError(...) invocation to
ThrowTypeError(...) with the same message to use the centralized error
constructor.
In `@units/Goccia.Values.StringObjectValue.pas`:
- Around line 381-399: The string-index fast path currently uses TryStrToInt on
AName which accepts non-canonical forms like "01" and "-0"; change the check to
accept only canonical non-negative integer indices (either "0" or a sequence
starting with '1'..'9' followed by only digits) before converting to an integer.
Constrain the branch around the existing TryStrToInt/Index logic (the block
referencing AName, Index, StringValue, and TGocciaPropertyDescriptorData) so it
only triggers for canonical decimal keys; fall back to the ordinary-property
path (e.g., the PROP_LENGTH / ordinary lookup code) for any other AName.
- Around line 323-373: The current String exotic overrides
(GetEnumerablePropertyNames, GetEnumerablePropertyValues,
GetEnumerablePropertyEntries, GetAllPropertyNames) replace the base
FProperties-backed own properties; change each to append the ordinary own
properties from FProperties after the synthesized index keys (and after 'length'
for GetAllPropertyNames) instead of replacing them: build the index-based arrays
as you already do, then iterate FProperties to add its keys/values/entries (for
GetEnumerable* only include enumerable properties) while skipping duplicates
(numeric index strings already emitted and skip adding 'length' again), and for
GetAllPropertyNames include all FProperties keys (non-enumerable too) after the
indices; use the existing FProperties container methods to fetch keys/values and
ensure duplicate suppression so expando props like s.extra are preserved.
---
Nitpick comments:
In `@tests/built-ins/Object/getPrototypeOf.js`:
- Around line 29-38: The tests named "with number coerces to wrapper — returns a
value" and "with boolean coerces to wrapper — returns a value" currently only
assert proto !== undefined; tighten these assertions to ensure the boxed wrapper
prototype isn't null by asserting the result of Object.getPrototypeOf(42) and
Object.getPrototypeOf(true) is neither undefined nor null (e.g., check proto !==
undefined && proto !== null or use two expectations) so regressions that return
null will fail.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 93380e0c-bc11-467f-9cce-0d35047173d2
📒 Files selected for processing (13)
tests/built-ins/Object/assign.jstests/built-ins/Object/entries.jstests/built-ins/Object/getOwnPropertyDescriptor.jstests/built-ins/Object/getOwnPropertyDescriptors.jstests/built-ins/Object/getOwnPropertyNames.jstests/built-ins/Object/getOwnPropertySymbols.jstests/built-ins/Object/getPrototypeOf.jstests/built-ins/Object/hasOwn.jstests/built-ins/Object/keys.jstests/built-ins/Object/setPrototypeOf.jstests/built-ins/Object/values.jsunits/Goccia.Builtins.GlobalObject.pasunits/Goccia.Values.StringObjectValue.pas
Contributor
Benchmark Results364 benchmarks Interpreted: 🟢 228 improved · 🔴 64 regressed · 72 unchanged · avg +2.2% arraybuffer.js — Interp: 🟢 12, 🔴 1, 1 unch. · avg +7.9% · Bytecode: 🟢 3, 🔴 3, 8 unch. · avg -0.2%
arrays.js — Interp: 🟢 18, 1 unch. · avg +6.8% · Bytecode: 🟢 4, 🔴 8, 7 unch. · avg +0.1%
async-await.js — Interp: 🟢 6 · avg +7.5% · Bytecode: 🟢 1, 🔴 2, 3 unch. · avg -0.0%
base64.js — Interp: 🟢 9, 1 unch. · avg +7.7% · Bytecode: 🟢 9, 1 unch. · avg +2.9%
classes.js — Interp: 🟢 24, 🔴 2, 5 unch. · avg +4.5% · Bytecode: 🟢 1, 🔴 3, 27 unch. · avg -0.1%
closures.js — Interp: 🟢 10, 1 unch. · avg +4.7% · Bytecode: 🟢 4, 7 unch. · avg +1.3%
collections.js — Interp: 🟢 10, 🔴 2 · avg +5.1% · Bytecode: 🔴 3, 9 unch. · avg -0.7%
destructuring.js — Interp: 🟢 9, 🔴 2, 11 unch. · avg +1.7% · Bytecode: 🟢 4, 🔴 5, 13 unch. · avg +0.6%
fibonacci.js — Interp: 🟢 7, 1 unch. · avg +4.9% · Bytecode: 🟢 4, 🔴 2, 2 unch. · avg -0.4%
float16array.js — Interp: 🟢 9, 🔴 19, 4 unch. · avg -6.1% · Bytecode: 🟢 9, 🔴 13, 10 unch. · avg +1.5%
for-of.js — Interp: 🟢 2, 5 unch. · avg +1.2% · Bytecode: 🔴 5, 2 unch. · avg -3.0%
helpers/bench-module.js — Interp: 0 · Bytecode: 0
iterators.js — Interp: 🟢 25, 🔴 11, 6 unch. · avg +0.8% · Bytecode: 🟢 7, 🔴 9, 26 unch. · avg +0.2%
json.js — Interp: 🟢 19, 1 unch. · avg +5.2% · Bytecode: 🟢 6, 14 unch. · avg +1.4%
jsx.jsx — Interp: 🟢 8, 🔴 1, 12 unch. · avg +0.7% · Bytecode: 🟢 10, 🔴 2, 9 unch. · avg +1.8%
modules.js — Interp: 🟢 4, 5 unch. · avg +2.6% · Bytecode: 🟢 8, 1 unch. · avg +1.8%
numbers.js — Interp: 🟢 10, 🔴 1 · avg +5.7% · Bytecode: 🟢 4, 🔴 4, 3 unch. · avg +1.7%
objects.js — Interp: 🟢 1, 🔴 5, 1 unch. · avg -2.3% · Bytecode: 🟢 4, 3 unch. · avg +1.8%
promises.js — Interp: 🟢 7, 5 unch. · avg +1.5% · Bytecode: 🟢 7, 🔴 2, 3 unch. · avg +1.9%
regexp.js — Interp: 🟢 10, 🔴 1 · avg +3.6% · Bytecode: 🟢 2, 🔴 5, 4 unch. · avg +0.6%
strings.js — Interp: 🟢 15, 🔴 1, 3 unch. · avg +2.5% · Bytecode: 🟢 3, 🔴 4, 12 unch. · avg +0.6%
typed-arrays.js — Interp: 🟢 5, 🔴 12, 5 unch. · avg -1.4% · Bytecode: 🟢 7, 🔴 3, 12 unch. · avg +4.4%
uint8array-encoding.js — Interp: 🟢 8, 🔴 6, 4 unch. · avg -0.2% · Bytecode: 🟢 3, 🔴 1, 14 unch. · avg +0.6%
Measured on ubuntu-latest x64. Benchmark ranges compare cached main-branch min/max ops/sec with the PR run; overlapping ranges are treated as unchanged noise. Percentage deltas are secondary context. |
Contributor
Suite Timing
Measured on ubuntu-latest x64. |
Three actionable issues fixed:
1. ThrowError → ThrowTypeError for invalid proto type in Object.setPrototypeOf
(Object.setPrototypeOf({}, 1) was throwing a generic Error instead of TypeError)
2. String enumeration overrides now append FProperties-backed own properties after
the synthesized index keys, so expando properties on String objects remain visible
to Object.keys / Object.values / Object.entries / Object.getOwnPropertyNames
3. Canonical string-index check added to GetOwnPropertyDescriptor and HasOwnProperty:
TryStrToInt matched non-canonical forms like "01" and "-0"; the additional
`AName = IntToStr(Index)` guard now ensures round-trip canonicality per
ES2026 §10.4.3
Nitpick addressed: tighten getPrototypeOf.js proto assertions from
`proto !== undefined` to `proto !== null && proto !== undefined`
Additional tests added:
- Object.setPrototypeOf({}, 1) → TypeError
- Object.getOwnPropertyDescriptor / hasOwn with non-canonical indices "01"/"-0"
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Owner
Author
|
All CodeRabbit feedback addressed in 88fc31a: 🟠 Major — 🟠 Major — String enumeration overrides preserve expando properties 🟡 Minor — Canonical string-index guard in 🧹 Nitpick — Tighten |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@units/Goccia.Builtins.GlobalObject.pas`:
- Around line 103-128: The ToObject function is returning a freshly boxed
primitive (via AValue.Box) that lives only on the Pascal stack and can be
collected by GC when callers perform allocations; protect that temporary boxed
wrapper with the engine's temp-root API. After Box returns and Boxed is assigned
in ToObject, call AddTempRoot(Boxed) immediately, ensure any early
ThrowTypeError paths still remove the temp root, and call RemoveTempRoot(Boxed)
just before returning the TGocciaObjectValue (use a try/finally around the
caller-facing work or ensure every Exit removes the temp root). Keep
ThrowTypeError behavior unchanged and only add AddTempRoot/RemoveTempRoot around
the AValue.Box / TGocciaObjectValue(Boxed) handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b6178612-93ca-4df0-b39d-5a1c1d45cc6d
📒 Files selected for processing (6)
tests/built-ins/Object/getOwnPropertyDescriptor.jstests/built-ins/Object/getPrototypeOf.jstests/built-ins/Object/hasOwn.jstests/built-ins/Object/setPrototypeOf.jsunits/Goccia.Builtins.GlobalObject.pasunits/Goccia.Values.StringObjectValue.pas
✅ Files skipped from review due to trivial changes (3)
- tests/built-ins/Object/getPrototypeOf.js
- tests/built-ins/Object/setPrototypeOf.js
- tests/built-ins/Object/hasOwn.js
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/built-ins/Object/getOwnPropertyDescriptor.js
- units/Goccia.Values.StringObjectValue.pas
When ToObject boxes a primitive (boolean, number, string) the resulting wrapper object lives only in a Pascal local and has no JavaScript scope root. If a subsequent allocation in the same method body triggers CollectIfNeeded, the wrapper could be swept before use. Add try/finally guards around the work in each of the ten Object.* methods that call ToObject — ObjectKeys, ObjectValues, ObjectEntries, ObjectAssign, ObjectHasOwn, ObjectGetOwnPropertyNames, ObjectGetOwnPropertyDescriptor, ObjectGetOwnPropertyDescriptors, ObjectGetOwnPropertySymbols, and ObjectGetPrototypeOf — following the AddTempRoot/RemoveTempRoot pattern already established in the codebase (Goccia.Values.MockFunction, Goccia.Builtins.GlobalArray, etc.). The guard is a no-op when the input was already a JS-scope object; it only matters for freshly boxed primitives. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
units/Goccia.Builtins.GlobalObject.pas (1)
349-352:⚠️ Potential issue | 🟠 MajorPreserve symbol property keys in
Object.hasOwn.Both code paths convert
PwithToStringLiteralinstead of checking for symbol keys. Lines 361–367 (ClassObj path) and lines 373–377 (normal Obj path) will incorrectly stringify symbol arguments, soObject.hasOwn(obj, sym)checks"Symbol(...)"instead of the actual symbol slot. The symbol-aware split already exists inObject.getOwnPropertyDescriptor(lines 432–437); apply the sameToPropertyKeyhandling to both paths inObject.hasOwn.🛠️ Proposed fix
var Obj: TGocciaObjectValue; PropertyName: string; ClassObj: TGocciaClassValue; + SymbolKey: TGocciaSymbolValue; begin TGocciaArgumentValidator.RequireExactly(AArgs, 2, 'Object.hasOwn', ThrowError); // Step 1: Let obj be ? ToObject(O) if AArgs.GetElement(0) is TGocciaClassValue then begin ClassObj := TGocciaClassValue(AArgs.GetElement(0)); // Step 2: Let key be ? ToPropertyKey(P) - PropertyName := AArgs.GetElement(1).ToStringLiteral.Value; - // Step 3: Return ? HasOwnProperty(obj, key) - if not (ClassObj.GetProperty(PropertyName) is TGocciaUndefinedLiteralValue) then - Result := TGocciaBooleanLiteralValue.TrueValue + if AArgs.GetElement(1) is TGocciaSymbolValue then + begin + SymbolKey := TGocciaSymbolValue(AArgs.GetElement(1)); + if ClassObj.GetOwnSymbolPropertyDescriptor(SymbolKey) <> nil then + Result := TGocciaBooleanLiteralValue.TrueValue + else + Result := TGocciaBooleanLiteralValue.FalseValue; + end else - Result := TGocciaBooleanLiteralValue.FalseValue; + begin + PropertyName := AArgs.GetElement(1).ToStringLiteral.Value; + if not (ClassObj.GetProperty(PropertyName) is TGocciaUndefinedLiteralValue) then + Result := TGocciaBooleanLiteralValue.TrueValue + else + Result := TGocciaBooleanLiteralValue.FalseValue; + end; Exit; end; Obj := ToObject(AArgs.GetElement(0)); @@ -373,11 +390,23 @@ try // Step 2: Let key be ? ToPropertyKey(P) - PropertyName := AArgs.GetElement(1).ToStringLiteral.Value; - - // Step 3: Return ? HasOwnProperty(obj, key) - if Obj.HasOwnProperty(PropertyName) then - Result := TGocciaBooleanLiteralValue.TrueValue + if AArgs.GetElement(1) is TGocciaSymbolValue then + begin + SymbolKey := TGocciaSymbolValue(AArgs.GetElement(1)); + if Obj.GetOwnSymbolPropertyDescriptor(SymbolKey) <> nil then + Result := TGocciaBooleanLiteralValue.TrueValue + else + Result := TGocciaBooleanLiteralValue.FalseValue; + end else - Result := TGocciaBooleanLiteralValue.FalseValue; + begin + PropertyName := AArgs.GetElement(1).ToStringLiteral.Value; + // Step 3: Return ? HasOwnProperty(obj, key) + if Obj.HasOwnProperty(PropertyName) then + Result := TGocciaBooleanLiteralValue.TrueValue + else + Result := TGocciaBooleanLiteralValue.FalseValue; + end; finally🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@units/Goccia.Builtins.GlobalObject.pas` around lines 349 - 352, The Object.hasOwn implementation currently converts the property argument P using ToStringLiteral which loses symbol keys; update both code paths (the ClassObj branch handling ClassObj: TGocciaClassValue and the normal Obj: TGocciaObjectValue branch) to use ToPropertyKey instead of ToStringLiteral so symbol property keys are preserved (mirror the symbol-aware logic used in Object.getOwnPropertyDescriptor). Replace the ToStringLiteral(P) usage where P is converted to a string with a call that obtains the property key via ToPropertyKey and then use that key for the subsequent property lookup/slot checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@units/Goccia.Builtins.GlobalObject.pas`:
- Around line 349-352: The Object.hasOwn implementation currently converts the
property argument P using ToStringLiteral which loses symbol keys; update both
code paths (the ClassObj branch handling ClassObj: TGocciaClassValue and the
normal Obj: TGocciaObjectValue branch) to use ToPropertyKey instead of
ToStringLiteral so symbol property keys are preserved (mirror the symbol-aware
logic used in Object.getOwnPropertyDescriptor). Replace the ToStringLiteral(P)
usage where P is converted to a string with a call that obtains the property key
via ToPropertyKey and then use that key for the subsequent property lookup/slot
checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 095b2975-afb4-499f-b4a9-1eb03faafb96
📒 Files selected for processing (1)
units/Goccia.Builtins.GlobalObject.pas
The benchmark runner already protects setup-returned objects with GC.AddTempRoot(SetupResult). When Object.keys(obj) was called in the benchmark run callback, the unconditional RemoveTempRoot(obj) inside the method stripped that protection mid-benchmark. On the next CollectIfNeeded call (line 246 of Goccia.Builtins.Benchmark.pas), the setup object was swept, causing an access violation on the subsequent iteration. Fix: the temp-root guards must fire only when ToObject actually boxed a primitive (i.e. the caller passed a non-object). Already-rooted arguments (TGocciaObjectValue in scope) need no extra protection and must not have their existing roots disturbed. All ten methods now gate AddTempRoot/RemoveTempRoot behind: not (AArgs.GetElement(0) is TGocciaObjectValue) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Both branches of ObjectHasOwn were calling ToStringLiteral on the property key argument, which coerces symbol keys to their string representation instead of checking the actual symbol property slot. Fix: mirror the symbol-aware branching used in ObjectGetOwnPropertyDescriptor. - For the TGocciaObjectValue path: check HasSymbolProperty for symbol keys (own-property-only check via FSymbolDescriptors) and HasOwnProperty for string keys — unchanged. - For the TGocciaClassValue path: check GetSymbolProperty for symbol keys and GetProperty for string keys — consistent with the existing chain-walk behaviour of that branch. Tests added for symbol key detection (own vs inherited vs absent). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
frostney
added a commit
that referenced
this pull request
Apr 11, 2026
Incorporates 5 commits from main: - TextEncoder and TextDecoder built-ins (#272) - Make import.meta tests account for Windows drive letters (#274) - Fixes tagged template object identity (#275) - Add Goccia.build platform metadata (#276) - Add ToObject coercion for primitives across all Object.* static methods (#271) Conflict resolution in 3 files (all "both sides added" — keep both): Goccia.Engine.pas / Goccia.Runtime.Bootstrap.pas: - Added ggTextEncoder and ggTextDecoder to TGocciaGlobalBuiltin enum - Added both to DefaultGlobals alongside ggURL - Added FBuiltinTextEncoder/FBuiltinTextDecoder fields, Free calls, registration blocks, Expose* helpers, and constructor TypeDef blocks alongside the existing URL equivalents Goccia.Values.ClassValue.pas: - Added TGocciaTextEncoderClassValue and TGocciaTextDecoderClassValue declarations, impl uses, and CreateNativeInstance implementations alongside the existing URL equivalents Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #263.
Per ES2026 §7.1.18
ToObject, allObject.*static methods must coerce their first argument to an object — wrapping primitives in their respective wrapper objects and only throwingTypeErrorfornullandundefined. Previously, GocciaScript threw for all non-objects, including numbers, strings, and booleans.Changes
Goccia.Builtins.GlobalObject.pasToObject()helper (ES2026 §7.1.18) that usesThrowTypeErrordirectly (ensuring errors are catchable asTypeErrorfrom JavaScripttry...catch)ToObjectinstead of throwing directly:Object.keys,Object.values,Object.entriesObject.getOwnPropertyNames,Object.getOwnPropertyDescriptor,Object.getOwnPropertyDescriptorsObject.getOwnPropertySymbols(previously returned[]for all non-objects includingnull/undefined)Object.getPrototypeOf,Object.hasOwn,Object.assign(target only)Object.setPrototypeOfto useRequireObjectCoerciblesemantics: throwTypeErrorfornull/undefined, silently return the primitive unchanged for other non-objects (e.g.Object.setPrototypeOf(42, null)→42)freeze,isFrozen,seal,isSealed,preventExtensions,isExtensibledefineProperty,definePropertiesGoccia.Values.StringObjectValue.pasGetEnumerablePropertyNames/GetEnumerablePropertyValues/GetEnumerablePropertyEntriesto expose character indices as own enumerable propertiesGetAllPropertyNamesto include character indices andlengthGetOwnPropertyDescriptorto return correct descriptors for indices (enumerable, non-writable, non-configurable) andlength(non-enumerable)HasOwnPropertyto correctly report ownership of indices andlengthThis makes
Object.keys('str')→['0','1','2'],Object.getOwnPropertyNames('str')→['0','1','2','length'], andObject.getOwnPropertyDescriptor('str', '0')→{ value: 's', enumerable: true, writable: false, configurable: false }.Expected behavior after fix
Testing checklist
tests/built-ins/Object/tests pass (100%)keys,values,entries,getOwnPropertyNames,getOwnPropertyDescriptor,getOwnPropertyDescriptors,getOwnPropertySymbols,getPrototypeOf,hasOwn,assign,setPrototypeOfgetOwnPropertyDescriptors.jsthat expectedTypeErrorfor primitive arguments🤖 Generated with Claude Code