Skip to content

Enum support#33

Merged
frostney merged 2 commits into
mainfrom
feat-enum-support
Feb 22, 2026
Merged

Enum support#33
frostney merged 2 commits into
mainfrom
feat-enum-support

Conversation

@frostney
Copy link
Copy Markdown
Owner

@frostney frostney commented Feb 22, 2026

Summary by CodeRabbit

  • New Features

    • Added TC39-style enum declarations with numeric, string, and symbol members and iteration/toStringTag support.
    • Improved String methods to handle Unicode surrogate pairs (isWellFormed, toWellFormed).
    • Exposed a DOMException constructor with DataCloneError semantics.
  • Bug Fixes

    • structuredClone now reads accessor getters and throws DOMException(DataCloneError) for non-cloneable values.
  • Documentation

    • Expanded ECMAScript feature tables, roadmap, structuredClone and DOMException docs, and test-runner options.
  • Tests

    • Extensive new and updated tests covering enums, string well-formedness, structuredClone, and DOMException.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 22, 2026

📝 Walkthrough

Walkthrough

This PR adds TC39-style enum support across the language stack (lexer, token, parser, AST, evaluator, interpreter, runtime EnumValue), introduces DOMException/DataCloneError plumbing for structuredClone errors, augments String well-formedness handling, and adds comprehensive docs and tests for enums and related built-ins.

Changes

Cohort / File(s) Summary
Language & Parser
units/Goccia.Token.pas, units/Goccia.Keywords.Reserved.pas, units/Goccia.Parser.pas
Introduce gttEnum token and KEYWORD_ENUM; add EnumDeclaration parser rule and integrate enum handling into statement/export flows.
AST
units/Goccia.AST.Statements.pas
Add TGocciaEnumMember, TGocciaEnumDeclaration, TGocciaExportEnumDeclaration types and constructors.
Evaluator / Interpreter
units/Goccia.Evaluator.pas, units/Goccia.Interpreter.pas
Add EvaluateEnumDeclaration and hook enum evaluation into statement dispatch; support exporting enum values from modules.
Runtime: Enum Value & Symbols
units/Goccia.Values.EnumValue.pas
New TGocciaEnumValue type with Entries/Name, iterator implementation and Symbol.toStringTag wiring via InitializeEnumSymbols.
Lexer / Unicode
units/Goccia.Lexer.pas
Extend ScanUnicodeEscape to combine surrogate pairs per spec; add enum keyword init.
Builtins & Error Handling
units/Goccia.Builtins.Globals.pas, units/Goccia.Values.ErrorHelper.pas, units/Goccia.Constants.ErrorNames.pas, units/Goccia.Constants.PropertyNames.pas
Add DOMException prototype and constructor wiring; add DATA_CLONE_ERROR_NAME/DOM_EXCEPTION_NAME and PROP_CODE constant; implement ThrowDataCloneError and use DOMException/DataCloneError for structuredClone error paths.
String Methods
units/Goccia.Values.StringObjectValue.pas
Add null/undefined receiver checks and surrogate-pair aware logic to isWellFormed and toWellFormed.
Tests — Enums
tests/language/enums/*, tests/language/types-as-comments/enum-annotations.js, tests/language/enums/helpers/direction-enum.js
Add comprehensive enum tests: basic behavior, errors for invalid value types, exports/imports, immutability, iteration, self-references, string-named members, Symbol.toStringTag, and type-annotation scenarios.
Tests — Builtins & structuredClone
tests/built-ins/structuredClone/errors.js, tests/built-ins/DOMException/constructor.js, tests/built-ins/String/prototype/isWellFormed.js, tests/built-ins/String/prototype/toWellFormed.js
Refine structuredClone tests to assert DOMException/DataCloneError semantics; add DOMException constructor tests; extend String well-formedness tests for surrogates and coercion.
Docs & Config
AGENTS.md, README.md, docs/*.md, vitest.config.js
Document enums, update structuredClone/DOMException docs, add test-runner options and build-step stats doc, and exclude enum tests in test config; README feature tables and roadmap updates.
Test Config Exclusions
vitest.config.js
Convert exclude array to multiline and add tests/**/enums/**/*.js exclusion.

Sequence Diagram

sequenceDiagram
    participant Parser
    participant Evaluator
    participant Scope
    participant EnumValue
    participant Symbols

    Parser->>Parser: Parse 'enum' declaration (name, members, optional initializers)
    Parser->>Evaluator: Emit TGocciaEnumDeclaration node

    Evaluator->>EnumValue: Create enum instance (name)
    Evaluator->>Scope: Create child scope for member evaluation
    Evaluator->>Evaluator: Evaluate each member initializer in child scope
    Evaluator->>EnumValue: Populate Entries array with [key,value] pairs
    Evaluator->>Scope: Bind lexical names to enum members / enum object

    EnumValue->>Symbols: Initialize Symbol.iterator (entries iterator)
    EnumValue->>Symbols: Initialize Symbol.toStringTag (enum name)

    Evaluator->>Scope: Bind enum into current scope (name -> EnumValue)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 I dug a hole in parser land,
I planted keys and ASTs by hand,
Enums sprouted, entries neat and round,
Symbols hop and iteration's sound,
A carrot-shaped DOMException bound! 🎉

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Enum support' is fully related to the main change—extensive enum language feature implementation including AST nodes, parser, evaluator, value types, and comprehensive test coverage.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-enum-support

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 22, 2026

Benchmark Results

179 benchmarks · 🟢 30 improved · 149 unchanged · avg +2.6%

