diff --git a/.agents/skills/uloop-hot-reload/SKILL.md b/.agents/skills/uloop-hot-reload/SKILL.md index 4a5fb70e06..5a341c1fb0 100644 --- a/.agents/skills/uloop-hot-reload/SKILL.md +++ b/.agents/skills/uloop-hot-reload/SKILL.md @@ -71,9 +71,9 @@ changed are patched (`UnchangedTotal` counts the rest). refused with a `Warnings` line naming the reason. Use from another assembly or from files outside the reload, reflection, serialization, and Unity message discovery still need `uloop compile`. -- Signature changes: a return-type change is `Skipped` unless this reload or an earlier one - patched every live compiled caller of the old signature; a rename or parameter change - applies as an added method and warns about the call sites left on the old signature. +- Signature changes: a return-type change is `Skipped` unless this or an earlier reload + patched every live compiled caller, none in another assembly; a rename or parameter change + applies as an added method and warns about call sites left on the old signature. - Constructors, operators, struct methods, compiled setter/init/indexer accessors, and event accessors are `Skipped`; finalizers and interface members are silently not applied. - A reload applies each file all-or-nothing: a `Failed` method leaves that file unapplied, 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 4f4b8019ad..d9f22e25c4 100644 --- a/.agents/skills/uloop-hot-reload/references/scope-and-limits.md +++ b/.agents/skills/uloop-hot-reload/references/scope-and-limits.md @@ -180,25 +180,34 @@ the assembly, the Editor-session illusion, and the `virtual`/generic/interface exclusions. A gate protects compiled callers: the change applies only when every live compiled -call site of the old signature is patched by the same reload. A caller this reload -did not edit — in another file, in another assembly, or an *unedited* method in the +call site of the old signature is in the same assembly and patched by the same reload. +A caller this reload did not edit — in another file or an *unedited* method in the edited file itself (an implicit `int`→`long` widening can leave a caller's source untouched) — would keep calling the old method silently, so the run reports the changed method and its edited callers as `Skipped` instead; land the change with -`uloop compile`. When every uncovered caller is in the edited file itself, the +`uloop compile`. A caller in another assembly gates the change even when this or an +earlier reload patched it: that patch is compiled against the compiled assembly, where +the old signature still exists. When every uncovered caller is in the edited file itself, the `Skipped` reason names those callers: editing their bodies and reloading again applies them together without `uloop compile`. Call sites inside methods that the same edit removes or re-signatures do not gate: those compiled bodies are already stale, and anything still reaching them stays on the consistent old behavior. -If an earlier reload already patched the compiled call sites, a later signature change applies without editing the callers; the response then carries a warning naming the call sites this run re-applied on the new signature. +If an earlier reload already patched the compiled call sites in the same assembly, a later signature change applies without editing the callers; the response then carries a warning naming the call sites this run re-applied on the new signature. Renaming a method or changing its parameter list follows the delete rules rather than the gate: the new signature is an ordinary added method, the old one is reported removed, and a `Warnings` entry names each compiled call site of the old signature that the reload leaves unpatched — those call sites keep the previous behavior until `uloop compile`. Deleting a method emits the same warning when -compiled callers remain. +compiled callers remain. A caller whose patch is active when the reload ends — +patched by this reload in any assembly, or kept from an earlier reload — is left out, +because it no longer runs its compiled body. The warning does not check what the +patched body calls, and two leftovers of the compiled caller can still reach the old +method: a copy the JIT inlined into another method before the patch, and a delegate to +the old method the caller created before it. A call inside a lambda or local function +stays listed under its compiler-generated name even when the method declaring it is +patched. Field declarations are stricter: when a compiled field's type — or its `static`/ `const` modifier — differs from the edited source, every edited method that reads diff --git a/.claude/skills/uloop-hot-reload/SKILL.md b/.claude/skills/uloop-hot-reload/SKILL.md index 4a5fb70e06..5a341c1fb0 100644 --- a/.claude/skills/uloop-hot-reload/SKILL.md +++ b/.claude/skills/uloop-hot-reload/SKILL.md @@ -71,9 +71,9 @@ changed are patched (`UnchangedTotal` counts the rest). refused with a `Warnings` line naming the reason. Use from another assembly or from files outside the reload, reflection, serialization, and Unity message discovery still need `uloop compile`. -- Signature changes: a return-type change is `Skipped` unless this reload or an earlier one - patched every live compiled caller of the old signature; a rename or parameter change - applies as an added method and warns about the call sites left on the old signature. +- Signature changes: a return-type change is `Skipped` unless this or an earlier reload + patched every live compiled caller, none in another assembly; a rename or parameter change + applies as an added method and warns about call sites left on the old signature. - Constructors, operators, struct methods, compiled setter/init/indexer accessors, and event accessors are `Skipped`; finalizers and interface members are silently not applied. - A reload applies each file all-or-nothing: a `Failed` method leaves that file unapplied, 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 4f4b8019ad..d9f22e25c4 100644 --- a/.claude/skills/uloop-hot-reload/references/scope-and-limits.md +++ b/.claude/skills/uloop-hot-reload/references/scope-and-limits.md @@ -180,25 +180,34 @@ the assembly, the Editor-session illusion, and the `virtual`/generic/interface exclusions. A gate protects compiled callers: the change applies only when every live compiled -call site of the old signature is patched by the same reload. A caller this reload -did not edit — in another file, in another assembly, or an *unedited* method in the +call site of the old signature is in the same assembly and patched by the same reload. +A caller this reload did not edit — in another file or an *unedited* method in the edited file itself (an implicit `int`→`long` widening can leave a caller's source untouched) — would keep calling the old method silently, so the run reports the changed method and its edited callers as `Skipped` instead; land the change with -`uloop compile`. When every uncovered caller is in the edited file itself, the +`uloop compile`. A caller in another assembly gates the change even when this or an +earlier reload patched it: that patch is compiled against the compiled assembly, where +the old signature still exists. When every uncovered caller is in the edited file itself, the `Skipped` reason names those callers: editing their bodies and reloading again applies them together without `uloop compile`. Call sites inside methods that the same edit removes or re-signatures do not gate: those compiled bodies are already stale, and anything still reaching them stays on the consistent old behavior. -If an earlier reload already patched the compiled call sites, a later signature change applies without editing the callers; the response then carries a warning naming the call sites this run re-applied on the new signature. +If an earlier reload already patched the compiled call sites in the same assembly, a later signature change applies without editing the callers; the response then carries a warning naming the call sites this run re-applied on the new signature. Renaming a method or changing its parameter list follows the delete rules rather than the gate: the new signature is an ordinary added method, the old one is reported removed, and a `Warnings` entry names each compiled call site of the old signature that the reload leaves unpatched — those call sites keep the previous behavior until `uloop compile`. Deleting a method emits the same warning when -compiled callers remain. +compiled callers remain. A caller whose patch is active when the reload ends — +patched by this reload in any assembly, or kept from an earlier reload — is left out, +because it no longer runs its compiled body. The warning does not check what the +patched body calls, and two leftovers of the compiled caller can still reach the old +method: a copy the JIT inlined into another method before the patch, and a delegate to +the old method the caller created before it. A call inside a lambda or local function +stays listed under its compiler-generated name even when the method declaring it is +patched. Field declarations are stricter: when a compiled field's type — or its `static`/ `const` modifier — differs from the edited source, every edited method that reads diff --git a/Assets/Tests/Editor/HotReload/HotReloadCrossAssemblyStaleSignatureE2ETests.cs b/Assets/Tests/Editor/HotReload/HotReloadCrossAssemblyStaleSignatureE2ETests.cs new file mode 100644 index 0000000000..30e3a95067 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadCrossAssemblyStaleSignatureE2ETests.cs @@ -0,0 +1,423 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +using NUnit.Framework; + +using UnityEngine; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; +using io.github.hatayama.UnityCliLoop.ToolContracts; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// End-to-end EditMode coverage for the stale-signature warning and the signature-change gate + /// when the only compiled callers of the edited host live in another assembly. + /// + public class HotReloadCrossAssemblyStaleSignatureE2ETests + { + private const string HostFileName = "HotReloadCrossAssemblyStaleSignatureHost.cs"; + private const string CallerFileName = "HotReloadCrossAssemblyStaleSignatureCaller.cs"; + private const string FixtureNamespace = "io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload."; + private const string RemovedSignatureKey = + FixtureNamespace + "HotReloadCrossAssemblyStaleSignatureHost::ToDelete(System.Int32)"; + private const string CallerKey = + FixtureNamespace + "HotReloadCrossAssemblyStaleSignatureCaller::CallDeleted(System.Int32)"; + private const string CallerLabel = + FixtureNamespace + "HotReloadCrossAssemblyStaleSignatureCaller.CallDeleted(System.Int32)"; + private const string ReturnTypeTargetLabel = + FixtureNamespace + "HotReloadCrossAssemblyStaleSignatureHost.ReturnTypeTarget(System.Int32)"; + + private const string HostUnrelatedAndToDeleteAnchor = + " public int Unrelated(int value)\n {\n return value;\n }\n\n" + + " [MethodImpl(MethodImplOptions.NoInlining)]\n" + + " public int ToDelete(int value)\n {\n return value;\n }"; + private const string HostUnrelatedEditedWithoutToDelete = + " public int Unrelated(int value)\n {\n return value + 1;\n }"; + private const string HostUnrelatedAnchor = + " public int Unrelated(int value)\n {\n return value;\n }"; + private const string HostUnrelatedEdited = + " public int Unrelated(int value)\n {\n return value + 1;\n }"; + private const string HostReturnTypeTargetAnchor = + " public int ReturnTypeTarget(int value)\n {\n return value;\n }"; + private const string HostReturnTypeTargetWidened = + " public long ReturnTypeTarget(int value)\n {\n return value + 1L;\n }"; + private const string CallerDeletedCallAnchor = + " return new HotReloadCrossAssemblyStaleSignatureHost().ToDelete(value);"; + private const string CallerReturnTypeTargetCallAnchor = + " return new HotReloadCrossAssemblyStaleSignatureHost().ReturnTypeTarget(value);"; + + private HotReloadDomainTestScope _scope; + + [SetUp] + public void SetUp() + { + _scope = new HotReloadDomainTestScope(); + HotReloadAutoRefreshHold.SyncToActiveChanges(); + } + + [TearDown] + public void TearDown() + { + _scope.Dispose(); + HotReloadAutoRefreshHold.SyncToActiveChanges(); + VibeLogger.ClearMemoryLogs(); + } + + /// + /// What: when the caller's group runs before the host's group in one reload, the caller it + /// patched is left out of the host's stale-signature warning, and the warning is gone + /// because no compiled caller remains. + /// + [Test] + public async Task Run_CrossAssemblyCallerPatchedInEarlierGroup_IsOmittedFromStaleSignatureWarning() + { + string hostPath = HostPath(); + string callerPath = CallerPath(); + + HotReloadOrchestratorResult result = await RunAsync( + new[] { callerPath, hostPath }, + new Dictionary + { + [callerPath] = WriteCallerWithoutDeletedCall("CrossAssemblyStaleCallerFirst.cs"), + [hostPath] = WriteHostWithoutToDelete("CrossAssemblyStaleHostSecond.cs") + }); + + AssertNoFailure(result); + AssertKind(result, HotReloadMethodOutcomeKind.Patched, "CallDeleted"); + AssertKind(result, HotReloadMethodOutcomeKind.Patched, "Unrelated"); + Assert.That(FindStaleSignatureWarnings(result), Is.Empty, FormatWarnings(result)); + } + + /// + /// What: when the host's group runs before the caller's group in one reload, the caller the + /// later group patched is still left out of the host's stale-signature warning. + /// + [Test] + public async Task Run_CrossAssemblyCallerPatchedInLaterGroup_IsOmittedFromStaleSignatureWarning() + { + string hostPath = HostPath(); + string callerPath = CallerPath(); + + HotReloadOrchestratorResult result = await RunAsync( + new[] { hostPath, callerPath }, + new Dictionary + { + [hostPath] = WriteHostWithoutToDelete("CrossAssemblyStaleHostFirst.cs"), + [callerPath] = WriteCallerWithoutDeletedCall("CrossAssemblyStaleCallerSecond.cs") + }); + + AssertNoFailure(result); + AssertKind(result, HotReloadMethodOutcomeKind.Patched, "CallDeleted"); + AssertKind(result, HotReloadMethodOutcomeKind.Patched, "Unrelated"); + Assert.That(FindStaleSignatureWarnings(result), Is.Empty, FormatWarnings(result)); + } + + /// + /// What: a reload of the host alone leaves out a caller that an earlier reload patched and + /// that is still active. + /// + [Test] + public async Task Run_HostOnlyReloadWithActiveCrossAssemblyCaller_IsOmittedFromStaleSignatureWarning() + { + string hostPath = HostPath(); + string callerPath = CallerPath(); + HotReloadOrchestratorResult callerRun = await RunAsync( + new[] { callerPath }, + new Dictionary + { + [callerPath] = WriteCallerWithoutDeletedCall("CrossAssemblyStaleCallerEarlier.cs") + }); + AssertKind(callerRun, HotReloadMethodOutcomeKind.Patched, "CallDeleted"); + + HotReloadOrchestratorResult hostRun = await RunAsync( + new[] { hostPath }, + new Dictionary + { + [hostPath] = WriteHostWithoutToDelete("CrossAssemblyStaleHostAlone.cs") + }); + + AssertNoFailure(hostRun); + AssertKind(hostRun, HotReloadMethodOutcomeKind.Patched, "Unrelated"); + Assert.That(FindStaleSignatureWarnings(hostRun), Is.Empty, FormatWarnings(hostRun)); + } + + /// + /// What: a reload of the host alone still names a caller in another assembly that no reload + /// patched, since it keeps running its compiled body. + /// + [Test] + public async Task Run_HostOnlyReloadWithUnpatchedCrossAssemblyCaller_StillWarns() + { + string hostPath = HostPath(); + + HotReloadOrchestratorResult result = await RunAsync( + new[] { hostPath }, + new Dictionary + { + [hostPath] = WriteHostWithoutToDelete("CrossAssemblyStaleHostUnpatchedCaller.cs") + }); + + AssertNoFailure(result); + Assert.That( + FindStaleSignatureWarnings(result), + Is.EqualTo(new[] { FormatStaleSignatureWarning() }), + FormatWarnings(result)); + } + + /// + /// What: a caller whose earlier patch is active when the host's group runs, but that a later + /// group of the same reload peels back to its compiled body, is still named, because the + /// warning reflects the patches that are active when the reload ends. + /// + [Test] + public async Task Run_CrossAssemblyCallerPeeledLaterInSameRun_StillWarns() + { + string hostPath = HostPath(); + string callerPath = CallerPath(); + HotReloadOrchestratorResult callerRun = await RunAsync( + new[] { callerPath }, + new Dictionary + { + [callerPath] = WriteCallerWithoutDeletedCall("CrossAssemblyStaleCallerToPeel.cs") + }); + AssertKind(callerRun, HotReloadMethodOutcomeKind.Patched, "CallDeleted"); + + // Why the caller keeps its on-disk content: it matches the compiled assembly, so its + // group peels the earlier patch after the host's group has already been gated. + HotReloadOrchestratorResult result = await RunAsync( + new[] { hostPath, callerPath }, + new Dictionary + { + [hostPath] = WriteHostWithoutToDelete("CrossAssemblyStaleHostBeforePeel.cs") + }); + + AssertNoFailure(result); + Assert.That(IsActive(CallerLabel), Is.False, "Precondition: the caller patch was peeled."); + Assert.That( + FindStaleSignatureWarnings(result), + Is.EqualTo(new[] { FormatStaleSignatureWarning() }), + FormatWarnings(result)); + } + + /// + /// What: an active caller patch is left out even when its own body still calls the removed + /// method, because the warning does not look inside patched bodies (documented limit). + /// + [Test] + public async Task Run_ActiveCrossAssemblyCallerStillCallingRemovedMethod_IsOmitted() + { + string hostPath = HostPath(); + string callerPath = CallerPath(); + HotReloadOrchestratorResult callerRun = await RunAsync( + new[] { callerPath }, + new Dictionary + { + [callerPath] = HotReloadTestSourceWriter.WriteEditedSource( + "CrossAssemblyStaleCallerStillCalling.cs", + ReplaceInSource( + File.ReadAllText(callerPath), + CallerDeletedCallAnchor, + " return new HotReloadCrossAssemblyStaleSignatureHost().ToDelete(value) + 7;")) + }); + AssertKind(callerRun, HotReloadMethodOutcomeKind.Patched, "CallDeleted"); + + HotReloadOrchestratorResult hostRun = await RunAsync( + new[] { hostPath }, + new Dictionary + { + [hostPath] = WriteHostWithoutToDelete("CrossAssemblyStaleHostAfterStillCalling.cs") + }); + + AssertNoFailure(hostRun); + Assert.That(FindStaleSignatureWarnings(hostRun), Is.Empty, FormatWarnings(hostRun)); + } + + /// + /// What: a return-type change is still refused when its only compiled caller lives in + /// another assembly and an earlier reload patched it, because that patch was compiled + /// against the old signature. + /// + [Test] + public async Task Run_ReturnTypeChangeWithOnlyCrossAssemblyActiveCaller_StaysGated() + { + string hostPath = HostPath(); + string callerPath = CallerPath(); + HotReloadOrchestratorResult callerRun = await RunAsync( + new[] { callerPath }, + new Dictionary + { + [callerPath] = HotReloadTestSourceWriter.WriteEditedSource( + "CrossAssemblyGatedCaller.cs", + ReplaceInSource( + File.ReadAllText(callerPath), + CallerReturnTypeTargetCallAnchor, + " return new HotReloadCrossAssemblyStaleSignatureHost().ReturnTypeTarget(value) + 7;")) + }); + AssertKind(callerRun, HotReloadMethodOutcomeKind.Patched, "CallReturnTypeTarget"); + string hostSource = ReplaceInSource( + ReplaceInSource(File.ReadAllText(hostPath), HostReturnTypeTargetAnchor, HostReturnTypeTargetWidened), + HostUnrelatedAnchor, + HostUnrelatedEdited); + + HotReloadOrchestratorResult hostRun = await RunAsync( + new[] { hostPath }, + new Dictionary + { + [hostPath] = HotReloadTestSourceWriter.WriteEditedSource("CrossAssemblyGatedHost.cs", hostSource) + }); + + HotReloadMethodOutcome skipped = + FindOutcome(hostRun, HotReloadMethodOutcomeKind.Skipped, "Host.ReturnTypeTarget"); + Assert.That( + skipped.Reason, + Is.EqualTo(string.Format(HotReloadConstants.SignatureChangedGateSkipReasonFormat, ReturnTypeTargetLabel))); + } + + private static async Task RunAsync( + string[] files, + Dictionary contentPathOverrideByFile) + { + return await HotReloadCompositionRoot.Services.Orchestrator.RunAsync( + files, + contentPathOverride: null, + CancellationToken.None, + contentPathOverrideByFile); + } + + private static string WriteHostWithoutToDelete(string fileName) + { + return HotReloadTestSourceWriter.WriteEditedSource( + fileName, + ReplaceInSource( + File.ReadAllText(HostPath()), + HostUnrelatedAndToDeleteAnchor, + HostUnrelatedEditedWithoutToDelete)); + } + + private static string WriteCallerWithoutDeletedCall(string fileName) + { + return HotReloadTestSourceWriter.WriteEditedSource( + fileName, + ReplaceInSource( + File.ReadAllText(CallerPath()), + CallerDeletedCallAnchor, + " return value + 7;")); + } + + private static List FindStaleSignatureWarnings(HotReloadOrchestratorResult result) + { + // Why the quoted key: only the stale-signature warning quotes the removed wire key. + string quotedKey = "'" + RemovedSignatureKey + "'"; + List warnings = new List(); + foreach (string warning in result.Warnings) + { + if (warning.Contains(quotedKey)) + { + warnings.Add(warning); + } + } + + return warnings; + } + + private static string FormatStaleSignatureWarning() + { + return string.Format( + HotReloadConstants.StaleSignatureCallersWarningFormat, + RemovedSignatureKey, + CallerKey); + } + + private static bool IsActive(string methodLabel) + { + foreach (HotReloadActivePatchInfo patch in HotReloadCompositionRoot.Services.Patcher.DescribeActivePatches()) + { + if (patch.MethodKey == methodLabel) + { + return true; + } + } + + return false; + } + + private static string ReplaceInSource(string source, string anchor, string replacement) + { + Assert.That(source, Does.Contain(anchor), "Precondition: anchor must exist: " + anchor); + return source.Replace(anchor, replacement, StringComparison.Ordinal); + } + + private static string HostPath() + { + return FixturePath(Path.Combine("HotReload", HostFileName)); + } + + private static string CallerPath() + { + return FixturePath(Path.Combine("HotReloadCallSiteCrossAssembly", CallerFileName)); + } + + private static string FixturePath(string relativePath) + { + string path = Path.GetFullPath(Path.Combine(Application.dataPath, "Tests", "Editor", relativePath)); + Assert.That(File.Exists(path), Is.True, "Fixture missing: " + path); + return path; + } + + private static void AssertNoFailure(HotReloadOrchestratorResult result) + { + foreach (HotReloadMethodOutcome outcome in result.Methods) + { + Assert.That( + outcome.Kind, + Is.Not.EqualTo(HotReloadMethodOutcomeKind.Failed), + "Unexpected failure.\n" + FormatOutcomes(result)); + } + } + + private static void AssertKind( + HotReloadOrchestratorResult result, + HotReloadMethodOutcomeKind kind, + string methodNamePart) + { + FindOutcome(result, kind, methodNamePart); + } + + private static HotReloadMethodOutcome FindOutcome( + HotReloadOrchestratorResult result, + HotReloadMethodOutcomeKind kind, + string methodNamePart) + { + foreach (HotReloadMethodOutcome outcome in result.Methods) + { + if (outcome.Kind == kind && outcome.Method != null && outcome.Method.Contains(methodNamePart)) + { + return outcome; + } + } + + Assert.Fail("Expected " + kind + " for " + methodNamePart + ".\n" + FormatOutcomes(result)); + return null; + } + + private static string FormatOutcomes(HotReloadOrchestratorResult result) + { + List lines = new List(); + foreach (HotReloadMethodOutcome outcome in result.Methods) + { + lines.Add(outcome.Kind + " " + outcome.Method + " @" + outcome.FilePath + " :: " + outcome.Reason); + } + + return string.Join("\n", lines); + } + + private static string FormatWarnings(HotReloadOrchestratorResult result) + { + return "Warnings:\n" + string.Join("\n", result.Warnings) + "\nMethods:\n" + FormatOutcomes(result); + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadCrossAssemblyStaleSignatureE2ETests.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadCrossAssemblyStaleSignatureE2ETests.cs.meta new file mode 100644 index 0000000000..ae46eb9d2c --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadCrossAssemblyStaleSignatureE2ETests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: aa157d2d76849499482859efd1020f7d +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/HotReloadCrossAssemblyStaleSignatureHost.cs b/Assets/Tests/Editor/HotReload/HotReloadCrossAssemblyStaleSignatureHost.cs new file mode 100644 index 0000000000..9e4fcf3d14 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadCrossAssemblyStaleSignatureHost.cs @@ -0,0 +1,30 @@ +using System.Runtime.CompilerServices; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// Compiled host with a deletable method, a return-type-change target, and an unrelated + /// method. Its only compiled callers live in another assembly, so a signature change here + /// has callers outside the host's group. + /// + public class HotReloadCrossAssemblyStaleSignatureHost + { + [MethodImpl(MethodImplOptions.NoInlining)] + public int Unrelated(int value) + { + return value; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public int ToDelete(int value) + { + return value; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public int ReturnTypeTarget(int value) + { + return value; + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadCrossAssemblyStaleSignatureHost.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadCrossAssemblyStaleSignatureHost.cs.meta new file mode 100644 index 0000000000..8b630b338c --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadCrossAssemblyStaleSignatureHost.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 02801d275b8664ee18a0d627db129492 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/HotReloadGroupProcessorTests.cs b/Assets/Tests/Editor/HotReload/HotReloadGroupProcessorTests.cs index 54eb5d1373..9fdaf29ccb 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadGroupProcessorTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadGroupProcessorTests.cs @@ -1065,7 +1065,7 @@ internal IDisposable Install() private static HotReloadSignatureChangeGate.SignatureChangeGateResult CreateGateResultWithoutExemptions() { return HotReloadSignatureChangeGate.SignatureChangeGateResult.WarningsOnly( - new List(), + new List(), new List { new HotReloadCallSiteScanner.CallSiteHit @@ -1084,7 +1084,7 @@ private static HotReloadSignatureChangeGate.SignatureChangeGateResult CreateGate private static HotReloadSignatureChangeGate.SignatureChangeGateResult CreateEmptyGateResult() { return HotReloadSignatureChangeGate.SignatureChangeGateResult.WarningsOnly( - new List(), + new List(), new List(), new HashSet()); } @@ -1097,7 +1097,7 @@ private static HotReloadSignatureChangeGate.SignatureChangeGateResult CreateGate new HotReloadQualifiedMethodIdentity(AssemblyName, CallerKey) }; return HotReloadSignatureChangeGate.SignatureChangeGateResult.WarningsOnly( - new List(), + new List(), new List { new HotReloadCallSiteScanner.CallSiteHit @@ -1644,7 +1644,7 @@ private static HotReloadGroupFile CreateFile( HotReloadGroupFile file = new HotReloadGroupFile( path, workerSourcePath, path, AssemblyName, compilationAssembly, HotReloadTypeHome.ScriptAssembliesUnderProject(projectRoot, AssemblyName), - projectRoot, new HotReloadFileSinks(new List(), null, displayedRemovedMembers), + projectRoot, new HotReloadFileSinks(new List(), null, new HotReloadRunStaleSignatureWarnings(), displayedRemovedMembers), newSourceMembershipEvidence); file.FileOutput = new TransformWorkerFileOutputDto { @@ -1781,7 +1781,7 @@ public void SignatureChangeGateResult_Retried_ReportsTheWorkerRetryWithoutFailin HotReloadSignatureChangeGate.SignatureChangeGateResult.Retried( null, new List(), - new List(), + new List(), new List(), new HashSet(), new List()); diff --git a/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeOutcomeSinkTests.cs b/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeOutcomeSinkTests.cs index c633429fbe..66c415753f 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeOutcomeSinkTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadIntroducedTypeOutcomeSinkTests.cs @@ -67,7 +67,7 @@ private static HotReloadGroupFile CreateFile(string projectRelativePath) FindCompilationAssembly(), HotReloadTypeHome.ScriptAssembliesUnderProject(ProjectRoot, AssemblyName), ProjectRoot, - new HotReloadFileSinks(new List(), null)); + new HotReloadFileSinks(new List(), null, new HotReloadRunStaleSignatureWarnings())); } private static string ProjectRoot => diff --git a/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs b/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs index 2a25894ce9..0eebbf6895 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs @@ -3283,7 +3283,7 @@ private static HotReloadApplyContext CreateApplyContext( compilationAssembly, HotReloadTypeHome.ScriptAssembliesUnderProject(projectRoot, assemblyName), projectRoot, - new HotReloadFileSinks(new List(), null)) + new HotReloadFileSinks(new List(), null, new HotReloadRunStaleSignatureWarnings())) { FileOutput = workerOutput.files[0], SnapshotLabels = new HashSet(), diff --git a/Assets/Tests/Editor/HotReload/HotReloadRetainedTypePatchReverterTests.cs b/Assets/Tests/Editor/HotReload/HotReloadRetainedTypePatchReverterTests.cs index c7239e7690..253d0a9764 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadRetainedTypePatchReverterTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadRetainedTypePatchReverterTests.cs @@ -137,7 +137,7 @@ private static HotReloadGroupFile ArrangeRetainedDeclarationWithActivePatch() private static HotReloadGroupFile CreateFileBoundToTheRetainedDeclaration() { - HotReloadFileSinks sinks = new HotReloadFileSinks(new List(), null); + HotReloadFileSinks sinks = new HotReloadFileSinks(new List(), null, new HotReloadRunStaleSignatureWarnings()); // The row the preparation leaves for a declaration a retained artifact serves and // whose method bodies the edited source matches again. sinks.IntroducedTypes.Add( diff --git a/Assets/Tests/Editor/HotReload/HotReloadRunStaleSignatureWarningsTests.cs b/Assets/Tests/Editor/HotReload/HotReloadRunStaleSignatureWarningsTests.cs new file mode 100644 index 0000000000..ae106ab652 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadRunStaleSignatureWarningsTests.cs @@ -0,0 +1,369 @@ +using System; +using System.Collections.Generic; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// Pure tests for the run-end stale-signature warnings: which recorded compiled callers are + /// dropped because their patch is active when the run ends, and how the rest are worded. + /// + public class HotReloadRunStaleSignatureWarningsTests + { + private const string EditedAssemblyName = "EditedAssembly"; + private const string ExternalAssemblyName = "ExternalAssembly"; + private const string RemovedKey = "Example.Target::Removed()"; + private const string OtherRemovedKey = "Example.Target::AlsoRemoved()"; + private const string CallerType = "Example.Caller"; + private const string OtherCallerType = "Example.OtherCaller"; + private const string ThirdCallerType = "Example.ThirdCaller"; + + /// + /// What: a run that recorded no removed signature appends no warning. + /// + [Test] + public void AppendTo_NothingRecorded_AppendsNothing() + { + HotReloadRunStaleSignatureWarnings staleWarnings = new HotReloadRunStaleSignatureWarnings(); + List warnings = new List(); + + staleWarnings.AppendTo(warnings, new List()); + + Assert.That(warnings, Is.Empty); + } + + /// + /// What: with no caller patched at the end of the run, the warning names every caller in + /// the order they were recorded, worded exactly as the stale-signature format. + /// + [Test] + public void AppendTo_NoCallerActive_KeepsTheExactWarningText() + { + HotReloadRunStaleSignatureWarnings staleWarnings = Record( + RemovedKey, + CreateHit(EditedAssemblyName, CallerType, "Call"), + CreateHit(ExternalAssemblyName, OtherCallerType, "Call")); + List warnings = new List(); + + staleWarnings.AppendTo(warnings, new List()); + + Assert.That( + warnings, + Is.EqualTo(new[] { FormatWarning(RemovedKey, "Example.Caller::Call()", "Example.OtherCaller::Call()") })); + } + + /// + /// What: a signature whose every caller is patched when the run ends — one in the removed + /// signature's own assembly and one in another assembly — appends no warning, whether the + /// patch came from this run or an earlier one. + /// + [Test] + public void AppendTo_AllCallersActive_AppendsNoWarning() + { + HotReloadRunStaleSignatureWarnings staleWarnings = Record( + RemovedKey, + CreateHit(EditedAssemblyName, CallerType, "Call"), + CreateHit(ExternalAssemblyName, OtherCallerType, "Call")); + List warnings = new List(); + + staleWarnings.AppendTo( + warnings, + new List + { + CreateActivePatch(EditedAssemblyName, "Example.Caller.Call()"), + CreateActivePatch(ExternalAssemblyName, "Example.OtherCaller.Call()") + }); + + Assert.That(warnings, Is.Empty); + } + + /// + /// What: when some callers are patched at the end of the run, the warning names only the + /// others, keeping their recorded order. + /// + [Test] + public void AppendTo_SomeCallersActive_ListsTheRestInOrder() + { + HotReloadRunStaleSignatureWarnings staleWarnings = Record( + RemovedKey, + CreateHit(EditedAssemblyName, CallerType, "Call"), + CreateHit(ExternalAssemblyName, OtherCallerType, "Call"), + CreateHit(ExternalAssemblyName, ThirdCallerType, "Call")); + List warnings = new List(); + + staleWarnings.AppendTo( + warnings, + new List + { + CreateActivePatch(ExternalAssemblyName, "Example.OtherCaller.Call()") + }); + + Assert.That( + warnings, + Is.EqualTo(new[] { FormatWarning(RemovedKey, "Example.Caller::Call()", "Example.ThirdCaller::Call()") })); + } + + /// + /// What: a patch on another method of the caller's type does not hide the caller, because + /// only a patch on the caller itself stops its compiled body from running. + /// + [Test] + public void AppendTo_CallerNotActive_KeepsItInWarning() + { + HotReloadRunStaleSignatureWarnings staleWarnings = Record( + RemovedKey, + CreateHit(ExternalAssemblyName, CallerType, "Call")); + List warnings = new List(); + + staleWarnings.AppendTo( + warnings, + new List + { + CreateActivePatch(ExternalAssemblyName, "Example.Caller.Unrelated()") + }); + + Assert.That(warnings, Is.EqualTo(new[] { FormatWarning(RemovedKey, "Example.Caller::Call()") })); + } + + /// + /// What: a patch on a method with the caller's label in another assembly does not hide the + /// caller, because the two assemblies declare different methods. + /// + [Test] + public void AppendTo_SameLabelActiveInOtherAssembly_KeepsCaller() + { + HotReloadRunStaleSignatureWarnings staleWarnings = Record( + RemovedKey, + CreateHit(ExternalAssemblyName, CallerType, "Call")); + List warnings = new List(); + + staleWarnings.AppendTo( + warnings, + new List + { + CreateActivePatch(EditedAssemblyName, "Example.Caller.Call()") + }); + + Assert.That(warnings, Is.EqualTo(new[] { FormatWarning(RemovedKey, "Example.Caller::Call()") })); + } + + /// + /// What: when two assemblies hold a caller with the same wire key and only one of them is + /// patched, the other still names the key once, whichever of the two was recorded first. + /// + [TestCase(true)] + [TestCase(false)] + public void AppendTo_SameWireKeyInTwoAssemblies_OneActive_ListsItOnce(bool activeCallerRecordedFirst) + { + HotReloadCallSiteScanner.CallSiteHit activeCaller = CreateHit(ExternalAssemblyName, CallerType, "Call"); + HotReloadCallSiteScanner.CallSiteHit compiledCaller = CreateHit(EditedAssemblyName, CallerType, "Call"); + HotReloadRunStaleSignatureWarnings staleWarnings = activeCallerRecordedFirst + ? Record(RemovedKey, activeCaller, compiledCaller) + : Record(RemovedKey, compiledCaller, activeCaller); + List warnings = new List(); + + staleWarnings.AppendTo( + warnings, + new List + { + CreateActivePatch(ExternalAssemblyName, "Example.Caller.Call()") + }); + + Assert.That(warnings, Is.EqualTo(new[] { FormatWarning(RemovedKey, "Example.Caller::Call()") })); + } + + /// + /// What: when both same-key callers are patched, no caller is left and no warning is added. + /// + [Test] + public void AppendTo_SameWireKeyInTwoAssemblies_BothActive_DropsWarning() + { + HotReloadRunStaleSignatureWarnings staleWarnings = Record( + RemovedKey, + CreateHit(ExternalAssemblyName, CallerType, "Call"), + CreateHit(EditedAssemblyName, CallerType, "Call")); + List warnings = new List(); + + staleWarnings.AppendTo( + warnings, + new List + { + CreateActivePatch(ExternalAssemblyName, "Example.Caller.Call()"), + CreateActivePatch(EditedAssemblyName, "Example.Caller.Call()") + }); + + Assert.That(warnings, Is.Empty); + } + + /// + /// What: the warning names a wire key shared by callers of two assemblies only once, since + /// it shows keys, while each caller is still judged on its own assembly. + /// + [Test] + public void AppendTo_SameWireKeyInTwoAssemblies_NeitherActive_ListsItOnce() + { + HotReloadRunStaleSignatureWarnings staleWarnings = Record( + RemovedKey, + CreateHit(EditedAssemblyName, CallerType, "Call"), + CreateHit(ExternalAssemblyName, CallerType, "Call")); + List warnings = new List(); + + staleWarnings.AppendTo(warnings, new List()); + + Assert.That(warnings, Is.EqualTo(new[] { FormatWarning(RemovedKey, "Example.Caller::Call()") })); + } + + /// + /// What: a patched caller of a nested type is matched even though the scanner spells the + /// type with the metadata '/' separator and the patch label uses reflection's '+'. + /// + [Test] + public void AppendTo_NestedTypeCallerActive_IsOmitted() + { + HotReloadRunStaleSignatureWarnings staleWarnings = Record( + RemovedKey, + CreateHit(ExternalAssemblyName, "Example.Outer/Inner", "Call")); + List warnings = new List(); + + staleWarnings.AppendTo( + warnings, + new List + { + CreateActivePatch(ExternalAssemblyName, "Example.Outer+Inner.Call()") + }); + + Assert.That(warnings, Is.Empty); + } + + /// + /// What: a patched caller is dropped even when its hit loads the removed method as a + /// function pointer, the same as a caller that the removed signature's group patches. + /// + [Test] + public void AppendTo_FunctionPointerLoadOfActiveCaller_IsOmitted() + { + HotReloadCallSiteScanner.CallSiteHit hit = CreateHit(ExternalAssemblyName, CallerType, "Call"); + hit.IsFunctionPointerLoad = true; + HotReloadRunStaleSignatureWarnings staleWarnings = Record(RemovedKey, hit); + List warnings = new List(); + + staleWarnings.AppendTo( + warnings, + new List + { + CreateActivePatch(ExternalAssemblyName, "Example.Caller.Call()") + }); + + Assert.That(warnings, Is.Empty); + } + + /// + /// What: a call inside a lambda stays named under its compiler-generated method even when + /// the method that declares the lambda is patched, because the scanner does not map + /// closures to their owner. + /// + [Test] + public void AppendTo_ClosureCallerOfActiveOwner_StaysListed() + { + HotReloadRunStaleSignatureWarnings staleWarnings = Record( + RemovedKey, + CreateHit(ExternalAssemblyName, "Example.Host/<>c", "b__0_0")); + List warnings = new List(); + + staleWarnings.AppendTo( + warnings, + new List + { + CreateActivePatch(ExternalAssemblyName, "Example.Host.Owner()") + }); + + Assert.That( + warnings, + Is.EqualTo(new[] { FormatWarning(RemovedKey, "Example.Host/<>c::b__0_0()") })); + } + + /// + /// What: signatures recorded by two groups are filtered independently, so a patched caller + /// of one does not hide a compiled caller of the other. + /// + [Test] + public void AppendTo_TwoSignatures_FiltersEachIndependently() + { + HotReloadRunStaleSignatureWarnings staleWarnings = Record( + RemovedKey, + CreateHit(ExternalAssemblyName, CallerType, "Call")); + staleWarnings.AddRange( + new List + { + new HotReloadStaleSignatureCallSites( + OtherRemovedKey, + new List + { + CreateHit(ExternalAssemblyName, OtherCallerType, "Call") + }) + }); + List warnings = new List(); + + staleWarnings.AppendTo( + warnings, + new List + { + CreateActivePatch(ExternalAssemblyName, "Example.Caller.Call()") + }); + + Assert.That(warnings, Is.EqualTo(new[] { FormatWarning(OtherRemovedKey, "Example.OtherCaller::Call()") })); + } + + private static HotReloadRunStaleSignatureWarnings Record( + string removedKey, + params HotReloadCallSiteScanner.CallSiteHit[] callers) + { + HotReloadRunStaleSignatureWarnings staleWarnings = new HotReloadRunStaleSignatureWarnings(); + staleWarnings.AddRange( + new List + { + new HotReloadStaleSignatureCallSites( + removedKey, + new List(callers)) + }); + return staleWarnings; + } + + private static HotReloadCallSiteScanner.CallSiteHit CreateHit( + string assemblyName, + string typeMetadataName, + string methodName) + { + return new HotReloadCallSiteScanner.CallSiteHit + { + CallerAssemblyName = assemblyName, + CallerTypeMetadataName = new HotReloadMetadataTypeName(typeMetadataName), + CallerMethodName = methodName, + CallerParameterTypeFullNames = Array.Empty(), + CallerGenericArity = 0, + CallerMethodKey = HotReloadMethodKeys.BuildMethodKeyParts( + typeMetadataName, + methodName, + Array.Empty(), + 0), + TargetMethodKey = RemovedKey + }; + } + + private static HotReloadActivePatchInfo CreateActivePatch(string assemblyName, string methodLabel) + { + return new HotReloadActivePatchInfo(methodLabel, "Assets/Example/Caller.cs", assemblyName); + } + + private static string FormatWarning(string removedKey, params string[] callerKeys) + { + return string.Format( + HotReloadConstants.StaleSignatureCallersWarningFormat, + removedKey, + string.Join(", ", callerKeys)); + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadRunStaleSignatureWarningsTests.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadRunStaleSignatureWarningsTests.cs.meta new file mode 100644 index 0000000000..077ddc6d92 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadRunStaleSignatureWarningsTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: a25afa6e0484d48018d334d9fa3bc7f2 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/HotReloadSignatureChangeCoverageTests.cs b/Assets/Tests/Editor/HotReload/HotReloadSignatureChangeCoverageTests.cs index d0487bb2e0..5bb0bb37ec 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadSignatureChangeCoverageTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadSignatureChangeCoverageTests.cs @@ -365,34 +365,32 @@ public void AppendSignatureChangeCallersRepatchedWarnings_ExternalSameKeyCaller_ } /// - /// What: stale warnings de-duplicate only their displayed caller keys, preserving distinct - /// cross-assembly caller identities for coverage decisions. + /// What: the stale call sites of a removed signature keep the first hit of each uncovered + /// caller identity, so callers with the same wire key in two assemblies both reach the + /// run-end filter, and a removed signature without an uncovered caller records nothing. /// [Test] - public void FormatUncoveredCallerMethodKeys_CrossAssemblySameKey_DeduplicatesDisplayOnly() + public void CollectStaleSignatureCallSites_CrossAssemblySameKey_KeepsFirstHitPerIdentity() { - List sameKeyCallers = - new List - { - new HotReloadQualifiedMethodIdentity(EditedAssemblyName, CallerKey), - new HotReloadQualifiedMethodIdentity(ExternalAssemblyName, CallerKey) - }; - List differentKeyCallers = - new List - { - new HotReloadQualifiedMethodIdentity(EditedAssemblyName, CallerKey), - new HotReloadQualifiedMethodIdentity( - ExternalAssemblyName, - "Example.OtherCaller::Call()") - }; + HotReloadCallSiteScanner.CallSiteHit editedHit = CreateHit(EditedAssemblyName); + HotReloadCallSiteScanner.CallSiteHit repeatedEditedHit = CreateHit(EditedAssemblyName); + HotReloadCallSiteScanner.CallSiteHit externalHit = CreateHit(ExternalAssemblyName); + List hits = + new List { editedHit, repeatedEditedHit, externalHit }; + Dictionary> callersByTarget = + HotReloadSignatureChangeCoverage.CollectUncoveredCallersByTarget( + hits, + new HashSet()); - Assert.That(sameKeyCallers, Has.Count.EqualTo(2)); - Assert.That( - CountOccurrences(CollectStaleWarning(sameKeyCallers), CallerKey), - Is.EqualTo(1)); - Assert.That( - CollectStaleWarning(differentKeyCallers), - Does.Contain("Example.Caller::Call(), Example.OtherCaller::Call()")); + List callSites = + HotReloadSignatureChangeCoverage.CollectStaleSignatureCallSites( + new[] { CreateTargetRemovedSignature("Call"), CreateTargetRemovedSignature("Uncalled") }, + hits, + callersByTarget); + + Assert.That(callSites, Has.Count.EqualTo(1)); + Assert.That(callSites[0].RemovedMethodKey, Is.EqualTo(ReplacementKey)); + Assert.That(callSites[0].Callers, Is.EqualTo(new[] { editedHit, externalHit })); } private static TransformWorkerEntryDto CreateReplacementEntry() @@ -431,45 +429,15 @@ private static TransformWorkerUnchangedMethodDto CreateCallerUnchangedMethod() }; } - private static string CollectStaleWarning( - List callers) + private static TransformWorkerRemovedMethodSignatureDto CreateTargetRemovedSignature(string methodName) { - TransformWorkerRemovedMethodSignatureDto removedSignature = - new TransformWorkerRemovedMethodSignatureDto - { - typeMetadataName = "Example.Target", - methodName = "Call", - parameterTypeFullNames = Array.Empty(), - genericArity = 0 - }; - Dictionary> callersByTarget = - new Dictionary>(StringComparer.Ordinal) - { - { ReplacementKey, callers } - }; - - List warnings = HotReloadSignatureChangeCoverage.CollectStaleSignatureWarnings( - new[] { removedSignature }, - callersByTarget); - - return warnings[0]; - } - - private static int CountOccurrences(string text, string value) - { - int count = 0; - int startIndex = 0; - while (true) + return new TransformWorkerRemovedMethodSignatureDto { - int occurrenceIndex = text.IndexOf(value, startIndex, StringComparison.Ordinal); - if (occurrenceIndex < 0) - { - return count; - } - - count++; - startIndex = occurrenceIndex + value.Length; - } + typeMetadataName = "Example.Target", + methodName = methodName, + parameterTypeFullNames = Array.Empty(), + genericArity = 0 + }; } private static TransformWorkerEntryDto CreateOrdinaryEntry() diff --git a/Assets/Tests/Editor/HotReload/HotReloadUnchangedPatchPeelTests.cs b/Assets/Tests/Editor/HotReload/HotReloadUnchangedPatchPeelTests.cs index 12f267fb38..80b8ee70f2 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadUnchangedPatchPeelTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadUnchangedPatchPeelTests.cs @@ -105,7 +105,7 @@ private static HotReloadGroupFile ArrangeUnchangedMethodWithActivePatch() original, shim, HotReloadPatchShape.Transplant, OwnerPath); Assert.That(patch.Success, Is.True, patch.ErrorMessage); - HotReloadFileSinks sinks = new HotReloadFileSinks(new List(), null); + HotReloadFileSinks sinks = new HotReloadFileSinks(new List(), null, new HotReloadRunStaleSignatureWarnings()); return new HotReloadGroupFile( OwnerPath, OwnerPath, diff --git a/Assets/Tests/Editor/HotReloadCallSiteCrossAssembly/HotReloadCrossAssemblyStaleSignatureCaller.cs b/Assets/Tests/Editor/HotReloadCallSiteCrossAssembly/HotReloadCrossAssemblyStaleSignatureCaller.cs new file mode 100644 index 0000000000..541fb4bb75 --- /dev/null +++ b/Assets/Tests/Editor/HotReloadCallSiteCrossAssembly/HotReloadCrossAssemblyStaleSignatureCaller.cs @@ -0,0 +1,23 @@ +using System.Runtime.CompilerServices; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// Compiled callers of the cross-assembly stale-signature host from a separate assembly, so + /// the host's group never holds them as entries. + /// + public class HotReloadCrossAssemblyStaleSignatureCaller + { + [MethodImpl(MethodImplOptions.NoInlining)] + public int CallDeleted(int value) + { + return new HotReloadCrossAssemblyStaleSignatureHost().ToDelete(value); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public int CallReturnTypeTarget(int value) + { + return new HotReloadCrossAssemblyStaleSignatureHost().ReturnTypeTarget(value); + } + } +} diff --git a/Assets/Tests/Editor/HotReloadCallSiteCrossAssembly/HotReloadCrossAssemblyStaleSignatureCaller.cs.meta b/Assets/Tests/Editor/HotReloadCallSiteCrossAssembly/HotReloadCrossAssemblyStaleSignatureCaller.cs.meta new file mode 100644 index 0000000000..00216bc6a5 --- /dev/null +++ b/Assets/Tests/Editor/HotReloadCallSiteCrossAssembly/HotReloadCrossAssemblyStaleSignatureCaller.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 5388d7fe1118e4e7096377f63109d8ba +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadFileSinks.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadFileSinks.cs index b6e1f6a399..7e227795a6 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadFileSinks.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadFileSinks.cs @@ -10,16 +10,18 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools /// /// Why separate from HotReloadApplyContext: these are the mutable half. Keeping them apart /// makes it visible at each call site which stage writes results and which only reads inputs. - /// The two injected lists span the whole run, not one file, so the caller owns them. + /// The injected collections span the whole run, not one file, so the caller owns them. /// internal sealed class HotReloadFileSinks { internal HotReloadFileSinks( List siblingDerivedWarnings, List oneShotCallerNoteCandidates, + HotReloadRunStaleSignatureWarnings staleSignatureWarnings, HotReloadRunDisplayedRemovedMembers displayedRemovedMembers = null) { Debug.Assert(siblingDerivedWarnings != null, "siblingDerivedWarnings must not be null."); + Debug.Assert(staleSignatureWarnings != null, "staleSignatureWarnings must not be null."); Outcomes = new List(); Warnings = new List(); @@ -29,6 +31,7 @@ internal HotReloadFileSinks( IntroducedTypes = new List(); SiblingDerivedWarnings = siblingDerivedWarnings; OneShotCallerNoteCandidates = oneShotCallerNoteCandidates; + StaleSignatureWarnings = staleSignatureWarnings; DisplayedRemovedMembers = displayedRemovedMembers; } @@ -61,6 +64,9 @@ internal HotReloadFileSinks( // Shared across the whole run; null when the caller collects no one-shot caller notes. internal List OneShotCallerNoteCandidates { get; } + // Shared across the whole run so the warning reflects the patches active when it ends. + internal HotReloadRunStaleSignatureWarnings StaleSignatureWarnings { get; } + // Shared across the whole run so the removed-member record is written once the run ends; // null when the caller keeps no such record. internal HotReloadRunDisplayedRemovedMembers DisplayedRemovedMembers { get; } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupProcessor.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupProcessor.cs index db742aff29..d8a7d44cc2 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupProcessor.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadGroupProcessor.cs @@ -301,9 +301,10 @@ internal static async Task GateAndCompileAsy } HotReloadGroupOutcomeRouter.AppendByFilePath(files, gateResult.SkippedOutcomes); - // Why one file's warning list: gate warnings name compiled call sites across the - // assembly, not one edited file, and the run merges every file's warnings anyway. - gateWarningSink.Sinks.Warnings.AddRange(gateResult.Warnings); + // Why the run's record rather than a warning now: a stale call site in another assembly + // may be patched by a later group of this run, so the warning is only built once every + // group has applied. + gateWarningSink.Sinks.StaleSignatureWarnings.AddRange(gateResult.StaleSignatureCallSites); HotReloadGroupCompileResult compile = await HotReloadShimFirstCompile.ResolveEntriesToPatchAsync( collaborators, diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadInputFileResolver.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadInputFileResolver.cs index 4fe52cd87b..8011a0f522 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadInputFileResolver.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadInputFileResolver.cs @@ -49,6 +49,7 @@ internal void ResolveInputFile( HotReloadFileSinks sinks = new HotReloadFileSinks( run.SiblingDerivedWarnings, run.OneShotCallerNoteCandidates, + run.StaleSignatureWarnings, run.DisplayedRemovedMembers); List alreadyActiveOutcomes = new List(); diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadRunAccumulator.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadRunAccumulator.cs index 8aca223c98..133b683069 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadRunAccumulator.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadRunAccumulator.cs @@ -83,6 +83,10 @@ public HotReloadRunAccumulator( public HotReloadRunDisplayedRemovedMembers DisplayedRemovedMembers { get; } = new HotReloadRunDisplayedRemovedMembers(); + /// Where each group's gate records stale call sites, turned into warnings once per run. + public HotReloadRunStaleSignatureWarnings StaleSignatureWarnings { get; } = + new HotReloadRunStaleSignatureWarnings(); + /// Where re-applied siblings report a missing baseline, summarized once per run. public HotReloadSiblingBaselineNotices SiblingBaselineNotices => _siblingBaselineNotices; @@ -220,6 +224,10 @@ public HotReloadOrchestratorResult BuildResult(string correlationId) // Why first: the per-file warnings of the re-applied files were merged last, so the // summary of their missing baselines lands right after them. _siblingBaselineNotices.AppendTo(_warnings); + // Why the patches active now rather than at each gate: a later group can patch a + // caller in another assembly or peel its earlier patch, and only the state after the + // last group says which callers still run the compiled body. + StaleSignatureWarnings.AppendTo(_warnings, _patcher.DescribeActivePatches()); AppendInlineRiskWarning(); AppendAddedFieldsLifetimeWarning(); AppendSerializedAddedFieldWarning(); diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadRunStaleSignatureWarnings.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadRunStaleSignatureWarnings.cs new file mode 100644 index 0000000000..cbf53adaad --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadRunStaleSignatureWarnings.cs @@ -0,0 +1,110 @@ +using System; +using System.Collections.Generic; + +using UnityEngine; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// Collects the removed signatures whose compiled callers each group's signature-change gate + /// left uncovered, and turns them into warnings once the run has applied every group. + /// + /// + /// Why at the end of the run: a gate sees only its own group's entries, and groups run one + /// assembly at a time, so a caller in another assembly may be patched by an earlier group, a + /// later group, or an earlier run. A caller whose patch is active when the run ends no longer + /// runs the compiled body that calls the old signature; one whose patch a later group peeled + /// runs it again. + /// + internal sealed class HotReloadRunStaleSignatureWarnings + { + private readonly List _callSites = + new List(); + + internal void AddRange(IReadOnlyList callSites) + { + Debug.Assert(callSites != null, "callSites must not be null."); + _callSites.AddRange(callSites); + } + + /// + /// Appends one warning per recorded signature that still has a caller running its compiled + /// body, leaving out every caller one of the active patches replaces. + /// + internal void AppendTo(List warnings, IReadOnlyList activePatches) + { + Debug.Assert(warnings != null, "warnings must not be null."); + Debug.Assert(activePatches != null, "activePatches must not be null."); + Dictionary> activeLabelsByAssembly = + CollectActiveLabelsByAssembly(activePatches); + foreach (HotReloadStaleSignatureCallSites callSites in _callSites) + { + List callerKeys = CollectCompiledCallerKeys(callSites, activeLabelsByAssembly); + if (callerKeys.Count == 0) + { + continue; + } + + warnings.Add( + string.Format( + HotReloadConstants.StaleSignatureCallersWarningFormat, + callSites.RemovedMethodKey, + string.Join(", ", callerKeys))); + } + } + + private static Dictionary> CollectActiveLabelsByAssembly( + IReadOnlyList activePatches) + { + Dictionary> activeLabelsByAssembly = + new Dictionary>(StringComparer.Ordinal); + foreach (HotReloadActivePatchInfo patch in activePatches) + { + if (!activeLabelsByAssembly.TryGetValue(patch.AssemblyName, out HashSet labels)) + { + labels = new HashSet(StringComparer.Ordinal); + activeLabelsByAssembly.Add(patch.AssemblyName, labels); + } + + labels.Add(patch.MethodKey); + } + + return activeLabelsByAssembly; + } + + // Why active callers are dropped before the display de-duplication: two assemblies can + // declare a caller with the same wire key, and the one still running its compiled body has + // to stay listed even when the other one, recorded first, is active. + private static List CollectCompiledCallerKeys( + HotReloadStaleSignatureCallSites callSites, + Dictionary> activeLabelsByAssembly) + { + List callerKeys = new List(); + HashSet seenCallerKeys = new HashSet(StringComparer.Ordinal); + foreach (HotReloadCallSiteScanner.CallSiteHit caller in callSites.Callers) + { + if (IsReplacedByActivePatch(caller, activeLabelsByAssembly)) + { + continue; + } + + if (seenCallerKeys.Add(caller.CallerMethodKey)) + { + callerKeys.Add(caller.CallerMethodKey); + } + } + + return callerKeys; + } + + // Why the label rather than the wire key: an active patch is described by the label of its + // resolved method, which the hit's label spells the same way, nested types included. + private static bool IsReplacedByActivePatch( + HotReloadCallSiteScanner.CallSiteHit caller, + Dictionary> activeLabelsByAssembly) + { + return activeLabelsByAssembly.TryGetValue(caller.CallerAssemblyName, out HashSet labels) + && labels.Contains(HotReloadSignatureChangeCoverage.FormatCallSiteCallerLabel(caller)); + } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadRunStaleSignatureWarnings.cs.meta b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadRunStaleSignatureWarnings.cs.meta new file mode 100644 index 0000000000..ce2c07ec53 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadRunStaleSignatureWarnings.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: f911707edded5409ba91e60272d53517 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSiblingRebindReporter.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSiblingRebindReporter.cs index 394f9c767c..761c19e98a 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSiblingRebindReporter.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSiblingRebindReporter.cs @@ -51,6 +51,7 @@ internal void AppendActiveSiblingsToGroup( new HotReloadFileSinks( run.SiblingDerivedWarnings, run.OneShotCallerNoteCandidates, + run.StaleSignatureWarnings, run.DisplayedRemovedMembers), inclusion.Evidence, run.SiblingBaselineNotices)); diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSignatureChangeCoverage.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSignatureChangeCoverage.cs index 859de3423e..def9dd4a9a 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSignatureChangeCoverage.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSignatureChangeCoverage.cs @@ -56,11 +56,16 @@ internal static Dictionary> Colle return uncoveredCallersByTarget; } - internal static List CollectStaleSignatureWarnings( + /// + /// One entry per removed signature that still has an uncovered compiled caller, holding the + /// first scan hit of each such caller in scan order. + /// + internal static List CollectStaleSignatureCallSites( TransformWorkerRemovedMethodSignatureDto[] removedSignatures, + IReadOnlyList hits, Dictionary> uncoveredCallersByTarget) { - List warnings = new List(); + List staleCallSites = new List(); foreach (TransformWorkerRemovedMethodSignatureDto signature in removedSignatures) { string methodKey = HotReloadMethodKeys.BuildMethodKeyParts( @@ -76,14 +81,39 @@ internal static List CollectStaleSignatureWarnings( continue; } - warnings.Add( - string.Format( - HotReloadConstants.StaleSignatureCallersWarningFormat, + staleCallSites.Add( + new HotReloadStaleSignatureCallSites( methodKey, - FormatUncoveredCallerMethodKeys(callers))); + CollectFirstHitPerCaller(methodKey, hits, callers))); + } + + return staleCallSites; + } + + // Why one hit per caller: a caller that calls the removed method twice is still one caller + // to name, and whether it still runs its compiled body is decided per caller. + private static List CollectFirstHitPerCaller( + string targetMethodKey, + IReadOnlyList hits, + List uncoveredCallers) + { + HashSet callersWithoutHit = + new HashSet(uncoveredCallers); + List callerHits = + new List(uncoveredCallers.Count); + foreach (HotReloadCallSiteScanner.CallSiteHit hit in hits) + { + if (string.Equals(hit.TargetMethodKey, targetMethodKey, StringComparison.Ordinal) + && callersWithoutHit.Remove(CreateCallerIdentity(hit))) + { + callerHits.Add(hit); + } } - return warnings; + Debug.Assert( + callersWithoutHit.Count == 0, + "Every uncovered caller comes from a hit on the removed signature."); + return callerHits; } // Why only already-patched callers of applied replacements: a caller the user @@ -160,7 +190,7 @@ internal static void AppendSignatureChangeCallersRepatchedWarnings( } } - private static string FormatCallSiteCallerLabel(HotReloadCallSiteScanner.CallSiteHit hit) + internal static string FormatCallSiteCallerLabel(HotReloadCallSiteScanner.CallSiteHit hit) { Debug.Assert(hit != null, "hit must not be null."); return HotReloadMethodKeys.FormatMethodLabelParts( @@ -380,24 +410,6 @@ internal static string FormatUncoveredCallerShortNames( return string.Join(", ", names); } - private static string FormatUncoveredCallerMethodKeys( - IReadOnlyList callers) - { - List methodKeys = new List(callers.Count); - HashSet seenMethodKeys = new HashSet(StringComparer.Ordinal); - foreach (HotReloadQualifiedMethodIdentity caller in callers) - { - if (!seenMethodKeys.Add(caller.MethodKey)) - { - continue; - } - - methodKeys.Add(caller.MethodKey); - } - - return string.Join(", ", methodKeys); - } - private static HotReloadQualifiedMethodIdentity CreateEntryIdentity( string assemblyName, TransformWorkerEntryDto entry) diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSignatureChangeGate.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSignatureChangeGate.cs index b849380ce9..012660997a 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSignatureChangeGate.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSignatureChangeGate.cs @@ -47,16 +47,18 @@ internal static async Task TryApplySignatureChangeGat Dictionary> uncoveredCallersByTarget = CollectInitialUncoveredCallers(context.AssemblyName, entries, hits, deletedCallerExemptions); - List staleWarnings = HotReloadSignatureChangeCoverage.CollectStaleSignatureWarnings( - removedSignatures, - uncoveredCallersByTarget); + List staleSignatureCallSites = + HotReloadSignatureChangeCoverage.CollectStaleSignatureCallSites( + removedSignatures, + hits, + uncoveredCallersByTarget); List gatedReplacements = CollectGatedReplacementEntries( replacementEntries, uncoveredCallersByTarget); if (gatedReplacements.Count == 0) { return SignatureChangeGateResult.WarningsOnly( - staleWarnings, + staleSignatureCallSites, hits, deletedCallerExemptions); } @@ -116,7 +118,7 @@ internal static async Task TryApplySignatureChangeGat return SignatureChangeGateResult.Retried( retry.Isolation, skippedOutcomes, - staleWarnings, + staleSignatureCallSites, hits, deletedCallerExemptions, gatedReplacementMethodKeys); @@ -351,7 +353,7 @@ internal sealed class SignatureChangeGateResult public bool DidScan { get; } public HotReloadShimIsolation.HotReloadShimIsolationResult Isolation { get; } public List SkippedOutcomes { get; } - public List Warnings { get; } + public List StaleSignatureCallSites { get; } public List Hits { get; } public HashSet DeletedCallerExemptions { get; } public List GatedReplacementMethodKeys { get; } @@ -365,7 +367,7 @@ private SignatureChangeGateResult( bool didScan, HotReloadShimIsolation.HotReloadShimIsolationResult isolation, List skippedOutcomes, - List warnings, + List staleSignatureCallSites, List hits, HashSet deletedCallerExemptions, List gatedReplacementMethodKeys) @@ -375,7 +377,7 @@ private SignatureChangeGateResult( DidScan = didScan; Isolation = isolation; SkippedOutcomes = skippedOutcomes ?? new List(); - Warnings = warnings ?? new List(); + StaleSignatureCallSites = staleSignatureCallSites ?? new List(); Hits = hits ?? new List(); DeletedCallerExemptions = deletedCallerExemptions ?? new HashSet(); @@ -390,13 +392,13 @@ public static SignatureChangeGateResult NoWork() } public static SignatureChangeGateResult WarningsOnly( - List warnings, + List staleSignatureCallSites, List hits, HashSet deletedCallerExemptions) { return new SignatureChangeGateResult( SignatureChangeGateOutcome.WarningsOnly, - null, true, null, null, warnings, hits, deletedCallerExemptions, null); + null, true, null, null, staleSignatureCallSites, hits, deletedCallerExemptions, null); } // Why didScan is true: this result is only built after FindCallSites has already run, @@ -421,7 +423,7 @@ public static SignatureChangeGateResult Failed( public static SignatureChangeGateResult Retried( HotReloadShimIsolation.HotReloadShimIsolationResult isolation, List skippedOutcomes, - List warnings, + List staleSignatureCallSites, List hits, HashSet deletedCallerExemptions, List gatedReplacementMethodKeys) @@ -432,7 +434,7 @@ public static SignatureChangeGateResult Retried( true, isolation, skippedOutcomes, - warnings, + staleSignatureCallSites, hits, deletedCallerExemptions, gatedReplacementMethodKeys); diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStaleSignatureCallSites.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStaleSignatureCallSites.cs new file mode 100644 index 0000000000..8a91bb6119 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStaleSignatureCallSites.cs @@ -0,0 +1,33 @@ +using System.Collections.Generic; + +using UnityEngine; + +namespace io.github.hatayama.UnityCliLoop.FirstPartyTools +{ + /// + /// One removed signature and the compiled call sites the signature-change gate left uncovered + /// for it, one scanner hit per caller in the order the scan found them. + /// + /// + /// Why hits rather than a formatted warning: a caller in another assembly may be patched by a + /// later group of the same run, so whether it still runs its compiled body is only known once + /// every group has applied. + /// + internal sealed class HotReloadStaleSignatureCallSites + { + internal HotReloadStaleSignatureCallSites( + string removedMethodKey, + IReadOnlyList callers) + { + Debug.Assert(!string.IsNullOrEmpty(removedMethodKey), "removedMethodKey must not be null or empty."); + Debug.Assert(callers != null && callers.Count > 0, "callers must hold at least one hit."); + RemovedMethodKey = removedMethodKey; + Callers = callers; + } + + // The wire key of the removed signature (Type::Method(params)), as the warning names it. + internal string RemovedMethodKey { get; } + + internal IReadOnlyList Callers { get; } + } +} diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStaleSignatureCallSites.cs.meta b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStaleSignatureCallSites.cs.meta new file mode 100644 index 0000000000..47d9f6a306 --- /dev/null +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStaleSignatureCallSites.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: c44330c7577924a2c82e0206e293bfa1 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadActivePatchInfo.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadActivePatchInfo.cs index 6a3576092d..1e914d38e4 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadActivePatchInfo.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadActivePatchInfo.cs @@ -1,18 +1,24 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools { /// - /// One active hot-reload patch for --status: method key plus the source file path - /// that was applied (project-relative when applied through the orchestrator). + /// One active hot-reload patch for --status and the stale-signature warning: method key + /// plus the source file path that was applied (project-relative when applied through the + /// orchestrator), and the name of the assembly that declares the patched method. /// internal sealed class HotReloadActivePatchInfo { public string MethodKey { get; } public string FilePath { get; } - public HotReloadActivePatchInfo(string methodKey, string filePath) + // Why kept beside the label: two assemblies can declare a method with the same label, and + // a compiled call site must be matched to the patch of its own assembly's method. + public string AssemblyName { get; } + + public HotReloadActivePatchInfo(string methodKey, string filePath, string assemblyName) { MethodKey = methodKey ?? string.Empty; FilePath = filePath ?? string.Empty; + AssemblyName = assemblyName ?? string.Empty; } } } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadDomain.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadDomain.cs index fe6267a794..5387b011bc 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadDomain.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadDomain.cs @@ -364,7 +364,8 @@ internal IReadOnlyList DescribeActivePatches() patches.Add( new HotReloadActivePatchInfo( HotReloadMethodKeys.FormatMethodLabel(methods[index]), - pair.Value.Path)); + pair.Value.Path, + methods[index].DeclaringType.Assembly.GetName().Name)); } } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/SKILL.md b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/SKILL.md index 4a5fb70e06..5a341c1fb0 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/SKILL.md +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/SKILL.md @@ -71,9 +71,9 @@ changed are patched (`UnchangedTotal` counts the rest). refused with a `Warnings` line naming the reason. Use from another assembly or from files outside the reload, reflection, serialization, and Unity message discovery still need `uloop compile`. -- Signature changes: a return-type change is `Skipped` unless this reload or an earlier one - patched every live compiled caller of the old signature; a rename or parameter change - applies as an added method and warns about the call sites left on the old signature. +- Signature changes: a return-type change is `Skipped` unless this or an earlier reload + patched every live compiled caller, none in another assembly; a rename or parameter change + applies as an added method and warns about call sites left on the old signature. - Constructors, operators, struct methods, compiled setter/init/indexer accessors, and event accessors are `Skipped`; finalizers and interface members are silently not applied. - A reload applies each file all-or-nothing: a `Failed` method leaves that file unapplied, 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 4f4b8019ad..d9f22e25c4 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 @@ -180,25 +180,34 @@ the assembly, the Editor-session illusion, and the `virtual`/generic/interface exclusions. A gate protects compiled callers: the change applies only when every live compiled -call site of the old signature is patched by the same reload. A caller this reload -did not edit — in another file, in another assembly, or an *unedited* method in the +call site of the old signature is in the same assembly and patched by the same reload. +A caller this reload did not edit — in another file or an *unedited* method in the edited file itself (an implicit `int`→`long` widening can leave a caller's source untouched) — would keep calling the old method silently, so the run reports the changed method and its edited callers as `Skipped` instead; land the change with -`uloop compile`. When every uncovered caller is in the edited file itself, the +`uloop compile`. A caller in another assembly gates the change even when this or an +earlier reload patched it: that patch is compiled against the compiled assembly, where +the old signature still exists. When every uncovered caller is in the edited file itself, the `Skipped` reason names those callers: editing their bodies and reloading again applies them together without `uloop compile`. Call sites inside methods that the same edit removes or re-signatures do not gate: those compiled bodies are already stale, and anything still reaching them stays on the consistent old behavior. -If an earlier reload already patched the compiled call sites, a later signature change applies without editing the callers; the response then carries a warning naming the call sites this run re-applied on the new signature. +If an earlier reload already patched the compiled call sites in the same assembly, a later signature change applies without editing the callers; the response then carries a warning naming the call sites this run re-applied on the new signature. Renaming a method or changing its parameter list follows the delete rules rather than the gate: the new signature is an ordinary added method, the old one is reported removed, and a `Warnings` entry names each compiled call site of the old signature that the reload leaves unpatched — those call sites keep the previous behavior until `uloop compile`. Deleting a method emits the same warning when -compiled callers remain. +compiled callers remain. A caller whose patch is active when the reload ends — +patched by this reload in any assembly, or kept from an earlier reload — is left out, +because it no longer runs its compiled body. The warning does not check what the +patched body calls, and two leftovers of the compiled caller can still reach the old +method: a copy the JIT inlined into another method before the patch, and a delegate to +the old method the caller created before it. A call inside a lambda or local function +stays listed under its compiler-generated name even when the method declaring it is +patched. Field declarations are stricter: when a compiled field's type — or its `static`/ `const` modifier — differs from the edited source, every edited method that reads