Repository navigation
fix: Hot reload now applies static methods that use nameof on an instance member - #3190
Conversation
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.
…-reload-nameof-in-static-shim
…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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (5)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe hot-reload shim rewriter now folds bound ChangesHot-reload nameof rewriting
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to 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)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
…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
f2b48b5
into
feature/hot-reload-large-project-feedback
Summary
nameofon an instance member of its type (for examplereturn nameof(_count).Length;). It used to fail withCS0103: 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.other is { Count: 3 }, whereotherhas the edited type) no longer makes the transform worker exit with anInvalidCastException.User Impact
private static int AddedNameLength() { return nameof(_stored).Length; }was reported asFailedwith CS0103, and a compiled method calling it was skipped. After: the method isAdded, the caller isPatched, and the call returns the same value the compiled code would.return Inner is { PublicSeed: 3 } ? 1 : value;in an edited method of the type that declaresPublicSeedmade the worker exit with code 1, and the whole file was reportedFailed. 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
__uloopInstance.Name. It did so insidenameof(...)as well. A static method's shim has no__uloopInstanceparameter, 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.Changes
nameof(...)in a shim body is folded to its string literal when its span has no error diagnostic.nameofis a compile-time constant, so the value is unchanged in every case. The existing fold for added members still runs first and is unchanged; anameofthat does not bind is left as it was.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.Input space
Where
nameof(operand)sits × what the operand is. Test labels are listed under Verification.nameof(__uloopInstance.x), no such parameter → CS0103 → Failed"x"→ Added"x"→ Patchednameof(__uloopInstance.M)→ Failed"M"nameof(__uloopInstance.P)→ Failed"P"nameof(__uloopInstance.x)(the parameter exists; compiles, same value)"x"(same value)this.xnameof(__uloopInstance.x)"x"nameof(value)kept"value"(same value)nameof(__uloopInstance.M)kept, naming a member the compiled type lacks"M"nameof(NoSuchName)kept → shim compile fails → FailedList<NoSuchType>)nameof(global::T.x)/nameof(T)(compiles)"x"(same value)nameof(__uloopInstance.P)(compiles)"P"(same value)nameof(...)keptNot supported (row 14): in a static method, such an operand that starts with an instance member still fails, as before.
Red and mutations
nameof(__uloopInstance._privateSeed)andnameof(__uloopInstance.PublicSeed)/nameof(__uloopInstance.ExistingValue)/nameof(__uloopInstance.ExistingGetter)with no__uloopInstanceparameter. W3 and W6 keptnameof((W6:nameof(__uloopInstance.AddedGenericProbe)). E1 reportedFailed AddedStoredNameLength() :: CS0103: The name '__uloopInstance' does not exist in the current context, withScaledskipped because it calls an added method whose shim failed to compile. W4 and W4b passed (they pin today's behavior).return "NoSuchName".Length + value;andreturn "List".Length + value;.System.InvalidCastException: Unable to cast object of type 'MemberAccessExpressionSyntax' to type 'IdentifierNameSyntax'inCSharpSyntaxRewriter.VisitNameColon). The fix makes it pass; forcing the new pattern check tofalsebrings the same exception back.false:Failed (file) :: Transform worker exited with code 1 ... InvalidCastException.Not covered
System.Func<bool> match = () => Inner is { _privateSeed: 3 };), still makes the worker exit withInvalidCastException: Unable to cast object of type 'InvocationExpressionSyntax' to type 'IdentifierNameSyntax'inCSharpSyntaxRewriter.VisitNameColon. This PR does not change it: run once without committing, the exception is identical with the new pattern check forced tofalse. An added method's body takes the same accessor path (read from the code, not run).nameofpositions (no dedicated test; the following is read from the code, not run): parameter default values and parameter attributes (kept in the shim; there anameofof 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,caselabels andconstlocal initializers, and interpolation holes. All of them go through the same rewrite, and the fold does not branch on position.Type.X,local.X,global::…, and generic names: not tested; the fold does not branch on shape (the value is the last name).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.{ Inner.PublicSeed: 3 }): C# 10 syntax, which Unity's default language version does not accept.Verification
New tests:
Rewrite_NameofInstanceFieldInAddedStaticMethod_FoldsToStringLiteralRewrite_NameofInstanceMembersInExistingStaticMethod_FoldsToStringLiteralsRewrite_NameofInInstanceMethod_FoldsToStringLiteralsRewrite_NameofUnboundName_IsNotFoldedRewrite_NameofWithUnboundTypeArgument_IsNotFoldedRewrite_PropertyPatternNamingCompiledMember_KeepsTheBareNameRewrite_NameofAddedMethodThatIsNotApplied_FoldsToStringLiteralRun_AddedStaticMethodNamingInstanceFieldWithNameof_IsAddedAndReturnsTheNameLength(compiles and runs the shim in the Editor, then calls the patched method)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 regexoverTransformWorkerAddedMemberTests,TransformWorkerAddedFieldTests,TransformWorkerAddedPropertyTests,TransformWorkerEventAccessorTests,TransformWorkerClientTests,TransformWorkerCrossFileTests,TransformWorkerSkippedWriterWarningTests,TransformWorkerIntroducedTypeTests,HotReloadAddedMemberAccessE2ETests,HotReloadCrossFileE2ETests, andHotReloadIntroducedTypeRetainedInternalReferrerE2ETests: all 424 tests pass (on this branch after merging the latest integration branch; 420 before that merge).HotReloadAddedMemberAccessE2ETests3/3.scripts/check-code-complexity.sh(C# CA1502 and Go cyclop, max 15) andscripts/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.