arrays.js — 19 unchanged · avg +0.2%
Benchmark Base (ops/sec) PR (ops/sec) Change
Array.from length 100 13,990 13,932 -0.4%
Array.from 10 elements 162,382 161,493 -0.5%
Array.of 10 elements 198,614 201,294 +1.3%
spread into new array 239,186 236,569 -1.1%
map over 50 elements 11,498 11,441 -0.5%
filter over 50 elements 9,855 9,852 -0.0%
reduce sum 50 elements 12,326 12,356 +0.2%
forEach over 50 elements 9,892 9,788 -1.0%
find in 50 elements 13,106 13,166 +0.5%
sort 20 elements 10,292 10,482 +1.8%
flat nested array 89,926 89,990 +0.1%
flatMap 59,617 58,845 -1.3%
map inside map (5x5) 16,719 16,746 +0.2%
filter inside map (5x10) 5,361 5,338 -0.4%
reduce inside map (5x10) 6,130 6,266 +2.2%
forEach inside forEach (5x10) 5,615 5,703 +1.6%
find inside some (10x10) 3,488 3,483 -0.1%
map+filter chain nested (5x20) 2,346 2,351 +0.2%
reduce flatten (10x5) 6,269 6,307 +0.6%
classes.js — 15 unchanged · avg +0.3%
Benchmark Base (ops/sec) PR (ops/sec) Change
simple class new 103,252 103,097 -0.2%
class with defaults 81,645 80,534 -1.4%
50 instances via Array.from 4,911 4,926 +0.3%
instance method call 46,273 48,507 +4.8%
static method call 82,870 83,227 +0.4%
single-level inheritance 40,889 40,798 -0.2%
two-level inheritance 36,140 35,886 -0.7%
private field access 47,408 47,324 -0.2%
private methods 54,865 54,812 -0.1%
getter/setter access 55,384 54,856 -1.0%
static getter read 99,712 100,066 +0.4%
static getter/setter pair 69,468 69,927 +0.7%
inherited static getter 55,461 55,859 +0.7%
inherited static setter 59,345 59,085 -0.4%
inherited static getter with this binding 47,085 47,945 +1.8%
closures.js — 11 unchanged · avg +1.3%
Benchmark Base (ops/sec) PR (ops/sec) Change
closure over single variable 86,461 87,402 +1.1%
closure over multiple variables 99,036 100,546 +1.5%
nested closures 109,116 111,040 +1.8%
function as argument 78,063 78,628 +0.7%
function returning function 98,242 100,312 +2.1%
compose two functions 60,014 60,613 +1.0%
fn.call 140,412 140,903 +0.3%
fn.apply 94,542 94,633 +0.1%
fn.bind 116,777 119,602 +2.4%
recursive sum to 50 8,198 8,291 +1.1%
recursive tree traversal 14,230 14,602 +2.6%
collections.js — 12 unchanged · avg -1.0%
Benchmark Base (ops/sec) PR (ops/sec) Change
add 50 elements 5,995 5,921 -1.2%
has lookup (50 elements) 9,493 9,236 -2.7%
delete elements 26,721 26,276 -1.7%
forEach iteration 5,952 5,891 -1.0%
spread to array 13,268 13,138 -1.0%
deduplicate array 41,722 41,231 -1.2%
set 50 entries 4,552 4,503 -1.1%
get lookup (50 entries) 4,690 4,673 -0.4%
has check 4,783 4,768 -0.3%
delete entries 12,877 12,748 -1.0%
forEach iteration 3,618 3,616 -0.0%
keys/values/entries 4,679 4,680 +0.0%
destructuring.js — 14 unchanged · avg -0.8%
Benchmark Base (ops/sec) PR (ops/sec) Change
simple array destructuring 299,238 297,950 -0.4%
with rest element 189,518 189,550 +0.0%
with defaults 299,790 298,933 -0.3%
skip elements 305,251 299,618 -1.8%
nested array destructuring 150,355 148,863 -1.0%
swap variables 359,090 357,475 -0.4%
simple object destructuring 248,047 249,722 +0.7%
with defaults 296,384 287,697 -2.9%
with renaming 295,293 293,622 -0.6%
nested object destructuring 147,804 147,794 -0.0%
rest properties 164,914 164,440 -0.3%
object parameter 87,495 85,915 -1.8%
array parameter 102,041 100,600 -1.4%
mixed destructuring in map 12,215 12,137 -0.6%
fibonacci.js — 6 unchanged · avg +0.5%
Benchmark Base (ops/sec) PR (ops/sec) Change
recursive fib(15) 222 229 +2.8%
recursive fib(20) 20 21 +3.5%
iterative fib(20) via reduce 9,974 9,970 -0.0%
iterator fib(20) 7,379 7,374 -0.1%
iterator fib(20) via Iterator.from + take 8,395 8,363 -0.4%
iterator fib(20) last value via reduce 7,020 6,810 -3.0%
iterators.js — 20 unchanged · avg +0.9%
Benchmark Base (ops/sec) PR (ops/sec) Change
Iterator.from({next}).toArray() — 20 elements 9,433 9,441 +0.1%
Iterator.from({next}).toArray() — 50 elements 3,998 4,003 +0.1%
spread pre-wrapped iterator — 20 elements 9,401 9,432 +0.3%
Iterator.from({next}).forEach — 50 elements 3,066 3,091 +0.8%
Iterator.from({next}).reduce — 50 elements 3,119 3,149 +1.0%
wrap array iterator 62,147 62,573 +0.7%
wrap plain {next()} object 6,450 6,564 +1.8%
map + toArray (50 elements) 2,756 2,794 +1.4%
filter + toArray (50 elements) 2,798 2,841 +1.5%
take(10) + toArray (50 element source) 15,418 15,553 +0.9%
drop(40) + toArray (50 element source) 3,870 3,906 +0.9%
chained map + filter + take (100 element source) 4,916 4,910 -0.1%
some + every (50 elements) 1,740 1,774 +1.9%
find (50 elements) 3,770 3,877 +2.8%
array.values().map().filter().toArray() 3,408 3,513 +3.1%
array.values().take(5).toArray() 18,598 18,719 +0.7%
array.values().drop(45).toArray() 11,430 11,627 +1.7%
map.entries() chained helpers 3,714 3,643 -1.9%
set.values() chained helpers 6,052 6,140 +1.5%
string iterator map + toArray 8,599 8,514 -1.0%
json.js — 20 unchanged · avg +1.0%
Benchmark Base (ops/sec) PR (ops/sec) Change
parse simple object 169,831 172,206 +1.4%
parse nested object 111,124 113,397 +2.0%
parse array of objects 58,684 59,183 +0.8%
parse large flat object 49,873 49,836 -0.1%
parse mixed types 73,418 73,830 +0.6%
stringify simple object 147,023 148,684 +1.1%
stringify nested object 81,116 81,858 +0.9%
stringify array of objects 16,899 17,221 +1.9%
stringify mixed types 67,600 66,839 -1.1%
reviver doubles numbers 34,117 35,150 +3.0%
reviver filters properties 31,714 33,164 +4.6%
reviver on nested object 41,300 43,385 +5.0%
reviver on array 21,495 22,453 +4.5%
replacer function doubles numbers 34,571 35,607 +3.0%
replacer function excludes properties 46,924 46,963 +0.1%
array replacer (allowlist) 93,835 93,630 -0.2%
stringify with 2-space indent 74,256 74,652 +0.5%
stringify with tab indent 75,879 74,624 -1.7%
parse then stringify 45,612 44,073 -3.4%
stringify then parse 17,426 17,001 -2.4%
jsx.jsx — 🟢 15 improved, 6 unchanged · avg +8.4%
Benchmark Base (ops/sec) PR (ops/sec) Change
simple element 191,694 198,255 +3.4%
self-closing element 188,653 206,273 🟢 +9.3%
element with string attribute 157,701 172,317 🟢 +9.3%
element with multiple attributes 131,298 143,930 🟢 +9.6%
element with expression attribute 138,790 150,765 🟢 +8.6%
text child 180,378 197,755 🟢 +9.6%
expression child 175,415 188,994 🟢 +7.7%
mixed text and expression 161,169 179,329 🟢 +11.3%
nested elements (3 levels) 66,253 76,051 🟢 +14.8%
sibling children 51,399 54,489 +6.0%
component element 126,023 138,559 🟢 +9.9%
component with children 77,132 85,096 🟢 +10.3%
dotted component 105,778 115,542 🟢 +9.2%
empty fragment 178,932 197,711 🟢 +10.5%
fragment with children 48,879 53,603 🟢 +9.7%
spread attributes 98,358 102,163 +3.9%
spread with overrides 83,958 87,221 +3.9%
shorthand props 137,822 145,444 +5.5%
nav bar structure 23,501 25,422 🟢 +8.2%
card component tree 28,030 29,559 +5.5%
10 list items via Array.from 12,729 13,911 🟢 +9.3%
numbers.js — 🟢 1 improved, 10 unchanged · avg +4.0%
Benchmark Base (ops/sec) PR (ops/sec) Change
integer arithmetic 320,244 334,432 +4.4%
floating point arithmetic 339,409 354,212 +4.4%
number coercion 150,303 156,181 +3.9%
toFixed 83,220 89,547 🟢 +7.6%
toString 126,743 132,570 +4.6%
valueOf 181,380 186,909 +3.0%
toPrecision 119,685 123,901 +3.5%
Number.isNaN 238,417 241,321 +1.2%
Number.isFinite 234,067 241,315 +3.1%
Number.isInteger 209,127 222,157 +6.2%
Number.parseInt and parseFloat 192,164 195,795 +1.9%
objects.js — 🟢 1 improved, 6 unchanged · avg +4.3%
Benchmark Base (ops/sec) PR (ops/sec) Change
create simple object 360,884 373,033 +3.4%
create nested object 184,012 200,290 🟢 +8.8%
create 50 objects via Array.from 9,207 9,506 +3.2%
property read 170,214 173,947 +2.2%
Object.keys 120,472 125,430 +4.1%
Object.entries 70,289 73,100 +4.0%
spread operator 154,823 161,523 +4.3%
promises.js — 🟢 10 improved, 2 unchanged · avg +9.1%
Benchmark Base (ops/sec) PR (ops/sec) Change
Promise.resolve(value) 407,534 434,483 +6.6%
new Promise(resolve => resolve(value)) 154,950 168,063 🟢 +8.5%
Promise.reject(reason) 428,717 462,134 🟢 +7.8%
resolve + then (1 handler) 134,663 141,968 +5.4%
resolve + then chain (3 deep) 54,110 58,567 🟢 +8.2%
resolve + then chain (10 deep) 17,432 18,989 🟢 +8.9%
reject + catch + then 79,167 85,907 🟢 +8.5%
resolve + finally + then 65,691 72,649 🟢 +10.6%
Promise.all (5 resolved) 26,816 28,795 🟢 +7.4%
Promise.race (5 resolved) 26,657 30,167 🟢 +13.2%
Promise.allSettled (5 mixed) 22,544 24,978 🟢 +10.8%
Promise.any (5 mixed) 25,552 28,963 🟢 +13.3%
strings.js — 🟢 3 improved, 8 unchanged · avg +5.4%
Benchmark Base (ops/sec) PR (ops/sec) Change
string concatenation 354,848 382,384 🟢 +7.8%
template literal 384,673 381,399 -0.9%
string repeat 349,184 365,965 +4.8%
split and join 126,697 130,148 +2.7%
indexOf and includes 139,176 146,142 +5.0%
toUpperCase and toLowerCase 206,917 232,053 🟢 +12.1%
slice and substring 118,305 123,114 +4.1%
trim operations 153,947 169,400 🟢 +10.0%
replace and replaceAll 174,158 184,579 +6.0%
startsWith and endsWith 116,105 122,208 +5.3%
padStart and padEnd 161,108 164,261 +2.0%

Measured on ubuntu-latest x64. Changes within ±7% are considered insignificant.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (2)
tests/built-ins/structuredClone/errors.js (1)

6-43: Consider aligning remaining error tests with the newer assertion style.

The refactored test at lines 28-32 uses the cleaner .toThrow(TypeError) pattern, but the other error tests (lines 6-15, 17-26, 34-43) still use the verbose try/catch approach. For consistency and readability, consider refactoring them as well.

♻️ Example refactor for "throws on function" test
 test("throws on function", () => {
-  let threw = false;
-  try {
-    structuredClone(() => {});
-  } catch (e) {
-    threw = true;
-    expect(e.message.includes("could not be cloned")).toBe(true);
-  }
-  expect(threw).toBe(true);
+  expect(() => {
+    structuredClone(() => {});
+  }).toThrow("could not be cloned");
 });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/built-ins/structuredClone/errors.js` around lines 6 - 43, Convert the
verbose try/catch assertions in the "throws on function", "throws on symbol",
and "throws on object containing a function" tests to the concise Jest form used
in the other test: replace the manual try/catch + flag pattern with expect(() =>
structuredClone(...)).toThrow(TypeError) or, if you want to assert the message,
use expect(() => structuredClone(...)).toThrow(/could not be cloned/); update
the tests named "throws on function", "throws on symbol", and "throws on object
containing a function" to use this pattern for consistency.
README.md (1)

150-150: Consider keeping the test-count wording resilient to drift.
Exact counts tend to go stale; a looser phrasing (e.g., “2k+”) is easier to maintain.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 150, The exact test count in the README sentence
"GocciaScript has 2300+ JavaScript unit tests covering language features,
built-in objects, and edge cases." is likely to become stale; update that line
to a looser, drift-resistant phrasing such as "2k+ JavaScript unit tests" or
"2,300+ JavaScript unit tests" (whichever team style prefers) so the README
remains accurate without frequent updates.
🤖 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.Globals.pas`:
- Around line 339-343: Add a brief inline comment above the block where
StructuredCloneValue(AObj.GetProperty(Keys[I]), AMemory) is converted into a
data property and where Descriptor.Flags is changed (the call to
Result.DefineProperty with TGocciaPropertyDescriptorData and the flag expression
Descriptor.Flags - [pfConfigurable, pfWritable] + [pfEnumerable]); explain that
accessor properties on enum members are intentionally converted to
non-configurable, non-writable data properties to enforce enum immutability and
reference the relevant structuredClone/enum spec section or HTML spec for
justification. Also add a unit test that structuredClones an enum-like object
and asserts the cloned property descriptor flags match the expected
non-configurable, non-writable, enumerable state.

In `@units/Goccia.Evaluator.pas`:
- Around line 1442-1444: Add the ECMAScript spec annotation comment immediately
above the function declaration for EvaluateEnumDeclaration: insert a single-line
comment in the format // ES2024 §X.Y.Z EvaluateEnumDeclaration(const
AEnumDeclaration: TGocciaEnumDeclaration; const AContext:
TGocciaEvaluationContext) (replace §X.Y.Z with the actual spec section number if
known); place it directly above the existing "function
EvaluateEnumDeclaration(...)" line so the evaluator entry point is annotated per
project convention.
- Around line 1442-1489: EvaluateEnumDeclaration builds EnumValue and
EnumEntries in Pascal-only variables without temp-rooting and never frees
ChildScope; protect temporary GC-visible values by calling AddTempRoot on
EnumValue and EnumEntries (and any EntryPair/MemberValue as needed) before
evaluating members and RemoveTempRoot after assigning EnumValue. Also ensure
ChildScope is freed after use (after AContext.Scope.DefineLexicalBinding and
before returning) so the enum init scope is released; reference the
EvaluateEnumDeclaration function, EnumValue, EnumEntries, ChildScope,
ChildContext, InitializeEnumSymbols and PreventExtensions when applying these
AddTempRoot/RemoveTempRoot and freeing fixes.

In `@units/Goccia.Lexer.pas`:
- Around line 242-265: When parsing UTF-16 surrogate pairs in the CodePoint
combine block (where SavedCurrent is used and Advance is called to consume '\'
and 'u'), also save the current column before advancing (e.g. SavedColumn :=
FColumn) and on any backtrack paths (both the IsAtEnd break and the LowSurrogate
invalid branch) restore FColumn along with FCurrent (FCurrent := SavedCurrent;
FColumn := SavedColumn) so token/error positions stay in sync; update all
backtrack sites in that CodePoint/Advance sequence to restore SavedColumn.

In `@units/Goccia.Values.EnumValue.pas`:
- Around line 58-63: EnumSymbolIterator currently uses Self.FEntries causing
detached/borrowed iterator calls to iterate the wrong enum; modify
TGocciaEnumValue.EnumSymbolIterator to use the call receiver AThisValue: add
Goccia.Values.ErrorHelper to the implementation uses clause, check if AThisValue
is TGocciaEnumValue and if so create the iterator with (AThisValue as
TGocciaEnumValue).FEntries and akValues, otherwise call ThrowError with the
"called on non-enum" message (use ThrowError, not ThrowTypeError) to match
Array.values() patterns.
- Around line 53-56: TGocciaEnumValue.TypeName currently returns FName which
violates the object-type contract; update the method TGocciaEnumValue.TypeName
to return the OBJECT_TYPE_NAME constant from Goccia.Constants.TypeNames
(matching other TGocciaObjectValue-derived types) so enum values report as the
`"object"` type rather than the enum name.

---

Nitpick comments:
In `@README.md`:
- Line 150: The exact test count in the README sentence "GocciaScript has 2300+
JavaScript unit tests covering language features, built-in objects, and edge
cases." is likely to become stale; update that line to a looser, drift-resistant
phrasing such as "2k+ JavaScript unit tests" or "2,300+ JavaScript unit tests"
(whichever team style prefers) so the README remains accurate without frequent
updates.

