Skip to content

fix: Hot reload now applies static methods that use nameof on an instance member - #3190

Merged
hatayama merged 8 commits into
feature/hot-reload-large-project-feedbackfrom
fix/hot-reload-nameof-in-static-shim
Oct 6, 2026
Merged

hatayama merged 8 commits into
feature/hot-reload-large-project-feedbackfrom
fix/hot-reload-nameof-in-static-shim

Conversation

@hatayama

@hatayama hatayama commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Hot reload now applies an added or edited static method whose body uses nameof on an instance member of its type (for example return nameof(_count).Length;). It used to fail with CS0103: The name '__uloopInstance' does not exist in the current context, and because a file is applied all-or-nothing, every other edit in that file stayed unapplied too.
  • A method body with a property pattern that names a member of the edited type (other is { Count: 3 }, where other has the edited type) no longer makes the transform worker exit with an InvalidCastException.

User Impact

  • Before: private static int AddedNameLength() { return nameof(_stored).Length; } was reported as Failed with CS0103, and a compiled method calling it was skipped. After: the method is Added, the caller is Patched, and the call returns the same value the compiled code would.
  • Before: return Inner is { PublicSeed: 3 } ? 1 : value; in an edited method of the type that declares PublicSeed made the worker exit with code 1, and the whole file was reported Failed. After: the method is patched. This holds for a pattern on a private field of the edited type too: an end-to-end test patches such a method and checks that a matching and a non-matching pattern evaluate correctly.

Cause

  • When the worker copies a method body into its shim (a static method in a separate assembly), it rewrites each bare instance member of the target type to __uloopInstance.Name. It did so inside nameof(...) as well. A static method's shim has no __uloopInstance parameter, so its compile failed with CS0103. The worker's binding checks read the body before this rewrite, so nothing caught it earlier. Edited existing static methods took the same rewrite, not only added ones.
  • The same qualification ran on the member name of a property pattern. That name is an identifier, not an expression, so the syntax rewriter could not put the qualified node back and threw.

Changes

  • Every nameof(...) in a shim body is folded to its string literal when its span has no error diagnostic. nameof is a compile-time constant, so the value is unchanged in every case. The existing fold for added members still runs first and is unchanged; a nameof that does not bind is left as it was.
  • Why "no error diagnostic" and not "has a constant value": Roslyn returns a constant even for an operand that does not bind (see mutation (m1) below: nameof(NoSuchName) became "NoSuchName", nameof(System.Collections.Generic.List<NoSuchType>) became "List"). Folding those would hide a compile error behind a name the original code never compiled with.
  • In the bare-name rewrite, a property pattern's member name is no longer qualified. Only that final qualification is skipped: the accessor-read and added-field rewrites earlier in the same path still run, so a pattern that names a member the shim cannot reach directly keeps today's behavior instead of producing a shim that compiles and then fails at run time.

Input space

Where nameof(operand) sits × what the operand is. Test labels are listed under Verification.

# Method Operand Before (shim → result) After Test
1 added, static instance field of the type (bare) nameof(__uloopInstance.x), no such parameter → CS0103 → Failed "x" → Added W1, E1
2 existing, static instance field (bare) same → Failed "x" → Patched W2
3 existing, static instance method (method group) nameof(__uloopInstance.M) → Failed "M" W2
3b existing, static instance property (bare) nameof(__uloopInstance.P) → Failed "P" W2
4 instance (added or existing) instance field (bare) nameof(__uloopInstance.x) (the parameter exists; compiles, same value) "x" (same value) W3
5 instance this.x nameof(__uloopInstance.x) "x" W3
6 any parameter or local nameof(value) kept "value" (same value) W3
7 any added method or field string literal (existing fold) same existing tests, unchanged
7b existing, instance added method that is skipped (a generic one) and so never registered as added nameof(__uloopInstance.M) kept, naming a member the compiled type lacks "M" W6
8 any event Skipped before the rewrite same existing tests, unchanged
9 any added property Skipped before the rewrite same existing tests, unchanged
10 existing, non-partial type name that does not bind nameof(NoSuchName) kept → shim compile fails → Failed same (not folded) W4
10b existing, non-partial type operand that binds only in part (List<NoSuchType>) kept → Failed same (not folded) W4b
11 added name that does not bind Skipped by the binding check before the rewrite same not reached
12 any static member or type name (bare) nameof(global::T.x) / nameof(T) (compiles) "x" (same value) same branch as row 4
13 instance instance property (bare) nameof(__uloopInstance.P) (compiles) "P" (same value) same branch as row 4
14 existing, non-partial type operand containing an internal member the worker cannot see rewritten nameof(...) kept same (error diagnostic, not folded) unchanged path

