diff --git a/.agents/skills/uloop-hot-reload/references/introduced-types.md b/.agents/skills/uloop-hot-reload/references/introduced-types.md index 009ee4e36..adbc05734 100644 --- a/.agents/skills/uloop-hot-reload/references/introduced-types.md +++ b/.agents/skills/uloop-hot-reload/references/introduced-types.md @@ -61,7 +61,7 @@ When an edited body in the same run names a refused type, its shim compile fails CS0234, or CS0426, or with CS0103 or CS0117 when the body reads a static member of it. That `Failed` row's `Reason` then ends with a note that quotes the refusal and says `uloop compile` clears it. -Three conditions produce a `Failed` row in `IntroducedTypes` instead, and a `Failed` row makes +These conditions produce a `Failed` row in `IntroducedTypes` instead, and a `Failed` row makes `Success` false and leaves every file that shares an assembly with the refused declaration unapplied — no method body of those files is patched in that run, files in other assemblies still apply, and patches from earlier reloads stay active: @@ -72,6 +72,7 @@ apply, and patches from earlier reloads stay active: | A member body of an already-introduced type changed and is neither an ordinary method body nor a getter-only property body | `Changed member body of introduced type requires a compile:` | | Two files of the reload declare the same type | `Introduced type is declared in more than one file of the group:` | | The artifact assembly did not compile | `Introduced-type compilation failed:` | +| This reload did not patch a body the artifact stubs (see "Calling members hot reload adds") | `Not introduced:` | ## Reading the response @@ -139,17 +140,46 @@ applied as `Added` rows. Constructor, setter, init, indexer and event accessor b initializer bodies, member removals, signature changes, and added constructors, operators, events, indexers or nested types still require a compile. +## Calling members hot reload adds + +A new type's ordinary methods and get-only properties can call a method, field or property that +hot reload adds, in the same reload or an earlier one, to a compiled type of the same assembly or +to a type an earlier reload introduced. The artifact compiles each such body as a stub that +throws, and the same reload activates the type only when it holds a patch for every stub, then +patches the real bodies in, so the response shows the type as `Introduced` and those bodies as +`Patched` rows. Later reloads that +include the file keep the type `AlreadyActive` and patch the body again. The file declaring the +addition has to be in the reload: passed, or unchanged since it was last applied, which the +reload pulls back in on its own. + +Constructors, initializers, setters, indexers, operators, event accessors and subscriptions to an +added event cannot be patched, so a call from them still fails the artifact compile (CS1061 or +CS0117) with a hint saying where such a call works. So does a call to an addition in another +assembly, or in a file that changed since it was last applied and is not passed. + +- When this reload does not patch a stubbed body (a generic method, or a method of a struct, is + `Skipped`), no type of that artifact is introduced: the stubbed type's row reads `Not + introduced: calls members that a hot reload added, …`, the other types of the batch + fail with it, and nothing of that assembly's files is applied. +- Another type the same reload introduces cannot name such a type in its member signatures. The + run is refused with `Introduced type '' calls members that a hot reload added, so its + method bodies run through hot reload patches, …`; run `uloop compile`, or name the type only + inside that other type's method bodies. Two types that both call additions this way may name + each other. +- After `--revert-all`, or when the reload that introduces the type fails to apply one of those + patches (that method's row is `Failed`), a stubbed body runs its stub, which throws + `InvalidOperationException` naming the file to reload; reloading that file patches the body in + again. + ## Still needs `uloop compile` Any refused shape above; use of the type from another assembly, from a file that is neither passed to this reload nor already hot-reloaded; anything that reaches the type through Unity (serialization, `[SerializeField]`, Inspector, `AddComponent`, `CreateInstance`, message discovery); a method body edit of an introduced -struct, which is `Skipped` like any struct method; a call to a member an earlier or the same -reload *added* to a compiled type or to an earlier introduced type, because introduced types -compile against the compiled assemblies and retained artifacts only, so the compile fails naming -the missing member and says a hot reload addition shares its name (reloading the addition first -does not help); an added method that passes a type declared from source in this reload to a +struct, which is `Skipped` like any struct method; a call to a member hot reload *added* from a +new type's body that is not an ordinary method or get-only property, or to an addition this +reload does not hold (see "Calling members hot reload adds"); an added method that passes a type declared from source in this reload to a member of an earlier introduced type whose signature was bound to the compiled copy, which is `Skipped` naming both types; and any new or changed `.asmdef` / `.asmref`. A snippet run by `uloop execute-dynamic-code` is the exception: every active artifact is referenced by that compilation, so the snippet can name an 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 be2d4032e..9d208ab92 100644 --- a/.agents/skills/uloop-hot-reload/references/scope-and-limits.md +++ b/.agents/skills/uloop-hot-reload/references/scope-and-limits.md @@ -95,12 +95,11 @@ by name through the wiring entry point, which is how a value or scene reference into an added `[SerializeField]` without a compile — see [added-field-wiring.md](added-field-wiring.md). -An introduced type cannot use members hot reload added to a compiled type, whether they -were added in the same reload or an earlier one: its artifact compiles against the -compiled assemblies and earlier introduced types only, so reloading the addition first -does not help. Such a reference fails with CS1061/CS0117 on a `Failed` `IntroducedTypes` -row, whose `Reason` notes that the missing name matches an addition; run `uloop compile`, -then rerun (issue #2695). +A type a reload introduces can call added members of a compiled type from its ordinary +methods and get-only properties: its artifact compiles those bodies as stubs, and the same +reload patches the real bodies in. From a constructor, initializer, setter, indexer, +operator or event accessor such a reference still fails with CS1061/CS0117 and needs a +compile — see [introduced-types.md](introduced-types.md). Added members are an Editor-session illusion. Any real compile or domain reload drops them all: added methods disappear from the ledger and added-field values are diff --git a/.claude/skills/uloop-hot-reload/references/introduced-types.md b/.claude/skills/uloop-hot-reload/references/introduced-types.md index 009ee4e36..adbc05734 100644 --- a/.claude/skills/uloop-hot-reload/references/introduced-types.md +++ b/.claude/skills/uloop-hot-reload/references/introduced-types.md @@ -61,7 +61,7 @@ When an edited body in the same run names a refused type, its shim compile fails CS0234, or CS0426, or with CS0103 or CS0117 when the body reads a static member of it. That `Failed` row's `Reason` then ends with a note that quotes the refusal and says `uloop compile` clears it. -Three conditions produce a `Failed` row in `IntroducedTypes` instead, and a `Failed` row makes +These conditions produce a `Failed` row in `IntroducedTypes` instead, and a `Failed` row makes `Success` false and leaves every file that shares an assembly with the refused declaration unapplied — no method body of those files is patched in that run, files in other assemblies still apply, and patches from earlier reloads stay active: @@ -72,6 +72,7 @@ apply, and patches from earlier reloads stay active: | A member body of an already-introduced type changed and is neither an ordinary method body nor a getter-only property body | `Changed member body of introduced type requires a compile:` | | Two files of the reload declare the same type | `Introduced type is declared in more than one file of the group:` | | The artifact assembly did not compile | `Introduced-type compilation failed:` | +| This reload did not patch a body the artifact stubs (see "Calling members hot reload adds") | `Not introduced:` | ## Reading the response @@ -139,17 +140,46 @@ applied as `Added` rows. Constructor, setter, init, indexer and event accessor b initializer bodies, member removals, signature changes, and added constructors, operators, events, indexers or nested types still require a compile. +## Calling members hot reload adds + +A new type's ordinary methods and get-only properties can call a method, field or property that +hot reload adds, in the same reload or an earlier one, to a compiled type of the same assembly or +to a type an earlier reload introduced. The artifact compiles each such body as a stub that +throws, and the same reload activates the type only when it holds a patch for every stub, then +patches the real bodies in, so the response shows the type as `Introduced` and those bodies as +`Patched` rows. Later reloads that +include the file keep the type `AlreadyActive` and patch the body again. The file declaring the +addition has to be in the reload: passed, or unchanged since it was last applied, which the +reload pulls back in on its own. + +Constructors, initializers, setters, indexers, operators, event accessors and subscriptions to an +added event cannot be patched, so a call from them still fails the artifact compile (CS1061 or +CS0117) with a hint saying where such a call works. So does a call to an addition in another +assembly, or in a file that changed since it was last applied and is not passed. + +- When this reload does not patch a stubbed body (a generic method, or a method of a struct, is + `Skipped`), no type of that artifact is introduced: the stubbed type's row reads `Not + introduced: calls members that a hot reload added, …`, the other types of the batch + fail with it, and nothing of that assembly's files is applied. +- Another type the same reload introduces cannot name such a type in its member signatures. The + run is refused with `Introduced type '' calls members that a hot reload added, so its + method bodies run through hot reload patches, …`; run `uloop compile`, or name the type only + inside that other type's method bodies. Two types that both call additions this way may name + each other. +- After `--revert-all`, or when the reload that introduces the type fails to apply one of those + patches (that method's row is `Failed`), a stubbed body runs its stub, which throws + `InvalidOperationException` naming the file to reload; reloading that file patches the body in + again. + ## Still needs `uloop compile` Any refused shape above; use of the type from another assembly, from a file that is neither passed to this reload nor already hot-reloaded; anything that reaches the type through Unity (serialization, `[SerializeField]`, Inspector, `AddComponent`, `CreateInstance`, message discovery); a method body edit of an introduced -struct, which is `Skipped` like any struct method; a call to a member an earlier or the same -reload *added* to a compiled type or to an earlier introduced type, because introduced types -compile against the compiled assemblies and retained artifacts only, so the compile fails naming -the missing member and says a hot reload addition shares its name (reloading the addition first -does not help); an added method that passes a type declared from source in this reload to a +struct, which is `Skipped` like any struct method; a call to a member hot reload *added* from a +new type's body that is not an ordinary method or get-only property, or to an addition this +reload does not hold (see "Calling members hot reload adds"); an added method that passes a type declared from source in this reload to a member of an earlier introduced type whose signature was bound to the compiled copy, which is `Skipped` naming both types; and any new or changed `.asmdef` / `.asmref`. A snippet run by `uloop execute-dynamic-code` is the exception: every active artifact is referenced by that compilation, so the snippet can name an 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 be2d4032e..9d208ab92 100644 --- a/.claude/skills/uloop-hot-reload/references/scope-and-limits.md +++ b/.claude/skills/uloop-hot-reload/references/scope-and-limits.md @@ -95,12 +95,11 @@ by name through the wiring entry point, which is how a value or scene reference into an added `[SerializeField]` without a compile — see [added-field-wiring.md](added-field-wiring.md). -An introduced type cannot use members hot reload added to a compiled type, whether they -were added in the same reload or an earlier one: its artifact compiles against the -compiled assemblies and earlier introduced types only, so reloading the addition first -does not help. Such a reference fails with CS1061/CS0117 on a `Failed` `IntroducedTypes` -row, whose `Reason` notes that the missing name matches an addition; run `uloop compile`, -then rerun (issue #2695). +A type a reload introduces can call added members of a compiled type from its ordinary +methods and get-only properties: its artifact compiles those bodies as stubs, and the same +reload patches the real bodies in. From a constructor, initializer, setter, indexer, +operator or event accessor such a reference still fails with CS1061/CS0117 and needs a +compile — see [introduced-types.md](introduced-types.md). Added members are an Editor-session illusion. Any real compile or domain reload drops them all: added methods disappear from the ledger and added-field values are diff --git a/Assets/Tests/Editor/HotReload/HotReloadEntryHomeResolverTests.cs b/Assets/Tests/Editor/HotReload/HotReloadEntryHomeResolverTests.cs index c8dc92955..d5ff91dc0 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadEntryHomeResolverTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadEntryHomeResolverTests.cs @@ -94,6 +94,51 @@ public void Resolve_RowNamesAnAssemblyNoArtifactCarries_Throws() () => resolver.Resolve(fileHome, "EntryHomeResolverUnknownAssembly")); } + /// + /// What: a row that names the artifact this run prepared, which nothing has activated yet, + /// resolves to that artifact's home, so its body can be patched in before activation. + /// + [Test] + public void Resolve_RowNamesThePreparedArtifact_ReturnsThatArtifactHome() + { + HotReloadIntroducedTypeArtifact prepared = CreateArtifact(); + _access.Domain.IntroducedTypes.RegisterPrepared(prepared); + HotReloadEntryHomeResolver resolver = new HotReloadEntryHomeResolver( + _access.Domain, + ResolveProjectRoot(), + prepared); + HotReloadTypeHome fileHome = HotReloadTypeHome.ScriptAssembliesUnderProject( + ResolveProjectRoot(), + ProjectAssemblyName); + + HotReloadTypeHome home = resolver.Resolve(fileHome, prepared.Assembly.GetName().Name); + + Assert.That(home.Kind, Is.EqualTo(HotReloadTypeHomeKind.RetainedArtifact)); + Assert.That(home.DllPath, Is.EqualTo(prepared.DllPath)); + HotReloadLoadedAssemblyResolution resolution = home.ResolveLoadedAssembly( + prepared.Assembly.ManifestModule.ModuleVersionId.ToString()); + Assert.That(resolution.State, Is.EqualTo(HotReloadLoadedAssemblyState.Loaded)); + Assert.That(resolution.Assembly, Is.SameAs(prepared.Assembly)); + } + + /// + /// What: without the prepared artifact handed in, a row naming it is refused like any + /// assembly this domain does not retain, because nothing has activated it. + /// + [Test] + public void Resolve_RowNamesAPreparedArtifactTheResolverWasNotGiven_Throws() + { + HotReloadIntroducedTypeArtifact prepared = CreateArtifact(); + _access.Domain.IntroducedTypes.RegisterPrepared(prepared); + HotReloadEntryHomeResolver resolver = CreateResolver(); + HotReloadTypeHome fileHome = HotReloadTypeHome.ScriptAssembliesUnderProject( + ResolveProjectRoot(), + ProjectAssemblyName); + + Assert.Throws( + () => resolver.Resolve(fileHome, prepared.Assembly.GetName().Name)); + } + private HotReloadEntryHomeResolver CreateResolver() { return new HotReloadEntryHomeResolver(_access.Domain, ResolveProjectRoot()); @@ -101,7 +146,15 @@ private HotReloadEntryHomeResolver CreateResolver() private HotReloadIntroducedTypeArtifact ActivateArtifact() { - HotReloadIntroducedTypeArtifact artifact = new HotReloadIntroducedTypeArtifact( + HotReloadIntroducedTypeArtifact artifact = CreateArtifact(); + _access.Domain.IntroducedTypes.RegisterPrepared(artifact); + _access.Domain.IntroducedTypes.Activate(artifact); + return artifact; + } + + private static HotReloadIntroducedTypeArtifact CreateArtifact() + { + return new HotReloadIntroducedTypeArtifact( CreateArtifactAssembly(), ArtifactDllPath, "entry-home-resolver-artifact.pdb", @@ -115,9 +168,6 @@ private HotReloadIntroducedTypeArtifact ActivateArtifact() "entry-home-resolver-fingerprint", "public class Introduced { }") }); - _access.Domain.IntroducedTypes.RegisterPrepared(artifact); - _access.Domain.IntroducedTypes.Activate(artifact); - return artifact; } // Why a generated name: an artifact assembly is compiled under a name of its own, so a diff --git a/Assets/Tests/Editor/HotReload/HotReloadGroupProcessorLeaveOutTests.cs b/Assets/Tests/Editor/HotReload/HotReloadGroupProcessorLeaveOutTests.cs index ad6ad81f0..dcb1859a5 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadGroupProcessorLeaveOutTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadGroupProcessorLeaveOutTests.cs @@ -306,7 +306,7 @@ private HotReloadGroupFile CreateDefaultFile(string path) _compilationAssembly, HotReloadTypeHome.ScriptAssembliesUnderProject(_projectRoot, AssemblyName), _projectRoot, - new HotReloadFileSinks(new List(), null, null)); + new HotReloadFileSinks(new List(), null, new HotReloadRunStaleSignatureWarnings())); file.IsDefaultSelected = true; return file; } @@ -317,7 +317,7 @@ private static HotReloadGroupFile CreateSibling(HotReloadGroupFile template, str template, path, WriteWorkerSource(path), - new HotReloadFileSinks(new List(), null, null), + new HotReloadFileSinks(new List(), null, new HotReloadRunStaleSignatureWarnings()), null, new HotReloadSiblingBaselineNotices()); } diff --git a/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeAddedMemberHintE2ETests.cs b/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeAddedMemberHintE2ETests.cs index aef827578..40c03f44c 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeAddedMemberHintE2ETests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeAddedMemberHintE2ETests.cs @@ -11,9 +11,9 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload { /// - /// End-to-end coverage of a new type whose body calls a member hot reload adds, in the same - /// reload or an earlier one, to a compiled type or to a type an earlier reload introduced. The - /// introduced-type compilation cannot see such a member, and the failure has to say so. + /// End-to-end coverage of a new type that names a member hot reload adds from a place no + /// patch can replace, such as a constructor body or an enum member. The introduced-type + /// compilation cannot see such a member, and the failure has to say so. /// /// /// Why the owners are not fixtures on disk: a .cs under Assets/ is compiled into the test @@ -21,19 +21,13 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload /// public class HotReloadIntroducedTypeAddedMemberHintE2ETests : HotReloadIntroducedTypeE2ETestBase { - private const string ValueOwnerPath = - "Assets/Tests/Editor/HotReload/UncompiledAddedMemberHintValueOwner.cs"; - private const string UserOwnerPath = "Assets/Tests/Editor/HotReload/UncompiledAddedMemberHintUserOwner.cs"; private const string Namespace = "io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload"; - private const string ValueSimpleName = "HotReloadAddedMemberHintValue"; private const string UserSimpleName = "HotReloadAddedMemberHintUser"; private const string HostValueAnchor = " public int Value()"; private const string CompiledTypeAddedMethodName = "AddedInTheSameReload"; - private const string IntroducedTypeAddedMethodName = "Pong"; - private const string NoExtraMembers = ""; private const string EnumLastMemberAnchor = " Second = 2"; private const string AddedEnumMemberName = "Third"; @@ -45,6 +39,11 @@ public class HotReloadIntroducedTypeAddedMemberHintE2ETests : HotReloadIntroduce private const string HintCore = "share a name with a hot reload addition, from this reload or an earlier one"; + // The part of the hint that says a constructor is one of the places no patch can reach. + private const string UnpatchableBodiesHintCore = + "Constructors, initializers, setters, indexers, operators, event accessors and " + + "subscriptions to an added event cannot."; + private static readonly string CompiledTypeAddedMember = " public int " + CompiledTypeAddedMethodName + "()\n" + " {\n" @@ -52,20 +51,14 @@ public class HotReloadIntroducedTypeAddedMemberHintE2ETests : HotReloadIntroduce + " }\n" + "\n"; - private static readonly string IntroducedTypeAddedMember = - "\n" - + " public int " + IntroducedTypeAddedMethodName + "()\n" - + " {\n" - + " return 9;\n" - + " }\n"; - /// - /// What: a new type calling a method the same reload adds to a compiled type fails to - /// compile, and the failure carries the hint that the introduced type cannot see a hot - /// reload addition, not only the bare CS1061. + /// What: a new type whose constructor calls a method the same reload adds to a compiled + /// type fails to compile, because no patch replaces a constructor body and the artifact + /// therefore cannot stub it, and the failure carries the hint rather than only the bare + /// CS1061. /// [Test] - public async Task Run_NewTypeCallsAMemberTheSameReloadAddsToACompiledType_FailsWithTheHint() + public async Task Run_NewTypeConstructorCallsAMemberTheSameReloadAdds_FailsWithTheHint() { string hostPath = FixturePath("HotReloadCrossFileAddedMemberHost.cs"); @@ -75,70 +68,10 @@ await RunInIntroducedTypeDomainAsync(async _ => new Dictionary { [hostPath] = InsertCompiledTypeMember(File.ReadAllText(hostPath)), - [UserOwnerPath] = BuildUserSource( + [UserOwnerPath] = BuildConstructorUserSource( "new HotReloadCrossFileAddedMemberHost()." + CompiledTypeAddedMethodName + "()") }, - "CompiledSameReload"); - - AssertFailsWithHint(result); - }); - } - - /// - /// What: a new type calling a method the same reload adds to a type an earlier reload - /// introduced fails to compile with the same hint. - /// - [Test] - public async Task Run_NewTypeCallsAMemberTheSameReloadAddsToAnIntroducedType_FailsWithTheHint() - { - await RunInIntroducedTypeDomainAsync(async _ => - { - HotReloadOrchestratorResult introducing = await RunAsync( - new Dictionary { [ValueOwnerPath] = BuildValueSource(NoExtraMembers) }, - "IntroducedSameReloadIntroducing"); - Assert.That(CountFailures(introducing), Is.EqualTo(0), DescribeOutcomes(introducing)); - - HotReloadOrchestratorResult result = await RunAsync( - new Dictionary - { - [ValueOwnerPath] = BuildValueSource(IntroducedTypeAddedMember), - [UserOwnerPath] = BuildUserSource( - "new " + ValueSimpleName + "()." + IntroducedTypeAddedMethodName + "()") - }, - "IntroducedSameReload"); - - AssertFailsWithHint(result); - }); - } - - /// - /// What: splitting the edit does not help. Once an earlier reload has added the method to - /// an introduced type, a later reload introducing a new type that calls it still fails to - /// compile, with the same hint. - /// - [Test] - public async Task Run_NewTypeCallsAMemberAnEarlierReloadAddedToAnIntroducedType_FailsWithTheHint() - { - await RunInIntroducedTypeDomainAsync(async _ => - { - HotReloadOrchestratorResult introducing = await RunAsync( - new Dictionary { [ValueOwnerPath] = BuildValueSource(NoExtraMembers) }, - "IntroducedSplitIntroducing"); - Assert.That(CountFailures(introducing), Is.EqualTo(0), DescribeOutcomes(introducing)); - - HotReloadOrchestratorResult addition = await RunAsync( - new Dictionary { [ValueOwnerPath] = BuildValueSource(IntroducedTypeAddedMember) }, - "IntroducedSplitAddition"); - Assert.That(CountFailures(addition), Is.EqualTo(0), DescribeOutcomes(addition)); - - HotReloadOrchestratorResult result = await RunAsync( - new Dictionary - { - [ValueOwnerPath] = BuildValueSource(IntroducedTypeAddedMember), - [UserOwnerPath] = BuildUserSource( - "new " + ValueSimpleName + "()." + IntroducedTypeAddedMethodName + "()") - }, - "IntroducedSplit"); + "ConstructorSameReload"); AssertFailsWithHint(result); }); @@ -182,6 +115,7 @@ private static void AssertFailsWithHint(HotReloadOrchestratorResult result) { string reason = FindIntroducedTypeFailureReason(result, "CS1061"); Assert.That(reason, Does.Contain(HintCore), DescribeOutcomes(result)); + Assert.That(reason, Does.Contain(UnpatchableBodiesHintCore), DescribeOutcomes(result)); } private static string FindIntroducedTypeFailureReason( @@ -270,32 +204,38 @@ private static string InsertCompiledTypeMember(string hostSource) StringComparison.Ordinal); } - private static string BuildValueSource(string extraMembers) + private static string BuildUserSource(string expression) { return "namespace " + Namespace + "\n" + "{\n" - + " public sealed class " + ValueSimpleName + "\n" + + " public sealed class " + UserSimpleName + "\n" + " {\n" - + " public int Ping()\n" + + " public int Run()\n" + " {\n" - + " return 4;\n" + + " return " + expression + ";\n" + " }\n" - + extraMembers + " }\n" + "}\n"; } - private static string BuildUserSource(string expression) + private static string BuildConstructorUserSource(string expression) { return "namespace " + Namespace + "\n" + "{\n" + " public sealed class " + UserSimpleName + "\n" + " {\n" + + " private readonly int _value;\n" + + "\n" + + " public " + UserSimpleName + "()\n" + + " {\n" + + " _value = " + expression + ";\n" + + " }\n" + + "\n" + " public int Run()\n" + " {\n" - + " return " + expression + ";\n" + + " return _value;\n" + " }\n" + " }\n" + "}\n"; diff --git a/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeCallsAddedMemberE2ETests.cs b/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeCallsAddedMemberE2ETests.cs new file mode 100644 index 000000000..14b5b66eb --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeCallsAddedMemberE2ETests.cs @@ -0,0 +1,686 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Reflection; +using System.Threading; +using System.Threading.Tasks; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// End-to-end coverage of a new type whose methods and getters call members hot reload adds, + /// in the same reload or an earlier one, to a compiled type or to a type an earlier reload + /// introduced. The artifact cannot compile those bodies, so it carries stubs, and the same + /// reload patches the real bodies in before the type is activated. A new type that names such + /// a type in its signatures is still refused, and that refusal has to say why. + /// + /// + /// Why the owners are not fixtures on disk: a .cs under Assets/ is compiled into the test + /// assembly, and a type the compiler already lists is never introduced. + /// + public class HotReloadIntroducedTypeCallsAddedMemberE2ETests : HotReloadIntroducedTypeE2ETestBase + { + private const string ValueOwnerPath = + "Assets/Tests/Editor/HotReload/UncompiledCallsAddedMemberValueOwner.cs"; + + private const string UserOwnerPath = + "Assets/Tests/Editor/HotReload/UncompiledCallsAddedMemberUserOwner.cs"; + + private const string FactoryOwnerPath = + "Assets/Tests/Editor/HotReload/UncompiledCallsAddedMemberFactoryOwner.cs"; + + private const string Namespace = "io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload"; + private const string ValueSimpleName = "HotReloadCallsAddedMemberValue"; + private const string UserSimpleName = "HotReloadCallsAddedMemberUser"; + private const string UserMetadataName = Namespace + "." + UserSimpleName; + private const string FactorySimpleName = "HotReloadCallsAddedMemberFactory"; + private const string FactoryMetadataName = Namespace + "." + FactorySimpleName; + private const string HostValueAnchor = " public int Value()"; + private const string CompiledTypeAddedMethodName = "AddedForTheNewType"; + private const int CompiledTypeAddedValue = 41; + private const string IntroducedTypeAddedMethodName = "Pong"; + private const int IntroducedTypeAddedValue = 9; + private const int PlainValue = 7; + private const int EditedOffset = 100; + private const string NoExtraMembers = ""; + private const string StubMessageCore = "calls members that a hot reload added"; + private const string MixedParametersMethodName = "Mixed"; + + private const string CompiledTypeAddedCall = + "new HotReloadCrossFileAddedMemberHost()." + CompiledTypeAddedMethodName + "()"; + + private const string IntroducedTypeAddedCall = + "new " + ValueSimpleName + "()." + IntroducedTypeAddedMethodName + "()"; + + private static readonly string CompiledTypeAddedMember = + " public int " + CompiledTypeAddedMethodName + "()\n" + + " {\n" + + " return " + CompiledTypeAddedValue + ";\n" + + " }\n" + + "\n"; + + private static readonly string IntroducedTypeAddedMember = + "\n" + + " public int " + IntroducedTypeAddedMethodName + "()\n" + + " {\n" + + " return " + IntroducedTypeAddedValue + ";\n" + + " }\n"; + + // A factory member naming the user type in its signature, and one naming it only inside + // its body. + private static readonly string FactoryMakeMember = + " public " + UserSimpleName + " Make()\n" + + " {\n" + + " return new " + UserSimpleName + "();\n" + + " }\n"; + + private static readonly string FactoryTotalMember = + " public int Total()\n" + + " {\n" + + " return new " + UserSimpleName + "().Run();\n" + + " }\n"; + + // A factory body that calls the addition itself, so the factory is stubbed as well. + private static readonly string FactoryScaledMember = + "\n" + + " public int Scaled()\n" + + " {\n" + + " return " + CompiledTypeAddedCall + " * 2;\n" + + " }\n"; + + // A user member naming the factory in its signature. + private static readonly string UserPairMember = + "\n" + + " public " + FactorySimpleName + " Pair()\n" + + " {\n" + + " return new " + FactorySimpleName + "();\n" + + " }\n"; + + // A stubbed user method with a parameter of each shape a method key spells in its own way: + // an array, a multi-dimensional array, a nested type, a by-ref value and a constructed + // generic type. + private static readonly string UserMixedParametersMember = + "\n" + + " public int " + MixedParametersMethodName + "(\n" + + " int[] values,\n" + + " int[,] grid,\n" + + " System.Environment.SpecialFolder folder,\n" + + " ref int counter,\n" + + " System.Collections.Generic.List list)\n" + + " {\n" + + " counter++;\n" + + " return " + CompiledTypeAddedCall + " + values.Length + grid.Length + list.Count;\n" + + " }\n"; + + /// + /// What: a new type calling a method the same reload adds to a compiled type is introduced; + /// its method and its getter run the added method, and a method that calls nothing added + /// keeps running the body the artifact was compiled with. + /// + [Test] + public async Task Run_NewTypeCallsAMemberTheSameReloadAddsToACompiledType_RunsTheAddition() + { + string hostPath = FixturePath("HotReloadCrossFileAddedMemberHost.cs"); + + await RunInIntroducedTypeDomainAsync(async readArtifact => + { + HotReloadOrchestratorResult result = await RunAsync( + new Dictionary + { + [hostPath] = WriteSource(hostPath, "CompiledSameReload", InsertCompiledTypeMember(File.ReadAllText(hostPath))), + [UserOwnerPath] = WriteSource(UserOwnerPath, "CompiledSameReload", BuildUserSource(CompiledTypeAddedCall)) + }); + + AssertIntroduced(result); + AssertMethodRow(result, HotReloadMethodOutcomeKind.Patched, UserSimpleName + ".Run("); + AssertMethodRow(result, HotReloadMethodOutcomeKind.Patched, UserSimpleName + ".get_Answer("); + Assert.That(Invoke(readArtifact(), "Run"), Is.EqualTo(CompiledTypeAddedValue), DescribeOutcomes(result)); + Assert.That(Invoke(readArtifact(), "get_Answer"), Is.EqualTo(CompiledTypeAddedValue + 1), DescribeOutcomes(result)); + Assert.That(Invoke(readArtifact(), "Plain"), Is.EqualTo(PlainValue), DescribeOutcomes(result)); + }); + } + + /// + /// What: a new type calling a method an earlier reload added is introduced when the file + /// that declares the addition is not passed but is pulled in as an unchanged sibling. + /// + [Test] + public async Task Run_NewTypeCallsAMemberAnEarlierReloadAddedInAnUnpassedSibling_RunsTheAddition() + { + string hostPath = FixturePath("HotReloadCrossFileAddedMemberHost.cs"); + string hostWithAddition = WriteSource( + hostPath, + "SiblingAddition", + InsertCompiledTypeMember(File.ReadAllText(hostPath))); + + await RunInIntroducedTypeDomainAsync(async readArtifact => + { + HotReloadOrchestratorResult addition = await RunAsync( + new Dictionary { [hostPath] = hostWithAddition }); + Assert.That(CountFailures(addition), Is.EqualTo(0), DescribeOutcomes(addition)); + + // Why the host is left out of the files but kept in the overrides: the run has to + // pull it in as the sibling an earlier reload changed, with the source that reload + // applied rather than the fixture on disk. + HotReloadOrchestratorResult result = await HotReloadCompositionRoot.Services.Orchestrator.RunAsync( + new[] { UserOwnerPath }, + contentPathOverride: null, + CancellationToken.None, + new Dictionary + { + [UserOwnerPath] = WriteSource(UserOwnerPath, "SiblingIntroducing", BuildUserSource(CompiledTypeAddedCall)), + [hostPath] = hostWithAddition + }); + + AssertIntroduced(result); + Assert.That(Invoke(readArtifact(), "Run"), Is.EqualTo(CompiledTypeAddedValue), DescribeOutcomes(result)); + }); + } + + /// + /// What: a new type calling a method the same reload adds to a type an earlier reload + /// introduced is introduced and runs that method. + /// + [Test] + public async Task Run_NewTypeCallsAMemberTheSameReloadAddsToAnIntroducedType_RunsTheAddition() + { + await RunInIntroducedTypeDomainAsync(async readArtifact => + { + HotReloadOrchestratorResult introducing = await RunAsync( + new Dictionary + { + [ValueOwnerPath] = WriteSource(ValueOwnerPath, "IntroducedSameReloadIntroducing", BuildValueSource(NoExtraMembers)) + }); + Assert.That(CountFailures(introducing), Is.EqualTo(0), DescribeOutcomes(introducing)); + + HotReloadOrchestratorResult result = await RunAsync( + new Dictionary + { + [ValueOwnerPath] = WriteSource(ValueOwnerPath, "IntroducedSameReload", BuildValueSource(IntroducedTypeAddedMember)), + [UserOwnerPath] = WriteSource(UserOwnerPath, "IntroducedSameReload", BuildUserSource(IntroducedTypeAddedCall)) + }); + + AssertIntroduced(result); + Assert.That(Invoke(readArtifact(), "Run"), Is.EqualTo(IntroducedTypeAddedValue), DescribeOutcomes(result)); + }); + } + + /// + /// What: once an earlier reload has added a method to an introduced type, a later reload + /// introducing a new type that calls it introduces that type and runs the method. + /// + [Test] + public async Task Run_NewTypeCallsAMemberAnEarlierReloadAddedToAnIntroducedType_RunsTheAddition() + { + await RunInIntroducedTypeDomainAsync(async readArtifact => + { + HotReloadOrchestratorResult introducing = await RunAsync( + new Dictionary + { + [ValueOwnerPath] = WriteSource(ValueOwnerPath, "IntroducedSplitIntroducing", BuildValueSource(NoExtraMembers)) + }); + Assert.That(CountFailures(introducing), Is.EqualTo(0), DescribeOutcomes(introducing)); + + string valueWithAddition = WriteSource( + ValueOwnerPath, + "IntroducedSplitAddition", + BuildValueSource(IntroducedTypeAddedMember)); + HotReloadOrchestratorResult addition = await RunAsync( + new Dictionary { [ValueOwnerPath] = valueWithAddition }); + Assert.That(CountFailures(addition), Is.EqualTo(0), DescribeOutcomes(addition)); + + HotReloadOrchestratorResult result = await RunAsync( + new Dictionary + { + [ValueOwnerPath] = valueWithAddition, + [UserOwnerPath] = WriteSource(UserOwnerPath, "IntroducedSplit", BuildUserSource(IntroducedTypeAddedCall)) + }); + + AssertIntroduced(result); + Assert.That(Invoke(readArtifact(), "Run"), Is.EqualTo(IntroducedTypeAddedValue), DescribeOutcomes(result)); + }); + } + + /// + /// What: a stubbed body the transform skips, here a generic method, leaves its stub + /// unpatched, so the type is not introduced, the row names the method, and the group is + /// left unapplied instead of shipping a type that throws where its source works. + /// + [Test] + public async Task Run_StubbedBodyTheTransformSkips_DoesNotIntroduceTheType() + { + string hostPath = FixturePath("HotReloadCrossFileAddedMemberHost.cs"); + + await RunInIntroducedTypeDomainAsync(async _ => + { + HotReloadOrchestratorResult result = await RunAsync( + new Dictionary + { + [hostPath] = WriteSource(hostPath, "SkippedStub", InsertCompiledTypeMember(File.ReadAllText(hostPath))), + [UserOwnerPath] = WriteSource(UserOwnerPath, "SkippedStub", BuildGenericUserSource(CompiledTypeAddedCall)) + }); + + HotReloadIntroducedTypeOutcome row = FindIntroducedTypeRow(result); + Assert.That(row.Kind, Is.EqualTo(HotReloadIntroducedTypeOutcomeKind.Failed), DescribeOutcomes(result)); + Assert.That( + row.Reason, + Does.StartWith("Not introduced: " + UserMetadataName + ".Run`1() " + StubMessageCore), + DescribeOutcomes(result)); + Assert.That(IsActive(UserMetadataName), Is.False, "The held-back type must not be active."); + Assert.That( + CountMethodRows(result, HotReloadMethodOutcomeKind.Added), + Is.EqualTo(0), + "The group must stay unapplied.\n" + DescribeOutcomes(result)); + }); + } + + /// + /// What: later reloads keep the stubbed body patched while nothing changes, and patch the + /// new body in when it is edited, because the record never reads the source as unchanged. + /// + [Test] + public async Task Run_LaterReloads_KeepThePatchedBodyAndTakeItsEdits() + { + string hostPath = FixturePath("HotReloadCrossFileAddedMemberHost.cs"); + string hostWithAddition = WriteSource( + hostPath, + "LaterReloads", + InsertCompiledTypeMember(File.ReadAllText(hostPath))); + string user = WriteSource(UserOwnerPath, "LaterReloads", BuildUserSource(CompiledTypeAddedCall)); + + await RunInIntroducedTypeDomainAsync(async readArtifact => + { + HotReloadOrchestratorResult introducing = await RunAsync( + new Dictionary { [hostPath] = hostWithAddition, [UserOwnerPath] = user }); + AssertIntroduced(introducing); + + HotReloadOrchestratorResult unchanged = await RunAsync( + new Dictionary { [hostPath] = hostWithAddition, [UserOwnerPath] = user }); + Assert.That(CountFailures(unchanged), Is.EqualTo(0), DescribeOutcomes(unchanged)); + Assert.That( + FindIntroducedTypeRow(unchanged).Kind, + Is.EqualTo(HotReloadIntroducedTypeOutcomeKind.AlreadyActive), + DescribeOutcomes(unchanged)); + Assert.That(Invoke(readArtifact(), "Run"), Is.EqualTo(CompiledTypeAddedValue), DescribeOutcomes(unchanged)); + + HotReloadOrchestratorResult edited = await RunAsync( + new Dictionary + { + [hostPath] = hostWithAddition, + [UserOwnerPath] = WriteSource( + UserOwnerPath, + "LaterReloadsEdited", + BuildUserSource(CompiledTypeAddedCall + " + " + EditedOffset)) + }); + Assert.That(CountFailures(edited), Is.EqualTo(0), DescribeOutcomes(edited)); + Assert.That( + Invoke(readArtifact(), "Run"), + Is.EqualTo(CompiledTypeAddedValue + EditedOffset), + DescribeOutcomes(edited)); + }); + } + + /// + /// What: after revert-all the stubbed body runs its stub, which says to reload the owner + /// file, and reloading it patches the body in again. + /// + [Test] + public async Task RevertAll_ThenReloadingTheOwnerFile_RunsTheAdditionAgain() + { + string hostPath = FixturePath("HotReloadCrossFileAddedMemberHost.cs"); + string hostWithAddition = WriteSource( + hostPath, + "RevertAll", + InsertCompiledTypeMember(File.ReadAllText(hostPath))); + string user = WriteSource(UserOwnerPath, "RevertAll", BuildUserSource(CompiledTypeAddedCall)); + + await RunInIntroducedTypeDomainAsync(async readArtifact => + { + HotReloadOrchestratorResult introducing = await RunAsync( + new Dictionary { [hostPath] = hostWithAddition, [UserOwnerPath] = user }); + AssertIntroduced(introducing); + + HotReloadCompositionRoot.Services.StatusExecutor.ExecuteRevertAll(); + TargetInvocationException stubbed = Assert.Throws( + () => Invoke(readArtifact(), "Run")); + Assert.That(stubbed.InnerException, Is.TypeOf()); + Assert.That(stubbed.InnerException.Message, Does.Contain(StubMessageCore)); + Assert.That(stubbed.InnerException.Message, Does.Contain("Reload '" + UserOwnerPath + "' again")); + + HotReloadOrchestratorResult reloaded = await RunAsync( + new Dictionary { [hostPath] = hostWithAddition, [UserOwnerPath] = user }); + Assert.That(CountFailures(reloaded), Is.EqualTo(0), DescribeOutcomes(reloaded)); + Assert.That(Invoke(readArtifact(), "Run"), Is.EqualTo(CompiledTypeAddedValue), DescribeOutcomes(reloaded)); + }); + } + + /// + /// What: a new type naming a stubbed new type in a member signature is refused with the + /// message that says the stubbed type runs through patches, not with the two-step advice + /// for a type an earlier reload loaded, and moving the name into a method body lets both + /// types be introduced. + /// + [Test] + public async Task Run_NewTypeNamesAStubbedTypeInASignature_RefusesUntilTheNameMovesIntoABody() + { + string hostPath = FixturePath("HotReloadCrossFileAddedMemberHost.cs"); + string hostWithAddition = WriteSource( + hostPath, + "SignatureReferrer", + InsertCompiledTypeMember(File.ReadAllText(hostPath))); + string user = WriteSource(UserOwnerPath, "SignatureReferrer", BuildUserSource(CompiledTypeAddedCall)); + + await RunInIntroducedTypeDomainAsync(async readArtifact => + { + HotReloadOrchestratorResult refused = await RunAsync( + new Dictionary + { + [hostPath] = hostWithAddition, + [UserOwnerPath] = user, + [FactoryOwnerPath] = WriteSource( + FactoryOwnerPath, + "SignatureReferrer", + BuildFactorySource(FactoryMakeMember)) + }); + + string description = DescribeOutcomes(refused); + Assert.That(CountFailures(refused), Is.GreaterThan(0), description); + Assert.That( + description, + Does.Contain("Introduced type '" + UserMetadataName + "' " + StubMessageCore + + ", so its method bodies run through hot reload patches, and it appears in member signatures of '" + + FactoryMetadataName + "'."), + description); + Assert.That( + description, + Does.Contain("name '" + UserMetadataName + "' only inside method bodies of '" + FactoryMetadataName + "'"), + description); + Assert.That(description, Does.Not.Contain("reload in two steps"), description); + Assert.That(IsActive(UserMetadataName), Is.False, "A refused run must introduce nothing."); + + HotReloadOrchestratorResult bodyOnly = await RunAsync( + new Dictionary + { + [hostPath] = hostWithAddition, + [UserOwnerPath] = user, + [FactoryOwnerPath] = WriteSource( + FactoryOwnerPath, + "BodyReferrer", + BuildFactorySource(FactoryTotalMember)) + }); + + AssertIntroduced(bodyOnly); + Assert.That(IsActive(FactoryMetadataName), Is.True, DescribeOutcomes(bodyOnly)); + Assert.That( + Invoke(readArtifact(), "Total", FactoryMetadataName), + Is.EqualTo(CompiledTypeAddedValue), + DescribeOutcomes(bodyOnly)); + }); + } + + /// + /// What: two new types that both call a member the reload adds and name each other in + /// their signatures are not refused, because both stay in the source together, and both + /// run their patched bodies. + /// + [Test] + public async Task Run_TwoStubbedNewTypesNameEachOtherInSignatures_IntroducesBoth() + { + string hostPath = FixturePath("HotReloadCrossFileAddedMemberHost.cs"); + + await RunInIntroducedTypeDomainAsync(async readArtifact => + { + HotReloadOrchestratorResult result = await RunAsync( + new Dictionary + { + [hostPath] = WriteSource(hostPath, "MutualReferrers", InsertCompiledTypeMember(File.ReadAllText(hostPath))), + [UserOwnerPath] = WriteSource( + UserOwnerPath, + "MutualReferrers", + BuildUserSource(CompiledTypeAddedCall, UserPairMember)), + [FactoryOwnerPath] = WriteSource( + FactoryOwnerPath, + "MutualReferrers", + BuildFactorySource(FactoryMakeMember + FactoryScaledMember)) + }); + + AssertIntroduced(result); + Assert.That(IsActive(FactoryMetadataName), Is.True, DescribeOutcomes(result)); + AssertMethodRow(result, HotReloadMethodOutcomeKind.Patched, UserSimpleName + ".Run("); + AssertMethodRow(result, HotReloadMethodOutcomeKind.Patched, FactorySimpleName + ".Scaled("); + Assert.That(Invoke(readArtifact(), "Run"), Is.EqualTo(CompiledTypeAddedValue), DescribeOutcomes(result)); + Assert.That( + Invoke(readArtifact(), "Scaled", FactoryMetadataName), + Is.EqualTo(CompiledTypeAddedValue * 2), + DescribeOutcomes(result)); + }); + } + + /// + /// What: a stubbed method taking an array, a multi-dimensional array, a nested type, a + /// by-ref value and a constructed generic type is introduced and runs the addition. The + /// artifact is activated only when the key the worker records for the stub is the key the + /// transform gives the method's entry, so either side spelling one of these shapes in its + /// own way leaves the type out. + /// + [Test] + public async Task Run_StubbedMethodWithParametersOfEveryKeyShape_RunsTheAddition() + { + string hostPath = FixturePath("HotReloadCrossFileAddedMemberHost.cs"); + + await RunInIntroducedTypeDomainAsync(async readArtifact => + { + HotReloadOrchestratorResult result = await RunAsync( + new Dictionary + { + [hostPath] = WriteSource(hostPath, "MixedParameters", InsertCompiledTypeMember(File.ReadAllText(hostPath))), + [UserOwnerPath] = WriteSource( + UserOwnerPath, + "MixedParameters", + BuildUserSource(CompiledTypeAddedCall, UserMixedParametersMember)) + }); + + AssertIntroduced(result); + AssertMethodRow( + result, + HotReloadMethodOutcomeKind.Patched, + UserSimpleName + "." + MixedParametersMethodName + "("); + object[] arguments = + { + new[] { 1, 2 }, + new int[2, 3], + Environment.SpecialFolder.Desktop, + 0, + new List { 5 } + }; + Assert.That( + Invoke(readArtifact(), MixedParametersMethodName, arguments: arguments), + Is.EqualTo(CompiledTypeAddedValue + 2 + 6 + 1), + DescribeOutcomes(result)); + Assert.That(arguments[3], Is.EqualTo(1), "The patched body must write through the by-ref parameter."); + }); + } + + private static void AssertIntroduced(HotReloadOrchestratorResult result) + { + Assert.That(CountFailures(result), Is.EqualTo(0), DescribeOutcomes(result)); + Assert.That( + FindIntroducedTypeRow(result).Kind, + Is.EqualTo(HotReloadIntroducedTypeOutcomeKind.Introduced), + DescribeOutcomes(result)); + } + + private static HotReloadIntroducedTypeOutcome FindIntroducedTypeRow(HotReloadOrchestratorResult result) + { + foreach (HotReloadIntroducedTypeOutcome outcome in result.IntroducedTypes) + { + if (outcome.MetadataName == UserMetadataName) + { + return outcome; + } + } + + Assert.Fail("No row for " + UserMetadataName + ".\n" + DescribeOutcomes(result)); + return null; + } + + private static void AssertMethodRow( + HotReloadOrchestratorResult result, + HotReloadMethodOutcomeKind kind, + string methodFragment) + { + foreach (HotReloadMethodOutcome outcome in result.Methods) + { + if (outcome.Kind == kind + && outcome.Method != null + && outcome.Method.Contains(methodFragment, StringComparison.Ordinal)) + { + return; + } + } + + Assert.Fail("No " + kind + " row mentions " + methodFragment + ".\n" + DescribeOutcomes(result)); + } + + private static int CountMethodRows(HotReloadOrchestratorResult result, HotReloadMethodOutcomeKind kind) + { + int count = 0; + foreach (HotReloadMethodOutcome outcome in result.Methods) + { + if (outcome.Kind == kind) + { + count++; + } + } + + return count; + } + + private static bool IsActive(string metadataName) + { + foreach (HotReloadIntroducedTypeDescriptor descriptor + in HotReloadCompositionRoot.Services.Domain.IntroducedTypes.DescribeActive()) + { + if (descriptor.MetadataName.Value == metadataName) + { + return true; + } + } + + return false; + } + + // Why the method is looked up by name, getters included: the patch replaces the method the + // artifact compiled, so calling it through reflection runs whatever body is patched in. + // Why the arguments are the caller's array: reflection writes a by-ref argument back into + // it, which is how a caller reads what the body wrote. + private static int Invoke( + HotReloadIntroducedTypeArtifact artifact, + string methodName, + string metadataName = UserMetadataName, + object[] arguments = null) + { + Assert.That(artifact, Is.Not.Null, "A reload had to prepare the type before this call."); + Type type = artifact.Assembly.GetType(metadataName, throwOnError: false); + Assert.That(type, Is.Not.Null, "The artifact must hold " + metadataName + "."); + MethodInfo method = type.GetMethod(methodName, BindingFlags.Instance | BindingFlags.Public); + Assert.That(method, Is.Not.Null, metadataName + " must declare " + methodName + "."); + return (int)method.Invoke(Activator.CreateInstance(type), arguments ?? Array.Empty()); + } + + private static Task RunAsync(Dictionary edits) + { + return HotReloadCompositionRoot.Services.Orchestrator.RunAsync( + new List(edits.Keys).ToArray(), + contentPathOverride: null, + CancellationToken.None, + edits); + } + + private static string WriteSource(string ownerPath, string label, string contents) + { + return HotReloadTestSourceWriter.WriteEditedSource( + "CallsAddedMember" + Path.GetFileNameWithoutExtension(ownerPath) + label + ".cs", + contents); + } + + private static string InsertCompiledTypeMember(string hostSource) + { + Assert.That(hostSource, Does.Contain(HostValueAnchor), "Precondition: host value anchor must exist."); + return hostSource.Replace( + HostValueAnchor, + CompiledTypeAddedMember + HostValueAnchor, + StringComparison.Ordinal); + } + + private static string BuildValueSource(string extraMembers) + { + return + "namespace " + Namespace + "\n" + + "{\n" + + " public sealed class " + ValueSimpleName + "\n" + + " {\n" + + " public int Ping()\n" + + " {\n" + + " return 4;\n" + + " }\n" + + extraMembers + + " }\n" + + "}\n"; + } + + private static string BuildUserSource(string expression, string extraMembers = NoExtraMembers) + { + return + "namespace " + Namespace + "\n" + + "{\n" + + " public sealed class " + UserSimpleName + "\n" + + " {\n" + + " public int Run()\n" + + " {\n" + + " return " + expression + ";\n" + + " }\n" + + "\n" + + " public int Answer => " + expression + " + 1;\n" + + "\n" + + " public int Plain()\n" + + " {\n" + + " return " + PlainValue + ";\n" + + " }\n" + + extraMembers + + " }\n" + + "}\n"; + } + + private static string BuildFactorySource(string members) + { + return + "namespace " + Namespace + "\n" + + "{\n" + + " public sealed class " + FactorySimpleName + "\n" + + " {\n" + + members + + " }\n" + + "}\n"; + } + + private static string BuildGenericUserSource(string expression) + { + return + "namespace " + Namespace + "\n" + + "{\n" + + " public sealed class " + UserSimpleName + "\n" + + " {\n" + + " public int Run()\n" + + " {\n" + + " return " + expression + ";\n" + + " }\n" + + " }\n" + + "}\n"; + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeCallsAddedMemberE2ETests.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeCallsAddedMemberE2ETests.cs.meta new file mode 100644 index 000000000..2adbfcb0c --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeCallsAddedMemberE2ETests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 558a7af6a77614edda7c966a424492e4 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeCompileFailureOutcomesTests.cs b/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeCompileFailureOutcomesTests.cs index 62c4120af..36ff4e809 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeCompileFailureOutcomesTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeCompileFailureOutcomesTests.cs @@ -201,8 +201,8 @@ public void Build_DescriptorWithoutDiagnostics_ReportsItAsNotCompiled() } /// - /// Verifies that a missing-member diagnostic naming a member hot reload added earlier - /// explains that an introduced type cannot see it, instead of reading as a typo. + /// Verifies that a missing-member diagnostic naming a member hot reload added explains + /// where a new type can and cannot call such an addition, instead of reading as a typo. /// [Test] public void Build_DiagnosticNamesAnActiveAddedMember_AppendsTheAddedMemberHint() @@ -229,11 +229,14 @@ public void Build_DiagnosticNamesAnActiveAddedMember_AppendsTheAddedMemberHint() rows[0].Reason, Does.EndWith( "One or more of the missing members share a name with a hot reload addition, " - + "from this reload or an earlier one. If the missing member is that addition, " - + "an introduced type cannot see it: it compiles against the compiled " - + "assemblies and earlier introduced types only, so reloading the addition " - + "first does not help. Run 'uloop compile' to make the added members " - + "compiled, then rerun.")); + + "from this reload or an earlier one. A new type can call such an addition " + + "only from its methods and get-only properties, and only when the file that " + + "declares the addition belongs to the same assembly and is part of this " + + "reload: passed, or unchanged since it was last applied. Constructors, " + + "initializers, setters, indexers, operators, event accessors and " + + "subscriptions to an added event cannot. Pass that file too or move the call " + + "into a method, or run 'uloop compile' to make the added members compiled, " + + "then rerun.")); } /// diff --git a/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeFingerprintTests.cs b/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeFingerprintTests.cs index a5c427a47..549b6d378 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeFingerprintTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeFingerprintTests.cs @@ -457,6 +457,97 @@ public void Compare_SeveralBodiesChanged_ListsEveryChangedBodyKey() Assert.That(comparison.ChangedBodyKeys, Is.EqualTo(new List { FieldKey, MethodKey })); } + /// + /// What: stubbing one member replaces only that member's body hash, with a canonical hash + /// the fingerprint recognises as a stub, and leaves every other hash as it was. + /// + [Test] + public void WithStubbedBodies_ListedMember_ReplacesOnlyThatBodyHash() + { + HotReloadIntroducedTypeFingerprint source = CreateFingerprint( + CreateMember(FieldKey, DeclarationHash, BodyHash), + CreateMember(MethodKey, DeclarationHash, BodyHash)); + + HotReloadIntroducedTypeFingerprint stubbed = source.WithStubbedBodies(new[] { MethodKey }); + + Assert.That(stubbed.HeaderHash, Is.EqualTo(HeaderHash)); + Assert.That(stubbed.DefinesHash, Is.EqualTo(DefinesHash)); + Assert.That(stubbed.MemberOrderHash, Is.EqualTo(OrderHash)); + Assert.That(stubbed.Members[0].Key, Is.EqualTo(FieldKey)); + Assert.That(stubbed.Members[0].BodyHash, Is.EqualTo(BodyHash)); + Assert.That(stubbed.Members[0].HasStubbedBody, Is.False); + Assert.That(stubbed.Members[1].Key, Is.EqualTo(MethodKey)); + Assert.That(stubbed.Members[1].DeclarationHash, Is.EqualTo(DeclarationHash)); + Assert.That(stubbed.Members[1].BodyHash, Is.Not.EqualTo(BodyHash)); + Assert.That(HotReloadIntroducedTypeFingerprint.IsCanonicalHash(stubbed.Members[1].BodyHash), Is.True); + Assert.That(stubbed.Members[1].HasStubbedBody, Is.True); + Assert.That(stubbed.HoldsStubbedBodies, Is.True); + Assert.That(source.HoldsStubbedBodies, Is.False); + } + + /// + /// What: the stub survives the wire, so a record read back in a later run still reports it. + /// + [Test] + public void WithStubbedBodies_AfterSerializeAndTryParse_StillHoldsTheStub() + { + HotReloadIntroducedTypeFingerprint stubbed = CreateFingerprint( + CreateMember(FieldKey, DeclarationHash, string.Empty), + CreateMember(MethodKey, DeclarationHash, BodyHash)) + .WithStubbedBodies(new[] { MethodKey }); + + bool parsed = HotReloadIntroducedTypeFingerprint.TryParse( + stubbed.Serialize(), + out HotReloadIntroducedTypeFingerprint value); + + Assert.That(parsed, Is.True); + Assert.That(value.HoldsStubbedBodies, Is.True); + Assert.That(value.Members[1].HasStubbedBody, Is.True); + Assert.That(value.Members[0].HasStubbedBody, Is.False); + } + + /// + /// What: a stubbed record compared with the fingerprint of the source it was built from is a + /// body-only difference on the stubbed member, so the unchanged source never reads as Identical. + /// + [Test] + public void Compare_SourceAgainstItsStubbedRecord_ReturnsBodyOnlyWithTheStubbedKey() + { + HotReloadIntroducedTypeFingerprint source = CreateFingerprint( + CreateMember(FieldKey, DeclarationHash, BodyHash), + CreateMember(MethodKey, DeclarationHash, BodyHash)); + HotReloadIntroducedTypeFingerprint record = source.WithStubbedBodies(new[] { MethodKey }); + + HotReloadIntroducedTypeFingerprintComparison comparison = HotReloadIntroducedTypeFingerprint.Compare(record, source); + + Assert.That(comparison.Kind, Is.EqualTo(HotReloadIntroducedTypeFingerprintDifference.BodyOnly)); + Assert.That(comparison.ChangedBodyKeys, Is.EqualTo(new List { MethodKey })); + } + + /// + /// What: asking to stub a key the fingerprint does not hold is refused. + /// + [Test] + public void WithStubbedBodies_KeyThatNamesNoMember_Throws() + { + HotReloadIntroducedTypeFingerprint source = CreateFingerprint(CreateMember(MethodKey, DeclarationHash, BodyHash)); + + Assert.Throws(() => source.WithStubbedBodies(new[] { "type:Missing()" })); + } + + /// + /// What: asking to stub a member that has no body is refused, since the artifact has no body to stub. + /// + [Test] + public void WithStubbedBodies_MemberWithoutBody_Throws() + { + HotReloadIntroducedTypeFingerprint source = CreateFingerprint( + CreateMember(FieldKey, DeclarationHash, string.Empty), + CreateMember(MethodKey, DeclarationHash, BodyHash)); + + Assert.Throws(() => source.WithStubbedBodies(new[] { FieldKey })); + } + private static HotReloadIntroducedTypeFingerprint CreateFingerprint( params HotReloadIntroducedTypeMemberFingerprint[] members) { diff --git a/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeMemberAdditionE2ETests.cs b/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeMemberAdditionE2ETests.cs index fbb10d3db..3ddeb9874 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeMemberAdditionE2ETests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeMemberAdditionE2ETests.cs @@ -32,20 +32,17 @@ public class HotReloadIntroducedTypeMemberAdditionE2ETests : HotReloadIntroduced private const string HostValueAnchor = " public int Value()"; private const string CompiledTypeAddedMethodName = "AddedForIntroducedType"; - private const string InvisibleAddedMemberTypeName = "HotReloadAddedMemberInvisibleIntroducedValue"; - private const string AddedMemberInvisibleHint = - "One or more of the missing members share a name with a hot reload addition, from this " - + "reload or an earlier one. If the missing member is that addition, an introduced type " - + "cannot see it: it compiles against the compiled assemblies and earlier introduced " - + "types only, so reloading the addition first does not help. Run 'uloop compile' to " - + "make the added members compiled, then rerun."; + private const int CompiledTypeAddedValue = 41; + private const string AddedMemberCallerTypeName = "HotReloadAddedMemberCallerIntroducedValue"; + private const string AddedMemberCallerMetadataName = + "io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload." + AddedMemberCallerTypeName; // The method the first reload adds to a compiled type. It exists only in that reload's // shim, never in the assembly on disk the introduced-type compilation reads. private static readonly string CompiledTypeAddedMember = " public int " + CompiledTypeAddedMethodName + "()\n" + " {\n" - + " return 41;\n" + + " return " + CompiledTypeAddedValue + ";\n" + " }\n" + "\n"; @@ -238,12 +235,12 @@ await RunInIntroducedTypeDomainAsync(async readArtifact => } /// - /// What: a type introduced by this reload whose body calls a member an earlier reload - /// added to a compiled type fails to compile, and the failure explains that an introduced - /// type cannot see a hot-reload addition instead of leaving the bare compiler error. + /// What: a type introduced by this reload whose method calls a member an earlier reload + /// added to a compiled type is introduced, and the method runs that member because the + /// reload patches its real body over the stub the artifact compiled. /// [Test] - public async Task Run_IntroducedTypeUsesAMemberAddedToACompiledType_FailsWithTheAddedMemberHint() + public async Task Run_IntroducedTypeUsesAMemberAddedToACompiledType_RunsTheAddedMember() { string hostPath = FixturePath("HotReloadCrossFileAddedMemberHost.cs"); string callerPath = FixturePath("HotReloadCrossFileAddedMemberCaller.cs"); @@ -261,34 +258,19 @@ await RunInIntroducedTypeDomainAsync(async readArtifact => callerPath, CreateIntroducedTypeUsingTheAddedMemberEdits(hostPath, callerPath)); - string reason = FindIntroducedTypeFailureReason(introduced); + Assert.That(CountFailures(introduced), Is.EqualTo(0), DescribeOutcomes(introduced)); + AssertOutcome( + introduced, + HotReloadMethodOutcomeKind.Patched, + AddedMemberCallerMetadataName + ".Compute()"); Assert.That( - reason, - Does.Contain("CS1061"), - "The compilation of the introduced type must report the member as missing.\n" - + DescribeOutcomes(introduced)); - Assert.That( - reason, - Does.EndWith(AddedMemberInvisibleHint), - "The failure must explain why the added member is invisible here.\n" + ReadComputedValue(readArtifact(), AddedMemberCallerMetadataName), + Is.EqualTo(CompiledTypeAddedValue), + "The introduced method must run the member the earlier reload added.\n" + DescribeOutcomes(introduced)); }); } - private static string FindIntroducedTypeFailureReason(HotReloadOrchestratorResult result) - { - foreach (HotReloadIntroducedTypeOutcome outcome in result.IntroducedTypes) - { - if (outcome.Kind == HotReloadIntroducedTypeOutcomeKind.Failed) - { - return outcome.Reason; - } - } - - Assert.Fail("No introduced type failed.\n" + DescribeOutcomes(result)); - return null; - } - private static Dictionary CreateCompiledTypeAdditionEdits( string hostPath, string callerPath) @@ -305,7 +287,8 @@ private static Dictionary CreateCompiledTypeAdditionEdits( } // The addition stays declared so it is still active, and a new type declared beside it - // calls it - which only the shim of the previous reload can answer. + // calls it, so the artifact compiles a stub for that body and this reload patches the + // real one in. private static Dictionary CreateIntroducedTypeUsingTheAddedMemberEdits( string hostPath, string callerPath) @@ -336,7 +319,7 @@ private static string InsertTypeUsingTheAddedMember(string hostSource) { Assert.That(hostSource, Does.Contain(HostTypeAnchor), "Precondition: host type anchor must exist."); string introduced = - " public sealed class " + InvisibleAddedMemberTypeName + "\n" + " public sealed class " + AddedMemberCallerTypeName + "\n" + " {\n" + " public int Compute()\n" + " {\n" @@ -399,11 +382,13 @@ private static int CountAddedMethods(HotReloadOrchestratorResult result) return count; } - private static int ReadComputedValue(HotReloadIntroducedTypeArtifact artifact) + private static int ReadComputedValue( + HotReloadIntroducedTypeArtifact artifact, + string metadataName = IntroducedTypeMetadataName) { Assert.That(artifact, Is.Not.Null, "A reload had to introduce the type before this check."); - Type introducedType = artifact.Assembly.GetType(IntroducedTypeMetadataName, throwOnError: false); - Assert.That(introducedType, Is.Not.Null, "The artifact must hold " + IntroducedTypeMetadataName + "."); + Type introducedType = artifact.Assembly.GetType(metadataName, throwOnError: false); + Assert.That(introducedType, Is.Not.Null, "The artifact must hold " + metadataName + "."); MethodInfo compute = introducedType.GetMethod( "Compute", BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); diff --git a/Assets/Tests/Editor/HotReload/HotReloadStubbedMemberCoverageTests.cs b/Assets/Tests/Editor/HotReload/HotReloadStubbedMemberCoverageTests.cs new file mode 100644 index 000000000..1f9709b63 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadStubbedMemberCoverageTests.cs @@ -0,0 +1,236 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Reflection; +using System.Reflection.Emit; + +using NUnit.Framework; + +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +using UnityCompilationAssembly = UnityEditor.Compilation.Assembly; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// EditMode coverage for the check that holds a prepared artifact back until this run patches + /// every body the artifact stubs: which files count as patching a body, and which rows the + /// artifact's types get when one stub is left in place. + /// + public class HotReloadStubbedMemberCoverageTests + { + private const string AssemblyName = "UnityCLILoop.Tests.Editor.HotReload"; + private const string OwnerPath = "Assets/StubbedCaller.cs"; + private const string SiblingOwnerPath = "Assets/StubbedSibling.cs"; + private const string CallerMetadataName = "Example.StubbedCaller"; + private const string SiblingMetadataName = "Example.StubbedSibling"; + private const string UsesKey = "Example.StubbedCaller::Uses()"; + private const string GetterKey = "Example.StubbedCaller::get_Value()"; + private const string NestedParameterMetadataName = "Example.Host/Inner"; + private const string NestedParameterKey = "Example.StubbedCaller::Uses(" + NestedParameterMetadataName + ")"; + + /// + /// What: an artifact whose every stub has an entry in a resolved file may be activated. + /// + [Test] + public void FindUncoveredTypes_EveryStubResolved_ReturnsNoRows() + { + HotReloadIntroducedTypeArtifact artifact = CreateArtifact(new[] { UsesKey, GetterKey }); + HotReloadPreparedGroupFile resolved = CreateResolved( + CreateEntry("Uses"), + CreateEntry("get_Value"), + CreateEntry("Unrelated")); + + List rows = + HotReloadStubbedMemberCoverage.FindUncoveredTypes(artifact, new[] { resolved }); + + Assert.That(rows, Is.Empty); + } + + /// + /// What: one stub without an entry holds the whole artifact back: the stubbed type's row + /// names the method left unpatched, and the sibling type fails with it. + /// + [Test] + public void FindUncoveredTypes_OneStubUnresolved_FailsEveryTypeOfTheArtifact() + { + HotReloadIntroducedTypeArtifact artifact = CreateArtifact(new[] { UsesKey, GetterKey }); + HotReloadPreparedGroupFile resolved = CreateResolved(CreateEntry("Uses")); + + List rows = + HotReloadStubbedMemberCoverage.FindUncoveredTypes(artifact, new[] { resolved }); + + Assert.That(rows, Has.Count.EqualTo(2)); + Assert.That(rows[0].Kind, Is.EqualTo(HotReloadIntroducedTypeOutcomeKind.Failed)); + Assert.That(rows[0].MetadataName, Is.EqualTo(CallerMetadataName)); + Assert.That(rows[0].OwnerProjectRelativePath, Is.EqualTo(OwnerPath)); + Assert.That( + rows[0].Reason, + Does.StartWith("Not introduced: Example.StubbedCaller.get_Value() calls members that a hot reload added")); + Assert.That(rows[0].Reason, Does.Not.Contain("Uses")); + Assert.That(rows[1].Kind, Is.EqualTo(HotReloadIntroducedTypeOutcomeKind.Failed)); + Assert.That(rows[1].MetadataName, Is.EqualTo(SiblingMetadataName)); + Assert.That(rows[1].Reason, Does.Contain("another declaration in the same introduced-type batch")); + } + + /// + /// What: an entry of a file the group skipped patches nothing, so it does not cover a stub. + /// + [Test] + public void FindUncoveredTypes_StubOnlyInASkippedFile_FailsTheArtifact() + { + HotReloadIntroducedTypeArtifact artifact = CreateArtifact(new[] { UsesKey }); + HotReloadPreparedGroupFile skipped = HotReloadPreparedGroupFile.SkippedByGroup(CreateFile(OwnerPath)); + HotReloadPreparedGroupFile noEntries = HotReloadPreparedGroupFile.NoEntriesToApply(CreateFile(SiblingOwnerPath)); + + List rows = + HotReloadStubbedMemberCoverage.FindUncoveredTypes(artifact, new[] { skipped, noEntries }); + + Assert.That(rows, Has.Count.EqualTo(2)); + Assert.That(rows[0].Reason, Does.StartWith("Not introduced: Example.StubbedCaller.Uses() calls")); + } + + /// + /// What: a run with no entry at all leaves every stub in place, so it fails the artifact. + /// + [Test] + public void FindUncoveredTypes_RunWithoutEntries_FailsTheArtifact() + { + HotReloadIntroducedTypeArtifact artifact = CreateArtifact(new[] { UsesKey, GetterKey }); + + List rows = HotReloadStubbedMemberCoverage.FindUncoveredTypes(artifact, null); + + Assert.That(rows, Has.Count.EqualTo(2)); + Assert.That( + rows[0].Reason, + Does.StartWith( + "Not introduced: Example.StubbedCaller.Uses(), Example.StubbedCaller.get_Value() call members")); + } + + /// + /// What: a stub whose parameter type is nested is covered by the entry the worker writes + /// for it, because both spell the nested type in metadata form. + /// + [Test] + public void FindUncoveredTypes_StubWithANestedParameterTypeResolved_ReturnsNoRows() + { + HotReloadIntroducedTypeArtifact artifact = CreateArtifact(new[] { NestedParameterKey }); + HotReloadPreparedGroupFile resolved = CreateResolved( + CreateEntry("Uses", new[] { NestedParameterMetadataName })); + + List rows = + HotReloadStubbedMemberCoverage.FindUncoveredTypes(artifact, new[] { resolved }); + + Assert.That(rows, Is.Empty); + } + + /// + /// What: the row of an unpatched stub whose parameter type is nested names that type in + /// reflection form, the way the method rows of the same run name it. + /// + [Test] + public void FindUncoveredTypes_StubWithANestedParameterTypeUnresolved_NamesItInReflectionForm() + { + HotReloadIntroducedTypeArtifact artifact = CreateArtifact(new[] { NestedParameterKey }); + + List rows = HotReloadStubbedMemberCoverage.FindUncoveredTypes(artifact, null); + + Assert.That( + rows[0].Reason, + Does.StartWith("Not introduced: Example.StubbedCaller.Uses(Example.Host+Inner) calls")); + } + + /// + /// What: an artifact that stubs nothing is never held back, even by a run with no entry. + /// + [Test] + public void FindUncoveredTypes_ArtifactWithoutStubs_ReturnsNoRows() + { + HotReloadIntroducedTypeArtifact artifact = CreateArtifact(Array.Empty()); + + List rows = HotReloadStubbedMemberCoverage.FindUncoveredTypes(artifact, null); + + Assert.That(rows, Is.Empty); + } + + private static HotReloadIntroducedTypeArtifact CreateArtifact(string[] callerStubKeys) + { + AssemblyName assemblyName = new AssemblyName("UloopIntroducedTypes_" + Guid.NewGuid().ToString("N")); + Assembly assembly = AssemblyBuilder.DefineDynamicAssembly(assemblyName, AssemblyBuilderAccess.Run); + return new HotReloadIntroducedTypeArtifact( + assembly, + "stubbed-member-coverage-artifact.dll", + "stubbed-member-coverage-artifact.pdb", + new List + { + new HotReloadIntroducedTypeDescriptor( + AssemblyName, + "original-mvid", + CallerMetadataName, + OwnerPath, + "caller-fingerprint", + "public class StubbedCaller { }", + callerStubKeys), + new HotReloadIntroducedTypeDescriptor( + AssemblyName, + "original-mvid", + SiblingMetadataName, + SiblingOwnerPath, + "sibling-fingerprint", + "public class StubbedSibling { }") + }); + } + + private static HotReloadPreparedGroupFile CreateResolved(params TransformWorkerEntryDto[] entries) + { + return HotReloadPreparedGroupFile.Resolved( + CreateFile(OwnerPath), + entries, + HotReloadEntryResolution.Result.Succeeded(new List())); + } + + private static TransformWorkerEntryDto CreateEntry(string methodName, string[] parameterTypeFullNames = null) + { + return new TransformWorkerEntryDto + { + typeMetadataName = CallerMetadataName, + methodName = methodName, + parameterTypeFullNames = parameterTypeFullNames ?? Array.Empty(), + genericArity = 0 + }; + } + + private static HotReloadGroupFile CreateFile(string projectRelativePath) + { + return new HotReloadGroupFile( + projectRelativePath, + projectRelativePath, + projectRelativePath, + AssemblyName, + FindCompilationAssembly(), + HotReloadTypeHome.ScriptAssembliesUnderProject(ProjectRoot, AssemblyName), + ProjectRoot, + new HotReloadFileSinks(new List(), null, new HotReloadRunStaleSignatureWarnings())); + } + + private static string ProjectRoot => + Path.GetFullPath(Path.Combine(Application.dataPath, "..")); + + private static UnityCompilationAssembly FindCompilationAssembly() + { + foreach (UnityCompilationAssembly assembly + in UnityEditor.Compilation.CompilationPipeline.GetAssemblies()) + { + if (assembly.name == AssemblyName) + { + return assembly; + } + } + + Assert.Fail("The test assembly must be a compilation assembly."); + return null; + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadStubbedMemberCoverageTests.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadStubbedMemberCoverageTests.cs.meta new file mode 100644 index 000000000..6c9509f64 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadStubbedMemberCoverageTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 2558fbd1a82fd47a5874d7aa2511df34 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/TransformWorkerClientTests.cs b/Assets/Tests/Editor/HotReload/TransformWorkerClientTests.cs index d70e4649f..763062d7e 100644 --- a/Assets/Tests/Editor/HotReload/TransformWorkerClientTests.cs +++ b/Assets/Tests/Editor/HotReload/TransformWorkerClientTests.cs @@ -3382,6 +3382,57 @@ public void InterpretOutput_PrepareDescriptorMatchingOwnerAndIdentity_ReturnsSuc Assert.That(result.Success, Is.True, result.ErrorMessage); } + /// + /// Verifies that preparation rejects a descriptor listing a blank stubbed method key, which + /// no entry of the run could ever match. + /// + [Test] + public void InterpretOutput_PrepareDescriptorBlankStubbedMethodKey_ReturnsFailure() + { + TransformWorkerInputDto input = CreatePreparationValidationInput(); + TransformWorkerOutputDto output = CreatePreparationValidationOutputWithStubbedKeys( + new[] { "Example.Introduced::Run()", " " }); + + TransformWorkerClientResult result = CreateOutputInterpreter().InterpretOutput(input, output); + + Assert.That(result.Success, Is.False); + Assert.That(result.ErrorMessage, Does.Contain("stubbedMethodKeys")); + } + + /// + /// Verifies that preparation rejects a descriptor listing one stubbed method key twice, + /// which would let one patch count for two stubs. + /// + [Test] + public void InterpretOutput_PrepareDescriptorRepeatedStubbedMethodKey_ReturnsFailure() + { + TransformWorkerInputDto input = CreatePreparationValidationInput(); + TransformWorkerOutputDto output = CreatePreparationValidationOutputWithStubbedKeys( + new[] { "Example.Introduced::Run()", "Example.Introduced::Run()" }); + + TransformWorkerClientResult result = CreateOutputInterpreter().InterpretOutput(input, output); + + Assert.That(result.Success, Is.False); + Assert.That(result.ErrorMessage, Does.Contain("stubbedMethodKeys")); + } + + /// + /// Verifies that preparation accepts a descriptor whose stubbed method keys are distinct + /// and hands them on unchanged. + /// + [Test] + public void InterpretOutput_PrepareDescriptorDistinctStubbedMethodKeys_ReturnsSuccess() + { + TransformWorkerInputDto input = CreatePreparationValidationInput(); + string[] keys = { "Example.Introduced::Run()", "Example.Introduced::get_Value()" }; + TransformWorkerOutputDto output = CreatePreparationValidationOutputWithStubbedKeys(keys); + + TransformWorkerClientResult result = CreateOutputInterpreter().InterpretOutput(input, output); + + Assert.That(result.Success, Is.True, result.ErrorMessage); + Assert.That(result.Output.files[0].introducedTypes[0].stubbedMethodKeys, Is.EqualTo(keys)); + } + /// /// Verifies that JSON containing a null preparation descriptor is rejected before a /// success-shaped response can reach artifact preparation. @@ -3776,5 +3827,16 @@ private static TransformWorkerOutputDto CreatePreparationValidationOutput( } }; } + + private static TransformWorkerOutputDto CreatePreparationValidationOutputWithStubbedKeys( + string[] stubbedMethodKeys) + { + TransformWorkerOutputDto output = CreatePreparationValidationOutput( + "Assembly", + "mvid", + "Assets/Edited.cs"); + output.files[0].introducedTypes[0].stubbedMethodKeys = stubbedMethodKeys; + return output; + } } } diff --git a/Assets/Tests/Editor/HotReload/TransformWorkerIntroducedTypeStubTests.cs b/Assets/Tests/Editor/HotReload/TransformWorkerIntroducedTypeStubTests.cs new file mode 100644 index 000000000..47ba95baa --- /dev/null +++ b/Assets/Tests/Editor/HotReload/TransformWorkerIntroducedTypeStubTests.cs @@ -0,0 +1,333 @@ +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// Verifies which bodies of a newly introduced type the preparation stubs because they call + /// members this reload adds to a type that already exists, and what the stubbed artifact + /// source, the stub keys and the recorded fingerprint look like. + /// + public sealed class TransformWorkerIntroducedTypeStubTests + { + private const string StubThrow = "throw new global::System.InvalidOperationException("; + + // The compiled host reopened with the members this reload adds: a method, an overload of + // a compiled name, a field, a property and an event. + private const string HostWithAdditionsSource = + "namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload\n" + + "{\n" + + " public sealed class HotReloadCrossFileAddedMemberHost\n" + + " {\n" + + " public int Value() { return 1; }\n" + + " public int Scaled(int factor) { return factor; }\n" + + " public int Scaled(int factor, int offset) { return factor + offset; }\n" + + " public int AddedValue() { return 42; }\n" + + " public int AddedField;\n" + + " public int AddedProperty { get { return 5; } }\n" + + " public event System.Action AddedEvent;\n" + + " }\n" + + "}\n"; + + private const string CallerUsings = "using io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload;\n"; + + /// + /// A method that calls an added method is stubbed in the artifact source and keyed the way + /// the transform keys its entry, its record marks the body as stubbed, and a method that + /// calls nothing added keeps its body. + /// + [Test] + public async Task PrepareIntroducedTypes_MethodCallingAnAddedMethod_StubsOnlyThatBody() + { + TransformWorkerIntroducedTypeDto introducedType = await PrepareSingleTypeAsync( + "MethodCallsAddedMethod", + CallerUsings + + "namespace Example.Stubs\n" + + "{\n" + + " public class Caller\n" + + " {\n" + + " public int Uses() { return new HotReloadCrossFileAddedMemberHost().AddedValue(); }\n" + + " public int Plain() { return 7; }\n" + + " }\n" + + "}\n"); + + Assert.That(introducedType.stubbedMethodKeys, Is.EqualTo(new[] { "Example.Stubs.Caller::Uses()" })); + Assert.That( + introducedType.source, + Does.Contain("public int Uses() { " + StubThrow + "\"'Example.Stubs.Caller.Uses' calls members that a hot reload added")); + Assert.That(introducedType.source, Does.Not.Contain("AddedValue")); + Assert.That(introducedType.source, Does.Contain("public int Plain() { return 7; }")); + Assert.That(introducedType.source, Does.Contain("Reload 'Assets/First.cs' again, or run 'uloop compile'.")); + Assert.That(ReadStubbedBodyCount(introducedType), Is.EqualTo(1)); + } + + /// + /// Expression-bodied methods and getters keep their arrow form and a get accessor keeps its + /// block, and each getter is keyed by its get method. + /// + [Test] + public async Task PrepareIntroducedTypes_ExpressionBodiesAndGetters_KeepTheirBodyForm() + { + TransformWorkerIntroducedTypeDto introducedType = await PrepareSingleTypeAsync( + "ExpressionBodies", + CallerUsings + + "namespace Example.Stubs\n" + + "{\n" + + " public class Caller\n" + + " {\n" + + " public int Arrow() => new HotReloadCrossFileAddedMemberHost().AddedValue();\n" + + " public int Getter => new HotReloadCrossFileAddedMemberHost().AddedValue();\n" + + " public int Accessor { get { return new HotReloadCrossFileAddedMemberHost().AddedValue(); } }\n" + + " }\n" + + "}\n"); + + Assert.That( + introducedType.stubbedMethodKeys, + Is.EqualTo(new[] + { + "Example.Stubs.Caller::Arrow()", + "Example.Stubs.Caller::get_Getter()", + "Example.Stubs.Caller::get_Accessor()" + })); + Assert.That(introducedType.source, Does.Contain("public int Arrow() => " + StubThrow)); + Assert.That(introducedType.source, Does.Contain("public int Getter => " + StubThrow)); + Assert.That(introducedType.source, Does.Contain("public int Accessor { get { " + StubThrow)); + Assert.That(introducedType.source, Does.Not.Contain("AddedValue")); + Assert.That(ReadStubbedBodyCount(introducedType), Is.EqualTo(3)); + } + + /// + /// Bodies the transform cannot patch onto an artifact keep calling the added member, so the + /// artifact keeps failing to compile on them instead of shipping a stub nothing replaces. + /// + [Test] + public async Task PrepareIntroducedTypes_BodiesTheTransformCannotPatch_AreNotStubbed() + { + TransformWorkerIntroducedTypeDto introducedType = await PrepareSingleTypeAsync( + "UnpatchableBodies", + CallerUsings + + "namespace Example.Stubs\n" + + "{\n" + + " public class Caller\n" + + " {\n" + + " private int _seed = new HotReloadCrossFileAddedMemberHost().AddedValue();\n" + + " public Caller() { _seed = new HotReloadCrossFileAddedMemberHost().AddedValue(); }\n" + + " public int WithSetter { get { return new HotReloadCrossFileAddedMemberHost().AddedValue(); } set { _seed = value; } }\n" + + " public int this[int index] { get { return new HotReloadCrossFileAddedMemberHost().AddedValue(); } }\n" + + " public static int operator +(Caller left, int right) { return new HotReloadCrossFileAddedMemberHost().AddedValue(); }\n" + + " }\n" + + "}\n"); + + Assert.That(introducedType.stubbedMethodKeys, Is.Empty); + Assert.That(introducedType.source, Does.Not.Contain(StubThrow)); + Assert.That(ReadStubbedBodyCount(introducedType), Is.EqualTo(0)); + } + + /// + /// A call that resolves to an overload this reload adds to a compiled name is stubbed, since + /// the compiled type holds only the other overload. + /// + [Test] + public async Task PrepareIntroducedTypes_CallToAnAddedOverload_IsStubbed() + { + TransformWorkerIntroducedTypeDto introducedType = await PrepareSingleTypeAsync( + "AddedOverload", + CallerUsings + + "namespace Example.Stubs\n" + + "{\n" + + " public class Caller\n" + + " {\n" + + " public int Uses() { return new HotReloadCrossFileAddedMemberHost().Scaled(2, 3); }\n" + + " }\n" + + "}\n"); + + Assert.That(introducedType.stubbedMethodKeys, Is.EqualTo(new[] { "Example.Stubs.Caller::Uses()" })); + } + + /// + /// A call that resolves to the overload the compiled type already holds is not stubbed, even + /// though this reload adds another overload of the same name. + /// + [Test] + public async Task PrepareIntroducedTypes_CallToTheCompiledOverload_IsNotStubbed() + { + TransformWorkerIntroducedTypeDto introducedType = await PrepareSingleTypeAsync( + "CompiledOverload", + CallerUsings + + "namespace Example.Stubs\n" + + "{\n" + + " public class Caller\n" + + " {\n" + + " public int Uses() { return new HotReloadCrossFileAddedMemberHost().Scaled(2); }\n" + + " }\n" + + "}\n"); + + Assert.That(introducedType.stubbedMethodKeys, Is.Empty); + Assert.That(introducedType.source, Does.Contain(".Scaled(2)")); + } + + /// + /// Reading an added field or an added property is stubbed the same way as calling an added method. + /// + [Test] + public async Task PrepareIntroducedTypes_AddedFieldAndProperty_AreStubbed() + { + TransformWorkerIntroducedTypeDto introducedType = await PrepareSingleTypeAsync( + "AddedFieldAndProperty", + CallerUsings + + "namespace Example.Stubs\n" + + "{\n" + + " public class Caller\n" + + " {\n" + + " public int UsesField() { return new HotReloadCrossFileAddedMemberHost().AddedField; }\n" + + " public int UsesProperty() { return new HotReloadCrossFileAddedMemberHost().AddedProperty; }\n" + + " }\n" + + "}\n"); + + Assert.That( + introducedType.stubbedMethodKeys, + Is.EqualTo(new[] { "Example.Stubs.Caller::UsesField()", "Example.Stubs.Caller::UsesProperty()" })); + } + + /// + /// A body that uses an added event is not stubbed, even when it also calls an added method: + /// the transform never patches such a body, so a stub would stay in place. + /// + [Test] + public async Task PrepareIntroducedTypes_BodyUsingAnAddedEvent_IsNotStubbed() + { + TransformWorkerIntroducedTypeDto introducedType = await PrepareSingleTypeAsync( + "AddedEvent", + CallerUsings + + "namespace Example.Stubs\n" + + "{\n" + + " public class Caller\n" + + " {\n" + + " public int Subscribes()\n" + + " {\n" + + " HotReloadCrossFileAddedMemberHost host = new HotReloadCrossFileAddedMemberHost();\n" + + " host.AddedEvent += () => { };\n" + + " return host.AddedValue();\n" + + " }\n" + + " }\n" + + "}\n"); + + Assert.That(introducedType.stubbedMethodKeys, Is.Empty); + Assert.That(introducedType.source, Does.Contain("host.AddedEvent += () => { };")); + } + + /// + /// A call into a type this same reload introduces is not stubbed: that type compiles into + /// the same artifact, so the call binds there. + /// + [Test] + public async Task PrepareIntroducedTypes_CallIntoATypeThisReloadIntroduces_IsNotStubbed() + { + string directory = TransformWorkerIntroducedTypeTestInputs.CreateSourceDirectory("StubSameRunType"); + string callerPath = Path.Combine(directory, "First.cs"); + string helperPath = Path.Combine(directory, "Second.cs"); + File.WriteAllText( + callerPath, + "namespace Example.Stubs { public class Caller { public int Uses() { return new Helper().Help(); } } }"); + File.WriteAllText(helperPath, "namespace Example.Stubs { public class Helper { public int Help() { return 3; } } }"); + + TransformWorkerClientResult result = await HotReloadCompositionRoot.Services.TransformWorkerClient.RunAsync( + TransformWorkerIntroducedTypeTestInputs.CreateInput(callerPath, helperPath), + CancellationToken.None); + + Assert.That(result.Success, Is.True, result.ErrorMessage); + Assert.That(result.Output.files[0].introducedTypes, Has.Length.EqualTo(1)); + Assert.That(result.Output.files[0].introducedTypes[0].stubbedMethodKeys, Is.Empty); + Assert.That(result.Output.files[1].introducedTypes[0].stubbedMethodKeys, Is.Empty); + } + + /// + /// A call whose added member is declared in a file this run does not compile does not bind, + /// so nothing tells it from a typo and the body is not stubbed. + /// + [Test] + public async Task PrepareIntroducedTypes_AdditionOutsideTheRun_IsNotStubbed() + { + string directory = TransformWorkerIntroducedTypeTestInputs.CreateSourceDirectory("StubAdditionOutsideRun"); + string callerPath = Path.Combine(directory, "First.cs"); + string unrelatedPath = Path.Combine(directory, "Second.cs"); + File.WriteAllText( + callerPath, + CallerUsings + + "namespace Example.Stubs { public class Caller { public int Uses() { return new HotReloadCrossFileAddedMemberHost().AddedValue(); } } }"); + File.WriteAllText(unrelatedPath, "namespace Unrelated { public class Untouched { } }"); + + TransformWorkerClientResult result = await HotReloadCompositionRoot.Services.TransformWorkerClient.RunAsync( + TransformWorkerIntroducedTypeTestInputs.CreateInput(callerPath, unrelatedPath), + CancellationToken.None); + + Assert.That(result.Success, Is.True, result.ErrorMessage); + TransformWorkerIntroducedTypeDto caller = result.Output.files[0].introducedTypes[0]; + Assert.That(caller.stubbedMethodKeys, Is.Empty); + Assert.That(caller.source, Does.Contain(".AddedValue()")); + } + + /// + /// A type declared outside any namespace is stubbed the same way, through the artifact + /// source that has no namespace to wrap it in. + /// + [Test] + public async Task PrepareIntroducedTypes_TypeWithoutNamespace_IsStubbed() + { + TransformWorkerIntroducedTypeDto introducedType = await PrepareSingleTypeAsync( + "NoNamespace", + CallerUsings + + "public class StubNoNamespaceCaller\n" + + "{\n" + + " public int Uses() { return new HotReloadCrossFileAddedMemberHost().AddedValue(); }\n" + + "}\n"); + + Assert.That(introducedType.stubbedMethodKeys, Is.EqualTo(new[] { "StubNoNamespaceCaller::Uses()" })); + Assert.That(introducedType.source, Does.Not.Contain("namespace")); + Assert.That(introducedType.source, Does.Contain("public int Uses() { " + StubThrow)); + } + + private static async Task PrepareSingleTypeAsync(string caseName, string callerSource) + { + string directory = TransformWorkerIntroducedTypeTestInputs.CreateSourceDirectory("Stub" + caseName); + string callerPath = Path.Combine(directory, "First.cs"); + string hostPath = Path.Combine(directory, "Second.cs"); + File.WriteAllText(callerPath, callerSource); + File.WriteAllText(hostPath, HostWithAdditionsSource); + + TransformWorkerClientResult result = await HotReloadCompositionRoot.Services.TransformWorkerClient.RunAsync( + TransformWorkerIntroducedTypeTestInputs.CreateInput(callerPath, hostPath), + CancellationToken.None); + + Assert.That(result.Success, Is.True, result.ErrorMessage); + Assert.That(result.Output.files[0].introducedTypeDiagnostics, Is.Empty); + Assert.That(result.Output.files[0].introducedTypes, Has.Length.EqualTo(1)); + Assert.That(result.Output.files[1].introducedTypes, Is.Empty); + return result.Output.files[0].introducedTypes[0]; + } + + private static int ReadStubbedBodyCount(TransformWorkerIntroducedTypeDto introducedType) + { + bool parsed = HotReloadIntroducedTypeFingerprint.TryParse( + introducedType.declarationFingerprint, + out HotReloadIntroducedTypeFingerprint fingerprint); + Assert.That(parsed, Is.True, "The record must carry a canonical fingerprint."); + int count = 0; + foreach (HotReloadIntroducedTypeMemberFingerprint member in fingerprint.Members) + { + if (member.HasStubbedBody) + { + count++; + } + } + + Assert.That(fingerprint.HoldsStubbedBodies, Is.EqualTo(count > 0)); + return count; + } + } +} diff --git a/Assets/Tests/Editor/HotReload/TransformWorkerIntroducedTypeStubTests.cs.meta b/Assets/Tests/Editor/HotReload/TransformWorkerIntroducedTypeStubTests.cs.meta new file mode 100644 index 000000000..5b8b538b4 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/TransformWorkerIntroducedTypeStubTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 4fc3870b052a8446ea8e90dc77841ce3 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEntryHomeResolver.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEntryHomeResolver.cs index d05a1fb66..0568fe24a 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEntryHomeResolver.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadEntryHomeResolver.cs @@ -1,4 +1,5 @@ using System; +using System.Reflection; using UnityEngine; @@ -14,14 +15,28 @@ internal sealed class HotReloadEntryHomeResolver { private readonly HotReloadDomain _domain; private readonly string _projectRoot; + private readonly HotReloadIntroducedTypeArtifact _preparedArtifact; + private readonly string _preparedArtifactName; - internal HotReloadEntryHomeResolver(HotReloadDomain domain, string projectRoot) + /// + /// The artifact this run prepared, or null. Rows name it when they patch a body the + /// artifact stubs, and they are resolved before the artifact is activated, which is the + /// point from which the domain would answer for it. + /// + internal HotReloadEntryHomeResolver( + HotReloadDomain domain, + string projectRoot, + HotReloadIntroducedTypeArtifact preparedArtifact = null) { Debug.Assert(domain != null, "domain must not be null."); Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty."); _domain = domain; _projectRoot = projectRoot; + _preparedArtifact = preparedArtifact; + _preparedArtifactName = preparedArtifact == null + ? null + : new AssemblyName(preparedArtifact.AssemblyFullName).Name; } /// @@ -38,6 +53,15 @@ internal HotReloadTypeHome Resolve(HotReloadTypeHome fileHome, string homeAssemb return fileHome; } + if (_preparedArtifact != null + && string.Equals(homeAssemblyName, _preparedArtifactName, StringComparison.Ordinal)) + { + return HotReloadTypeHome.RetainedArtifact( + _preparedArtifactName, + _preparedArtifact.DllPath, + _preparedArtifact.Assembly); + } + HotReloadTypeHome home = _domain.ResolveTypeHome(_projectRoot, homeAssemblyName); if (home.Kind != HotReloadTypeHomeKind.RetainedArtifact) { diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupCommitStage.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupCommitStage.cs index 4df219699..c19b4d5ae 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupCommitStage.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupCommitStage.cs @@ -85,6 +85,7 @@ internal List BuildResolutionFailedResults( /// The commit point of a group run and everything that follows it: the introduced types /// become active, the patches this run supersedes are peeled, and the resolved entries are /// applied. A failure after the commit point leaves the types active and fails methods. + /// A run that leaves a body the prepared artifact stubs unpatched is refused before it. /// internal IReadOnlyList Commit( HotReloadApplyContext context, @@ -96,6 +97,16 @@ internal IReadOnlyList Commit( HotReloadPreparedIntroducedTypes prepared = context.PreparedIntroducedTypes; if (prepared != null) { + // Why here and not per file: whether a stub is patched depends on every file of + // the group, and this is the last point at which refusing changes nothing. + List uncoveredTypes = + HotReloadStubbedMemberCoverage.FindUncoveredTypes(prepared.Artifact, preparedFiles); + if (uncoveredTypes.Count > 0) + { + HotReloadIntroducedTypeOutcomeSink.Append(files, uncoveredTypes); + return _fileEntryApplier.BuildUnappliedGroupResults(files); + } + _domain.IntroducedTypes.Activate(prepared.Artifact); // Why after the activation and not at preparation: only a type the boundary // published is introduced, so a run that never reached here must report none. diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupEntryPreparation.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupEntryPreparation.cs index d2c29c6f8..509628708 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupEntryPreparation.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupEntryPreparation.cs @@ -38,8 +38,10 @@ internal static IReadOnlyList PrepareGroup( // Why once for the group: every file of the group is resolved against the same // domain and project, and a row that names an artifact assembly must resolve to the // same retained home no matter which file it came from. - HotReloadEntryHomeResolver homeResolver = - new HotReloadEntryHomeResolver(collaborators.Domain, context.ProjectRoot); + HotReloadEntryHomeResolver homeResolver = new HotReloadEntryHomeResolver( + collaborators.Domain, + context.ProjectRoot, + context.PreparedIntroducedTypes?.Artifact); // Why once for the group: a body can call an added member another file of the group // declares, and only the whole group's entries name every added member it may call. HotReloadAddedCalleeIndex addedCallees = new HotReloadAddedCalleeIndex(entriesToPatch); diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadIntroducedTypePreparation.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadIntroducedTypePreparation.cs index 926215ba1..eb6db8ff4 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadIntroducedTypePreparation.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadIntroducedTypePreparation.cs @@ -435,7 +435,8 @@ private static List CollectDescriptors( introducedType.metadataName, introducedType.ownerProjectRelativePath, introducedType.declarationFingerprint, - introducedType.source)); + introducedType.source, + introducedType.stubbedMethodKeys)); } } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStubbedMemberCoverage.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStubbedMemberCoverage.cs new file mode 100644 index 000000000..f59c6b931 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStubbedMemberCoverage.cs @@ -0,0 +1,134 @@ +using System; +using System.Collections.Generic; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Holds the activation of a prepared artifact back until this run patches every body the + /// artifact stubs. A stubbed body throws instead of running the source, and the artifact's + /// types can be called the moment it is activated, so activating it with a stub left in place + /// would introduce a type that fails where its source works. + /// + internal static class HotReloadStubbedMemberCoverage + { + // Why the sibling types fail too: the batch is one assembly that is activated as a whole, + // so holding one type back holds every type of it back. + private const string SiblingNotCoveredReason = + "Not introduced: another declaration in the same introduced-type batch has a body this " + + "reload did not patch, so this type was not introduced either. Fix that declaration and rerun."; + + /// + /// The rows of every type the artifact holds when a body it stubs has no resolved entry in + /// this run, or an empty list when every stubbed body is patched. A null + /// is a run with no entry at all. + /// + internal static List FindUncoveredTypes( + HotReloadIntroducedTypeArtifact artifact, + IReadOnlyList preparedFiles) + { + if (artifact == null) + { + throw new ArgumentNullException(nameof(artifact)); + } + + HashSet patchedKeys = CollectResolvedEntryKeys(preparedFiles); + Dictionary> uncoveredLabels = + new Dictionary>(); + foreach (HotReloadIntroducedTypeDescriptor descriptor in artifact.Descriptors) + { + List labels = CollectUncoveredLabels(descriptor, patchedKeys); + if (labels.Count > 0) + { + uncoveredLabels[descriptor] = labels; + } + } + + List rows = new List(); + if (uncoveredLabels.Count == 0) + { + return rows; + } + + foreach (HotReloadIntroducedTypeDescriptor descriptor in artifact.Descriptors) + { + string reason = uncoveredLabels.TryGetValue(descriptor, out List labels) + ? BuildUncoveredReason(labels) + : SiblingNotCoveredReason; + rows.Add( + HotReloadIntroducedTypeOutcome.Failed( + descriptor.MetadataName.Value, + descriptor.OriginalAssemblyName, + descriptor.OwnerProjectRelativePath, + reason)); + } + + return rows; + } + + // Only a resolved file is applied, so an entry of a file the group skipped or could not + // resolve patches nothing and must not count. + private static HashSet CollectResolvedEntryKeys(IReadOnlyList preparedFiles) + { + HashSet keys = new HashSet(StringComparer.Ordinal); + if (preparedFiles == null) + { + return keys; + } + + foreach (HotReloadPreparedGroupFile prepared in preparedFiles) + { + if (prepared.Kind != HotReloadGroupFilePreparationKind.Resolved) + { + continue; + } + + foreach (TransformWorkerEntryDto entry in prepared.Entries) + { + keys.Add(HotReloadMethodKeys.BuildMethodKey(entry)); + } + } + + return keys; + } + + private static List CollectUncoveredLabels( + HotReloadIntroducedTypeDescriptor descriptor, + HashSet patchedKeys) + { + List labels = new List(); + foreach (string stubbedMethodKey in descriptor.StubbedMethodKeys) + { + if (!patchedKeys.Contains(stubbedMethodKey)) + { + labels.Add(ToMethodLabel(stubbedMethodKey)); + } + } + + return labels; + } + + // The label is the key in reflection form: the separator before the method name becomes + // '.', and every nested-type separator becomes '+'. Only a parameter type can carry one, + // because an introduced type is never nested. + private static string ToMethodLabel(string methodKey) + { + return methodKey.Replace("::", ".").Replace('/', '+'); + } + + private static string BuildUncoveredReason(List labels) + { + if (labels.Count == 1) + { + return "Not introduced: " + labels[0] + + " calls members that a hot reload added, so its body runs only once this reload " + + "patches it in, and this reload did not. Fix what kept it from being patched and " + + "rerun, or run 'uloop compile' to apply this edit."; + } + + return "Not introduced: " + string.Join(", ", labels) + + " call members that a hot reload added, so their bodies run only once this reload " + + "patches them in, and this reload did not. Fix what kept them from being patched and " + + "rerun, or run 'uloop compile' to apply this edit."; + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStubbedMemberCoverage.cs.meta b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStubbedMemberCoverage.cs.meta new file mode 100644 index 000000000..1ed56f9f1 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStubbedMemberCoverage.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 9beaa65f9a9a94b8eab2574c0d215768 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/IntroducedType/HotReloadIntroducedTypeCompileFailureOutcomes.cs b/Packages/src/Editor/FirstPartyTools/HotReload/IntroducedType/HotReloadIntroducedTypeCompileFailureOutcomes.cs index dcc407fcc..05990c3e5 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/IntroducedType/HotReloadIntroducedTypeCompileFailureOutcomes.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/IntroducedType/HotReloadIntroducedTypeCompileFailureOutcomes.cs @@ -20,16 +20,21 @@ internal static class HotReloadIntroducedTypeCompileFailureOutcomes // Why this has to be spelled out: an introduced type compiles against the compiled // assemblies on disk and the retained artifacts, so a member hot reload added, in this - // reload or an earlier one, is genuinely absent there. The bare compiler error reads as a - // typo and sends the reader looking for one. Why worded as a condition: only the name is - // matched, so a missing member of another type can share it. Why splitting is ruled out: - // reloading the addition first still leaves it outside both, which is the obvious retry. + // reload or an earlier one, is genuinely absent there. A method or getter body naming it + // is compiled as a stub and patched in by the same reload, so the error only reaches a + // body no patch replaces, or a call whose declaring file the reload's compilation does + // not hold. The bare compiler error reads as a typo and sends the reader looking for one. + // Why worded as a condition: only the name is matched, so a missing member of another + // type can share it. private const string AddedMemberInvisibleHint = "One or more of the missing members share a name with a hot reload addition, from this " - + "reload or an earlier one. If the missing member is that addition, an introduced type " - + "cannot see it: it compiles against the compiled assemblies and earlier introduced " - + "types only, so reloading the addition first does not help. Run 'uloop compile' to " - + "make the added members compiled, then rerun."; + + "reload or an earlier one. A new type can call such an addition only from its methods " + + "and get-only properties, and only when the file that declares the addition belongs " + + "to the same assembly and is part of this reload: passed, or unchanged since it was " + + "last applied. Constructors, initializers, setters, indexers, operators, event " + + "accessors and subscriptions to an added event cannot. Pass that file too or move the " + + "call into a method, or run 'uloop compile' to make the added members compiled, then " + + "rerun."; // Why it points at the warning: the enum-member warning of the same run already carries // the cast that avoids the member, and repeating the value here would need the enum too. diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/IntroducedType/HotReloadIntroducedTypeDescriptor.cs b/Packages/src/Editor/FirstPartyTools/HotReload/IntroducedType/HotReloadIntroducedTypeDescriptor.cs index a61080f1b..332d7446f 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/IntroducedType/HotReloadIntroducedTypeDescriptor.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/IntroducedType/HotReloadIntroducedTypeDescriptor.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; namespace io.github.hatayama.UnityCliLoop.FirstPartyTools { @@ -19,13 +20,21 @@ internal sealed class HotReloadIntroducedTypeDescriptor public string Source { get; } + /// + /// Entry keys of the methods whose bodies stubs because they call + /// members a hot reload adds; empty when it stubs none. Only the run that prepares the + /// artifact reads them, to hold the activation back until each one is patched. + /// + public IReadOnlyList StubbedMethodKeys { get; } + public HotReloadIntroducedTypeDescriptor( string originalAssemblyName, string originalAssemblyMvid, string metadataName, string ownerProjectRelativePath, string declarationFingerprint, - string source) + string source, + IReadOnlyList stubbedMethodKeys = null) { // BuildIdentity concatenates the first three parts and HasSameDefinition compares the // fingerprint, so a missing part would collapse distinct declarations onto one identity @@ -40,6 +49,33 @@ public HotReloadIntroducedTypeDescriptor( OwnerProjectRelativePath = ownerProjectRelativePath; DeclarationFingerprint = declarationFingerprint; Source = source; + StubbedMethodKeys = CopyStubbedMethodKeys(stubbedMethodKeys); + } + + // A blank or repeated key would let the activation check count a key it can never match, + // or one patch for two stubs. + private static string[] CopyStubbedMethodKeys(IReadOnlyList stubbedMethodKeys) + { + if (stubbedMethodKeys == null) + { + return Array.Empty(); + } + + HashSet seen = new HashSet(StringComparer.Ordinal); + string[] copy = new string[stubbedMethodKeys.Count]; + for (int index = 0; index < stubbedMethodKeys.Count; index++) + { + string key = stubbedMethodKeys[index]; + RequireValue(key, nameof(stubbedMethodKeys)); + if (!seen.Add(key)) + { + throw new ArgumentException("A stubbed method key must not repeat: " + key, nameof(stubbedMethodKeys)); + } + + copy[index] = key; + } + + return copy; } private static void RequireValue(string value, string parameterName) diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadIntroducedTypeFingerprint.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadIntroducedTypeFingerprint.cs index cd01e93df..ee0d90661 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadIntroducedTypeFingerprint.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadIntroducedTypeFingerprint.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Globalization; +using System.Security.Cryptography; using System.Text; // This file is compiled twice: into the Unity editor assembly (host side) and into the @@ -64,6 +65,17 @@ internal HotReloadIntroducedTypeMemberFingerprint(string key, string declaration /// Empty for a member that has no body at all, such as a field or an enum member. internal string BodyHash { get; } + + /// + /// True when the body hash is the stub sentinel that + /// records for this key. + /// + internal bool HasStubbedBody => + BodyHash.Length > 0 + && string.Equals( + BodyHash, + HotReloadIntroducedTypeFingerprint.ComputeStubbedBodyHash(Key), + StringComparison.Ordinal); } /// @@ -101,6 +113,7 @@ internal sealed class HotReloadIntroducedTypeFingerprint { private const string FormatVersionLine = "v1"; private const int HashLength = 64; + private const string StubbedBodyHashInputPrefix = "hot-reload-stub|"; internal HotReloadIntroducedTypeFingerprint( string headerHash, @@ -162,6 +175,88 @@ internal HotReloadIntroducedTypeFingerprint( /// Members in ascending ordinal key order, each key appearing once. internal IReadOnlyList Members { get; } + /// True when any member carries the stub sentinel in place of its body hash. + internal bool HoldsStubbedBodies + { + get + { + for (int index = 0; index < Members.Count; index++) + { + if (Members[index].HasStubbedBody) + { + return true; + } + } + + return false; + } + } + + /// + /// Returns a copy in which each listed member's body hash is the stub sentinel for its key. + /// An artifact that stubs a body does not run the body its source spells, so its record must + /// not hash to that body: otherwise the unchanged source would compare Identical and the + /// stub would stay in place with nothing patching the real body in. + /// + internal HotReloadIntroducedTypeFingerprint WithStubbedBodies(IReadOnlyCollection memberKeys) + { + if (memberKeys == null) + { + throw new ArgumentException("memberKeys must not be null.", nameof(memberKeys)); + } + + HashSet remainingKeys = new HashSet(memberKeys, StringComparer.Ordinal); + List members = + new List(Members.Count); + for (int index = 0; index < Members.Count; index++) + { + HotReloadIntroducedTypeMemberFingerprint member = Members[index]; + if (!remainingKeys.Remove(member.Key)) + { + members.Add(member); + continue; + } + + // A member without a body has nothing an artifact could stub, so the request and + // this fingerprint describe different declarations. + if (member.BodyHash.Length == 0) + { + throw new ArgumentException("memberKeys must name members that have a body: " + member.Key, nameof(memberKeys)); + } + + members.Add(new HotReloadIntroducedTypeMemberFingerprint( + member.Key, + member.DeclarationHash, + ComputeStubbedBodyHash(member.Key))); + } + + if (remainingKeys.Count > 0) + { + throw new ArgumentException( + "memberKeys must name members of this fingerprint: " + string.Join(", ", remainingKeys), + nameof(memberKeys)); + } + + return new HotReloadIntroducedTypeFingerprint(HeaderHash, DefinesHash, MemberOrderHash, members); + } + + // The worker hashes a body as a list of ":" token entries, so a real body + // input always starts with a digit. This input starts with a letter, which keeps the + // sentinel from ever being the hash of a body. + internal static string ComputeStubbedBodyHash(string memberKey) + { + byte[] inputBytes = Encoding.UTF8.GetBytes(StubbedBodyHashInputPrefix + memberKey); + using SHA256 sha256 = SHA256.Create(); + byte[] hashBytes = sha256.ComputeHash(inputBytes); + StringBuilder builder = new StringBuilder(hashBytes.Length * 2); + for (int index = 0; index < hashBytes.Length; index++) + { + builder.Append(hashBytes[index].ToString("x2", CultureInfo.InvariantCulture)); + } + + return builder.ToString(); + } + /// /// Writes the canonical text that travels on the wire. Member keys are length prefixed, so /// a key containing the field separator cannot be misread. diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/TransformWorkerDtos.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/TransformWorkerDtos.cs index 5b5f91f09..f98808ebd 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/TransformWorkerDtos.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/TransformWorkerDtos.cs @@ -254,6 +254,11 @@ internal sealed class TransformWorkerIntroducedTypeDto public string ownerProjectRelativePath; public string declarationFingerprint; public string source; + + // Entry keys of the methods whose bodies source stubs; the artifact may only be activated + // once this run patches every one of them. Null/omitted deserializes as empty after + // client coalesce. + public string[] stubbedMethodKeys; } /// diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/TransformWorkerOutputInterpreter.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/TransformWorkerOutputInterpreter.cs index 60e4a2feb..b4b416b26 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/TransformWorkerOutputInterpreter.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/TransformWorkerOutputInterpreter.cs @@ -124,6 +124,7 @@ internal void CoalesceOutput(TransformWorkerOutputDto output) fileOutput.addedConstNames ??= Array.Empty(); fileOutput.addedEnumMemberNames ??= Array.Empty(); fileOutput.introducedTypes ??= Array.Empty(); + CoalesceIntroducedTypes(fileOutput.introducedTypes); fileOutput.introducedTypeDiagnostics ??= Array.Empty(); fileOutput.introducedTypeReuses ??= Array.Empty(); fileOutput.plannedAddedMemberNames ??= Array.Empty(); @@ -144,6 +145,19 @@ internal void CoalesceOutput(TransformWorkerOutputDto output) } } + private static void CoalesceIntroducedTypes(TransformWorkerIntroducedTypeDto[] introducedTypes) + { + foreach (TransformWorkerIntroducedTypeDto introducedType in introducedTypes) + { + if (introducedType == null) + { + continue; + } + + introducedType.stubbedMethodKeys ??= Array.Empty(); + } + } + internal bool TryValidateOutput( TransformWorkerInputDto input, TransformWorkerOutputDto output, diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/TransformWorkerOutputValidator.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/TransformWorkerOutputValidator.cs index 2beb7b2c0..4453530c4 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/TransformWorkerOutputValidator.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/TransformWorkerOutputValidator.cs @@ -439,8 +439,36 @@ private bool TryValidatePreparationDescriptor( return false; } + if (!HoldsDistinctStubbedMethodKeys(introducedType.stubbedMethodKeys)) + { + errorMessage = "Preparation descriptor stubbedMethodKeys must be non-blank and distinct."; + return false; + } + errorMessage = string.Empty; return true; } + + // Why refused rather than trimmed: a blank key would let the activation check wait for a + // patch no entry can name, and a repeated one would let one patch count for two stubs. + // Why null passes: coalescing reads an omitted list as a type with no stubs. + private bool HoldsDistinctStubbedMethodKeys(string[] stubbedMethodKeys) + { + if (stubbedMethodKeys == null) + { + return true; + } + + HashSet seen = new HashSet(StringComparer.Ordinal); + foreach (string key in stubbedMethodKeys) + { + if (string.IsNullOrWhiteSpace(key) || !seen.Add(key)) + { + return false; + } + } + + return true; + } } } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/introduced-types.md b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/introduced-types.md index 009ee4e36..adbc05734 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/introduced-types.md +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/introduced-types.md @@ -61,7 +61,7 @@ When an edited body in the same run names a refused type, its shim compile fails CS0234, or CS0426, or with CS0103 or CS0117 when the body reads a static member of it. That `Failed` row's `Reason` then ends with a note that quotes the refusal and says `uloop compile` clears it. -Three conditions produce a `Failed` row in `IntroducedTypes` instead, and a `Failed` row makes +These conditions produce a `Failed` row in `IntroducedTypes` instead, and a `Failed` row makes `Success` false and leaves every file that shares an assembly with the refused declaration unapplied — no method body of those files is patched in that run, files in other assemblies still apply, and patches from earlier reloads stay active: @@ -72,6 +72,7 @@ apply, and patches from earlier reloads stay active: | A member body of an already-introduced type changed and is neither an ordinary method body nor a getter-only property body | `Changed member body of introduced type requires a compile:` | | Two files of the reload declare the same type | `Introduced type is declared in more than one file of the group:` | | The artifact assembly did not compile | `Introduced-type compilation failed:` | +| This reload did not patch a body the artifact stubs (see "Calling members hot reload adds") | `Not introduced:` | ## Reading the response @@ -139,17 +140,46 @@ applied as `Added` rows. Constructor, setter, init, indexer and event accessor b initializer bodies, member removals, signature changes, and added constructors, operators, events, indexers or nested types still require a compile. +## Calling members hot reload adds + +A new type's ordinary methods and get-only properties can call a method, field or property that +hot reload adds, in the same reload or an earlier one, to a compiled type of the same assembly or +to a type an earlier reload introduced. The artifact compiles each such body as a stub that +throws, and the same reload activates the type only when it holds a patch for every stub, then +patches the real bodies in, so the response shows the type as `Introduced` and those bodies as +`Patched` rows. Later reloads that +include the file keep the type `AlreadyActive` and patch the body again. The file declaring the +addition has to be in the reload: passed, or unchanged since it was last applied, which the +reload pulls back in on its own. + +Constructors, initializers, setters, indexers, operators, event accessors and subscriptions to an +added event cannot be patched, so a call from them still fails the artifact compile (CS1061 or +CS0117) with a hint saying where such a call works. So does a call to an addition in another +assembly, or in a file that changed since it was last applied and is not passed. + +- When this reload does not patch a stubbed body (a generic method, or a method of a struct, is + `Skipped`), no type of that artifact is introduced: the stubbed type's row reads `Not + introduced: calls members that a hot reload added, …`, the other types of the batch + fail with it, and nothing of that assembly's files is applied. +- Another type the same reload introduces cannot name such a type in its member signatures. The + run is refused with `Introduced type '' calls members that a hot reload added, so its + method bodies run through hot reload patches, …`; run `uloop compile`, or name the type only + inside that other type's method bodies. Two types that both call additions this way may name + each other. +- After `--revert-all`, or when the reload that introduces the type fails to apply one of those + patches (that method's row is `Failed`), a stubbed body runs its stub, which throws + `InvalidOperationException` naming the file to reload; reloading that file patches the body in + again. + ## Still needs `uloop compile` Any refused shape above; use of the type from another assembly, from a file that is neither passed to this reload nor already hot-reloaded; anything that reaches the type through Unity (serialization, `[SerializeField]`, Inspector, `AddComponent`, `CreateInstance`, message discovery); a method body edit of an introduced -struct, which is `Skipped` like any struct method; a call to a member an earlier or the same -reload *added* to a compiled type or to an earlier introduced type, because introduced types -compile against the compiled assemblies and retained artifacts only, so the compile fails naming -the missing member and says a hot reload addition shares its name (reloading the addition first -does not help); an added method that passes a type declared from source in this reload to a +struct, which is `Skipped` like any struct method; a call to a member hot reload *added* from a +new type's body that is not an ordinary method or get-only property, or to an addition this +reload does not hold (see "Calling members hot reload adds"); an added method that passes a type declared from source in this reload to a member of an earlier introduced type whose signature was bound to the compiled copy, which is `Skipped` naming both types; and any new or changed `.asmdef` / `.asmref`. A snippet run by `uloop execute-dynamic-code` is the exception: every active artifact is referenced by that compilation, so the snippet can name an 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 be2d4032e..9d208ab92 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 @@ -95,12 +95,11 @@ by name through the wiring entry point, which is how a value or scene reference into an added `[SerializeField]` without a compile — see [added-field-wiring.md](added-field-wiring.md). -An introduced type cannot use members hot reload added to a compiled type, whether they -were added in the same reload or an earlier one: its artifact compiles against the -compiled assemblies and earlier introduced types only, so reloading the addition first -does not help. Such a reference fails with CS1061/CS0117 on a `Failed` `IntroducedTypes` -row, whose `Reason` notes that the missing name matches an addition; run `uloop compile`, -then rerun (issue #2695). +A type a reload introduces can call added members of a compiled type from its ordinary +methods and get-only properties: its artifact compiles those bodies as stubs, and the same +reload patches the real bodies in. From a constructor, initializer, setter, indexer, +operator or event accessor such a reference still fails with CS1061/CS0117 and needs a +compile — see [introduced-types.md](introduced-types.md). Added members are an Editor-session illusion. Any real compile or domain reload drops them all: added methods disappear from the ledger and added-field values are diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/AddedMemberReferenceClassifier.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/AddedMemberReferenceClassifier.cs new file mode 100644 index 000000000..081e69a38 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/AddedMemberReferenceClassifier.cs @@ -0,0 +1,172 @@ +using System.Collections.Generic; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; + +// What a body names that a hot reload added to a type that already exists. +internal enum AddedMemberUse +{ + None, + MethodsFieldsOrProperties, + Event +} + +// Tells whether a body names a member that the sources of this run add to a type that already +// exists - compiled, or served by an artifact an earlier reload retained - and that the existing +// type does not hold. The name binds in the planning compilation, which sees the adding source, +// but not in an artifact, which compiles against the existing type alone. +internal sealed class AddedMemberReferenceClassifier +{ + private readonly CSharpCompilation compilation; + private readonly WorkerTypeHome home; + private readonly IntroducedTypeArtifactMap artifactMap; + private readonly string targetAssemblyMvid; + + internal AddedMemberReferenceClassifier( + CSharpCompilation compilation, + WorkerTypeHome home, + IntroducedTypeArtifactMap artifactMap, + string targetAssemblyMvid) + { + this.compilation = compilation; + this.home = home; + this.artifactMap = artifactMap; + this.targetAssemblyMvid = targetAssemblyMvid; + } + + /// + /// Event when any name in the bodies is an added event, otherwise whether any names an added + /// method, field or property. An added event wins because a body that uses one is never + /// transformed, so it has to keep failing the way it fails without a stub. + /// + internal AddedMemberUse Classify(IReadOnlyList bodyNodes, SemanticModel semanticModel) + { + bool namesAddedMember = false; + foreach (SyntaxNode bodyNode in bodyNodes) + { + foreach (SimpleNameSyntax name in bodyNode.DescendantNodesAndSelf().OfType()) + { + AddedMemberUse use = ClassifyName(semanticModel.GetSymbolInfo(name)); + if (use == AddedMemberUse.Event) + { + return AddedMemberUse.Event; + } + + if (use == AddedMemberUse.MethodsFieldsOrProperties) + { + namesAddedMember = true; + } + } + } + + return namesAddedMember ? AddedMemberUse.MethodsFieldsOrProperties : AddedMemberUse.None; + } + + // Overload resolution that fails still reports its candidates, and a body naming an added + // overload whose arguments do not fit yet should read the same as one that binds. + private AddedMemberUse ClassifyName(SymbolInfo symbolInfo) + { + if (symbolInfo.Symbol != null) + { + return ClassifySymbol(symbolInfo.Symbol); + } + + AddedMemberUse strongest = AddedMemberUse.None; + foreach (ISymbol candidate in symbolInfo.CandidateSymbols) + { + AddedMemberUse use = ClassifySymbol(candidate); + if (use == AddedMemberUse.Event) + { + return AddedMemberUse.Event; + } + + if (use == AddedMemberUse.MethodsFieldsOrProperties) + { + strongest = use; + } + } + + return strongest; + } + + private AddedMemberUse ClassifySymbol(ISymbol symbol) + { + ISymbol definition = ToDefinition(symbol); + INamedTypeSymbol containingType = definition.ContainingType; + // Why an enum is left out: hot reload never adds an enum member, so a body naming one has + // to keep failing with the hint that says so rather than being stubbed and patched. + if (containingType == null + || containingType.TypeKind == TypeKind.Enum + || !SymbolEqualityComparer.Default.Equals(containingType.ContainingAssembly, compilation.Assembly)) + { + return AddedMemberUse.None; + } + + INamedTypeSymbol existingType = PlannedAddedMemberNames.FindExistingType( + containingType, + home, + compilation, + artifactMap, + home.AssemblyName, + targetAssemblyMvid); + if (existingType == null) + { + return AddedMemberUse.None; + } + + return ClassifyAgainstExistingType(definition, existingType); + } + + private static AddedMemberUse ClassifyAgainstExistingType(ISymbol definition, INamedTypeSymbol existingType) + { + switch (definition) + { + case IMethodSymbol method when method.MethodKind == MethodKind.Ordinary: + return CompiledMemberMatcher.MatchCompiledOrdinaryMethod(existingType, method) == CompiledMethodMatch.Matched + ? AddedMemberUse.None + : AddedMemberUse.MethodsFieldsOrProperties; + case IFieldSymbol field when !field.IsImplicitlyDeclared: + return CompiledMemberMatcher.MatchCompiledField(existingType, field) == CompiledFieldMatch.Matched + ? AddedMemberUse.None + : AddedMemberUse.MethodsFieldsOrProperties; + case IPropertySymbol property when !property.IsIndexer: + return HoldsMemberOfKind(existingType, property.Name, SymbolKind.Property) + ? AddedMemberUse.None + : AddedMemberUse.MethodsFieldsOrProperties; + case IEventSymbol addedEvent: + return HoldsMemberOfKind(existingType, addedEvent.Name, SymbolKind.Event) + ? AddedMemberUse.None + : AddedMemberUse.Event; + default: + return AddedMemberUse.None; + } + } + + // A reduced extension method names the static method it was declared as, and a member of a + // constructed generic type names the member of the generic definition; the existing type is + // looked up and matched in those terms. + private static ISymbol ToDefinition(ISymbol symbol) + { + if (symbol is IMethodSymbol method) + { + IMethodSymbol declared = method.ReducedFrom ?? method; + return declared.OriginalDefinition; + } + + return symbol.OriginalDefinition; + } + + private static bool HoldsMemberOfKind(INamedTypeSymbol existingType, string name, SymbolKind kind) + { + foreach (ISymbol member in existingType.GetMembers(name)) + { + if (member.Kind == kind) + { + return true; + } + } + + return false; + } +} diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypeAddedMemberStubs.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypeAddedMemberStubs.cs new file mode 100644 index 000000000..6781daaee --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypeAddedMemberStubs.cs @@ -0,0 +1,127 @@ +using System; +using System.Collections.Generic; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Text; + +// Picks the bodies of a newly introduced declaration that its artifact cannot compile because they +// name members this reload's sources add to a type that already exists, and plans a throwing stub +// for each. The artifact binds against the compiled assembly and the retained artifacts, neither +// of which holds those members; the transform of the same run patches the real body onto the +// artifact instead, exactly as it patches a body edit of a type an earlier reload introduced. +// Only the bodies that patch can replace are stubbed - ordinary methods, and properties whose +// getter is the only accessor with a body - so no other body is ever left throwing. +internal static class IntroducedTypeAddedMemberStubs +{ + private const string StubExceptionTypeName = "global::System.InvalidOperationException"; + + internal static IntroducedTypeStubPlan Plan( + BaseTypeDeclarationSyntax declaration, + INamedTypeSymbol typeSymbol, + SemanticModel semanticModel, + AddedMemberReferenceClassifier classifier, + string ownerProjectRelativePath) + { + IntroducedTypeDeclarationMemberIndex memberIndex = IntroducedTypeDeclarationMemberIndex.Build( + declaration, + CecilTypeNames.ToMetadataName(typeSymbol)); + IReadOnlyList members = IntroducedTypeMemberRegions.CollectMembers(declaration); + List fingerprintKeys = new List(); + List methodKeys = new List(); + List bodyChanges = new List(); + for (int index = 0; index < memberIndex.OrderedKeys.Count; index++) + { + string memberKey = memberIndex.OrderedKeys[index]; + IMethodSymbol patchedMethod = FindPatchableMethod(members[index], memberKey, memberIndex, semanticModel); + if (patchedMethod == null) + { + continue; + } + + IReadOnlyList bodyNodes = IntroducedTypeMemberRegions.CollectBodyNodes(members[index]); + if (classifier.Classify(bodyNodes, semanticModel) != AddedMemberUse.MethodsFieldsOrProperties) + { + continue; + } + + // Two members spelling one entry key, such as a getter and a method named after it, do + // not compile; the later one keeps its body so the artifact reports that error. + string methodKey = WorkerMethodKeys.BuildMethodKeyFromSymbol(patchedMethod); + if (methodKeys.Contains(methodKey)) + { + continue; + } + + fingerprintKeys.Add(memberKey); + methodKeys.Add(methodKey); + string literal = SyntaxFactory.Literal( + BuildStubMessage(typeSymbol, patchedMethod, ownerProjectRelativePath)).Text; + foreach (SyntaxNode bodyNode in bodyNodes) + { + bodyChanges.Add(BuildStubChange(bodyNode, literal)); + } + } + + if (fingerprintKeys.Count == 0) + { + return IntroducedTypeStubPlan.None; + } + + return new IntroducedTypeStubPlan(fingerprintKeys, methodKeys, bodyChanges); + } + + // The method whose patch replaces the member's body: the method itself, or the getter of a + // property whose getter is its only accessor with a body. Null for every other member, which + // is the answer the fingerprint comparison gives when it reads the member's body as one the + // reload cannot patch. + private static IMethodSymbol FindPatchableMethod( + MemberDeclarationSyntax member, + string memberKey, + IntroducedTypeDeclarationMemberIndex memberIndex, + SemanticModel semanticModel) + { + if (member is MethodDeclarationSyntax method && memberIndex.FindSyntaxMethodKey(memberKey) != null) + { + return semanticModel.GetDeclaredSymbol(method); + } + + if (member is PropertyDeclarationSyntax property && memberIndex.FindSyntaxGetterPropertyKey(memberKey) != null) + { + return semanticModel.GetDeclaredSymbol(property)?.GetMethod; + } + + return null; + } + + private static string BuildStubMessage( + INamedTypeSymbol typeSymbol, + IMethodSymbol patchedMethod, + string ownerProjectRelativePath) + { + string memberName = patchedMethod.AssociatedSymbol?.Name ?? patchedMethod.Name; + return "'" + typeSymbol.ToDisplayString() + "." + memberName + + "' calls members that a hot reload added, so its body runs only while hot reload patches it in. Reload '" + + ownerProjectRelativePath + "' again, or run 'uloop compile'."; + } + + // Keeps the form of the body it replaces, so an expression-bodied member keeps the semicolon + // that closes it and a block keeps the braces the surrounding trivia is laid out around. + private static TextChange BuildStubChange(SyntaxNode bodyNode, string messageLiteral) + { + string throwExpression = "throw new " + StubExceptionTypeName + "(" + messageLiteral + ")"; + if (bodyNode is BlockSyntax block) + { + return new TextChange(block.Span, "{ " + throwExpression + "; }"); + } + + if (bodyNode is ArrowExpressionClauseSyntax arrow) + { + return new TextChange(arrow.Span, "=> " + throwExpression); + } + + throw new ArgumentException( + "Only a block or an expression body can be stubbed: " + bodyNode.Kind(), + nameof(bodyNode)); + } +} diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypePlanner.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypePlanner.cs index 02302db19..2f796e198 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypePlanner.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypePlanner.cs @@ -26,7 +26,8 @@ internal static void Plan( string targetAssemblyMvid, IntroducedTypeArtifactMap artifactMap, IReadOnlyList defineSymbols, - IReadOnlyList assemblyGlobalUsings) + IReadOnlyList assemblyGlobalUsings, + AddedMemberReferenceClassifier addedMemberClassifier) { foreach (BaseTypeDeclarationSyntax declaration in unit.Root.DescendantNodes().OfType()) { @@ -82,7 +83,7 @@ internal static void Plan( } string metadataName = CecilTypeNames.ToMetadataName(typeSymbol); - string declarationFingerprint = IntroducedTypeFingerprint.Compute( + HotReloadIntroducedTypeFingerprint fingerprint = IntroducedTypeFingerprint.Compute( declaration, defineSymbols, typeSymbol, @@ -90,7 +91,8 @@ internal static void Plan( home.AssemblySymbol, home.AssemblyName, targetAssemblyMvid, - artifactMap).Serialize(); + artifactMap); + string declarationFingerprint = fingerprint.Serialize(); if (IntroducedTypeReuseDecider.IsAlreadyIntroduced( unit, artifactMap, @@ -103,6 +105,15 @@ internal static void Plan( continue; } + // Why the record carries the stubbed fingerprint: the artifact does not run the stubbed + // bodies the source spells, so every later comparison has to read them as edited bodies + // for the transform to patch, the run that introduces the type included. + IntroducedTypeStubPlan stubs = IntroducedTypeAddedMemberStubs.Plan( + declaration, + typeSymbol, + unit.SemanticModel, + addedMemberClassifier, + unit.Input.ProjectRelativePath); unit.IntroducedTypes.Add( new WorkerIntroducedType { @@ -110,8 +121,9 @@ internal static void Plan( OriginalAssemblyMvid = targetAssemblyMvid ?? string.Empty, MetadataName = metadataName, OwnerProjectRelativePath = unit.Input.ProjectRelativePath, - DeclarationFingerprint = declarationFingerprint, - Source = BuildTypeSource(unit.Root, typeSymbol, declaration, assemblyGlobalUsings) + DeclarationFingerprint = fingerprint.WithStubbedBodies(stubs.FingerprintKeys).Serialize(), + Source = BuildTypeSource(unit.Root, typeSymbol, declaration, assemblyGlobalUsings, stubs.BodyChanges), + StubbedMethodKeys = stubs.MethodKeys.ToArray() }); } @@ -270,7 +282,8 @@ private static string BuildTypeSource( CompilationUnitSyntax root, INamedTypeSymbol typeSymbol, BaseTypeDeclarationSyntax declaration, - IReadOnlyList assemblyGlobalUsings) + IReadOnlyList assemblyGlobalUsings, + IReadOnlyList bodyChanges) { StringBuilder builder = new StringBuilder(); foreach (ExternAliasDirectiveSyntax externAlias in root.Externs) @@ -297,7 +310,7 @@ private static string BuildTypeSource( } } - builder.Append(IntroducedTypeSourceAccessibility.ToArtifactDeclarationText(declaration)); + builder.Append(IntroducedTypeSourceAccessibility.ToArtifactDeclarationText(declaration, bodyChanges)); for (int index = 0; index < namespaceDeclarations.Count; index++) { builder.AppendLine("}"); @@ -306,7 +319,7 @@ private static string BuildTypeSource( } AppendRootUsings(builder, root, assemblyGlobalUsings); - builder.Append(IntroducedTypeSourceAccessibility.ToArtifactDeclarationText(declaration)); + builder.Append(IntroducedTypeSourceAccessibility.ToArtifactDeclarationText(declaration, bodyChanges)); return builder.ToString(); } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypePreparation.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypePreparation.cs index 8310d7e69..a68398cff 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypePreparation.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypePreparation.cs @@ -112,6 +112,11 @@ internal static WorkerOutput Prepare(WorkerInput input) syntaxTrees, references, compilation); + AddedMemberReferenceClassifier addedMemberClassifier = new AddedMemberReferenceClassifier( + compilation, + home, + artifactMap, + input.TargetAssemblyMvid); WorkerFileOutput[] files = new WorkerFileOutput[units.Count]; string[][] plannedAddedMemberNames = new string[units.Count][]; string[][] plannedAddedEnumMemberNames = new string[units.Count][]; @@ -133,7 +138,8 @@ internal static WorkerOutput Prepare(WorkerInput input) input.TargetAssemblyMvid, artifactMap, input.Defines, - assemblyGlobalUsings); + assemblyGlobalUsings, + addedMemberClassifier); plannedAddedMemberNames[index] = PlannedAddedMemberNames.Collect( unit, home, diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypeSourceAccessibility.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypeSourceAccessibility.cs index 348f90acc..2d0a88ef1 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypeSourceAccessibility.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypeSourceAccessibility.cs @@ -1,5 +1,7 @@ using System; +using System.Collections.Generic; using System.Linq; +using System.Text; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -7,9 +9,10 @@ // The accessibility an introduced declaration is compiled with. The artifact is an assembly of // its own, so a type the user left internal, or wrote without an access modifier, has to be public -// there for the compiled assembly to name it. Only that one access token changes: comments, -// attributes, directives and line endings reach the artifact exactly as the user wrote them, and -// the fingerprint keeps describing the declaration as written. +// there for the compiled assembly to name it. Only that one access token changes, apart from the +// bodies IntroducedTypeAddedMemberStubs replaces: comments, attributes, directives and line endings +// reach the artifact exactly as the user wrote them, and the fingerprint keeps describing the +// declaration as written. internal static class IntroducedTypeSourceAccessibility { private const string FileModifierText = "file"; @@ -24,18 +27,33 @@ internal static bool IsFileLocal(BaseTypeDeclarationSyntax declaration) return declaration.Modifiers.Any(modifier => modifier.ValueText == FileModifierText); } - /// The declaration text with its implicit or internal accessibility made public. - internal static string ToArtifactDeclarationText(BaseTypeDeclarationSyntax declaration) + /// + /// The declaration text with its implicit or internal accessibility made public and each of + /// the body changes applied. The body changes sit at positions of the tree that holds the + /// declaration, inside its members, so none of them overlaps the accessibility change. + /// + internal static string ToArtifactDeclarationText( + BaseTypeDeclarationSyntax declaration, + IReadOnlyList bodyChanges) { - string original = declaration.ToFullString(); + List changes = new List(bodyChanges); TextChange? promotion = PromotionChange(declaration); - if (promotion == null) + if (promotion != null) + { + changes.Add(promotion.Value); + } + + // Applied from the last position back, so every change still finds the text at the offset + // it was computed for. + changes.Sort((left, right) => right.Span.Start.CompareTo(left.Span.Start)); + StringBuilder text = new StringBuilder(declaration.ToFullString()); + foreach (TextChange change in changes) { - return original; + int start = change.Span.Start - declaration.FullSpan.Start; + text.Remove(start, change.Span.Length).Insert(start, change.NewText); } - int start = promotion.Value.Span.Start - declaration.FullSpan.Start; - return original.Remove(start, promotion.Value.Span.Length).Insert(start, promotion.Value.NewText); + return text.ToString(); } /// diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypeStubPlan.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypeStubPlan.cs new file mode 100644 index 000000000..c4bfd1855 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/IntroducedTypeStubPlan.cs @@ -0,0 +1,32 @@ +using System; +using System.Collections.Generic; +using Microsoft.CodeAnalysis.Text; + +// The bodies one introduced declaration's artifact stubs: the fingerprint keys its record marks as +// stubbed, the method keys the transform has to patch before the artifact may be activated, and +// the changes that replace each body in the declaration's text. +internal sealed class IntroducedTypeStubPlan +{ + internal static readonly IntroducedTypeStubPlan None = new IntroducedTypeStubPlan( + Array.Empty(), + Array.Empty(), + Array.Empty()); + + internal IntroducedTypeStubPlan( + IReadOnlyList fingerprintKeys, + IReadOnlyList methodKeys, + IReadOnlyList bodyChanges) + { + FingerprintKeys = fingerprintKeys; + MethodKeys = methodKeys; + BodyChanges = bodyChanges; + } + + internal IReadOnlyList FingerprintKeys { get; } + + /// Keys in the form the transform's entries are keyed with, one per stubbed member. + internal IReadOnlyList MethodKeys { get; } + + /// Changes at positions of the tree that holds the declaration. + internal IReadOnlyList BodyChanges { get; } +} diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/PlannedAddedMemberNames.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/PlannedAddedMemberNames.cs index 2d54e7af5..958835f2d 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/PlannedAddedMemberNames.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/PlannedAddedMemberNames.cs @@ -78,9 +78,16 @@ internal static string[] CollectCompiledEnumMembers( return names.ToArray(); } - // Why only top-level types are looked up in the artifacts: hot reload introduces top-level - // types only, so a retained artifact never serves a nested one. - private static INamedTypeSymbol FindExistingType( + /// + /// The existing type a source type describes: its compiled counterpart, or the type a retained + /// artifact serves under it. Null for a type neither holds, which includes every type this run + /// introduces. + /// + /// + /// Why only top-level types are looked up in the artifacts: hot reload introduces top-level + /// types only, so a retained artifact never serves a nested one. + /// + internal static INamedTypeSymbol FindExistingType( INamedTypeSymbol sourceType, WorkerTypeHome home, CSharpCompilation compilation, diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/RetainedTypeSignatureReferenceFinder.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/RetainedTypeSignatureReferenceFinder.cs index cadafceb6..6e5b22ea9 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/RetainedTypeSignatureReferenceFinder.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/RetainedTypeSignatureReferenceFinder.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using io.github.hatayama.UnityCliLoop.FirstPartyTools; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -46,6 +47,14 @@ internal static string FindRefusal( List referrers = FindReferrers(BuildIdentity(verdict.Record), signatureIdentitiesByReferrer); List retainedReferrers = referrers.FindAll(referrer => !preparedReferrers.Contains(referrer)); List sameRunReferrers = referrers.FindAll(referrer => preparedReferrers.Contains(referrer)); + // Why before the other two: a stubbed type stays in the source on every run that + // holds its file, whatever that run changes in it, so neither the two steps nor the + // applied-changes reading describes what keeps it there. + if (sameRunReferrers.Count > 0 && HoldsStubbedBodies(verdict.Record)) + { + return FormatStubbedTypeReferrerRefusal(verdict.MetadataName, retainedReferrers, sameRunReferrers); + } + if (sameRunReferrers.Count > 0 && verdict.HoldsOnlyAppliedChanges) { return FormatAppliedChangesReferrerRefusal(verdict.MetadataName, retainedReferrers, sameRunReferrers); @@ -148,6 +157,45 @@ private static string FormatAppliedChangesReferrerRefusal( + "."; } + // A record holding stubbed bodies never matches its source, because its bodies reach the + // artifact only as patches, so the type is kept in the source and a type this run introduces, + // compiled against the artifact's definition, is split from it on every such run. Why a + // retained referrer adds an edit to the alternative: once the new type names the stubbed one + // only inside its bodies, the retained referrer still splits it unless this reload edits it. + private static string FormatStubbedTypeReferrerRefusal( + string metadataName, + List retainedReferrers, + List sameRunReferrers) + { + string sameRunList = FormatNameList(sameRunReferrers); + string retainedClause = retainedReferrers.Count == 0 + ? string.Empty + : FormatNameList(retainedReferrers) + ", which an earlier reload retained and this edit leaves unchanged, and of "; + string retainedEdit = retainedReferrers.Count == 0 + ? string.Empty + : " and also edit " + FormatNameList(retainedReferrers) + " in this same reload (a method body change is enough)"; + return "Introduced type '" + metadataName + + "' calls members that a hot reload added, so its method bodies run through hot reload patches, " + + "and it appears in member signatures of " + + retainedClause + + sameRunList + + ". A type this reload introduces cannot name such a type in its signatures until 'uloop compile' runs. " + + "Run 'uloop compile' to apply this edit, or name '" + + metadataName + + "' only inside method bodies of " + + sameRunList + + retainedEdit + + "."; + } + + private static bool HoldsStubbedBodies(WorkerIntroducedTypeArtifactType record) + { + return HotReloadIntroducedTypeFingerprint.TryParse( + record.DeclarationFingerprint, + out HotReloadIntroducedTypeFingerprint recorded) + && recorded.HoldsStubbedBodies; + } + private static string FormatNameList(List names) { return "'" + string.Join("', '", names) + "'"; diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/WorkerIntroducedType.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/WorkerIntroducedType.cs index 37b7ae160..79ce8851f 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/WorkerIntroducedType.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/WorkerIntroducedType.cs @@ -28,4 +28,9 @@ internal sealed class WorkerIntroducedType public string DeclarationFingerprint { get; set; } public string Source { get; set; } + + // The methods whose bodies the source stubs because they call members this reload adds, keyed + // the way the transform's entries are. The artifact may only be activated once every one of + // them is patched, since until then those bodies throw instead of running. + public string[] StubbedMethodKeys { get; set; } = Array.Empty(); } diff --git a/docs/hot-reload-introduced-types.md b/docs/hot-reload-introduced-types.md index 996f5f4ea..adc33ea9a 100644 --- a/docs/hot-reload-introduced-types.md +++ b/docs/hot-reload-introduced-types.md @@ -56,7 +56,7 @@ merely re-read from source after an earlier reload retained it. This includes pr and event accessors. An inaccessible internal base does not hide Unity object ancestry: those descendants still require a compile. -Three conditions are reported as `Failed` rows in `IntroducedTypes` instead, because the run +These conditions are reported as `Failed` rows in `IntroducedTypes` instead, because the run cannot proceed as if the declaration were absent. A `Failed` row stops that assembly before any of its method bodies is transformed: the files sharing the assembly report no `Methods` rows and nothing from them is applied, while files in other assemblies still apply. @@ -68,6 +68,11 @@ nothing from them is applied, while files in other assemblies still apply. | Two files of the same reload declare the same type | `Introduced type is declared in more than one file of the group: .` | | The artifact assembly failed to compile | `Introduced-type compilation failed: ` | +One more `Failed` row comes later, after the method bodies are transformed: `Not introduced: + calls members that a hot reload added, …` when the reload leaves a body the artifact +stubs unpatched (see "When a compile is still required"). The other types of that artifact fail +with it, and nothing from that assembly's files is applied. + ## Access to the target assembly's internals An introduced type can use what the assembly it belongs to keeps `internal`: members declared @@ -172,16 +177,29 @@ recompiled rather than reused once that generation is gone. - Anything that reads the type through Unity: serialization, `[SerializeField]`, Inspector display, `AddComponent`, `ScriptableObject.CreateInstance`, Unity message discovery. - A new or changed `.asmdef` / `.asmref`. Assembly layout is decided at compile time. -- A call to a member an earlier or the same reload *added* to a compiled type (an `Added` row). +- A call to a member an earlier or the same reload *added* (an `Added` row) from a new type's + constructor, initializer, setter, indexer, operator or event accessor, or to an addition in + another assembly or in a file that is neither passed nor unchanged since it was applied. Introduced types compile against the compiled assemblies and the retained artifacts only, so - the artifact compilation fails with the compiler error naming the missing member. + the artifact compilation fails with the compiler error naming the missing member. Ordinary + methods and get-only properties can make the call: the artifact compiles their bodies as + throwing stubs, and the same reload activates the type only when it holds a patch for every + stub, then applies those patches right after the activation. If one of them fails to apply + there, the type stays active, that method's row is `Failed`, and the body runs its stub until + a later reload patches it in. A stubbed body the reload does not patch (a generic method, a struct method) is reported as + `Not introduced: calls members that a hot reload added, …` and keeps every type of + that artifact out, and a type the same reload introduces cannot name a stubbed type in its + member signatures (`Introduced type '' calls members that a hot reload added, so its + method bodies run through hot reload patches, …`). ## Lifecycle - `uloop hot-reload --revert-all` reverts patched methods and added members. It does **not** unload an introduced type: the response says how many stayed (`N introduced type(s) stay loaded until the next Domain Reload; a revert cannot unload the - assembly that carries them.`). + assembly that carries them.`). A body the reload patched over a stub runs the stub again, + which throws `InvalidOperationException` naming the file to reload until that file is + reloaded. - Auto Refresh stays held while any introduced type is active, so returning focus to the Editor does not recompile. `--revert-all` releases the hold only when no introduced type remains; `uloop compile` always releases it.