In `@tests/built-ins/structuredClone/errors.js`:
- Around line 6-43: Convert the verbose try/catch assertions in the "throws on
function", "throws on symbol", and "throws on object containing a function"
tests to the concise Jest form used in the other test: replace the manual
try/catch + flag pattern with expect(() =>
structuredClone(...)).toThrow(TypeError) or, if you want to assert the message,
use expect(() => structuredClone(...)).toThrow(/could not be cloned/); update
the tests named "throws on function", "throws on symbol", and "throws on object
containing a function" to use this pattern for consistency.

Comment on lines +339 to +343
begin
ClonedValue := StructuredCloneValue(AObj.GetProperty(Keys[I]), AMemory);
Result.DefineProperty(Keys[I],
TGocciaPropertyDescriptorData.Create(ClonedValue, Descriptor.Flags - [pfConfigurable, pfWritable] + [pfEnumerable]));
end;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there are tests covering accessor property cloning in structuredClone
rg -n "structuredClone.*get|accessor" tests/ 2>/dev/null || echo "No matches found"

# Check for any enum-related structuredClone tests
rg -n "structuredClone" tests/language/enums/ 2>/dev/null || echo "No enum tests found"

# Check broader test coverage for structuredClone
rg -l "structuredClone" tests/ | head -20

Repository: frostney/GocciaScript

Length of output: 1000


🏁 Script executed:

#!/bin/bash
# Read the full context of the CloneObject function around lines 339-343
wc -l units/Goccia.Builtins.Globals.pas

Repository: frostney/GocciaScript

Length of output: 102


🏁 Script executed:

#!/bin/bash
# Read the accessor cloning test
sed -n '40,70p' tests/built-ins/structuredClone/errors.js

Repository: frostney/GocciaScript

Length of output: 957


🏁 Script executed:

#!/bin/bash
# Read the code context around lines 339-343
sed -n '320,360p' units/Goccia.Builtins.Globals.pas

Repository: frostney/GocciaScript

Length of output: 1375


🏁 Script executed:

#!/bin/bash
# Look at more context - check for comments explaining the flag choices
sed -n '310,350p' units/Goccia.Builtins.Globals.pas | cat -n

Repository: frostney/GocciaScript

Length of output: 1779


🏁 Script executed:

#!/bin/bash
# Check the test more carefully - does it verify the descriptor flags of the cloned property?
sed -n '40,65p' tests/built-ins/structuredClone/errors.js | cat -n

Repository: frostney/GocciaScript

Length of output: 912


🏁 Script executed:

#!/bin/bash
# Search for any documentation or comments about accessor cloning behavior
rg -B5 -A5 "TGocciaPropertyDescriptorAccessor" units/Goccia.Builtins.Globals.pas | head -40

Repository: frostney/GocciaScript

Length of output: 624


🏁 Script executed:

#!/bin/bash
# Check if there are tests validating descriptor flags in structuredClone
rg -n "configurable|writable|enumerable" tests/built-ins/structuredClone/ -A2 -B2

Repository: frostney/GocciaScript

Length of output: 448


🏁 Script executed:

#!/bin/bash
# Search for any spec comments or references in the globals file
rg -n "structuredClone|spec|HTML" units/Goccia.Builtins.Globals.pas | head -20

Repository: frostney/GocciaScript

Length of output: 822


🏁 Script executed:

#!/bin/bash
# Check git history or comments for this specific change
git log --oneline -20 -- units/Goccia.Builtins.Globals.pas 2>/dev/null || echo "Git history unavailable"

Repository: frostney/GocciaScript

Length of output: 143


🏁 Script executed:

#!/bin/bash
# Check what the full enum support commit changed
git show ab39941 --stat 2>/dev/null | head -30

Repository: frostney/GocciaScript

Length of output: 1757


🏁 Script executed:

#!/bin/bash
# Look for any enum-related tests that might validate descriptor flags
find tests/ -name "*enum*" -type f

Repository: frostney/GocciaScript

Length of output: 558


🏁 Script executed:

#!/bin/bash
# Check if there are other places in code that manipulate flags similarly for enums
rg -n "pfConfigurable.*pfWritable.*pfEnumerable|pfEnumerable.*pfConfigurable" units/

Repository: frostney/GocciaScript

Length of output: 4247


🏁 Script executed:

#!/bin/bash
# Check the enum-immutability test to understand the expected behavior
cat tests/language/enums/enum-immutability.js

Repository: frostney/GocciaScript

Length of output: 741


🏁 Script executed:

#!/bin/bash
# Search for comments or documentation about enum property descriptors
rg -n "enum|immutable" units/Goccia.Builtins.Globals.pas -B2 -A2

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Let's verify what flags are set for enum accessor properties
rg -n "enum.*Accessor|DefineProperty.*PROP_" tests/language/enums/basic-enum.js -B2 -A2

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Check if structuredClone is used on enums in any tests
rg -n "structuredClone" tests/language/enums/

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

#!/bin/bash
# Look at the broader context of how accessor properties are created for enums in the implementation
rg -B5 -A5 "DefineProperty.*Accessor.*pfEnumerable.*pfConfigurable.*pfWritable" units/ | head -50

Repository: frostney/GocciaScript

Length of output: 2701


🏁 Script executed:

#!/bin/bash
# Check if there are any comments in the CloneObject function explaining the enum behavior
sed -n '306,350p' units/Goccia.Builtins.Globals.pas | cat -n

Repository: frostney/GocciaScript

Length of output: 2117


🏁 Script executed:

#!/bin/bash
# Check if enum objects use accessor properties or data properties
rg -n "DefineProperty.*enum|enum.*DefineProperty" units/ -B2 -A2 | head -40

Repository: frostney/GocciaScript

Length of output: 47


Add documentation explaining accessor property cloning behavior for enums.

The code correctly invokes the getter and clones the resulting value as a data property. However, the flag manipulation (removing pfConfigurable and pfWritable, adding pfEnumerable) lacks explanation. This appears intentional to support enum immutability—converting accessor properties to non-configurable, non-writable data properties—but should be documented with a comment referencing the enum behavior and/or the HTML structuredClone spec section. Additionally, consider adding a test validating structuredClone on enum objects to ensure descriptor flags match expectations.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@units/Goccia.Builtins.Globals.pas` around lines 339 - 343, Add a brief inline
comment above the block where StructuredCloneValue(AObj.GetProperty(Keys[I]),
AMemory) is converted into a data property and where Descriptor.Flags is changed
(the call to Result.DefineProperty with TGocciaPropertyDescriptorData and the
flag expression Descriptor.Flags - [pfConfigurable, pfWritable] +
[pfEnumerable]); explain that accessor properties on enum members are
intentionally converted to non-configurable, non-writable data properties to
enforce enum immutability and reference the relevant structuredClone/enum spec
section or HTML spec for justification. Also add a unit test that
structuredClones an enum-like object and asserts the cloned property descriptor
flags match the expected non-configurable, non-writable, enumerable state.

Comment on lines +1442 to +1444
// TC39 proposal-enum
function EvaluateEnumDeclaration(const AEnumDeclaration: TGocciaEnumDeclaration; const AContext: TGocciaEvaluationContext): TGocciaValue;
var
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add the ES spec annotation for the new evaluator entry point.
Please add the // ES<YYYY> §X.Y.Z EvaluateEnumDeclaration(...) comment immediately above the function body.

Based on learnings “When implementing ECMAScript-specified behavior, annotate each function or method with a comment referencing the relevant spec section using the format // ES §X.Y.Z MethodName(params) where YYYY is the current edition year.”

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@units/Goccia.Evaluator.pas` around lines 1442 - 1444, Add the ECMAScript spec
annotation comment immediately above the function declaration for
EvaluateEnumDeclaration: insert a single-line comment in the format // ES2024
§X.Y.Z EvaluateEnumDeclaration(const AEnumDeclaration: TGocciaEnumDeclaration;
const AContext: TGocciaEvaluationContext) (replace §X.Y.Z with the actual spec
section number if known); place it directly above the existing "function
EvaluateEnumDeclaration(...)" line so the evaluator entry point is annotated per
project convention.

Comment on lines +1442 to +1489
// TC39 proposal-enum
function EvaluateEnumDeclaration(const AEnumDeclaration: TGocciaEnumDeclaration; const AContext: TGocciaEvaluationContext): TGocciaValue;
var
EnumValue: TGocciaEnumValue;
EnumEntries: TGocciaArrayValue;
ChildScope: TGocciaScope;
ChildContext: TGocciaEvaluationContext;
I: Integer;
MemberValue: TGocciaValue;
EntryPair: TGocciaArrayValue;
begin
EnumValue := TGocciaEnumValue.Create(AEnumDeclaration.Name);
EnumEntries := TGocciaArrayValue.Create;

ChildScope := AContext.Scope.CreateChild(skBlock);
ChildContext := AContext;
ChildContext.Scope := ChildScope;

ChildScope.DefineLexicalBinding(AEnumDeclaration.Name, EnumValue, dtLet);

for I := 0 to Length(AEnumDeclaration.Members) - 1 do
begin
MemberValue := EvaluateExpression(AEnumDeclaration.Members[I].Initializer, ChildContext);

if not ((MemberValue is TGocciaNumberLiteralValue) or
(MemberValue is TGocciaStringLiteralValue) or
(MemberValue is TGocciaSymbolValue)) then
AContext.OnError('Enum member initializer must evaluate to a Number, String, or Symbol value',
AEnumDeclaration.Line, AEnumDeclaration.Column);

EnumValue.DefineProperty(AEnumDeclaration.Members[I].Name,
TGocciaPropertyDescriptorData.Create(MemberValue, [pfEnumerable]));

ChildScope.DefineLexicalBinding(AEnumDeclaration.Members[I].Name, MemberValue, dtLet);

EntryPair := TGocciaArrayValue.Create;
EntryPair.Elements.Add(TGocciaStringLiteralValue.Create(AEnumDeclaration.Members[I].Name));
EntryPair.Elements.Add(MemberValue);
EnumEntries.Elements.Add(EntryPair);
end;

EnumValue.Entries := EnumEntries;
InitializeEnumSymbols(EnumValue);
EnumValue.PreventExtensions;

AContext.Scope.DefineLexicalBinding(AEnumDeclaration.Name, EnumValue, dtLet);
Result := EnumValue;
end;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Protect enum construction from GC and free the child scope.
Enum entries are built while only held in Pascal variables; they should be temp-rooted, and the enum init scope should be freed after use to avoid leaks.

🔧 Suggested fix
 function EvaluateEnumDeclaration(const AEnumDeclaration: TGocciaEnumDeclaration; const AContext: TGocciaEvaluationContext): TGocciaValue;
 var
   EnumValue: TGocciaEnumValue;
   EnumEntries: TGocciaArrayValue;
   ChildScope: TGocciaScope;
   ChildContext: TGocciaEvaluationContext;
   I: Integer;
   MemberValue: TGocciaValue;
   EntryPair: TGocciaArrayValue;
 begin
   EnumValue := TGocciaEnumValue.Create(AEnumDeclaration.Name);
   EnumEntries := TGocciaArrayValue.Create;
+  TGocciaGarbageCollector.Instance.AddTempRoot(EnumValue);
+  TGocciaGarbageCollector.Instance.AddTempRoot(EnumEntries);

   ChildScope := AContext.Scope.CreateChild(skBlock);
-  ChildContext := AContext;
-  ChildContext.Scope := ChildScope;
-
-  ChildScope.DefineLexicalBinding(AEnumDeclaration.Name, EnumValue, dtLet);
-
-  for I := 0 to Length(AEnumDeclaration.Members) - 1 do
-  begin
-    MemberValue := EvaluateExpression(AEnumDeclaration.Members[I].Initializer, ChildContext);
-    ...
-  end;
-
-  EnumValue.Entries := EnumEntries;
-  InitializeEnumSymbols(EnumValue);
-  EnumValue.PreventExtensions;
-
-  AContext.Scope.DefineLexicalBinding(AEnumDeclaration.Name, EnumValue, dtLet);
-  Result := EnumValue;
+  try
+    ChildContext := AContext;
+    ChildContext.Scope := ChildScope;
+
+    ChildScope.DefineLexicalBinding(AEnumDeclaration.Name, EnumValue, dtLet);
+
+    for I := 0 to Length(AEnumDeclaration.Members) - 1 do
+    begin
+      MemberValue := EvaluateExpression(AEnumDeclaration.Members[I].Initializer, ChildContext);
+      ...
+    end;
+
+    EnumValue.Entries := EnumEntries;
+    InitializeEnumSymbols(EnumValue);
+    EnumValue.PreventExtensions;
+
+    AContext.Scope.DefineLexicalBinding(AEnumDeclaration.Name, EnumValue, dtLet);
+    Result := EnumValue;
+  finally
+    ChildScope.Free;
+    TGocciaGarbageCollector.Instance.RemoveTempRoot(EnumEntries);
+    TGocciaGarbageCollector.Instance.RemoveTempRoot(EnumValue);
+  end;
 end;

Based on learnings “Values held only by Pascal code must be protected with AddTempRoot/RemoveTempRoot.”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// TC39 proposal-enum
function EvaluateEnumDeclaration(const AEnumDeclaration: TGocciaEnumDeclaration; const AContext: TGocciaEvaluationContext): TGocciaValue;
var
EnumValue: TGocciaEnumValue;
EnumEntries: TGocciaArrayValue;
ChildScope: TGocciaScope;
ChildContext: TGocciaEvaluationContext;
I: Integer;
MemberValue: TGocciaValue;
EntryPair: TGocciaArrayValue;
begin
EnumValue := TGocciaEnumValue.Create(AEnumDeclaration.Name);
EnumEntries := TGocciaArrayValue.Create;
ChildScope := AContext.Scope.CreateChild(skBlock);
ChildContext := AContext;
ChildContext.Scope := ChildScope;
ChildScope.DefineLexicalBinding(AEnumDeclaration.Name, EnumValue, dtLet);
for I := 0 to Length(AEnumDeclaration.Members) - 1 do
begin
MemberValue := EvaluateExpression(AEnumDeclaration.Members[I].Initializer, ChildContext);
if not ((MemberValue is TGocciaNumberLiteralValue) or
(MemberValue is TGocciaStringLiteralValue) or
(MemberValue is TGocciaSymbolValue)) then
AContext.OnError('Enum member initializer must evaluate to a Number, String, or Symbol value',
AEnumDeclaration.Line, AEnumDeclaration.Column);
EnumValue.DefineProperty(AEnumDeclaration.Members[I].Name,
TGocciaPropertyDescriptorData.Create(MemberValue, [pfEnumerable]));
ChildScope.DefineLexicalBinding(AEnumDeclaration.Members[I].Name, MemberValue, dtLet);
EntryPair := TGocciaArrayValue.Create;
EntryPair.Elements.Add(TGocciaStringLiteralValue.Create(AEnumDeclaration.Members[I].Name));
EntryPair.Elements.Add(MemberValue);
EnumEntries.Elements.Add(EntryPair);
end;
EnumValue.Entries := EnumEntries;
InitializeEnumSymbols(EnumValue);
EnumValue.PreventExtensions;
AContext.Scope.DefineLexicalBinding(AEnumDeclaration.Name, EnumValue, dtLet);
Result := EnumValue;
end;
// TC39 proposal-enum
function EvaluateEnumDeclaration(const AEnumDeclaration: TGocciaEnumDeclaration; const AContext: TGocciaEvaluationContext): TGocciaValue;
var
EnumValue: TGocciaEnumValue;
EnumEntries: TGocciaArrayValue;
ChildScope: TGocciaScope;
ChildContext: TGocciaEvaluationContext;
I: Integer;
MemberValue: TGocciaValue;
EntryPair: TGocciaArrayValue;
begin
EnumValue := TGocciaEnumValue.Create(AEnumDeclaration.Name);
EnumEntries := TGocciaArrayValue.Create;
TGocciaGarbageCollector.Instance.AddTempRoot(EnumValue);
TGocciaGarbageCollector.Instance.AddTempRoot(EnumEntries);
ChildScope := AContext.Scope.CreateChild(skBlock);
try
ChildContext := AContext;
ChildContext.Scope := ChildScope;
ChildScope.DefineLexicalBinding(AEnumDeclaration.Name, EnumValue, dtLet);
for I := 0 to Length(AEnumDeclaration.Members) - 1 do
begin
MemberValue := EvaluateExpression(AEnumDeclaration.Members[I].Initializer, ChildContext);
if not ((MemberValue is TGocciaNumberLiteralValue) or
(MemberValue is TGocciaStringLiteralValue) or
(MemberValue is TGocciaSymbolValue)) then
AContext.OnError('Enum member initializer must evaluate to a Number, String, or Symbol value',
AEnumDeclaration.Line, AEnumDeclaration.Column);
EnumValue.DefineProperty(AEnumDeclaration.Members[I].Name,
TGocciaPropertyDescriptorData.Create(MemberValue, [pfEnumerable]));
ChildScope.DefineLexicalBinding(AEnumDeclaration.Members[I].Name, MemberValue, dtLet);
EntryPair := TGocciaArrayValue.Create;
EntryPair.Elements.Add(TGocciaStringLiteralValue.Create(AEnumDeclaration.Members[I].Name));
EntryPair.Elements.Add(MemberValue);
EnumEntries.Elements.Add(EntryPair);
end;
EnumValue.Entries := EnumEntries;
InitializeEnumSymbols(EnumValue);
EnumValue.PreventExtensions;
AContext.Scope.DefineLexicalBinding(AEnumDeclaration.Name, EnumValue, dtLet);
Result := EnumValue;
finally
ChildScope.Free;
TGocciaGarbageCollector.Instance.RemoveTempRoot(EnumEntries);
TGocciaGarbageCollector.Instance.RemoveTempRoot(EnumValue);
end;
end;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@units/Goccia.Evaluator.pas` around lines 1442 - 1489, EvaluateEnumDeclaration
builds EnumValue and EnumEntries in Pascal-only variables without temp-rooting
and never frees ChildScope; protect temporary GC-visible values by calling
AddTempRoot on EnumValue and EnumEntries (and any EntryPair/MemberValue as
needed) before evaluating members and RemoveTempRoot after assigning EnumValue.
Also ensure ChildScope is freed after use (after
AContext.Scope.DefineLexicalBinding and before returning) so the enum init scope
is released; reference the EvaluateEnumDeclaration function, EnumValue,
EnumEntries, ChildScope, ChildContext, InitializeEnumSymbols and
PreventExtensions when applying these AddTempRoot/RemoveTempRoot and freeing
fixes.

Comment thread units/Goccia.Lexer.pas
Comment on lines +53 to +56
function TGocciaEnumValue.TypeName: string;
begin
Result := FName;
end;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n units/Goccia.Values.EnumValue.pas | head -80

Repository: frostney/GocciaScript

Length of output: 2660


🏁 Script executed:

# Check if Goccia.Constants.TypeNames exists and what constants are available
fd -e pas 'Constants' | grep -i typename
cat -n "units/Goccia.Constants.TypeNames.pas" 2>/dev/null || find . -name "*Constants*" -type f | head -10

Repository: frostney/GocciaScript

Length of output: 600


🏁 Script executed:

# Search for TypeName implementations across the codebase
rg -n "function.*TypeName" --type=pas

Repository: frostney/GocciaScript

Length of output: 92


🏁 Script executed:

# Search for how TypeName is used (especially in typeof)
rg -n "\.TypeName\b" --type=pas -A 2 -B 2 | head -100

Repository: frostney/GocciaScript

Length of output: 92


🏁 Script executed:

# Look for typeof implementation
rg -n "typeof" --type=pas -i -A 3 | head -100

Repository: frostney/GocciaScript

Length of output: 92


🏁 Script executed:

# Check TGocciaValue base class to understand TypeName contract
rg -n "class TGocciaValue\|procedure.*TypeName\|function.*TypeName" --type=pas -A 5 | head -150

Repository: frostney/GocciaScript

Length of output: 92


🏁 Script executed:

# Find TypeName implementations across value types
rg "function.*TypeName.*:" units/Goccia.Values -A 3

Repository: frostney/GocciaScript

Length of output: 124


🏁 Script executed:

# Check TGocciaObjectValue implementation
rg -n "class TGocciaObjectValue\|function TypeName" units/Goccia.Values.ObjectValue.pas | head -30

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Search for typeof operator implementation
rg -n "typeof\|TypeOf\|TypeName" units/Goccia.Evaluator.pas | head -50

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Look at base TGocciaValue class
rg -n "class TGocciaValue\|function TypeName" units/Goccia.Values.Primitives.pas -B 2 -A 5 | head -80

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Check how other primitive values implement TypeName
rg -A 3 "function TGoccia.*Value\.TypeName" units/Goccia.Values.Primitives.pas

Repository: frostney/GocciaScript

Length of output: 550


🏁 Script executed:

# Check TGocciaObjectValue.TypeName implementation
rg -A 3 "function TGocciaObjectValue\.TypeName" units/Goccia.Values.ObjectValue.pas

Repository: frostney/GocciaScript

Length of output: 143


🏁 Script executed:

# Check TGocciaArrayValue.TypeName implementation
rg -A 3 "function TGocciaArrayValue\.TypeName" units/Goccia.Values.ArrayValue.pas

Repository: frostney/GocciaScript

Length of output: 142


🏁 Script executed:

# Verify the pattern: search all TypeName implementations
rg -A 2 "function.*TypeName.*string" --glob="units/*.pas" | head -80

Repository: frostney/GocciaScript

Length of output: 4375


🏁 Script executed:

# Check if StringObjectValue uses constant or hardcoded 'object'
rg -B 2 -A 2 "function TGocciaStringObjectValue\.TypeName" units/Goccia.Values.StringObjectValue.pas

Repository: frostney/GocciaScript

Length of output: 183


🏁 Script executed:

# Verify that FunctionBase also follows the pattern
rg -A 3 "function TGoccia.*FunctionBase.*TypeName\|function.*FunctionValue\.TypeName" units/Goccia.Values.FunctionBase.pas

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Check if there's a comment or documentation about TypeName vs TypeOf difference
rg -B 5 "TypeName\|TypeOf" units/Goccia.Values.Primitives.pas | head -80

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Verify how TypeOf differs from TypeName (seems there's both methods)
rg -B 3 -A 5 "function TypeOf.*string" units/Goccia.Values.Primitives.pas | head -60

Repository: frostney/GocciaScript

Length of output: 2324


🏁 Script executed:

# Check TGocciaEnumValue for TypeOf implementation
rg -B 3 -A 5 "function.*TypeOf" units/Goccia.Values.EnumValue.pas

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Verify ObjectValue's TypeOf to understand the distinction
rg -B 1 -A 4 "function TGocciaObjectValue\.TypeOf" units/Goccia.Values.ObjectValue.pas

Repository: frostney/GocciaScript

Length of output: 143


🏁 Script executed:

# Check if EnumValue has its own TypeOf implementation
cat -n units/Goccia.Values.EnumValue.pas | grep -A 5 "TypeOf"

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Check if enums are supposed to be treated specially anywhere
rg -B 3 -A 3 "TGocciaEnumValue\|Enum" units/Goccia.Evaluator.pas | head -80

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Verify what JavaScript semantics say about enum types
rg -B 2 -A 2 "enum" units/Goccia.Values.EnumValue.pas

Repository: frostney/GocciaScript

Length of output: 293


🏁 Script executed:

# Double-check: search for any special handling of TypeName for enums
rg "EnumValue\|TGocciaEnumValue" units/ -l | xargs grep -l "typeof\|TypeName" 2>/dev/null

Repository: frostney/GocciaScript

Length of output: 49


🏁 Script executed:

# Verify parent class relationship and check if EnumValue breaks Liskov substitution
rg -B 5 "class TGocciaEnumValue" units/Goccia.Values.EnumValue.pas

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Check if there's any special code path that expects EnumValue.TypeName to return the enum name
rg "TypeName" units/ --glob="*.pas" -B 2 -A 2 | grep -A 5 -B 5 "FName\|EnumValue"

Repository: frostney/GocciaScript

Length of output: 1349


🏁 Script executed:

# Verify ObjectValue parent implementation once more
cat -n units/Goccia.Values.ObjectValue.pas | grep -B 10 -A 5 "function TGocciaObjectValue.TypeName"

Repository: frostney/GocciaScript

Length of output: 462


🏁 Script executed:

# Check if there are tests for EnumValue TypeName
rg -B 5 -A 5 "EnumValue.*TypeName\|TypeName.*Enum" units/ --glob="*.Test.pas"

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Check ClassValue.TypeName to see if it follows a different pattern
rg -B 2 -A 5 "function TGocciaClassValue\.TypeName" units/Goccia.Values.ClassValue.pas

Repository: frostney/GocciaScript

Length of output: 202


🏁 Script executed:

# Verify if hardcoded 'object' strings exist elsewhere and whether they should be constants
rg "Result := 'object'" units/ --glob="*.pas" -B 2 -A 1

Repository: frostney/GocciaScript

Length of output: 1021


TypeName should return OBJECT_TYPE_NAME constant instead of enum name.

As TGocciaEnumValue extends TGocciaObjectValue, returning FName breaks the type contract—enums must report as objects like all other object-type values. JavaScript semantics require typeof enum to yield "object", not the enum name. Additionally, per coding guidelines, use the OBJECT_TYPE_NAME constant from Goccia.Constants.TypeNames instead of hardcoded strings.

Suggested fix
 uses
   Goccia.Arguments.Collection,
+  Goccia.Constants.TypeNames,
   Goccia.Values.ArrayValue,
   Goccia.Values.ObjectValue,
   Goccia.Values.Primitives;
@@
 function TGocciaEnumValue.TypeName: string;
 begin
-  Result := FName;
+  Result := OBJECT_TYPE_NAME;
 end;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function TGocciaEnumValue.TypeName: string;
begin
Result := FName;
end;
function TGocciaEnumValue.TypeName: string;
begin
Result := OBJECT_TYPE_NAME;
end;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@units/Goccia.Values.EnumValue.pas` around lines 53 - 56,
TGocciaEnumValue.TypeName currently returns FName which violates the object-type
contract; update the method TGocciaEnumValue.TypeName to return the
OBJECT_TYPE_NAME constant from Goccia.Constants.TypeNames (matching other
TGocciaObjectValue-derived types) so enum values report as the `"object"` type
rather than the enum name.

Comment on lines +58 to +63
// TC39 proposal-enum: Symbol.iterator yields [key, value] entries in declaration order
function TGocciaEnumValue.EnumSymbolIterator(const AArgs: TGocciaArgumentsCollection;
const AThisValue: TGocciaValue): TGocciaValue;
begin
Result := TGocciaArrayIteratorValue.Create(FEntries, akValues);
end;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n units/Goccia.Values.EnumValue.pas | sed -n '58,63p'

Repository: frostney/GocciaScript

Length of output: 406


🏁 Script executed:

# Check the complete EnumSymbolIterator implementation
rg -n -A 10 'function TGocciaEnumValue.EnumSymbolIterator' units/Goccia.Values.EnumValue.pas

Repository: frostney/GocciaScript

Length of output: 494


🏁 Script executed:

# Check how native functions are invoked with AThisValue
rg -n 'InvokeCallable\|CallNative' units/Goccia.Values.NativeFunction.pas | head -20

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Check the Entries property definition
rg -n 'FEntries\|property.*Entries' units/Goccia.Values.EnumValue.pas

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Check similar iterator implementations in other value types
rg -n 'EnumSymbolIterator\|TGocciaArrayIteratorValue.Create' units/ --type pas | head -20

Repository: frostney/GocciaScript

Length of output: 92


🏁 Script executed:

# Look for field declarations in EnumValue
rg -n 'FEntries' units/Goccia.Values.EnumValue.pas -A 2 -B 2

Repository: frostney/GocciaScript

Length of output: 796


🏁 Script executed:

# Check how symbol iterators are invoked - look in evaluator
rg -n 'Symbol.iterator\|GetIteratorFromValue' units/ -l

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Look for native function invocation pattern
fd -e pas | xargs grep -l 'InvokeCallable\|CallFunction' | head -5

Repository: frostney/GocciaScript

Length of output: 248


🏁 Script executed:

# Check error handling patterns in similar methods
rg -n 'ThrowTypeError\|TypeErrorBroadcast' units/Goccia.Values.EnumValue.pas

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Look for how iterators are typically called with AThisValue binding
rg -n 'TGocciaArrayIteratorValue.Create' units/ -B 5 -A 2 | head -40

Repository: frostney/GocciaScript

Length of output: 2641


🏁 Script executed:

# Check error handling in EnumValue
rg -n 'ThrowError\|ThrowTypeError' units/Goccia.Values.EnumValue.pas

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Check what error method is used in ErrorHelper or similar
rg -n 'ThrowTypeError' units/Goccia.Values.ErrorHelper.pas

Repository: frostney/GocciaScript

Length of output: 170


🏁 Script executed:

# Check how other value types handle this in iterator context
rg -n 'not.*is TGoccia.*then.*Throw' units/Goccia.Values.ArrayValue.pas | head -5

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Check exact error call in Array.values
rg -n -A 2 'Array.values called on non-array' units/Goccia.Values.ArrayValue.pas

Repository: frostney/GocciaScript

Length of output: 224


🏁 Script executed:

# Check the import/uses in ArrayValue to see how it handles errors
rg -n 'uses' units/Goccia.Values.ArrayValue.pas | head -1

Repository: frostney/GocciaScript

Length of output: 71


🏁 Script executed:

# Check if EnumValue imports ErrorHelper
rg -n 'uses' units/Goccia.Values.EnumValue.pas

Repository: frostney/GocciaScript

Length of output: 79


🏁 Script executed:

# Get full uses section of EnumValue
rg -n -A 10 '^uses' units/Goccia.Values.EnumValue.pas | head -20

Repository: frostney/GocciaScript

Length of output: 565


🏁 Script executed:

# Get full uses section of ArrayValue
rg -n -A 15 '^uses' units/Goccia.Values.ArrayValue.pas | head -25

Repository: frostney/GocciaScript

Length of output: 851


🏁 Script executed:

# Check what ThrowError is - is it from a base class or global
rg -n 'procedure ThrowError' units/ --type pas | head -5

Repository: frostney/GocciaScript

Length of output: 92


🏁 Script executed:

# Check Goccia.Error to see ThrowError definition
rg -n 'procedure ThrowError' units/Goccia.Error.pas

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Check what TGocciaObjectValue provides - ThrowError might be a method
rg -n 'procedure ThrowError' units/Goccia.Values.ObjectValue.pas

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Look for ThrowError in base classes
fd -e pas | xargs grep -l 'procedure ThrowError' | head -10

Repository: frostney/GocciaScript

Length of output: 188


🏁 Script executed:

# Check ErrorHelper for ThrowError definition
rg -n 'procedure ThrowError\|procedure ThrowTypeError' units/Goccia.Values.ErrorHelper.pas -A 2

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Check if ArrayValue imports ErrorHelper
rg -n 'ErrorHelper' units/Goccia.Values.ArrayValue.pas

Repository: frostney/GocciaScript

Length of output: 97


🏁 Script executed:

# Check the full uses section of ArrayValue implementation
sed -n '107,130p' units/Goccia.Values.ArrayValue.pas

Repository: frostney/GocciaScript

Length of output: 610


🏁 Script executed:

# Check what's exported from ErrorHelper
rg -n 'procedure\|function' units/Goccia.Values.ErrorHelper.pas | head -20

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Verify that EnumValue does not currently import ErrorHelper
cat -n units/Goccia.Values.EnumValue.pas | sed -n '7,36p'

Repository: frostney/GocciaScript

Length of output: 1111


🏁 Script executed:

# Check the exact signature and behavior of ThrowError in ErrorHelper
cat units/Goccia.Values.ErrorHelper.pas

Repository: frostney/GocciaScript

Length of output: 2333


🏁 Script executed:

# Verify the pattern by checking another method that uses AThisValue
rg -n -B 5 -A 10 'not.*AThisValue.*is.*Goccia' units/Goccia.Values.ArrayValue.pas | head -30

Repository: frostney/GocciaScript

Length of output: 1114


Use AThisValue as the iterator receiver instead of Self.

EnumSymbolIterator currently uses the method host's FEntries instead of the call receiver. Detached calls (or borrowed iterators) will iterate the wrong enum, violating the iterator protocol. This should either iterate the correct enum or throw if this isn't an enum.

The pattern is established in Array.values() and similar Array methods, which check AThisValue is TGocciaArrayValue before proceeding. The fix requires adding Goccia.Values.ErrorHelper to the implementation uses clause and wrapping the iterator creation with a type check.

Note: Use ThrowError (not ThrowTypeError) for consistency with existing Array method patterns for "called on non-X" errors.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@units/Goccia.Values.EnumValue.pas` around lines 58 - 63, EnumSymbolIterator
currently uses Self.FEntries causing detached/borrowed iterator calls to iterate
the wrong enum; modify TGocciaEnumValue.EnumSymbolIterator to use the call
receiver AThisValue: add Goccia.Values.ErrorHelper to the implementation uses
clause, check if AThisValue is TGocciaEnumValue and if so create the iterator
with (AThisValue as TGocciaEnumValue).FEntries and akValues, otherwise call
ThrowError with the "called on non-enum" message (use ThrowError, not
ThrowTypeError) to match Array.values() patterns.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@docs/embedding.md`:
- Line 208: Update the docs table entry for ggConsole to list the full set of
exported console methods to match the implementation in
Goccia.Builtins.Console.pas: replace the abbreviated "etc." with the missing
methods so the list reads: log, warn, error, info, debug, dir, assert, count,
countReset, time, timeEnd, timeLog, clear, group, groupEnd, trace, table; ensure
ggConsole and reference to Goccia.Builtins.Console.pas are used to locate the
implementation if needed.

---

Duplicate comments:
In `@units/Goccia.Evaluator.pas`:
- Around line 1442-1443: Add the required ES-spec-style annotation comment above
the evaluator function for the TC39 enum proposal: insert a line like "// TC39
proposal-enum §2.1 EvaluateEnumDeclaration(AEnumDeclaration, AContext)"
immediately above the function declaration for EvaluateEnumDeclaration so the
evaluator function is annotated in the project's required format.
- Around line 1442-1493: EvaluateEnumDeclaration currently risks GC and leaks:
temp-root EnumValue before any EvaluateExpression calls (use
TGocciaGarbageCollector.Instance.AddTempRoot(EnumValue) and RemoveTempRoot in
finally), temp-root each EntryPair while constructing it (AddTempRoot before
populating and RemoveTempRoot after adding to EnumEntries), and free ChildScope
(call ChildScope.Free in the finally block after using it). Update the procedure
to AddTempRoot(EnumValue) right after creating EnumValue, Add/RemoveTempRoot
around each EntryPair creation, and ensure ChildScope.Free is called in the same
finally where you RemoveTempRoot(EnumEntries)/EnumValue to avoid leaks and
premature collection.

Comment thread docs/embedding.md
| Flag | Provides | Notes |
|------|----------|-------|
| `ggConsole` | `console.log` | Output to stdout |
| `ggConsole` | `console.*` | `log`, `warn`, `error`, `info`, `debug`, `dir`, `assert`, `count`, `time`, etc. |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, find all Pascal files and search for ggConsole registration/implementation
find . -name "*.pas" -o -name "*.dpr" | head -20

Repository: frostney/GocciaScript

Length of output: 657


🏁 Script executed:

# Search for ggConsole definition and console method implementations
rg -n "ggConsole" --type=pascal -A 5 -B 5

Repository: frostney/GocciaScript

Length of output: 2031


🏁 Script executed:

# Also search more broadly for console object and method definitions
rg -n "console\." --type=pascal | head -30

Repository: frostney/GocciaScript

Length of output: 47


🏁 Script executed:

# Look for where console methods are registered (log, warn, error, etc.)
rg -n "(TMethod|procedure|function).*(log|warn|error|info|debug|dir|assert|count|time)" --type=pascal -i

Repository: frostney/GocciaScript

Length of output: 50377


🏁 Script executed:

# Get the actual Console implementation file to list registered methods
cat -n units/Goccia.Builtins.Console.pas | head -150

Repository: frostney/GocciaScript

Length of output: 7519


🏁 Script executed:

# Search for RegisterNativeMethod calls in Console.pas to see all registered methods
rg -n "RegisterNativeMethod" units/Goccia.Builtins.Console.pas -A 1 | head -60

Repository: frostney/GocciaScript

Length of output: 1846


🏁 Script executed:

# Check the context around line 208 in embedding.md to see the full table
sed -n '200,230p' docs/embedding.md

Repository: frostney/GocciaScript

Length of output: 1610


Expand the console.* method list to match the implementation.

The listed methods (log, warn, error, info, debug, dir, assert, count, time, etc.) are accurate but incomplete. The actual implementation in Goccia.Builtins.Console.pas exports 17 methods: log, warn, error, info, debug, dir, assert, count, countReset, time, timeEnd, timeLog, clear, group, groupEnd, trace, and table. Update the table to include countReset, timeEnd, timeLog, clear, group, groupEnd, trace, and table instead of relying on "etc."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/embedding.md` at line 208, Update the docs table entry for ggConsole to
list the full set of exported console methods to match the implementation in
Goccia.Builtins.Console.pas: replace the abbreviated "etc." with the missing
methods so the list reads: log, warn, error, info, debug, dir, assert, count,
countReset, time, timeEnd, timeLog, clear, group, groupEnd, trace, table; ensure
ggConsole and reference to Goccia.Builtins.Console.pas are used to locate the
implementation if needed.

@frostney frostney merged commit 00a564a into main Feb 22, 2026
4 checks passed
@frostney frostney deleted the feat-enum-support branch February 22, 2026 23:28
@frostney frostney added the new feature New feature or request label Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant