diff --git a/.agents/skills/uloop-hot-reload/references/mechanism-and-lifecycle.md b/.agents/skills/uloop-hot-reload/references/mechanism-and-lifecycle.md index 5cb481e577..c0be60faa4 100644 --- a/.agents/skills/uloop-hot-reload/references/mechanism-and-lifecycle.md +++ b/.agents/skills/uloop-hot-reload/references/mechanism-and-lifecycle.md @@ -3,7 +3,7 @@ ## How a Reload Applies 1. Resolves each file to its compiled assembly via `CompilationPipeline`, then groups the files by that assembly. Each group runs the worker once and produces one shim assembly, so the members one file adds are visible to the bodies edited in its siblings; groups are processed one after another and never affect each other. -2. Rewrites each editable method body of the group into a static shim in an out-of-process Roslyn worker. When an async, iterator, lambda, local-function, or LINQ-query body touches private/internal members, those accesses are rewritten to accessor delegates so the body can compile and run from the shim assembly (the delegation shape in step 4). +2. Rewrites each editable method body of the group into a static shim in an out-of-process Roslyn worker. When an async, iterator, lambda, local-function, or LINQ-query body touches private/internal members, those accesses are rewritten to accessor delegates so the body can compile and run from the shim assembly (the delegation shape in step 4). An `internal` member of a type the reload was not given cannot be rewritten this way, so such a body is `Skipped` (`scope-and-limits.md`). 3. Compiles the group's shims into one assembly against publicized reference copies, loads the result into the Editor domain, and binds every shim type's accessor delegates (`__BindAccessors`) before any patch is applied. 4. Patches each original method with a Harmony transpiler (ID `io.github.hatayama.uloop.hot-reload`) in one of two shapes: transplant copies the shim's IL into the original method, while delegation rewrites the original to forward its arguments to the shim, which runs as normally compiled code. diff --git a/.agents/skills/uloop-hot-reload/references/scope-and-limits.md b/.agents/skills/uloop-hot-reload/references/scope-and-limits.md index 5553989c40..be4772af36 100644 --- a/.agents/skills/uloop-hot-reload/references/scope-and-limits.md +++ b/.agents/skills/uloop-hot-reload/references/scope-and-limits.md @@ -404,7 +404,8 @@ source on disk. When a run skips a method it had patched before, `Warnings` name | Method on a `partial` type when another part of the type changed since the last compile and was not passed, or when a file that names the type has syntax errors (passed or not) | Hot reload binds against the compiled type; pass that file with `--files` too, or run `uloop compile`. For a file with syntax errors, fix it and run hot reload again | | Method on a `partial` type when the other parts could not be checked against the last compile (no source snapshot yet, or more than 50 changed files in the assembly) | Run `uloop compile` | | Method on a `partial` type whose body names a member no source file of the assembly declares | A part generated at compile time (a source generator's output) is not visible to hot reload; run `uloop compile` | -| Method or getter on a `partial` type whose body uses an `internal` member of a type the reload was not given, by its bare name, as a method passed as a delegate, inside a lambda, local function, query, iterator or async method, in a body where a lambda, local function or query works with the member's result, or in a body patched through a delegating shim | Hot reload reaches such a member only as a field, a property or a method call in the method's own statements, written with its receiver (`this.Name`, `Type.Name`, `value.Name`); qualify a bare name, or run `uloop compile` | +| Method or getter whose body uses an `internal` member of a type the reload was not given, by its bare name, inside a lambda, local function, query, iterator or async method, in a body where a lambda, local function or query works with a value hot reload could not resolve (such as the member's result), or in a body patched through a delegating shim | Hot reload reaches such a member only in the method's own statements, written with its receiver (`this.Name`, `Type.Name`, `value.Name`); qualify a bare name, or run `uloop compile` | +| Method or getter on a `partial` type whose body passes an `internal` method of such a type as a delegate, uses an `internal` event of it, or names an `internal` member of it in an object initializer or a property pattern | On a `partial` type hot reload patches such a member only as a field, a property or a method call written with its receiver; run `uloop compile` | | Method or getter on a `partial` type whose body uses an `internal` member of a type in another assembly (through `InternalsVisibleTo`) | Reported as a name no source file of the `partial` type declares; hot reload does not patch this use from a `partial` type yet. Run `uloop compile` | | Method on a struct (value type) | Value-type patching is out of scope | | Generic method, or method on a generic type | Harmony cannot safely patch open generics | diff --git a/.claude/skills/uloop-hot-reload/references/mechanism-and-lifecycle.md b/.claude/skills/uloop-hot-reload/references/mechanism-and-lifecycle.md index 5cb481e577..c0be60faa4 100644 --- a/.claude/skills/uloop-hot-reload/references/mechanism-and-lifecycle.md +++ b/.claude/skills/uloop-hot-reload/references/mechanism-and-lifecycle.md @@ -3,7 +3,7 @@ ## How a Reload Applies 1. Resolves each file to its compiled assembly via `CompilationPipeline`, then groups the files by that assembly. Each group runs the worker once and produces one shim assembly, so the members one file adds are visible to the bodies edited in its siblings; groups are processed one after another and never affect each other. -2. Rewrites each editable method body of the group into a static shim in an out-of-process Roslyn worker. When an async, iterator, lambda, local-function, or LINQ-query body touches private/internal members, those accesses are rewritten to accessor delegates so the body can compile and run from the shim assembly (the delegation shape in step 4). +2. Rewrites each editable method body of the group into a static shim in an out-of-process Roslyn worker. When an async, iterator, lambda, local-function, or LINQ-query body touches private/internal members, those accesses are rewritten to accessor delegates so the body can compile and run from the shim assembly (the delegation shape in step 4). An `internal` member of a type the reload was not given cannot be rewritten this way, so such a body is `Skipped` (`scope-and-limits.md`). 3. Compiles the group's shims into one assembly against publicized reference copies, loads the result into the Editor domain, and binds every shim type's accessor delegates (`__BindAccessors`) before any patch is applied. 4. Patches each original method with a Harmony transpiler (ID `io.github.hatayama.uloop.hot-reload`) in one of two shapes: transplant copies the shim's IL into the original method, while delegation rewrites the original to forward its arguments to the shim, which runs as normally compiled code. diff --git a/.claude/skills/uloop-hot-reload/references/scope-and-limits.md b/.claude/skills/uloop-hot-reload/references/scope-and-limits.md index 5553989c40..be4772af36 100644 --- a/.claude/skills/uloop-hot-reload/references/scope-and-limits.md +++ b/.claude/skills/uloop-hot-reload/references/scope-and-limits.md @@ -404,7 +404,8 @@ source on disk. When a run skips a method it had patched before, `Warnings` name | Method on a `partial` type when another part of the type changed since the last compile and was not passed, or when a file that names the type has syntax errors (passed or not) | Hot reload binds against the compiled type; pass that file with `--files` too, or run `uloop compile`. For a file with syntax errors, fix it and run hot reload again | | Method on a `partial` type when the other parts could not be checked against the last compile (no source snapshot yet, or more than 50 changed files in the assembly) | Run `uloop compile` | | Method on a `partial` type whose body names a member no source file of the assembly declares | A part generated at compile time (a source generator's output) is not visible to hot reload; run `uloop compile` | -| Method or getter on a `partial` type whose body uses an `internal` member of a type the reload was not given, by its bare name, as a method passed as a delegate, inside a lambda, local function, query, iterator or async method, in a body where a lambda, local function or query works with the member's result, or in a body patched through a delegating shim | Hot reload reaches such a member only as a field, a property or a method call in the method's own statements, written with its receiver (`this.Name`, `Type.Name`, `value.Name`); qualify a bare name, or run `uloop compile` | +| Method or getter whose body uses an `internal` member of a type the reload was not given, by its bare name, inside a lambda, local function, query, iterator or async method, in a body where a lambda, local function or query works with a value hot reload could not resolve (such as the member's result), or in a body patched through a delegating shim | Hot reload reaches such a member only in the method's own statements, written with its receiver (`this.Name`, `Type.Name`, `value.Name`); qualify a bare name, or run `uloop compile` | +| Method or getter on a `partial` type whose body passes an `internal` method of such a type as a delegate, uses an `internal` event of it, or names an `internal` member of it in an object initializer or a property pattern | On a `partial` type hot reload patches such a member only as a field, a property or a method call written with its receiver; run `uloop compile` | | Method or getter on a `partial` type whose body uses an `internal` member of a type in another assembly (through `InternalsVisibleTo`) | Reported as a name no source file of the `partial` type declares; hot reload does not patch this use from a `partial` type yet. Run `uloop compile` | | Method on a struct (value type) | Value-type patching is out of scope | | Generic method, or method on a generic type | Harmony cannot safely patch open generics | diff --git a/Assets/Tests/Editor/HotReload/HotReloadInternalMemberHost.cs b/Assets/Tests/Editor/HotReload/HotReloadInternalMemberHost.cs index 2ae28ebefb..922679a62d 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadInternalMemberHost.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadInternalMemberHost.cs @@ -65,6 +65,30 @@ internal static HotReloadInternalMemberHost[] InternalHosts() return new HotReloadInternalMemberHost[] { new HotReloadInternalMemberHost() }; } + internal event System.Action InternalEvent; + + /// + /// Calls the handlers subscribed to and returns how many there + /// were, so a test can tell that a subscription took effect. + /// + [MethodImpl(MethodImplOptions.NoInlining)] + public int RaiseInternalEvent() + { + System.Action handlers = InternalEvent; + if (handlers == null) + { + return 0; + } + + handlers(); + return handlers.GetInvocationList().Length; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void NoOp() + { + } + /// /// A type nested in the host, so a test can name an internal member through the nested type. /// diff --git a/Assets/Tests/Editor/HotReload/HotReloadPartialDerivedFixture.cs b/Assets/Tests/Editor/HotReload/HotReloadPartialDerivedFixture.cs index b5b1ce1fcb..8209232bd4 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadPartialDerivedFixture.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadPartialDerivedFixture.cs @@ -57,5 +57,14 @@ public async System.Threading.Tasks.Task AsyncValue() await System.Threading.Tasks.Task.CompletedTask; return 50; } + + public event Action DerivedEvent; + + [MethodImpl(MethodImplOptions.NoInlining)] + public int RaiseDerivedEvent() + { + DerivedEvent?.Invoke(); + return 60; + } } } diff --git a/Assets/Tests/Editor/HotReload/HotReloadPlainDerivedFixture.cs b/Assets/Tests/Editor/HotReload/HotReloadPlainDerivedFixture.cs index 2956f22c0a..34ce3c5dbf 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadPlainDerivedFixture.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadPlainDerivedFixture.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Runtime.CompilerServices; namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload @@ -50,5 +51,21 @@ public int DerivedProperty [MethodImpl(MethodImplOptions.NoInlining)] get { return 40; } } + + [MethodImpl(MethodImplOptions.NoInlining)] + public async System.Threading.Tasks.Task AsyncValue() + { + await System.Threading.Tasks.Task.CompletedTask; + return 50; + } + + public event Action DerivedEvent; + + [MethodImpl(MethodImplOptions.NoInlining)] + public int RaiseDerivedEvent() + { + DerivedEvent?.Invoke(); + return 60; + } } } diff --git a/Assets/Tests/Editor/HotReload/HotReloadUnpassedInternalMemberE2ETests.cs b/Assets/Tests/Editor/HotReload/HotReloadUnpassedInternalMemberE2ETests.cs index 41cbbd3af6..8250f37e64 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadUnpassedInternalMemberE2ETests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadUnpassedInternalMemberE2ETests.cs @@ -18,9 +18,11 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload /// the reload was not given: what each row reports, and what the method does when called. /// /// - /// Why two groups of edits: a straight-line body runs as IL copied into the patched method, + /// Why separate groups of edits: a straight-line body runs as IL copied into the patched method, /// while a lambda or iterator body runs in code the shim assembly compiles on its own, and a /// name inherited by simple name has to be qualified before the shim can compile it at all. + /// A partial type also skips some uses that run in the patched method, such as a method passed + /// as a delegate, so those have a group of their own. /// public class HotReloadUnpassedInternalMemberE2ETests { @@ -37,6 +39,8 @@ public class HotReloadUnpassedInternalMemberE2ETests private const string IteratorValues = "IteratorValues"; private const string ClosureSeedPlusValue = "ClosureSeedPlusValue"; private const string DerivedPropertyGetter = "get_DerivedProperty"; + private const string AsyncValue = "AsyncValue"; + private const string RaiseDerivedEvent = "RaiseDerivedEvent"; // Public only because a test case argument has to be as visible as the test method. public enum FixtureKind @@ -91,8 +95,11 @@ private static IEnumerable EditsThatRunInThePatchedMethod() } // Edits the shim cannot run as written: a simple name it cannot qualify, or a use inside a - // lambda or iterator, which the shim assembly compiles as ordinary code of its own, also when - // the lambda reaches the member through the result of another internal member. + // lambda, local function, anonymous method, iterator or async method, or in a getter or a + // method that runs through a delegating shim, which the shim assembly compiles as ordinary + // code of its own, also when the lambda reaches the member through the result of another + // internal member. A lambda that only uses such a result as a value is here too, because the + // worker cannot tell it from one that reaches the member through the result. private static IEnumerable EditsThatDoNotRunInThePatchedMethod() { yield return "InheritedInternalMethodBySimpleName"; @@ -104,6 +111,24 @@ private static IEnumerable EditsThatDoNotRunInThePatchedMethod() yield return "InternalStaticMethodInIterator"; yield return "InternalInstanceMethodInLambdaOverAnInternalResult"; yield return "InternalFieldInLambdaParameterFromAnInternalResult"; + yield return "InternalStaticMethodInLocalFunction"; + yield return "InternalStaticMethodInAnonymousMethod"; + yield return "InternalStaticMethodInAsyncMethod"; + yield return "InternalStaticMethodNextToLambdaReadingOwnPrivateFieldInGetter"; + yield return "InternalStaticMethodNextToOwnEventRaise"; + yield return "InternalFieldReadIntoAVarCapturedByALambda"; + } + + // Edits whose body runs as IL copied into the patched method but uses an internal member in a + // form a partial type does not patch: a method passed as a delegate, an event, or a member + // named in an object initializer or a property pattern. + private static IEnumerable EditsThatOnlyAPlainTypeRunsInThePatchedMethod() + { + yield return "InternalStaticMethodPassedAsDelegate"; + yield return "InternalInstanceMethodPassedAsDelegate"; + yield return "InternalEventSubscription"; + yield return "InternalFieldInObjectInitializer"; + yield return "InternalPropertyInPropertyPattern"; } /// @@ -116,7 +141,7 @@ public async Task Run_PlainTypeBodyUsingInternalMemberOfUnpassedType_PatchesBeha BodyEdit edit = FindEdit(editName); HotReloadOrchestratorResult result = await RunEditAsync(FixtureKind.Plain, edit); - AssertPatchedAsEdited(result, FixtureKind.Plain, edit); + await AssertPatchedAsEditedAsync(result, FixtureKind.Plain, edit); } /// @@ -130,7 +155,7 @@ public async Task Run_PartialTypeBodyUsingInternalMemberOfUnpassedType_PatchesBe BodyEdit edit = FindEdit(editName); HotReloadOrchestratorResult result = await RunEditAsync(FixtureKind.Partial, edit); - AssertPatchedAsEdited(result, FixtureKind.Partial, edit); + await AssertPatchedAsEditedAsync(result, FixtureKind.Partial, edit); } /// @@ -144,7 +169,7 @@ public async Task Run_PartialTypeBodyUsingInternalMemberOfATypeOfAnotherAssembly BodyEdit edit = FindEdit("InternalMethodOfPublicTypeOfAnotherAssemblyThroughInternalsVisibleTo"); HotReloadOrchestratorResult result = await RunEditAsync(FixtureKind.Partial, edit); - AssertAppliedAsEditedOrSkipped(result, FixtureKind.Partial, edit); + await AssertAppliedAsEditedOrSkippedAsync(result, FixtureKind.Partial, edit); } /// @@ -158,7 +183,36 @@ public async Task Run_PartialTypeBodyUsingInternalMemberOfUnpassedType_IsApplied BodyEdit edit = FindEdit(editName); HotReloadOrchestratorResult result = await RunEditAsync(FixtureKind.Partial, edit); - AssertAppliedAsEditedOrSkipped(result, FixtureKind.Partial, edit); + await AssertAppliedAsEditedOrSkippedAsync(result, FixtureKind.Partial, edit); + } + + /// + /// What: the same edits on a plain type are either patched and return the edited value, or + /// skipped and keep the compiled behavior. They are never reported as patched and then fail + /// when called, and never fail the file. + /// + [TestCaseSource(nameof(EditsThatDoNotRunInThePatchedMethod))] + public async Task Run_PlainTypeBodyUsingInternalMemberOfUnpassedType_IsAppliedAsEditedOrSkipped(string editName) + { + BodyEdit edit = FindEdit(editName); + HotReloadOrchestratorResult result = await RunEditAsync(FixtureKind.Plain, edit); + + await AssertAppliedAsEditedOrSkippedAsync(result, FixtureKind.Plain, edit); + } + + /// + /// What: an edited body of a plain type that passes an internal method of a compiled type the + /// reload was not given as a delegate, subscribes to its internal event, or names its internal + /// member in an object initializer or a property pattern is patched, and the method returns + /// the edited value. + /// + [TestCaseSource(nameof(EditsThatOnlyAPlainTypeRunsInThePatchedMethod))] + public async Task Run_PlainTypeBodyUsingInternalMemberAsADelegateEventInitializerOrPattern_PatchesBehavior(string editName) + { + BodyEdit edit = FindEdit(editName); + HotReloadOrchestratorResult result = await RunEditAsync(FixtureKind.Plain, edit); + + await AssertPatchedAsEditedAsync(result, FixtureKind.Plain, edit); } /// @@ -177,7 +231,7 @@ public async Task Run_GetterUsingInternalMemberOfUnpassedType_PatchesBehavior(Fi 101); HotReloadOrchestratorResult result = await RunEditAsync(fixture, edit); - AssertPatchedAsEdited(result, fixture, edit); + await AssertPatchedAsEditedAsync(result, fixture, edit); } /// @@ -326,6 +380,52 @@ private static BodyEdit FindEdit(string editName) return BodyEdit.OfDerivedValue( "return System.Array.Exists(HotReloadInternalMemberHost.InternalHosts(), host => host.InternalField > 0) ? 100 : 0;", 100); + case "InternalStaticMethodInLocalFunction": + return BodyEdit.OfDerivedValue( + "int Read() { return HotReloadInternalMemberHost.InternalStaticValue() + 100; }\n" + + " return Read();", + 101); + case "InternalStaticMethodInAnonymousMethod": + return BodyEdit.OfDerivedValue( + "System.Func read = delegate { return HotReloadInternalMemberHost.InternalStaticValue() + 100; };\n" + + " return read();", + 101); + case "InternalStaticMethodInAsyncMethod": + return new BodyEdit(AsyncValue, "return 50;", "return HotReloadInternalMemberHost.InternalStaticValue() + 100;", 101); + case "InternalStaticMethodNextToLambdaReadingOwnPrivateFieldInGetter": + return new BodyEdit( + DerivedPropertyGetter, + "return 40;", + "System.Func read = () => this._seed; return read() + HotReloadInternalMemberHost.InternalStaticValue();", + 1001); + case "InternalStaticMethodNextToOwnEventRaise": + return new BodyEdit(RaiseDerivedEvent, "return 60;", "return 60 + HotReloadInternalMemberHost.InternalStaticValue();", 61); + case "InternalFieldReadIntoAVarCapturedByALambda": + return BodyEdit.OfDerivedValue( + "var seed = new HotReloadInternalMemberHost().InternalField;\n" + + " System.Func read = () => seed + 100;\n" + + " return read();", + 103); + case "InternalStaticMethodPassedAsDelegate": + return BodyEdit.OfDerivedValue( + "System.Func read = HotReloadInternalMemberHost.InternalStaticValue;\n" + + " return read() + 100;", + 101); + case "InternalInstanceMethodPassedAsDelegate": + return BodyEdit.OfDerivedValue( + "System.Func read = new HotReloadInternalMemberHost().InternalInstanceValue;\n" + + " return read() + 100;", + 102); + case "InternalEventSubscription": + return BodyEdit.OfDerivedValue( + "HotReloadInternalMemberHost host = new HotReloadInternalMemberHost();\n" + + " host.InternalEvent += HotReloadInternalMemberHost.NoOp;\n" + + " return host.RaiseInternalEvent() + 100;", + 101); + case "InternalFieldInObjectInitializer": + return BodyEdit.OfDerivedValue("return new HotReloadInternalMemberHost { InternalField = 150 }.InternalField;", 150); + case "InternalPropertyInPropertyPattern": + return BodyEdit.OfDerivedValue("return new HotReloadInternalMemberHost() is { InternalProperty: 4 } ? 104 : 0;", 104); default: throw new ArgumentException("Unknown edit: " + editName); } @@ -343,15 +443,16 @@ private static async Task RunEditAsync(FixtureKind }); } - private static void AssertPatchedAsEdited(HotReloadOrchestratorResult result, FixtureKind fixture, BodyEdit edit) + private static async Task AssertPatchedAsEditedAsync(HotReloadOrchestratorResult result, FixtureKind fixture, BodyEdit edit) { Assert.That(FindRow(result, FixtureTypeName(fixture), edit.MethodName).Kind, Is.EqualTo(HotReloadMethodOutcomeKind.Patched), FormatOutcomes(result)); - Assert.That(CallFixture(fixture, edit.MethodName), Is.EqualTo(edit.EditedValue), FormatOutcomes(result)); + int value = await CallFixtureAsync(fixture, edit.MethodName); + Assert.That(value, Is.EqualTo(edit.EditedValue), FormatOutcomes(result)); } // Why the value follows the row: a skipped row keeps the compiled body, and a patched row // must run the edited one. A call that throws fails the test, which is the point. - private static void AssertAppliedAsEditedOrSkipped(HotReloadOrchestratorResult result, FixtureKind fixture, BodyEdit edit) + private static async Task AssertAppliedAsEditedOrSkippedAsync(HotReloadOrchestratorResult result, FixtureKind fixture, BodyEdit edit) { HotReloadMethodOutcomeKind kind = FindRow(result, FixtureTypeName(fixture), edit.MethodName).Kind; Assert.That( @@ -359,7 +460,8 @@ private static void AssertAppliedAsEditedOrSkipped(HotReloadOrchestratorResult r Is.EqualTo(HotReloadMethodOutcomeKind.Patched).Or.EqualTo(HotReloadMethodOutcomeKind.Skipped), FormatOutcomes(result)); int expected = kind == HotReloadMethodOutcomeKind.Patched ? edit.EditedValue : CompiledValue(fixture, edit.MethodName); - Assert.That(CallFixture(fixture, edit.MethodName), Is.EqualTo(expected), FormatOutcomes(result)); + int value = await CallFixtureAsync(fixture, edit.MethodName); + Assert.That(value, Is.EqualTo(expected), FormatOutcomes(result)); } private static int CompiledValue(FixtureKind fixture, string methodName) @@ -372,12 +474,20 @@ private static int CompiledValue(FixtureKind fixture, string methodName) return 30; case ClosureSeedPlusValue: return 1007; + case AsyncValue: + return 50; + case DerivedPropertyGetter: + return 40; + case RaiseDerivedEvent: + return 60; default: return 1000; } } - private static int CallFixture(FixtureKind fixture, string methodName) + // Why async: the async fixture method is awaited rather than waited on, because an EditMode + // test must not block the main thread on a task. + private static async Task CallFixtureAsync(FixtureKind fixture, string methodName) { if (fixture == FixtureKind.Plain) { @@ -396,6 +506,10 @@ private static int CallFixture(FixtureKind fixture, string methodName) return First(plain.IteratorValues()); case DerivedPropertyGetter: return plain.DerivedProperty; + case AsyncValue: + return await plain.AsyncValue(); + case RaiseDerivedEvent: + return plain.RaiseDerivedEvent(); } } else @@ -415,6 +529,10 @@ private static int CallFixture(FixtureKind fixture, string methodName) return First(partial.IteratorValues()); case DerivedPropertyGetter: return partial.DerivedProperty; + case AsyncValue: + return await partial.AsyncValue(); + case RaiseDerivedEvent: + return partial.RaiseDerivedEvent(); } } diff --git a/Assets/Tests/Editor/HotReload/TransformWorkerPartialTypeTests.cs b/Assets/Tests/Editor/HotReload/TransformWorkerPartialTypeTests.cs index a36ab6f650..da0c1005b3 100644 --- a/Assets/Tests/Editor/HotReload/TransformWorkerPartialTypeTests.cs +++ b/Assets/Tests/Editor/HotReload/TransformWorkerPartialTypeTests.cs @@ -683,9 +683,10 @@ public async Task Run_DerivedPartialTypeBodyUsingBaseMemberOfUnpassedType_Binds( /// /// What: a body of a partial type that uses an internal member of a type the run was not given /// where the patched method cannot run it in place (inside a lambda, a local function or a - /// query, in an iterator or async method, as a method passed as a delegate, by a bare name, or - /// next to a lambda, local function or query that works with the member's result) is skipped - /// with a reason that says the member is internal, not that a part is missing. + /// query, in an iterator or async method, in a method that runs through a delegating shim, as + /// a method passed as a delegate, by a bare name, or next to a lambda, local function or query + /// that works with the member's result) is skipped with a reason that says the member is + /// internal, not that a part is missing. /// [TestCase("Lambda", "OwnOnly", null, "System.Func read = () => HotReloadInternalMemberHost.InternalStaticValue(); return read();")] [TestCase("LocalFunction", "OwnOnly", null, "int Read() { return HotReloadInternalMemberHost.InternalStaticValue(); } return Read();")] @@ -697,6 +698,7 @@ public async Task Run_DerivedPartialTypeBodyUsingBaseMemberOfUnpassedType_Binds( [TestCase("MethodPassedAsDelegate", "OwnOnly", null, "System.Func read = HotReloadInternalMemberHost.InternalStaticValue; return read();")] [TestCase("Iterator", "IteratorValues", "yield return _seed;", "yield return HotReloadInternalMemberHost.InternalStaticValue();")] [TestCase("Async", "AsyncValue", "return 50;", "return HotReloadInternalMemberHost.InternalStaticValue();")] + [TestCase("MethodRaisingItsOwnEvent", "RaiseDerivedEvent", "return 60;", "return 60 + HotReloadInternalMemberHost.InternalStaticValue();")] [TestCase("BareName", "DerivedValue", PartialDerivedValueBody, "return InternalInstanceValue();")] public async Task Skip_PartialTypeBodyUsingInternalMemberWhereItCannotBePatchedInPlace_SaysWhy( string form, @@ -716,6 +718,102 @@ public async Task Skip_PartialTypeBodyUsingInternalMemberWhereItCannotBePatchedI Assert.That(reason, Does.Not.Contain("generated at compile time"), FormatSkipped(result)); } + /// + /// What: a body of a plain type that uses an internal member of a type the run was not given + /// where the patched method cannot reach it (inside a lambda, a local function, an anonymous + /// method or a query, in an iterator or async method, in a getter or a method that runs + /// through a delegating shim, by a bare name, or next to a lambda, local function or query that + /// works with the member's result) is skipped with a reason that says the member is internal. + /// + [TestCase("Lambda", "DerivedValue", PlainDerivedValueBody, "System.Func read = () => HotReloadInternalMemberHost.InternalStaticValue(); return read();")] + [TestCase("LocalFunction", "DerivedValue", PlainDerivedValueBody, "int Read() { return HotReloadInternalMemberHost.InternalStaticValue(); } return Read();")] + [TestCase("AnonymousMethod", "DerivedValue", PlainDerivedValueBody, "System.Func read = delegate { return HotReloadInternalMemberHost.InternalStaticValue(); }; return read();")] + [TestCase("Query", "DerivedValue", PlainDerivedValueBody, "return (from value in new[] { 1 } select value + HotReloadInternalMemberHost.InternalStaticValue()).First();")] + [TestCase("Iterator", "IteratorValues", "yield return _seed;", "yield return HotReloadInternalMemberHost.InternalStaticValue();")] + [TestCase("Async", "AsyncValue", "return 50;", "return HotReloadInternalMemberHost.InternalStaticValue();")] + [TestCase("BareName", "DerivedValue", PlainDerivedValueBody, "return InternalInstanceValue();")] + [TestCase("LambdaUsingTheResult", "DerivedValue", PlainDerivedValueBody, "var host = HotReloadInternalMemberHost.InternalSelf(); System.Func read = () => host.InternalInstanceValue(); return read();")] + [TestCase("LambdaParameterFromTheResult", "DerivedValue", PlainDerivedValueBody, "return System.Array.Exists(HotReloadInternalMemberHost.InternalHosts(), host => host.InternalField > 0) ? 1 : 0;")] + [TestCase("LocalFunctionUsingTheResult", "DerivedValue", PlainDerivedValueBody, "var host = HotReloadInternalMemberHost.InternalSelf(); int Read() { return host.InternalInstanceValue(); } return Read();")] + [TestCase("QueryOverTheResult", "DerivedValue", PlainDerivedValueBody, "var hosts = HotReloadInternalMemberHost.InternalHosts(); return (from host in hosts select host.InternalField).First();")] + [TestCase("GetterWithALambdaReadingAPrivateMember", "get_DerivedProperty", "return 40;", "System.Func read = () => this._seed; return read() + HotReloadInternalMemberHost.InternalStaticValue();")] + [TestCase("MethodRaisingItsOwnEvent", "RaiseDerivedEvent", "return 60;", "return 60 + HotReloadInternalMemberHost.InternalStaticValue();")] + [TestCase("LambdaNextToAnUnresolvedName", "DerivedValue", PlainDerivedValueBody, "System.Func read = () => HotReloadInternalMemberHost.InternalStaticValue(); return read() + NothingDeclaresThisName();")] + public async Task Skip_PlainTypeBodyUsingInternalMemberWhereThePatchedMethodCannotReachIt_SaysWhy( + string form, + string methodName, + string fragment, + string replacement) + { + TransformWorkerClientResult result = await RunWorkerOnSourcesAsync(new[] + { + BuildEditedFixtureSource(PlainDerivedFileName, "PlainInternalOutOfReach" + form + ".cs", fragment, replacement) + }); + + Assert.That(result.Success, Is.True, result.ErrorMessage); + Assert.That(AssertSkipped(result, methodName), Does.Contain("is internal to 'HotReloadInternalMemberHost'"), FormatSkipped(result)); + } + + /// + /// What: a body of a plain type that would work if patched, but that the worker cannot tell + /// from a use the patched method cannot reach, is skipped with the same internal-member reason: + /// a lambda that only uses an internal member's result as a value, an internal member in the + /// first source expression of a query, which runs as the method's own statement, and an + /// internal member named in 'nameof' inside a lambda, which compiles to a constant. The skip is + /// a deliberate choice; a change that lets one of these through rewrites its case. + /// + [TestCase("LambdaCapturingTheResultAsAValue", "var seed = new HotReloadInternalMemberHost().InternalField; System.Func read = () => seed + 100; return read();")] + [TestCase("QuerySourceExpression", "return (from host in HotReloadInternalMemberHost.InternalHosts() select 1).Count();")] + [TestCase("LambdaUsingNameofOfInternalMember", "System.Func name = () => nameof(HotReloadInternalMemberHost.InternalField); return name().Length;")] + public async Task Skip_PlainTypeBodyTheGuardCannotTellFromAnOutOfReachUse_IsSkippedToo(string form, string replacement) + { + TransformWorkerClientResult result = await RunWorkerOnSourcesAsync(new[] + { + BuildEditedFixtureSource(PlainDerivedFileName, "PlainInternalIndistinguishable" + form + ".cs", PlainDerivedValueBody, replacement) + }); + + Assert.That(result.Success, Is.True, result.ErrorMessage); + Assert.That(AssertSkipped(result, "DerivedValue"), Does.Contain("is internal to 'HotReloadInternalMemberHost'"), FormatSkipped(result)); + } + + /// + /// What: a body of a plain type that uses an internal member of a type the run was not given + /// where the patched method runs the use itself is emitted: a method passed as a delegate, an + /// event subscription, or a member named in an object initializer or a property pattern, which + /// a partial type skips, and a result held in an explicitly typed local that a lambda captures. + /// + [TestCase("MethodPassedAsDelegate", "System.Func read = HotReloadInternalMemberHost.InternalStaticValue; return read();")] + [TestCase("EventSubscription", "HotReloadInternalMemberHost host = new HotReloadInternalMemberHost(); host.InternalEvent += HotReloadInternalMemberHost.NoOp; return host.RaiseInternalEvent();")] + [TestCase("ObjectInitializer", "return new HotReloadInternalMemberHost { InternalField = 150 }.InternalField;")] + [TestCase("PropertyPattern", "return new HotReloadInternalMemberHost() is { InternalProperty: 4 } ? 104 : 0;")] + [TestCase("ExplicitlyTypedLocalCapturedByALambda", "int seed = new HotReloadInternalMemberHost().InternalField; System.Func read = () => seed + 100; return read();")] + public async Task Run_PlainTypeBodyUsingInternalMemberInAFormOnlyAPlainTypeEmits_EmitsEntry(string form, string replacement) + { + TransformWorkerClientResult result = await RunWorkerOnSourcesAsync(new[] + { + BuildEditedFixtureSource(PlainDerivedFileName, "PlainInternalInReach" + form + ".cs", PlainDerivedValueBody, replacement) + }); + + Assert.That(result.Success, Is.True, result.ErrorMessage); + AssertEmitted(result, "DerivedValue"); + } + + /// + /// What: a body of a plain type that uses a name nothing declares is still emitted, with no + /// skipped row, so the internal-member guard does not skip it with either of its reasons. + /// + [Test] + public async Task Run_PlainTypeBodyUsingANameNothingDeclares_IsNotSkippedByTheInternalMemberGuard() + { + TransformWorkerClientResult result = await RunWorkerOnSourcesAsync(new[] + { + BuildEditedFixtureSource(PlainDerivedFileName, "PlainUnresolvedName.cs", PlainDerivedValueBody, "return NothingDeclaresThisName() + 1;") + }); + + Assert.That(result.Success, Is.True, result.ErrorMessage); + AssertEmitted(result, "DerivedValue"); + } + /// /// What: a getter of a partial type whose lambda reads a private member, so the whole getter /// runs through a delegating shim, is skipped with a reason that says the internal member it @@ -873,6 +971,30 @@ public async Task Run_SiblingBroughtBack_GetterUsingInternalMemberOfUnpassedType AssertEmitted(result, "get_CallerProperty"); } + /// + /// What: a getter of a plain file brought back to re-bind its active patches that uses an + /// internal member of an unpassed type inside a lambda is skipped with the internal-member + /// reason, not the sibling's reason: passing the file would not help, because the lambda still + /// runs in the shim assembly. + /// + [Test] + public async Task Skip_PlainSiblingBroughtBack_GetterUsingInternalMemberInsideALambda_SaysTheMemberIsInternal() + { + TransformWorkerClientResult result = await RunWorkerOnSourcesAsync(new[] + { + BuildFixtureOwnOnlyEdit("SiblingGetterLambdaPassed.cs", "2"), + AsReappliedSibling(BuildEditedFixtureSource( + CallerFileName, + "SiblingGetterLambdaCaller.cs", + "get { return 30; }", + "get { System.Func read = () => HotReloadInternalMemberHost.InternalStaticValue(); return read(); }")) + }); + + string reason = AssertSkipped(result, "get_CallerProperty"); + Assert.That(reason, Does.Contain("is internal to 'HotReloadInternalMemberHost'"), FormatSkipped(result)); + Assert.That(reason, Does.Not.Contain("brought back to re-bind"), FormatSkipped(result)); + } + /// /// What: a method of a file brought back to re-bind its active patches whose body uses an /// internal member of an unpassed type inside a lambda keeps the sibling's skip reason. diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/mechanism-and-lifecycle.md b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/mechanism-and-lifecycle.md index 5cb481e577..c0be60faa4 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/mechanism-and-lifecycle.md +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/mechanism-and-lifecycle.md @@ -3,7 +3,7 @@ ## How a Reload Applies 1. Resolves each file to its compiled assembly via `CompilationPipeline`, then groups the files by that assembly. Each group runs the worker once and produces one shim assembly, so the members one file adds are visible to the bodies edited in its siblings; groups are processed one after another and never affect each other. -2. Rewrites each editable method body of the group into a static shim in an out-of-process Roslyn worker. When an async, iterator, lambda, local-function, or LINQ-query body touches private/internal members, those accesses are rewritten to accessor delegates so the body can compile and run from the shim assembly (the delegation shape in step 4). +2. Rewrites each editable method body of the group into a static shim in an out-of-process Roslyn worker. When an async, iterator, lambda, local-function, or LINQ-query body touches private/internal members, those accesses are rewritten to accessor delegates so the body can compile and run from the shim assembly (the delegation shape in step 4). An `internal` member of a type the reload was not given cannot be rewritten this way, so such a body is `Skipped` (`scope-and-limits.md`). 3. Compiles the group's shims into one assembly against publicized reference copies, loads the result into the Editor domain, and binds every shim type's accessor delegates (`__BindAccessors`) before any patch is applied. 4. Patches each original method with a Harmony transpiler (ID `io.github.hatayama.uloop.hot-reload`) in one of two shapes: transplant copies the shim's IL into the original method, while delegation rewrites the original to forward its arguments to the shim, which runs as normally compiled code. diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/scope-and-limits.md b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/scope-and-limits.md index 5553989c40..be4772af36 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/scope-and-limits.md +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/scope-and-limits.md @@ -404,7 +404,8 @@ source on disk. When a run skips a method it had patched before, `Warnings` name | Method on a `partial` type when another part of the type changed since the last compile and was not passed, or when a file that names the type has syntax errors (passed or not) | Hot reload binds against the compiled type; pass that file with `--files` too, or run `uloop compile`. For a file with syntax errors, fix it and run hot reload again | | Method on a `partial` type when the other parts could not be checked against the last compile (no source snapshot yet, or more than 50 changed files in the assembly) | Run `uloop compile` | | Method on a `partial` type whose body names a member no source file of the assembly declares | A part generated at compile time (a source generator's output) is not visible to hot reload; run `uloop compile` | -| Method or getter on a `partial` type whose body uses an `internal` member of a type the reload was not given, by its bare name, as a method passed as a delegate, inside a lambda, local function, query, iterator or async method, in a body where a lambda, local function or query works with the member's result, or in a body patched through a delegating shim | Hot reload reaches such a member only as a field, a property or a method call in the method's own statements, written with its receiver (`this.Name`, `Type.Name`, `value.Name`); qualify a bare name, or run `uloop compile` | +| Method or getter whose body uses an `internal` member of a type the reload was not given, by its bare name, inside a lambda, local function, query, iterator or async method, in a body where a lambda, local function or query works with a value hot reload could not resolve (such as the member's result), or in a body patched through a delegating shim | Hot reload reaches such a member only in the method's own statements, written with its receiver (`this.Name`, `Type.Name`, `value.Name`); qualify a bare name, or run `uloop compile` | +| Method or getter on a `partial` type whose body passes an `internal` method of such a type as a delegate, uses an `internal` event of it, or names an `internal` member of it in an object initializer or a property pattern | On a `partial` type hot reload patches such a member only as a field, a property or a method call written with its receiver; run `uloop compile` | | Method or getter on a `partial` type whose body uses an `internal` member of a type in another assembly (through `InternalsVisibleTo`) | Reported as a name no source file of the `partial` type declares; hot reload does not patch this use from a `partial` type yet. Run `uloop compile` | | Method on a struct (value type) | Value-type patching is out of scope | | Generic method, or method on a generic type | Harmony cannot safely patch open generics | diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/OrdinaryMethodQueue.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/OrdinaryMethodQueue.cs index d270524907..450816aa6d 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/OrdinaryMethodQueue.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/OrdinaryMethodQueue.cs @@ -317,7 +317,7 @@ internal static MethodTransformDecision DecideOrdinaryMethodTransform( // added-member binding check, with its own reason. if (!isAddedMethod && decision.SkipReason == null) { - WorkerReason partialSkip = PartialTypeBodyGuard.DescribeSkipOrNull( + WorkerReason unresolvedNameSkip = UnresolvedBodyNameGuard.DescribeSkipOrNull( typeState.TypeDeclaration, semanticModel, methodBodyNode, @@ -325,9 +325,9 @@ internal static MethodTransformDecision DecideOrdinaryMethodTransform( decision, typeState.TypeSymbol, typeState.TargetAssembly); - if (partialSkip != null) + if (unresolvedNameSkip != null) { - decision = MethodTransformDecision.Skip(partialSkip); + decision = MethodTransformDecision.Skip(unresolvedNameSkip); } } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/PropertyGetterClassifier.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/PropertyGetterClassifier.cs index d9c50ae260..3f705d9658 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/PropertyGetterClassifier.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/PropertyGetterClassifier.cs @@ -94,7 +94,7 @@ internal static (bool SkipGetter, MethodTransformDecision Decision) TrySkipPrope return (true, decision); } - WorkerReason partialSkip = PartialTypeBodyGuard.DescribeSkipOrNull( + WorkerReason unresolvedNameSkip = UnresolvedBodyNameGuard.DescribeSkipOrNull( typeDeclaration, semanticModel, getterBodyNode, @@ -102,15 +102,15 @@ internal static (bool SkipGetter, MethodTransformDecision Decision) TrySkipPrope decision, typeSymbol, targetAssembly); - if (partialSkip != null) + if (unresolvedNameSkip != null) { skipped.Add(new WorkerSkipped { SourceProjectRelativePath = sourceProjectRelativePath, Method = WorkerMethodKeys.FormatMethodLabel(getterSymbol), - Reason = partialSkip + Reason = unresolvedNameSkip }); - return (true, MethodTransformDecision.Skip(partialSkip)); + return (true, MethodTransformDecision.Skip(unresolvedNameSkip)); } (WorkerReason addedCallSiteSkip, string calledAddedMethodKey) = AddedCallSiteGuard.EvaluateAddedCallSiteSkipReason( diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/UnpassedInternalMemberUse.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/UnpassedInternalMemberUse.cs index 1a4ba1d812..f32ae221f2 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/UnpassedInternalMemberUse.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/UnpassedInternalMemberUse.cs @@ -17,13 +17,21 @@ /// internal sealed class UnpassedInternalMemberUse { + private const string SimpleNameLookupDiagnosticId = "CS0103"; + private static readonly HashSet MemberNotFoundDiagnosticIds = - new HashSet(StringComparer.Ordinal) { "CS0103", "CS1061", "CS0117" }; + new HashSet(StringComparer.Ordinal) { SimpleNameLookupDiagnosticId, "CS1061", "CS0117" }; - private UnpassedInternalMemberUse(INamedTypeSymbol declaringType, bool canBePatchedInPlace) + private UnpassedInternalMemberUse( + INamedTypeSymbol declaringType, + bool canBePatchedInPlace, + bool mayRunOutsideThePatchedMethod, + bool isSimpleNameLookup) { DeclaringType = declaringType; CanBePatchedInPlace = canBePatchedInPlace; + MayRunOutsideThePatchedMethod = mayRunOutsideThePatchedMethod; + IsSimpleNameLookup = isSimpleNameLookup; } /// The compiled type that declares the member, read with every member visible. @@ -32,6 +40,20 @@ private UnpassedInternalMemberUse(INamedTypeSymbol declaringType, bool canBePatc /// True when the patched method itself runs the use, so a guard may let the body through. internal bool CanBePatchedInPlace { get; } + /// + /// True when the use runs outside the patched method (in a closure, an async or iterator state + /// machine, or a delegating shim), or when a closure in the body works with a value the worker + /// could not resolve, so the worker cannot rule out that the closure reaches the member. + /// + internal bool MayRunOutsideThePatchedMethod { get; } + + /// + /// True when the lookup of a bare name failed (CS0103), such as an inherited member named + /// without 'this.'. Not a member name of an object initializer or a property pattern, which has + /// no receiver either but is looked up on the type being created or matched. + /// + internal bool IsSimpleNameLookup { get; } + /// /// The use reports, or null when the error is not a missing member that /// turns out to be an internal member of a compiled type of the target assembly the run does @@ -80,14 +102,21 @@ internal static UnpassedInternalMemberUse FindOrNull( return null; } + bool mayRunOutsideThePatchedMethod = + RunsOutsideThePatchedMethod(name, bodyNode, methodDeclarationOrNull, decision) + || HasAClosureOverAnUnresolvedValue(bodyNode, semanticModel); + // Why a bare name is out of reach: the shim is a static method, and it qualifies a bare // member name only when the worker binds the name. This member never binds there, so the // name would reach the shim compile unqualified and fail the whole file. bool canBePatchedInPlace = hasReceiver && IsPatchableKind(member, name) - && !RunsOutsideThePatchedMethod(name, bodyNode, methodDeclarationOrNull, decision) - && !HasAClosureOverAnUnresolvedValue(bodyNode, semanticModel); - return new UnpassedInternalMemberUse(member.ContainingType, canBePatchedInPlace); + && !mayRunOutsideThePatchedMethod; + return new UnpassedInternalMemberUse( + member.ContainingType, + canBePatchedInPlace, + mayRunOutsideThePatchedMethod, + error.Id == SimpleNameLookupDiagnosticId); } private static SimpleNameSyntax FindReportedNameOrNull(Diagnostic error, SyntaxNode bodyNode) diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/PartialTypeBodyGuard.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/UnresolvedBodyNameGuard.cs similarity index 51% rename from Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/PartialTypeBodyGuard.cs rename to Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/UnresolvedBodyNameGuard.cs index fab773df84..9dee9ab115 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/PartialTypeBodyGuard.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/UnresolvedBodyNameGuard.cs @@ -7,13 +7,14 @@ using io.github.hatayama.UnityCliLoop.FirstPartyTools; /// -/// Skips a body on a partial type that names something no visible part declares, instead of -/// letting it fail the whole file in the shim compile: a part generated at compile time is -/// invisible to the worker, so the name may be perfectly valid. An internal member of a compiled -/// type the run was not given looks just as missing to the worker; such a use goes through when -/// the patched method runs it itself, and is skipped with a reason that says why otherwise. +/// Skips an edited body that names something the worker cannot resolve, where emitting it would +/// break. On a partial type, a name no visible part declares is skipped instead of failing the +/// whole file in the shim compile: a part generated at compile time is invisible to the worker, +/// so the name may be perfectly valid. An internal member of a compiled type the run was not given +/// looks just as missing to the worker, on a partial or a plain type; a use of it the patched +/// method cannot reach is skipped with a reason that says why. /// -internal static class PartialTypeBodyGuard +internal static class UnresolvedBodyNameGuard { // Why only these: other binding errors are expected (a compiled API still expects the // compiled copy of a type the run declares from source) and the shim compile settles @@ -24,8 +25,9 @@ internal static class PartialTypeBodyGuard new HashSet(StringComparer.Ordinal) { "CS0103", "CS1061", "CS0117", "CS0246" }; /// - /// The skip reason for the body, or null when the type is not partial, or when the body names - /// nothing unresolved apart from internal members the patched method can reach. + /// The skip reason for the body, or null when every name in it the worker cannot resolve is an + /// internal member the patched method can reach, or, on a plain type, is not such a member at + /// all, which the shim compile settles as before. /// internal static WorkerReason DescribeSkipOrNull( TypeDeclarationSyntax typeDeclaration, @@ -36,11 +38,13 @@ internal static WorkerReason DescribeSkipOrNull( INamedTypeSymbol typeSymbol, IAssemblySymbol targetAssembly) { - if (bodyNode == null || !PartialTypeParts.IsPartialOrNestedInPartial(typeDeclaration)) + if (bodyNode == null) { return null; } + bool isPartial = PartialTypeParts.IsPartialOrNestedInPartial(typeDeclaration); + // Why every error and not the first: an internal member the patched method can reach says // nothing about the next error, which may be a name nothing declares. foreach (Diagnostic diagnostic in semanticModel.GetDiagnostics(bodyNode.Span)) @@ -66,10 +70,18 @@ internal static WorkerReason DescribeSkipOrNull( targetAssembly); if (use == null) { - return WorkerReason.Of(HotReloadWorkerReasonCode.MethodTransformPartialBodyUnbound, diagnosticText); + if (isPartial) + { + return WorkerReason.Of(HotReloadWorkerReasonCode.MethodTransformPartialBodyUnbound, diagnosticText); + } + + // Why a plain type goes on: some of these names bind in the shim compile, such as an + // internal member of another assembly that grants access through InternalsVisibleTo, + // and such a body is patched and runs today. The shim compile reports the rest. + continue; } - if (use.CanBePatchedInPlace) + if (IsWithinReach(use, isPartial)) { continue; } @@ -82,4 +94,21 @@ internal static WorkerReason DescribeSkipOrNull( return null; } + + // Why the two rules differ: a partial type skips a body whose names do not resolve, and lets + // through only the uses a run has shown to work in place. A plain type emits such a body, so + // only the uses known to break are closed there: one that runs outside the patched method, + // where the runtime checks access and the call throws, and a bare name, which reaches the shim + // compile unqualified and fails the whole file. A method passed as a delegate, an event, and a + // member named in an object initializer or a property pattern stay patched on a plain type, + // as they were. + private static bool IsWithinReach(UnpassedInternalMemberUse use, bool isPartial) + { + if (isPartial) + { + return use.CanBePatchedInPlace; + } + + return !use.MayRunOutsideThePatchedMethod && !use.IsSimpleNameLookup; + } }