Not supported (row 14): in a static method, such an operand that starts with an instance member still fails, as before.

Red and mutations

  • Red: the static shims of W1 and W2 contained nameof(__uloopInstance._privateSeed) and nameof(__uloopInstance.PublicSeed) / nameof(__uloopInstance.ExistingValue) / nameof(__uloopInstance.ExistingGetter) with no __uloopInstance parameter. W3 and W6 kept nameof( (W6: nameof(__uloopInstance.AddedGenericProbe)). E1 reported Failed AddedStoredNameLength() :: CS0103: The name '__uloopInstance' does not exist in the current context, with Scaled skipped because it calls an added method whose shim failed to compile. W4 and W4b passed (they pin today's behavior).
  • (m1) error-diagnostic check removed, folding on the constant alone: both W4 and W4b fail. The shims read return "NoSuchName".Length + value; and return "List".Length + value;.
  • (m2) new fold call removed: W1, W2, W3, W6, and E1 fail.
  • W5 failed before its fix (the worker exited with System.InvalidCastException: Unable to cast object of type 'MemberAccessExpressionSyntax' to type 'IdentifierNameSyntax' in CSharpSyntaxRewriter.VisitNameColon). The fix makes it pass; forcing the new pattern check to false brings the same exception back.
  • E2 fails the same way with the new pattern check forced to false: Failed (file) :: Transform worker exited with code 1 ... InvalidCastException.
  • No existing test expectation changed.

Not covered

  • A property pattern that names a private member where the accessor rewrite applies, for example inside a lambda (System.Func<bool> match = () => Inner is { _privateSeed: 3 };), still makes the worker exit with InvalidCastException: Unable to cast object of type 'InvocationExpressionSyntax' to type 'IdentifierNameSyntax' in CSharpSyntaxRewriter.VisitNameColon. This PR does not change it: run once without committing, the exception is identical with the new pattern check forced to false. An added method's body takes the same accessor path (read from the code, not run).
  • Unverified nameof positions (no dedicated test; the following is read from the code, not run): parameter default values and parameter attributes (kept in the shim; there a nameof of an event or an added property also reaches this rewrite and would be folded to its name, the same value as compiled code), method attributes (dropped with the declaration), local functions and lambdas including static ones, query expressions, case labels and const local initializers, and interpolation holes. All of them go through the same rewrite, and the fold does not branch on position.
  • Unverified operand shapes Type.X, local.X, global::…, and generic names: not tested; the fold does not branch on shape (the value is the last name).
  • A chain that starts with an instance member inside a static method (nameof(_field.Length)): not tested. Unity's default language version is expected (unverified) to reject it in user code. If the worker's compilation reports an error for it, it is not folded and behaves as before; otherwise it is folded to its name. Neither is worse than before.
  • Extended property patterns ({ Inner.PublicSeed: 3 }): C# 10 syntax, which Unity's default language version does not accept.

Verification

New tests:

  • W1 Rewrite_NameofInstanceFieldInAddedStaticMethod_FoldsToStringLiteral
  • W2 Rewrite_NameofInstanceMembersInExistingStaticMethod_FoldsToStringLiterals
  • W3 Rewrite_NameofInInstanceMethod_FoldsToStringLiterals
  • W4 Rewrite_NameofUnboundName_IsNotFolded
  • W4b Rewrite_NameofWithUnboundTypeArgument_IsNotFolded
  • W5 Rewrite_PropertyPatternNamingCompiledMember_KeepsTheBareName
  • W6 Rewrite_NameofAddedMethodThatIsNotApplied_FoldsToStringLiteral
  • E1 Run_AddedStaticMethodNamingInstanceFieldWithNameof_IsAddedAndReturnsTheNameLength (compiles and runs the shim in the Editor, then calls the patched method)
  • E2 Run_PropertyPatternOnCompiledPrivateField_IsPatchedAndEvaluatesThePatterns (same, for an edited method that matches property patterns against a compiled private field)

Run locally:

  • uloop compile: 0 errors.
  • uloop run-tests --filter-type regex over TransformWorkerAddedMemberTests, TransformWorkerAddedFieldTests, TransformWorkerAddedPropertyTests, TransformWorkerEventAccessorTests, TransformWorkerClientTests, TransformWorkerCrossFileTests, TransformWorkerSkippedWriterWarningTests, TransformWorkerIntroducedTypeTests, HotReloadAddedMemberAccessE2ETests, HotReloadCrossFileE2ETests, and HotReloadIntroducedTypeRetainedInternalReferrerE2ETests: all 424 tests pass (on this branch after merging the latest integration branch; 420 before that merge).
  • After E2 was added: HotReloadAddedMemberAccessE2ETests 3/3.
  • scripts/check-code-complexity.sh (C# CA1502 and Go cyclop, max 15) and scripts/check-file-length.sh (max 500 SLOC): no findings.

This pull request targets the integration branch, so CI runs only the Complexity Report, File Length Report, and Dead Code Gate jobs.

The worker qualifies a bare instance member as __uloopInstance.Name even
inside nameof, and a static method's shim has no __uloopInstance parameter,
so an added or edited static method that names an instance member with
nameof fails its shim compile with CS0103. These tests pin the folded
string literal for static and instance methods, a skipped added method,
and the end-to-end result, and pin that nameof operands that do not bind
are left alone.
nameof is a compile-time constant, so the shim does not need its operand
at all. Rewriting the operand instead qualified bare instance members as
__uloopInstance.Name, which a static method's shim has no parameter for,
so the shim failed to compile. A nameof is folded only when its span has
no error diagnostic: an operand that does not bind, or binds only in part,
can still yield a constant that is not the name the code compiles with.
The worker qualifies a bare member of the target type as
__uloopInstance.Name even when the name is the member name of a property
pattern (x is { Member: 1 }). A pattern member name is not an expression,
so the syntax rewriter cannot put the qualified node back and the worker
exits with an InvalidCastException, failing the whole reload.
A property pattern names a member of the matched value, and its name is
not an expression, so qualifying it with __uloopInstance crashed the
worker. Only the final qualification is skipped: the accessor-read and
added-field rewrites earlier in the same path still run, so a pattern
that names a member the shim cannot reach directly keeps today's
behavior instead of compiling to a shim that fails at run time.
…ation

Moving it up to the name-side checks would skip the accessor-read and
added-field rewrites too, which the comment now says at the call site.
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: hatayama/unity-cli-loop/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7485d532-eeb1-4347-a81c-26db6921e1d5
📥 Commits

Reviewing files that changed from the base of the PR and between 4ed563f and c8c5b22.

📒 Files selected for processing (5)
  • Assets/Tests/Editor/HotReload/HotReloadAddedMemberAccessE2ETests.cs
  • Assets/Tests/Editor/HotReload/TransformWorkerAddedMemberTests.cs
  • Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/HarmonyAccessorShimRewrite.cs
  • Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/NameofRules.cs
  • Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimBodyRewriter.cs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The hot-reload shim rewriter now folds bound nameof expressions to string literals and leaves unresolved expressions unchanged. It also preserves bare member names in property patterns. Tests cover added and edited methods, and an end-to-end test checks a patched call using an added static method.

Changes

Hot-reload nameof rewriting

Layer / File(s) Summary
Fold bound nameof expressions
Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/NameofRules.cs, Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimBodyRewriter.cs, Assets/Tests/Editor/HotReload/TransformWorkerAddedMemberTests.cs, Assets/Tests/Editor/HotReload/HotReloadAddedMemberAccessE2ETests.cs
The rewriter folds bound nameof values to string literals and leaves expressions with error diagnostics or no string constant unfurled. Tests cover compiled and added members, parameters, and unbound names. The end-to-end test checks a patched call that uses an added static method.
Preserve property-pattern member names
Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/HarmonyAccessorShimRewrite.cs, Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimBodyRewriter.cs, Assets/Tests/Editor/HotReload/TransformWorkerAddedMemberTests.cs
The rewriter identifies names in property subpatterns and leaves them bare. A test checks that the shim does not add an instance receiver prefix.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to c8c5b

The remaining property-pattern limitation predates this change. No actionable merge-blocking risk from this PR is established.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the primary change: hot reload now applies static methods that use nameof on an instance member.
Description check ✅ Passed The description directly explains the nameof and property-pattern changes, their causes, test coverage, verification results, and known limitations.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

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

…d runs

With the pattern's member name kept bare, an edited method that matches a
property pattern against a private field of its own type is now patched
instead of crashing the worker. This end-to-end test compiles and runs
that shim and checks that a matching and a non-matching pattern evaluate
correctly, so a shim that compiled but read the field wrongly would fail.
…-feedback' into fix/hot-reload-nameof-in-static-shim
@hatayama
hatayama merged commit f2b48b5 into feature/hot-reload-large-project-feedback Oct 6, 2026
5 checks passed
@hatayama
hatayama deleted the fix/hot-reload-nameof-in-static-shim branch October 6, 2026 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant