fix: Handle execute-dynamic-code transpiler constraints for static locals and byte casts#1723
Conversation
Skip literal hoisting inside static local function bodies so CS8421 no longer fires for inline constants, and add transpiler constraint hints plus SKILL.md guidance for int-to-byte failures after integer hoisting. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughThe dynamic-code transpiler now tracks static local function scopes to avoid hoisting literals there. Compilation responses provide guidance for static-local literal and integer-to-byte errors, with matching tests and documentation updates. ChangesDynamic transpiler constraints
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DynamicCodeLiteralHoister
participant LiteralHoistScopeTracker
participant StaticLocalFunctionHeaderScanner
DynamicCodeLiteralHoister->>LiteralHoistScopeTracker: inspect static local function
LiteralHoistScopeTracker->>StaticLocalFunctionHeaderScanner: parse function header
StaticLocalFunctionHeaderScanner-->>LiteralHoistScopeTracker: return body classification
LiteralHoistScopeTracker-->>DynamicCodeLiteralHoister: apply suppression state
DynamicCodeLiteralHoister->>DynamicCodeLiteralHoister: preserve or hoist literal
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeLiteralHoisterTests.cs (1)
25-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCoverage gap: top-level static local function. Both tests nest the
statichelper inside another function (braceDepth ≥ 1). Adding a case where the static local function is declared at the snippet's top level would expose the_braceDepth < 1gap flagged inLiteralHoistScopeTracker.cs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeLiteralHoisterTests.cs` around lines 25 - 59, Add test coverage for a static local function declared directly at snippet top level, where brace depth is zero, using both integer and string literal scenarios as appropriate. Anchor the tests near Rewrite_WhenIntegerLiteralIsInsideStaticLocalFunction and Rewrite_WhenStringLiteralIsInsideStaticLocalFunctionBlockBody, and assert literals inside the static function remain inline while eligible top-level literals retain the expected hoisting and bindings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/LiteralHoistScopeTracker.cs`:
- Around line 58-61: Update the brace-depth guard in the scope-tracking method
to allow brace depth 0, so top-level static local functions are recognized and
their literals are not hoisted; retain rejection only for negative depths.
---
Nitpick comments:
In `@Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeLiteralHoisterTests.cs`:
- Around line 25-59: Add test coverage for a static local function declared
directly at snippet top level, where brace depth is zero, using both integer and
string literal scenarios as appropriate. Anchor the tests near
Rewrite_WhenIntegerLiteralIsInsideStaticLocalFunction and
Rewrite_WhenStringLiteralIsInsideStaticLocalFunctionBlockBody, and assert
literals inside the static function remain inline while eligible top-level
literals retain the expected hoisting and bindings.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f1c44c1b-515f-4e0b-a48a-8c5c1df92a68
⛔ Files ignored due to path filters (4)
Assets/Tests/Editor/DynamicCodeToolTests/DynamicCodeTranspilerConstraintHintsTests.cs.metais excluded by none and included by nonePackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeTranspilerConstraintHints.cs.metais excluded by none and included by nonePackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/LiteralHoistScopeTracker.cs.metais excluded by none and included by nonePackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/StaticLocalFunctionHeaderScanner.cs.metais excluded by none and included by none
📒 Files selected for processing (11)
.agents/skills/uloop-execute-dynamic-code/SKILL.md.claude/skills/uloop-execute-dynamic-code/SKILL.mdAssets/Tests/Editor/DynamicCodeToolTests/DynamicCodeExecutionResponseFactoryTests.csAssets/Tests/Editor/DynamicCodeToolTests/DynamicCodeLiteralHoisterTests.csAssets/Tests/Editor/DynamicCodeToolTests/DynamicCodeTranspilerConstraintHintsTests.csPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeExecutionResponseFactory.csPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeLiteralHoister.csPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeTranspilerConstraintHints.csPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/LiteralHoistScopeTracker.csPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/StaticLocalFunctionHeaderScanner.csPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Skill/SKILL.md
Allow static local function scope tracking at brace depth zero, add CS8820 guidance, return hint tuples instead of out parameters, unify the literal prefix constant, and document scanner limitations in code and SKILL.md. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeLiteralHoister.cs (1)
592-631: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting shared regular-string scanning logic.
TryCopyRegularStringLiteralduplicates the prefix check, escape advancement, and closing-quote detection fromTryHoistRegularStringLiteral(lines 633-682) andTryAdvanceRegularStringLiteral(lines 487-522). A bug fix in the scanning logic would need to be applied in three places. Consider extracting a sharedTryAdvanceRegularStringLiteral(which already exists at line 487) and having bothTryCopyandTryHoistdelegate to it for the scanning phase, then differ only in what they append.♻️ Proposed refactor: delegate to existing TryAdvanceRegularStringLiteral
private static bool TryCopyRegularStringLiteral( string source, StringBuilder rewrittenSource, ref int index) { - if (source[index] != '"') - { - return false; - } - - if (index > 0 && (source[index - 1] == '@' || source[index - 1] == '$')) - { - return false; - } - int start = index; - index++; - - while (index < source.Length) - { - char current = source[index]; - if (current == '\\') - { - AdvanceEscapedLiteralSequence(source, ref index); - continue; - } - - if (current == '"') - { - index++; - rewrittenSource.Append(source, start, index - start); - return true; - } - - index++; - } - - index = start; - return false; + if (!TryAdvanceRegularStringLiteral(source, ref index)) + { + return false; + } + rewrittenSource.Append(source, start, index - start); + return true; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeLiteralHoister.cs` around lines 592 - 631, Refactor TryCopyRegularStringLiteral and TryHoistRegularStringLiteral to reuse the existing TryAdvanceRegularStringLiteral for regular-string prefix validation, escape advancement, and closing-quote detection. Keep each method’s distinct append behavior, and remove duplicated scanning logic so future fixes are centralized in TryAdvanceRegularStringLiteral.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeLiteralHoister.cs`:
- Around line 592-631: Refactor TryCopyRegularStringLiteral and
TryHoistRegularStringLiteral to reuse the existing
TryAdvanceRegularStringLiteral for regular-string prefix validation, escape
advancement, and closing-quote detection. Keep each method’s distinct append
behavior, and remove duplicated scanning logic so future fixes are centralized
in TryAdvanceRegularStringLiteral.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1ae93513-267d-4203-848a-6d370b595f3b
📒 Files selected for processing (10)
.agents/skills/uloop-execute-dynamic-code/SKILL.md.claude/skills/uloop-execute-dynamic-code/SKILL.mdAssets/Tests/Editor/DynamicCodeToolTests/DynamicCodeLiteralHoisterTests.csAssets/Tests/Editor/DynamicCodeToolTests/DynamicCodeTranspilerConstraintHintsTests.csPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeExecutionResponseFactory.csPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeLiteralHoister.csPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/DynamicCodeTranspilerConstraintHints.csPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/LiteralHoistScopeTracker.csPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/StaticLocalFunctionHeaderScanner.csPackages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Skill/SKILL.md
💤 Files with no reviewable changes (1)
- Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/LiteralHoistScopeTracker.cs
✅ Files skipped from review due to trivial changes (3)
- .agents/skills/uloop-execute-dynamic-code/SKILL.md
- Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/Skill/SKILL.md
- .claude/skills/uloop-execute-dynamic-code/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (2)
- Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCodeExecutionResponseFactory.cs
- Packages/src/Editor/FirstPartyTools/ExecuteDynamicCode/DynamicCompilation/StaticLocalFunctionHeaderScanner.cs
Summary
Address execute-dynamic-code transpiler constraints (B-9):
staticfrom lambda).Hint+Suggestionsand SKILL.md guidance for explicit(byte)casts.Follow-up (separate PR 3-5, before Group 3 integration): Color32 repro
Line=1/ caret onusingline is a PR 3-3 lineage bug (using extraction shrinks#lineregion while Context uses original lines). Track in PR 3-5 with placeholder-line or line-offset mapping.Pre-implementation verification
Repro 1a: nested static local (before fix)
Command:
uloop execute-dynamic-code --code $'int Compute(int value)\n{\n static int Double(int x) => x * 2;\n return Double(value);\n}\nreturn Compute(3);'Raw
Diagnostics[0](before fix):{ "Message": "CS8421: A static local function cannot contain a reference to '__uloop_literal_0'.", "Line": 3, "Column": 37, "ErrorCode": "CS8421", "Hint": "", "Suggestions": [], "Context": "L1:int Compute(int value)\nL2:{\nL3: static int Double(int x) => x * 2;\n ^\nL4: return Double(value);\nL5:}\nL6:return Compute(3);\n", "PointerColumn": 37 }Repro 1b: top-level static local — B-9 primary case (before fix)
Command:
uloop execute-dynamic-code --code $'static int Double(int x) => x * 2;\nreturn Double(3);'Raw
Diagnostics[0](before fix;_braceDepth < 1guard blocked hoister scope):{ "Message": "CS8421: A static local function cannot contain a reference to '__uloop_literal_0'.", "Line": 1, "Column": 37, "ErrorCode": "CS8421", "Hint": "", "Suggestions": [], "Context": "L1:static int Double(int x) => x * 2;\n ^\nL2:return Double(3);\n", "PointerColumn": 37 }Investigation: Integer literals inside static local bodies were hoisted to
__uloop_literal_N. Top-level static locals were missed becauseLiteralHoistScopeTrackerrequired_braceDepth >= 1.Decision: Fix — remove depth guard; keep literals inline inside recognized static local headers. CS8421 Hint/Suggestion targets remaining scanner gaps (remove
static).Repro 2: Color32 int literals (constraint)
Command:
uloop execute-dynamic-code --code $'using UnityEngine;\nreturn new Color32(255, 0, 0, 255);'Raw
Diagnostics[0](before fix, no Hint):{ "Message": "CS1503: Argument 1: cannot convert from 'int' to 'byte'", "Line": 1, "Column": 20, "ErrorCode": "CS1503", "Hint": "", "Suggestions": [], "Context": "L1:using UnityEngine;\n ^\nL2:return new Color32(255, 0, 0, 255);\n", "PointerColumn": 20 }Plain C# note:
new Color32(255, 0, 0, 255)compiles in normal Unity scripts because numeric literals are compile-time constants with implicit narrowing tobyte. Literal hoisting promotes them tointparameters.Decision: Known constraint; Hint + SKILL.md + explicit cast workaround. Line/caret mismatch deferred to PR 3-5.
Repro 3: static lambda CS8820 (constraint + Hint)
Command:
uloop execute-dynamic-code --code $'System.Func<int,int> f = static x => x * 2;\nreturn f(3);'Raw
Diagnostics[0](after Hint fix):{ "Message": "CS8820: A static anonymous function cannot contain a reference to '__uloop_literal_0'.", "Line": 1, "Column": 42, "ErrorCode": "CS8820", "Hint": "Static lambdas cannot reference hoisted literal parameters. Remove the `static` modifier from the lambda, or use a non-static local function instead.", "Suggestions": [ "Remove the `static` modifier from the lambda." ], "Context": "L1:System.Func<int,int> f = static x => x * 2;\n ^\nL2:return f(3);\n", "PointerColumn": 42 }Convention checklist
.claude//.agents/regenerated viauloop skills install(byte-identical)Implementation
LiteralHoistScopeTracker: remove_braceDepth < 1guard; suppress hoisting inside recognized static local bodiesStaticLocalFunctionHeaderScanner: document unsupported header shapes (where, tuple returns, statement lambdas in expression bodies)DynamicCodeLiteralHoister.LiteralParameterPrefix: shared internal constantDynamicCodeTranspilerConstraintHints: CS8421 / CS8820 / CS1503 guidance via tuple returnSkill/SKILL.md: updated Known transpiler constraintsVerification
dist/darwin-arm64/uloop compile— 0 errors / 0 warningsDynamicCodeLiteralHoisterTests+DynamicCodeTranspilerConstraintHintsTests+DynamicCodeExecutionResponseFactoryTests— 15 passedPost-fix repro 1b (top-level static local — B-9 primary)
{ "Success": true, "Result": "6", "Logs": ["Execution completed successfully"], "CompilationErrors": [], "ErrorMessage": "", "Diagnostics": [] }Post-fix repro 2 (Color32 — constraint with Hint)
Raw
Diagnostics[0]:{ "Message": "CS1503: Argument 1: cannot convert from 'int' to 'byte'", "Line": 1, "Column": 20, "ErrorCode": "CS1503", "Hint": "Literal hoisting promotes integer literals to int values. Unity APIs such as Color32 expect byte components and do not accept implicit int-to-byte conversion after hoisting, even though plain numeric literals compile in normal Unity scripts.", "Suggestions": [ "Cast each component explicitly, for example: new Color32((byte)255, (byte)0, (byte)0, (byte)255)." ], "Context": "L1:using UnityEngine;\n ^\nL2:return new Color32(255, 0, 0, 255);\n", "PointerColumn": 20 }Workaround verified:
{ "Success": true, "Result": "RGBA(255, 0, 0, 255)", "Logs": ["Execution completed successfully"] }