From 811da24017ec5d6d5d89243e6fcd0ce2ce8d537c Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 29 Sep 2026 06:59:31 +0900 Subject: [PATCH 1/3] Spell hot reload method labels with metadata parameter types Labels built from a resolved MethodBase used Type.ToString for parameter types (List`1[System.Int32], System.Int32[,]), while worker rows and call-site hits carry Cecil's spelling (List`1, System.Int32[0...,0...]). The ledgers compare labels by ordinal equality, so methods with constructed generic or multidimensional-array parameters silently missed their matches: the skipped-writer warning claimed the property keeps its default value although an earlier patch still assigns it, pause points found no unapplied row, and the stale, superseded, and signature-change lookups could miss (#2903). - FormatMethodLabel walks the parameter type structurally (byref, pointer, array rank, generic parameter, generic type from its definition's FullName plus argument names, otherwise FullName) and keeps reflection's '+' for nested types; FormatMethodLabelParts and the worker are unchanged. - A Cecil parity test compares the MethodBase label with the label built from the same method's metadata spelling for 23 parameter shapes, and two end-to-end tests repeat the skipped-writer-with-earlier-patch case with List and int[,] writers; restoring ToString makes both fail with the default-value wording. - The pause-point domain test and the patcher label test expect the metadata spelling, outdated comments about the mismatch are updated, and the skill output reference says how Method spells parameter types. --- .../uloop-hot-reload/references/output.md | 2 +- .../uloop-hot-reload/references/output.md | 2 +- .../Editor/HotReload/HotReloadDomainTests.cs | 10 +- .../HotReloadLabelShapeWriterHost.cs | 31 +++ .../HotReloadLabelShapeWriterHost.cs.meta | 11 + .../HotReloadMethodLabelParityTests.cs | 229 ++++++++++++++++++ .../HotReloadMethodLabelParityTests.cs.meta | 11 + .../HotReload/HotReloadOrchestratorTests.cs | 125 ++++++++++ .../Editor/HotReload/HotReloadPatcherTests.cs | 11 +- .../HotReloadSignatureChangeCoverage.cs | 3 +- .../Patching/HotReloadPausePointPort.cs | 6 +- .../HotReload/Shared/HotReloadMethodKeys.cs | 88 ++++++- .../HotReload/Skill/references/output.md | 2 +- 13 files changed, 508 insertions(+), 23 deletions(-) create mode 100644 Assets/Tests/Editor/HotReload/HotReloadLabelShapeWriterHost.cs create mode 100644 Assets/Tests/Editor/HotReload/HotReloadLabelShapeWriterHost.cs.meta create mode 100644 Assets/Tests/Editor/HotReload/HotReloadMethodLabelParityTests.cs create mode 100644 Assets/Tests/Editor/HotReload/HotReloadMethodLabelParityTests.cs.meta diff --git a/.agents/skills/uloop-hot-reload/references/output.md b/.agents/skills/uloop-hot-reload/references/output.md index 3a640d368f..fed7ea4a2c 100644 --- a/.agents/skills/uloop-hot-reload/references/output.md +++ b/.agents/skills/uloop-hot-reload/references/output.md @@ -5,7 +5,7 @@ Returns JSON with: - `Success` (boolean): `false` on parameter validation failure or when any method outcome is `Failed`, or when any `IntroducedTypes` row is `Failed`. `Skipped` outcomes alone never force `false` - `ErrorCode` (string, optional): Present on parameter validation failure. Values are `HOT_RELOAD_FILES_REQUIRED` when an omitted apply has no compile snapshots, `HOT_RELOAD_NO_CHANGED_FILES` when snapshots contain no changed `.cs` files, `HOT_RELOAD_INVALID_FILES` when `--files` contains a null or empty path, and `HOT_RELOAD_STATUS_CONFLICT` when `--status` is combined with `--files` or `--revert-all`. - `NextActions` (array, optional): Ordered recovery steps, present only with `ErrorCode` on a parameter validation failure. Omitted from every other response, including successful apply, plain `--status`, and `--revert-all` runs. -- `Methods` (array): Per-method `{ Kind, Method, Reason, FilePath, InvocationCount, LifecycleNote, ReappliedFromSibling }` where `Kind` is `Patched`, `Skipped`, `Failed`, `Added`, `AlreadyActive`, or `Stale` on apply runs, and `Active`, `Added`, or `AddedField` on `--status` runs; empty on `--revert-all` runs. `AlreadyActive` means this file's source matched the last fully applied reload (a run with no Skipped or Failed outcomes), so the existing patch was left in place and the row carries the live `InvocationCount`. `Stale` means the method was deleted from the edited source while its patch is still installed: compiled callers keep running the patched body until `uloop compile`, `--revert-all`, or a later reload whose source restores the method to the compiled baseline clears it; a reload that declares the method with a different body replaces the patch instead of clearing it. Stale rows keep counting toward `ActivePatchTotal`, and the Message summary includes `Stale=N`. `InvocationCount` is meaningful on `Active` rows and on `AlreadyActive` and `Stale` apply rows (calls since the current patch was applied); it is `0` on other apply/revert outcomes. On `--status`, an `Active` row with `InvocationCount` 0 sets `Reason` to explain that the method has not run since the patch: finished calls do not re-run, the patched body takes effect on the next call, and how to retrigger an initialization-only path. When the edited source later declares a different signature, that `Active` row's `Reason` instead explains it is superseded by a new declaration of that signature and is no longer the entry point for new calls (superseded wins over the never-invoked sentence). Added-member rows always show InvocationCount 0 — added-member calls are not instrumented, and the row's Reason says so. `AddedField` rows list a live added field as `Type.field` with an empty `Reason`; they are not method patches. `LifecycleNote` is set when a patched method is a Unity one-shot lifecycle message (`private void Awake`/`Start`/`OnEnable`/`OnDisable`/`OnDestroy` on a `MonoBehaviour`), or when every compiled call path into the patched method (callers of callers are followed a few levels within the compiled assemblies) starts at such a message; empty otherwise — it does not change `Kind`. On an `Added` row whose method is a Unity message, `LifecycleNote` instead says whether the engine will reach it: a forwarded message (`Start`, `Update`, the collision/trigger/mouse messages, and the rest listed in [scope-and-limits.md](scope-and-limits.md)) carries the note that a hot-reload proxy component delivers it to live instances while Play Mode runs, that the proxy is rebuilt only when a later reload changes which messages the type adds or their signatures, that execution order relative to other components is not guaranteed, and that it is gone on any compile or domain reload (only an added `Start` row also says that it runs once on each existing instance when the proxy attaches, and again when the proxy is rebuilt); a message this feature leaves to the compiler (`Awake`, `OnEnable`, `OnDisable`, `OnDestroy`, the editor-only messages, and any non-void message) carries the note that the engine does not invoke it until `uloop compile`, and the run adds one `Warnings` line naming every such message together. `Added` rows carry the added member's signature and file; their `InvocationCount` is always `0` (added-member calls are not instrumented). `ReappliedFromSibling` is `true` on every apply row, whatever its `Kind`, that belongs to a sibling file the run pulled in to re-apply changes from earlier reloads rather than to a file passed in `--files`; it is `false` on the other rows and on every `--status` and `--revert-all` row. Message's re-applied count covers only the `Patched` and `Added` rows among them. Example `--status` row: `{ "Kind": "Added", "Method": "Ns.Host.NewHelper(System.Int32)", "Reason": "Added-member calls are not instrumented, so InvocationCount is always 0 for this row.", "FilePath": "Assets/Scripts/Host.cs", "InvocationCount": 0, "LifecycleNote": "", "ReappliedFromSibling": false }` +- `Methods` (array): Per-method `{ Kind, Method, Reason, FilePath, InvocationCount, LifecycleNote, ReappliedFromSibling }` where `Kind` is `Patched`, `Skipped`, `Failed`, `Added`, `AlreadyActive`, or `Stale` on apply runs, and `Active`, `Added`, or `AddedField` on `--status` runs; empty on `--revert-all` runs. `AlreadyActive` means this file's source matched the last fully applied reload (a run with no Skipped or Failed outcomes), so the existing patch was left in place and the row carries the live `InvocationCount`. `Stale` means the method was deleted from the edited source while its patch is still installed: compiled callers keep running the patched body until `uloop compile`, `--revert-all`, or a later reload whose source restores the method to the compiled baseline clears it; a reload that declares the method with a different body replaces the patch instead of clearing it. Stale rows keep counting toward `ActivePatchTotal`, and the Message summary includes `Stale=N`. `InvocationCount` is meaningful on `Active` rows and on `AlreadyActive` and `Stale` apply rows (calls since the current patch was applied); it is `0` on other apply/revert outcomes. On `--status`, an `Active` row with `InvocationCount` 0 sets `Reason` to explain that the method has not run since the patch: finished calls do not re-run, the patched body takes effect on the next call, and how to retrigger an initialization-only path. When the edited source later declares a different signature, that `Active` row's `Reason` instead explains it is superseded by a new declaration of that signature and is no longer the entry point for new calls (superseded wins over the never-invoked sentence). Added-member rows always show InvocationCount 0 — added-member calls are not instrumented, and the row's Reason says so. `AddedField` rows list a live added field as `Type.field` with an empty `Reason`; they are not method patches. `LifecycleNote` is set when a patched method is a Unity one-shot lifecycle message (`private void Awake`/`Start`/`OnEnable`/`OnDisable`/`OnDestroy` on a `MonoBehaviour`), or when every compiled call path into the patched method (callers of callers are followed a few levels within the compiled assemblies) starts at such a message; empty otherwise — it does not change `Kind`. On an `Added` row whose method is a Unity message, `LifecycleNote` instead says whether the engine will reach it: a forwarded message (`Start`, `Update`, the collision/trigger/mouse messages, and the rest listed in [scope-and-limits.md](scope-and-limits.md)) carries the note that a hot-reload proxy component delivers it to live instances while Play Mode runs, that the proxy is rebuilt only when a later reload changes which messages the type adds or their signatures, that execution order relative to other components is not guaranteed, and that it is gone on any compile or domain reload (only an added `Start` row also says that it runs once on each existing instance when the proxy attaches, and again when the proxy is rebuilt); a message this feature leaves to the compiler (`Awake`, `OnEnable`, `OnDisable`, `OnDestroy`, the editor-only messages, and any non-void message) carries the note that the engine does not invoke it until `uloop compile`, and the run adds one `Warnings` line naming every such message together. `Added` rows carry the added member's signature and file; their `InvocationCount` is always `0` (added-member calls are not instrumented). `ReappliedFromSibling` is `true` on every apply row, whatever its `Kind`, that belongs to a sibling file the run pulled in to re-apply changes from earlier reloads rather than to a file passed in `--files`; it is `false` on the other rows and on every `--status` and `--revert-all` row. Message's re-applied count covers only the `Patched` and `Added` rows among them. `Method` spells parameter types as .NET metadata does, the same on every method row: a constructed generic as ``System.Collections.Generic.List`1``, a multidimensional array as `System.Int32[0...,0...]`, and a nested type with `+`. Example `--status` row: `{ "Kind": "Added", "Method": "Ns.Host.NewHelper(System.Int32)", "Reason": "Added-member calls are not instrumented, so InvocationCount is always 0 for this row.", "FilePath": "Assets/Scripts/Host.cs", "InvocationCount": 0, "LifecycleNote": "", "ReappliedFromSibling": false }` - `Warnings` (array): Non-fatal notes — one aggregated line listing the patched methods at risk of being already JIT-inlined into existing callers — those marked `[AggressiveInlining]`, plus (only when Code Optimization is Release) those with tiny pre-patch bodies — meaning the change may not show at those call sites, the pause-point interaction (see [pause-point-interaction.md](pause-point-interaction.md)), and the const drift, outside-body drift, and missing-baseline entries described in [scope-and-limits.md](scope-and-limits.md). Skipped outcomes are echoed here as `Skipped : `, or as one `Skipped N methods: ()` line per reason when several share it, so checking `Warnings` alone is enough to see that an edit was not applied. When a reload re-applies unchanged files so their patches bind to this run's shim, Warnings includes `Also re-applied N unchanged file(s) with active patches in assembly '...' so their patches bind to this reload's shim: ...`. When a pulled-in sibling fails in that reload, Warnings includes `'...' was pulled in to re-bind its active patches but this reload failed for it; see its rows for which patches changed and run uloop compile to clear the run.` instead of the re-applied line. When every row of that sibling was `Skipped` and none failed, Warnings instead includes `'...' was pulled in to re-bind its active patches, but every method there was Skipped this time; see its rows for the reasons. Any earlier patches there stay active until uloop compile clears the run.` When the reload stopped before re-applying anything at all, so that sibling has no rows, Warnings instead includes `'...' was pulled in to re-bind its active patches, but this reload stopped before re-applying them, so its active patches are unchanged. Fix the refused declaration and rerun, or run uloop compile to clear the run.` When the whole reload was refused, so that sibling's only rows are `Method` = `(file)` `Failed` rows repeating the refusal and none of its unchanged patches were reverted, Warnings instead includes `'...' was pulled in to re-bind its active patches, but the whole reload was refused before re-applying them, so its active patches are unchanged; its rows repeat the refusal reason. Fix that and rerun, or run uloop compile to clear the run.` When a sibling still has active patches but its source changed since they were applied, Warnings includes `'...' has active patches but its source changed since they were applied, so it was not re-applied; pass it to hot-reload to update it.` When a run carries two or more warnings and all of them are hot reload warnings, the Message ends with "A single 'uloop compile' clears all of them at once when you want them gone; none of them has to be cleared before you keep working." — it is the shortest recovery, not an obligation to compile immediately. Pause-point warnings carry their own recovery steps, so that line does not appear when they are present. Nor does it appear when an `IntroducedTypes` row is `Failed` or a warning says a declared type requires a compile, because that type does not exist until one. It is also left off when any `Methods` row is `Failed`, or when a `Methods` row of a file you passed, or of a sibling retried after an earlier Skip, is `Skipped`: that body is not running yet, so it needs a fix or a compile before you keep working. A `Skipped` row of a sibling pulled in only to re-bind its active patches does not leave it off, because the earlier patches there keep running. - `PatchedTotal` (number): Methods patched in this run - `AddedFields` (array): source-level names ("Type.field") of fields this reload added; their values live outside the compiled type until 'uloop compile'. Every run that adds fields also carries one warning stating that the values live outside the compiled assembly and last only until the next 'uloop compile' or domain reload; the warning names exactly the fields listed in AddedFields. An active added field declared with `[SerializeField]`, `[SerializeReference]`, or `[FormerlySerializedAs]` is also named, as `Namespace.Type.field` (nested types joined with `.`), in one `Added field(s) with a serialization attribute will not appear in the Inspector or serialize until 'uloop compile': ...` warning that points at [added-field-wiring.md](added-field-wiring.md). Only the run that first leaves the field active names it; a file that is Skipped or Failed names none of its fields, and a field is named again only after it stopped being active or after `--revert-all`. Pause-point `CapturedVariables` never includes these fields; `enable-pause-point` warns when the resolved type has any. diff --git a/.claude/skills/uloop-hot-reload/references/output.md b/.claude/skills/uloop-hot-reload/references/output.md index 3a640d368f..fed7ea4a2c 100644 --- a/.claude/skills/uloop-hot-reload/references/output.md +++ b/.claude/skills/uloop-hot-reload/references/output.md @@ -5,7 +5,7 @@ Returns JSON with: - `Success` (boolean): `false` on parameter validation failure or when any method outcome is `Failed`, or when any `IntroducedTypes` row is `Failed`. `Skipped` outcomes alone never force `false` - `ErrorCode` (string, optional): Present on parameter validation failure. Values are `HOT_RELOAD_FILES_REQUIRED` when an omitted apply has no compile snapshots, `HOT_RELOAD_NO_CHANGED_FILES` when snapshots contain no changed `.cs` files, `HOT_RELOAD_INVALID_FILES` when `--files` contains a null or empty path, and `HOT_RELOAD_STATUS_CONFLICT` when `--status` is combined with `--files` or `--revert-all`. - `NextActions` (array, optional): Ordered recovery steps, present only with `ErrorCode` on a parameter validation failure. Omitted from every other response, including successful apply, plain `--status`, and `--revert-all` runs. -- `Methods` (array): Per-method `{ Kind, Method, Reason, FilePath, InvocationCount, LifecycleNote, ReappliedFromSibling }` where `Kind` is `Patched`, `Skipped`, `Failed`, `Added`, `AlreadyActive`, or `Stale` on apply runs, and `Active`, `Added`, or `AddedField` on `--status` runs; empty on `--revert-all` runs. `AlreadyActive` means this file's source matched the last fully applied reload (a run with no Skipped or Failed outcomes), so the existing patch was left in place and the row carries the live `InvocationCount`. `Stale` means the method was deleted from the edited source while its patch is still installed: compiled callers keep running the patched body until `uloop compile`, `--revert-all`, or a later reload whose source restores the method to the compiled baseline clears it; a reload that declares the method with a different body replaces the patch instead of clearing it. Stale rows keep counting toward `ActivePatchTotal`, and the Message summary includes `Stale=N`. `InvocationCount` is meaningful on `Active` rows and on `AlreadyActive` and `Stale` apply rows (calls since the current patch was applied); it is `0` on other apply/revert outcomes. On `--status`, an `Active` row with `InvocationCount` 0 sets `Reason` to explain that the method has not run since the patch: finished calls do not re-run, the patched body takes effect on the next call, and how to retrigger an initialization-only path. When the edited source later declares a different signature, that `Active` row's `Reason` instead explains it is superseded by a new declaration of that signature and is no longer the entry point for new calls (superseded wins over the never-invoked sentence). Added-member rows always show InvocationCount 0 — added-member calls are not instrumented, and the row's Reason says so. `AddedField` rows list a live added field as `Type.field` with an empty `Reason`; they are not method patches. `LifecycleNote` is set when a patched method is a Unity one-shot lifecycle message (`private void Awake`/`Start`/`OnEnable`/`OnDisable`/`OnDestroy` on a `MonoBehaviour`), or when every compiled call path into the patched method (callers of callers are followed a few levels within the compiled assemblies) starts at such a message; empty otherwise — it does not change `Kind`. On an `Added` row whose method is a Unity message, `LifecycleNote` instead says whether the engine will reach it: a forwarded message (`Start`, `Update`, the collision/trigger/mouse messages, and the rest listed in [scope-and-limits.md](scope-and-limits.md)) carries the note that a hot-reload proxy component delivers it to live instances while Play Mode runs, that the proxy is rebuilt only when a later reload changes which messages the type adds or their signatures, that execution order relative to other components is not guaranteed, and that it is gone on any compile or domain reload (only an added `Start` row also says that it runs once on each existing instance when the proxy attaches, and again when the proxy is rebuilt); a message this feature leaves to the compiler (`Awake`, `OnEnable`, `OnDisable`, `OnDestroy`, the editor-only messages, and any non-void message) carries the note that the engine does not invoke it until `uloop compile`, and the run adds one `Warnings` line naming every such message together. `Added` rows carry the added member's signature and file; their `InvocationCount` is always `0` (added-member calls are not instrumented). `ReappliedFromSibling` is `true` on every apply row, whatever its `Kind`, that belongs to a sibling file the run pulled in to re-apply changes from earlier reloads rather than to a file passed in `--files`; it is `false` on the other rows and on every `--status` and `--revert-all` row. Message's re-applied count covers only the `Patched` and `Added` rows among them. Example `--status` row: `{ "Kind": "Added", "Method": "Ns.Host.NewHelper(System.Int32)", "Reason": "Added-member calls are not instrumented, so InvocationCount is always 0 for this row.", "FilePath": "Assets/Scripts/Host.cs", "InvocationCount": 0, "LifecycleNote": "", "ReappliedFromSibling": false }` +- `Methods` (array): Per-method `{ Kind, Method, Reason, FilePath, InvocationCount, LifecycleNote, ReappliedFromSibling }` where `Kind` is `Patched`, `Skipped`, `Failed`, `Added`, `AlreadyActive`, or `Stale` on apply runs, and `Active`, `Added`, or `AddedField` on `--status` runs; empty on `--revert-all` runs. `AlreadyActive` means this file's source matched the last fully applied reload (a run with no Skipped or Failed outcomes), so the existing patch was left in place and the row carries the live `InvocationCount`. `Stale` means the method was deleted from the edited source while its patch is still installed: compiled callers keep running the patched body until `uloop compile`, `--revert-all`, or a later reload whose source restores the method to the compiled baseline clears it; a reload that declares the method with a different body replaces the patch instead of clearing it. Stale rows keep counting toward `ActivePatchTotal`, and the Message summary includes `Stale=N`. `InvocationCount` is meaningful on `Active` rows and on `AlreadyActive` and `Stale` apply rows (calls since the current patch was applied); it is `0` on other apply/revert outcomes. On `--status`, an `Active` row with `InvocationCount` 0 sets `Reason` to explain that the method has not run since the patch: finished calls do not re-run, the patched body takes effect on the next call, and how to retrigger an initialization-only path. When the edited source later declares a different signature, that `Active` row's `Reason` instead explains it is superseded by a new declaration of that signature and is no longer the entry point for new calls (superseded wins over the never-invoked sentence). Added-member rows always show InvocationCount 0 — added-member calls are not instrumented, and the row's Reason says so. `AddedField` rows list a live added field as `Type.field` with an empty `Reason`; they are not method patches. `LifecycleNote` is set when a patched method is a Unity one-shot lifecycle message (`private void Awake`/`Start`/`OnEnable`/`OnDisable`/`OnDestroy` on a `MonoBehaviour`), or when every compiled call path into the patched method (callers of callers are followed a few levels within the compiled assemblies) starts at such a message; empty otherwise — it does not change `Kind`. On an `Added` row whose method is a Unity message, `LifecycleNote` instead says whether the engine will reach it: a forwarded message (`Start`, `Update`, the collision/trigger/mouse messages, and the rest listed in [scope-and-limits.md](scope-and-limits.md)) carries the note that a hot-reload proxy component delivers it to live instances while Play Mode runs, that the proxy is rebuilt only when a later reload changes which messages the type adds or their signatures, that execution order relative to other components is not guaranteed, and that it is gone on any compile or domain reload (only an added `Start` row also says that it runs once on each existing instance when the proxy attaches, and again when the proxy is rebuilt); a message this feature leaves to the compiler (`Awake`, `OnEnable`, `OnDisable`, `OnDestroy`, the editor-only messages, and any non-void message) carries the note that the engine does not invoke it until `uloop compile`, and the run adds one `Warnings` line naming every such message together. `Added` rows carry the added member's signature and file; their `InvocationCount` is always `0` (added-member calls are not instrumented). `ReappliedFromSibling` is `true` on every apply row, whatever its `Kind`, that belongs to a sibling file the run pulled in to re-apply changes from earlier reloads rather than to a file passed in `--files`; it is `false` on the other rows and on every `--status` and `--revert-all` row. Message's re-applied count covers only the `Patched` and `Added` rows among them. `Method` spells parameter types as .NET metadata does, the same on every method row: a constructed generic as ``System.Collections.Generic.List`1``, a multidimensional array as `System.Int32[0...,0...]`, and a nested type with `+`. Example `--status` row: `{ "Kind": "Added", "Method": "Ns.Host.NewHelper(System.Int32)", "Reason": "Added-member calls are not instrumented, so InvocationCount is always 0 for this row.", "FilePath": "Assets/Scripts/Host.cs", "InvocationCount": 0, "LifecycleNote": "", "ReappliedFromSibling": false }` - `Warnings` (array): Non-fatal notes — one aggregated line listing the patched methods at risk of being already JIT-inlined into existing callers — those marked `[AggressiveInlining]`, plus (only when Code Optimization is Release) those with tiny pre-patch bodies — meaning the change may not show at those call sites, the pause-point interaction (see [pause-point-interaction.md](pause-point-interaction.md)), and the const drift, outside-body drift, and missing-baseline entries described in [scope-and-limits.md](scope-and-limits.md). Skipped outcomes are echoed here as `Skipped : `, or as one `Skipped N methods: ()` line per reason when several share it, so checking `Warnings` alone is enough to see that an edit was not applied. When a reload re-applies unchanged files so their patches bind to this run's shim, Warnings includes `Also re-applied N unchanged file(s) with active patches in assembly '...' so their patches bind to this reload's shim: ...`. When a pulled-in sibling fails in that reload, Warnings includes `'...' was pulled in to re-bind its active patches but this reload failed for it; see its rows for which patches changed and run uloop compile to clear the run.` instead of the re-applied line. When every row of that sibling was `Skipped` and none failed, Warnings instead includes `'...' was pulled in to re-bind its active patches, but every method there was Skipped this time; see its rows for the reasons. Any earlier patches there stay active until uloop compile clears the run.` When the reload stopped before re-applying anything at all, so that sibling has no rows, Warnings instead includes `'...' was pulled in to re-bind its active patches, but this reload stopped before re-applying them, so its active patches are unchanged. Fix the refused declaration and rerun, or run uloop compile to clear the run.` When the whole reload was refused, so that sibling's only rows are `Method` = `(file)` `Failed` rows repeating the refusal and none of its unchanged patches were reverted, Warnings instead includes `'...' was pulled in to re-bind its active patches, but the whole reload was refused before re-applying them, so its active patches are unchanged; its rows repeat the refusal reason. Fix that and rerun, or run uloop compile to clear the run.` When a sibling still has active patches but its source changed since they were applied, Warnings includes `'...' has active patches but its source changed since they were applied, so it was not re-applied; pass it to hot-reload to update it.` When a run carries two or more warnings and all of them are hot reload warnings, the Message ends with "A single 'uloop compile' clears all of them at once when you want them gone; none of them has to be cleared before you keep working." — it is the shortest recovery, not an obligation to compile immediately. Pause-point warnings carry their own recovery steps, so that line does not appear when they are present. Nor does it appear when an `IntroducedTypes` row is `Failed` or a warning says a declared type requires a compile, because that type does not exist until one. It is also left off when any `Methods` row is `Failed`, or when a `Methods` row of a file you passed, or of a sibling retried after an earlier Skip, is `Skipped`: that body is not running yet, so it needs a fix or a compile before you keep working. A `Skipped` row of a sibling pulled in only to re-bind its active patches does not leave it off, because the earlier patches there keep running. - `PatchedTotal` (number): Methods patched in this run - `AddedFields` (array): source-level names ("Type.field") of fields this reload added; their values live outside the compiled type until 'uloop compile'. Every run that adds fields also carries one warning stating that the values live outside the compiled assembly and last only until the next 'uloop compile' or domain reload; the warning names exactly the fields listed in AddedFields. An active added field declared with `[SerializeField]`, `[SerializeReference]`, or `[FormerlySerializedAs]` is also named, as `Namespace.Type.field` (nested types joined with `.`), in one `Added field(s) with a serialization attribute will not appear in the Inspector or serialize until 'uloop compile': ...` warning that points at [added-field-wiring.md](added-field-wiring.md). Only the run that first leaves the field active names it; a file that is Skipped or Failed names none of its fields, and a field is named again only after it stopped being active or after `--revert-all`. Pause-point `CapturedVariables` never includes these fields; `enable-pause-point` warns when the resolved type has any. diff --git a/Assets/Tests/Editor/HotReload/HotReloadDomainTests.cs b/Assets/Tests/Editor/HotReload/HotReloadDomainTests.cs index 5cdc9ce362..86ad594ede 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadDomainTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadDomainTests.cs @@ -970,11 +970,12 @@ public void FindUnappliedRowForMethod_NullMethod_ReturnsNull() } /// - /// What: a worker row spells a constructed generic parameter type the way Cecil does, so a - /// method taking one finds no row; the match is exact and does not convert the spelling. + /// What: a worker row spells a constructed generic parameter type the way metadata does, and + /// the label of the resolved method spells it the same way, so a method taking one finds + /// its own row. /// [Test] - public void FindUnappliedRowForMethod_MethodWithAConstructedGenericParameter_ReturnsNull() + public void FindUnappliedRowForMethod_MethodWithAConstructedGenericParameter_FindsItsWorkerRow() { string workerLabel = HotReloadMethodKeys.FormatMethodLabelParts( new HotReloadMetadataTypeName(typeof(RowLabelHost).FullName.Replace('+', '/')), @@ -992,7 +993,8 @@ public void FindUnappliedRowForMethod_MethodWithAConstructedGenericParameter_Ret HotReloadUnappliedRow row = port.FindUnappliedRowForMethod(FileOne, RowLabelHostMethod(nameof(RowLabelHost.TakeList))); - Assert.That(row, Is.Null); + Assert.That(row, Is.Not.Null); + Assert.That(row.Label, Is.EqualTo(workerLabel)); } // Why a generated name: an artifact assembly is compiled under a name of its own, so a diff --git a/Assets/Tests/Editor/HotReload/HotReloadLabelShapeWriterHost.cs b/Assets/Tests/Editor/HotReload/HotReloadLabelShapeWriterHost.cs new file mode 100644 index 0000000000..31e5a51297 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadLabelShapeWriterHost.cs @@ -0,0 +1,31 @@ +using System.Collections.Generic; +using System.Runtime.CompilerServices; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// Compiled host whose writers take a constructed generic and a multidimensional array, the + /// parameter shapes reflection and metadata spell differently, and a reader the hot reload + /// tests edit to read what a writer assigns. + /// + public class HotReloadLabelShapeWriterHost + { + [MethodImpl(MethodImplOptions.NoInlining)] + public int WriteFromList(List values) + { + return values.Count; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public int WriteFromGrid(int[,] values) + { + return values.Length; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public int Read(int value) + { + return value; + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadLabelShapeWriterHost.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadLabelShapeWriterHost.cs.meta new file mode 100644 index 0000000000..b2eb86f927 --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadLabelShapeWriterHost.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: d2dcc96ca79844ceaab57c94ebf80464 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/HotReloadMethodLabelParityTests.cs b/Assets/Tests/Editor/HotReload/HotReloadMethodLabelParityTests.cs new file mode 100644 index 0000000000..d45043abcd --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadMethodLabelParityTests.cs @@ -0,0 +1,229 @@ +using System; +using System.Collections.Generic; +using System.Reflection; + +using Mono.Cecil; +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.FirstPartyTools; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload +{ + /// + /// EditMode coverage for the method label every hot reload row and ledger shares: the label + /// built from a resolved method must equal the label built from the same method's metadata + /// spelling, which is what worker rows and call-site hits carry. + /// + public class HotReloadMethodLabelParityTests + { + private const BindingFlags FixtureMethodFlags = + BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly; + + private AssemblyDefinition _fixtureAssembly; + + [OneTimeSetUp] + public void OneTimeSetUp() + { + _fixtureAssembly = AssemblyDefinition.ReadAssembly( + typeof(HotReloadLabelShapeHost).Assembly.Location, + new ReaderParameters { InMemory = true }); + } + + [OneTimeTearDown] + public void OneTimeTearDown() + { + _fixtureAssembly?.Dispose(); + _fixtureAssembly = null; + } + + private static IEnumerable ParameterShapeMethods() + { + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeInt)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeNested)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeList)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeDictionaryOfLists)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeGrid)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeCube)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeGridOfVectors)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeVector)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeJagged)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeVectorOfLists)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeListOfGrids)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeRefList)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeOutGrid)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeNullable)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeTuple)); + yield return Shape( + typeof(HotReloadLabelShapeHost), + nameof(HotReloadLabelShapeHost.TakeNestedOfConstructedOuter)); + yield return Shape( + typeof(HotReloadLabelShapeHost), + nameof(HotReloadLabelShapeHost.TakeNestedGenericOfConstructedOuter)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeListOfNested)); + yield return Shape(typeof(HotReloadLabelShapeHost), nameof(HotReloadLabelShapeHost.TakeGeneric)); + yield return Shape( + typeof(HotReloadLabelShapeGenericHost<>), + nameof(HotReloadLabelShapeGenericHost.TakeTypeParameterList)); + yield return Shape( + typeof(HotReloadLabelShapeGenericHost<>), + nameof(HotReloadLabelShapeGenericHost.TakeSelf)); + yield return Shape( + typeof(HotReloadLabelShapeGenericHost<>), + nameof(HotReloadLabelShapeGenericHost.TakeOwnInner)); + yield return Shape( + typeof(HotReloadLabelShapeGenericHost<>), + nameof(HotReloadLabelShapeGenericHost.TakeOwnInnerOf)); + } + + /// + /// Verifies that for each parameter shape a worker row or a call-site hit can carry, the + /// label built from the resolved method reads the same as the label built from Cecil's + /// spelling of that method, so a label made in either world matches the other. + /// + [TestCaseSource(nameof(ParameterShapeMethods))] + public void FormatMethodLabel_ParameterShape_MatchesTheLabelBuiltFromTheMetadataSpelling( + Type hostType, + string methodName) + { + MethodInfo method = hostType.GetMethod(methodName, FixtureMethodFlags); + Assert.That(method, Is.Not.Null, "Precondition: the fixture method must exist."); + string fromMetadata = FormatLabelFromMetadataSpelling(method); + + string fromMethod = HotReloadMethodKeys.FormatMethodLabel(method); + + Assert.That(fromMethod, Is.EqualTo(fromMetadata)); + } + + private static TestCaseData Shape(Type hostType, string methodName) + { + return new TestCaseData(hostType, methodName); + } + + private string FormatLabelFromMetadataSpelling(MethodInfo method) + { + MethodDefinition definition = (MethodDefinition)_fixtureAssembly.MainModule.LookupToken(method.MetadataToken); + string[] parameterTypeFullNames = new string[definition.Parameters.Count]; + for (int index = 0; index < definition.Parameters.Count; index++) + { + parameterTypeFullNames[index] = definition.Parameters[index].ParameterType.FullName; + } + + return HotReloadMethodKeys.FormatMethodLabelParts( + new HotReloadMetadataTypeName(definition.DeclaringType.FullName), + definition.Name, + parameterTypeFullNames, + definition.GenericParameters.Count); + } + } + + public sealed class HotReloadLabelShapeHost + { + public sealed class Inner + { + } + + public void TakeInt(int value) + { + } + + public void TakeNested(Inner value) + { + } + + public void TakeList(List values) + { + } + + public void TakeDictionaryOfLists(Dictionary> values) + { + } + + public void TakeGrid(int[,] values) + { + } + + public void TakeCube(int[,,] values) + { + } + + public void TakeGridOfVectors(int[,][] values) + { + } + + public void TakeVector(int[] values) + { + } + + public void TakeJagged(int[][] values) + { + } + + public void TakeVectorOfLists(List[] values) + { + } + + public void TakeListOfGrids(List values) + { + } + + public void TakeRefList(ref List values) + { + } + + public void TakeOutGrid(out int[,] values) + { + values = null; + } + + public void TakeNullable(int? value) + { + } + + public void TakeTuple((int, string) value) + { + } + + public void TakeNestedOfConstructedOuter(HotReloadLabelShapeGenericHost.Inner value) + { + } + + public void TakeNestedGenericOfConstructedOuter(HotReloadLabelShapeGenericHost.InnerOf value) + { + } + + public void TakeListOfNested(List values) + { + } + + public void TakeGeneric(TItem value, List values) + { + } + } + + public sealed class HotReloadLabelShapeGenericHost + { + public sealed class Inner + { + } + + public sealed class InnerOf + { + } + + public void TakeTypeParameterList(List values) + { + } + + public void TakeSelf(HotReloadLabelShapeGenericHost other) + { + } + + public void TakeOwnInner(Inner inner) + { + } + + public void TakeOwnInnerOf(InnerOf inner) + { + } + } +} diff --git a/Assets/Tests/Editor/HotReload/HotReloadMethodLabelParityTests.cs.meta b/Assets/Tests/Editor/HotReload/HotReloadMethodLabelParityTests.cs.meta new file mode 100644 index 0000000000..bc407b203c --- /dev/null +++ b/Assets/Tests/Editor/HotReload/HotReloadMethodLabelParityTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: a27af35bbcd074d13ba89d7a6d4d8ce6 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs b/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs index d866d9e519..2a25894ce9 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadOrchestratorTests.cs @@ -5555,6 +5555,116 @@ public async Task Run_SkippedWriterWithEarlierPatch_WarningNamesTheEarlierPatch( allWarnings); } + /// + /// What: when the skipped writer with an earlier patch takes a constructed generic + /// parameter, the skipped-writer warning still names the earlier patch: the Editor's + /// active-patch label spells the parameter the way the worker's skipped row does. + /// + [Test] + public async Task Run_SkippedWriterTakingAConstructedGeneric_WarningNamesTheEarlierPatch() + { + await AssertSkippedWriterWarningNamesTheEarlierPatch( + nameof(HotReloadLabelShapeWriterHost.WriteFromList), + "List values", + "values.Count", + host => host.WriteFromList(new List { 1, 2, 3 }), + 3); + } + + /// + /// What: when the skipped writer with an earlier patch takes a multidimensional array, + /// the skipped-writer warning still names the earlier patch: the Editor's active-patch + /// label spells the array rank the way the worker's skipped row does. + /// + [Test] + public async Task Run_SkippedWriterTakingAMultidimensionalArray_WarningNamesTheEarlierPatch() + { + await AssertSkippedWriterWarningNamesTheEarlierPatch( + nameof(HotReloadLabelShapeWriterHost.WriteFromGrid), + "int[,] values", + "values.Length", + host => host.WriteFromGrid(new int[2, 3]), + 6); + } + + // Runs the two reloads of the skipped-writer-with-earlier-patch case against one writer + // of the label-shape host: the first applies the writer assigning an added auto-property + // that Read reads, the second makes the worker skip that writer. + private static async Task AssertSkippedWriterWarningNamesTheEarlierPatch( + string writerName, + string writerParameterList, + string writerResult, + Func invokeWriter, + int expectedAssigned) + { + string fixturePath = ResolveLabelShapeWriterHostPath(); + string onDisk = File.ReadAllText(fixturePath); + string writerHeader = " public int " + writerName + "(" + writerParameterList + ")\n {\n"; + string writerOriginal = writerHeader + " return " + writerResult + ";\n }"; + const string readOriginal = + " public int Read(int value)\n {\n return value;\n }"; + const string readingRead = + " public int Read(int value)\n {\n return AddedCount + value;\n }\n\n" + + " public int AddedCount { get; private set; }"; + Assert.That(onDisk, Does.Contain(writerOriginal), "Precondition: the writer must be on disk as written."); + Assert.That(onDisk, Does.Contain(readOriginal), "Precondition: Read must be on disk as written."); + string firstEdit = onDisk + .Replace( + writerOriginal, + writerHeader + " AddedCount = " + writerResult + ";\n" + + " return " + writerResult + ";\n }", + StringComparison.Ordinal) + .Replace(readOriginal, readingRead, StringComparison.Ordinal); + + HotReloadOrchestratorResult first = await HotReloadCompositionRoot.Services.Orchestrator.RunAsync( + new[] { fixturePath }, + WriteEditedSource("LabelShape" + writerName + "1.cs", firstEdit), + CancellationToken.None); + AssertNoFileLevelFailure(first); + AssertHasPatched(first, writerName); + + // Why base.GetHashCode(): a base call is a worker-side skip, so the worker itself + // sees this writer as skipped while the first run's patch stays what runs. + string secondEdit = onDisk + .Replace( + writerOriginal, + writerHeader + " AddedCount = " + writerResult + ";\n" + + " return " + writerResult + " + base.GetHashCode() * 0;\n }", + StringComparison.Ordinal) + .Replace(readOriginal, readingRead, StringComparison.Ordinal); + + HotReloadOrchestratorResult second = await HotReloadCompositionRoot.Services.Orchestrator.RunAsync( + new[] { fixturePath }, + WriteEditedSource("LabelShape" + writerName + "2.cs", secondEdit), + CancellationToken.None); + AssertNoFileLevelFailure(second); + Assert.That(FindSkippedReason(second, writerName), Is.Not.Null, FormatOutcomes(second)); + + // The first run's writer patch still assigns the property the applied Read reads. + HotReloadLabelShapeWriterHost host = new HotReloadLabelShapeWriterHost(); + invokeWriter(host); + Assert.That(host.Read(0), Is.EqualTo(expectedAssigned)); + + string writerLabel = HotReloadMethodKeys.FormatMethodLabel( + typeof(HotReloadLabelShapeWriterHost).GetMethod(writerName)); + List writerWarnings = new List(); + foreach (string warning in second.Warnings) + { + if (warning.Contains("'AddedCount'", StringComparison.Ordinal)) + { + writerWarnings.Add(warning); + } + } + + string allWarnings = "Warnings were:\n" + string.Join("\n", second.Warnings); + Assert.That(writerWarnings, Has.Count.EqualTo(1), allWarnings); + Assert.That(writerWarnings[0], Does.Contain("an earlier hot reload applied " + writerLabel), allWarnings); + Assert.That( + writerWarnings[0], + Does.Not.Contain("reads it; the property keeps its default value"), + allWarnings); + } + /// /// What: when the only writer of an added field is an added method that an earlier /// reload added and this reload skips, the skipped-writer warning names that earlier @@ -7762,6 +7872,21 @@ private static string ResolveSignatureChangeExternalHostPath() return Path.GetFullPath(path); } + private static string ResolveLabelShapeWriterHostPath() + { + string path = Path.Combine( + Application.dataPath, + "Tests", + "Editor", + "HotReload", + "HotReloadLabelShapeWriterHost.cs"); + Assert.That( + File.Exists(path), + Is.True, + "Label-shape writer host source missing: " + path); + return Path.GetFullPath(path); + } + private static string ResolveSignatureChangeUnchangedCallerFixturePath() { string path = Path.Combine( diff --git a/Assets/Tests/Editor/HotReload/HotReloadPatcherTests.cs b/Assets/Tests/Editor/HotReload/HotReloadPatcherTests.cs index 73d792a6e8..459588393a 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadPatcherTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadPatcherTests.cs @@ -451,19 +451,20 @@ public void FormatMethodKey_DistinguishesOverloadsByParameterTypes() } /// - /// What: constructed generic parameters use Type.ToString (List`1[System.Int32]), not - /// assembly-qualified FullName (FullName would embed Version/PublicKeyToken). + /// What: constructed generic parameters are spelled the way metadata spells them, with the + /// type arguments in angle brackets, never with Type.ToString's square brackets or the + /// assembly-qualified FullName (which would embed Version/PublicKeyToken). /// [Test] - public void FormatMethodKey_ConstructedGenericParameter_OmitsAssemblyQualification() + public void FormatMethodKey_ConstructedGenericParameter_UsesMetadataSpellingWithoutAssemblyQualification() { MethodInfo take = AccessTools.Method( typeof(HotReloadGenericKeyFixture), nameof(HotReloadGenericKeyFixture.Take)); string key = HotReloadMethodKeys.FormatMethodLabel(take); - Assert.That(key, Does.Contain("System.Collections.Generic.List`1[System.Int32]")); - Assert.That(key, Does.Contain("System.Collections.Generic.Dictionary`2[System.String,System.Int32]")); + Assert.That(key, Does.Contain("System.Collections.Generic.List`1")); + Assert.That(key, Does.Contain("System.Collections.Generic.Dictionary`2")); Assert.That(key, Does.Not.Contain("Version=")); Assert.That(key, Does.Not.Contain("PublicKeyToken=")); Assert.That(key, Does.Not.Contain("mscorlib")); diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSignatureChangeCoverage.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSignatureChangeCoverage.cs index a9712e52ba..859de3423e 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSignatureChangeCoverage.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSignatureChangeCoverage.cs @@ -93,8 +93,7 @@ internal static List CollectStaleSignatureWarnings( // removals, so restrict to replacements that remain in the apply set. Known // limits: an already-patched caller that is also edited this run is // over-reported (the text is still true); a caller that stayed Skipped and - // drifted is missed; constructed generics can miss when the label space differs - // from the wire key (same constraint as IsActiveMember). + // drifted is missed. internal static void AppendSignatureChangeCallersRepatchedWarnings( List warnings, string assemblyName, diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadPausePointPort.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadPausePointPort.cs index bb02062d38..75c0009293 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadPausePointPort.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Patching/HotReloadPausePointPort.cs @@ -118,9 +118,9 @@ public HotReloadLatestFileReload GetLatestReloadOfFile(string file) return new HotReloadLatestFileReload(fileChangedSince, record.UnappliedRows); } - // Why the label match is exact: a worker row spells a constructed generic parameter type - // the way Cecil does (List`1) while a MethodBase spells it the way the CLR - // does (List`1[System.Int32]); such a method finds no row rather than a guessed one. + // Why the label match is exact: the label built from the MethodBase spells its parameter + // types the way the worker row does (List`1), so a method whose row is + // absent finds none rather than a guessed one. public HotReloadUnappliedRow FindUnappliedRowForMethod(string file, MethodBase method) { if (method == null || method.DeclaringType == null) diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadMethodKeys.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadMethodKeys.cs index 09869aac96..c932306425 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadMethodKeys.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadMethodKeys.cs @@ -12,6 +12,8 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools /// Single Unity-side formatter for the two method identifiers hot reload uses: the wire key /// (Type::Method`N(params), exchanged with the worker and the call-site scanner) and the /// display label (Type.Method`N(params), shown in Methods[].Method and --status rows). + /// Both spell parameter types the way metadata does (List`1<System.Int32>), whether they + /// are built from worker fields or from a resolved MethodBase. /// Mirrored on the worker side by WorkerMethodKeys — keep the two files in sync. /// internal static class HotReloadMethodKeys @@ -46,10 +48,13 @@ internal static string BuildMethodKeyParts( // What: status / counter label from a resolved MethodBase (apply outcomes use the same // helper after Resolve so --status rows match Patched Methods[].Method). - // Why parameter ToString (+ generic arity): MethodBase ledger entries distinguish - // overloads, so a name-only key would merge counts and let Revert of one overload - // zero the other's counter. FullName embeds assembly Version/PublicKeyToken for - // constructed generics (List`1[[Int32, mscorlib, ...]]), which bloated labels. + // Parameter types (+ generic arity) are part of the label because MethodBase ledger + // entries distinguish overloads, so a name-only key would merge counts and let Revert + // of one overload zero the other's counter. They are spelled the way metadata spells + // them because worker rows and call-site hits carry that spelling and the ledgers match + // labels by ordinal equality: Type.ToString writes List`1[System.Int32] and + // System.Int32[,] where Cecil writes List`1 and System.Int32[0...,0...], + // and FullName embeds assembly Version/PublicKeyToken for constructed generics. internal static string FormatMethodLabel(MethodBase method) { Debug.Assert(method != null, "method must not be null."); @@ -60,7 +65,7 @@ internal static string FormatMethodLabel(MethodBase method) string[] parameterTypeFullNames = new string[parameters.Length]; for (int index = 0; index < parameters.Length; index++) { - parameterTypeFullNames[index] = parameters[index].ParameterType.ToString(); + parameterTypeFullNames[index] = FormatParameterTypeName(parameters[index].ParameterType); } int genericArity = 0; @@ -76,6 +81,77 @@ internal static string FormatMethodLabel(MethodBase method) genericArity); } + // Spells a parameter type the way Cecil's TypeReference.FullName spells it, except that a + // nested type keeps reflection's '+', which is what every label turns Cecil's '/' into. + // A structural walk rather than a rewrite of Type.ToString, because a '[' in its output + // opens either type arguments or an array rank and only the Type knows which. + private static string FormatParameterTypeName(Type type) + { + if (type.IsByRef) + { + return FormatParameterTypeName(type.GetElementType()) + "&"; + } + + if (type.IsPointer) + { + return FormatParameterTypeName(type.GetElementType()) + "*"; + } + + if (type.IsArray) + { + return FormatParameterTypeName(type.GetElementType()) + FormatArrayRankSuffix(type.GetArrayRank()); + } + + if (type.IsGenericParameter) + { + return type.Name; + } + + if (type.IsGenericType) + { + return FormatGenericTypeName(type); + } + + return type.FullName; + } + + // Rank alone decides, as it does in the worker's CecilTypeNames: C# declares a rank-1 + // array only as a vector, and metadata gives each dimension of a C# multidimensional + // array a zero lower bound, which Cecil spells "0...". + private static string FormatArrayRankSuffix(int rank) + { + if (rank == 1) + { + return "[]"; + } + + string[] dimensions = new string[rank]; + for (int index = 0; index < rank; index++) + { + dimensions[index] = "0..."; + } + + return "[" + string.Join(",", dimensions) + "]"; + } + + // Every generic type takes this path, definitions included: FullName is null for a + // constructed type that still holds a type parameter (List), and a definition, which + // is what a generic type's method gets for its own type or a type nested in it, has a + // FullName without the type parameters metadata still spells (Box`1). + // GetGenericArguments lists a nested type's arguments outer type first, which is the + // order metadata writes them in. + private static string FormatGenericTypeName(Type genericType) + { + Type[] arguments = genericType.GetGenericArguments(); + string[] argumentNames = new string[arguments.Length]; + for (int index = 0; index < arguments.Length; index++) + { + argumentNames[index] = FormatParameterTypeName(arguments[index]); + } + + return genericType.GetGenericTypeDefinition().FullName + "<" + string.Join(",", argumentNames) + ">"; + } + // What: display label from worker DTO fields (pre-Resolve failures) using the same shape // as FormatMethodLabel. Cecil nested separators ('/') are normalized to reflection ('+'). // Keep in sync with WorkerMethodKeys.FormatMethodLabelParts. @@ -94,7 +170,7 @@ internal static string FormatMethodLabelParts( // What: the shared assembly of a label once every name it holds is in reflection form. // Why a parameter type is converted here too: a worker row spells a nested parameter type - // in metadata form, while a resolved MethodBase already spells it as reflection does, so + // in metadata form, while FormatParameterTypeName already spells it as reflection does, so // the conversion is a no-op on the reflection path and both paths produce one string. private static string FormatMethodLabelFromReflection( HotReloadReflectionTypeName typeReflectionName, diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md index 3a640d368f..fed7ea4a2c 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/output.md @@ -5,7 +5,7 @@ Returns JSON with: - `Success` (boolean): `false` on parameter validation failure or when any method outcome is `Failed`, or when any `IntroducedTypes` row is `Failed`. `Skipped` outcomes alone never force `false` - `ErrorCode` (string, optional): Present on parameter validation failure. Values are `HOT_RELOAD_FILES_REQUIRED` when an omitted apply has no compile snapshots, `HOT_RELOAD_NO_CHANGED_FILES` when snapshots contain no changed `.cs` files, `HOT_RELOAD_INVALID_FILES` when `--files` contains a null or empty path, and `HOT_RELOAD_STATUS_CONFLICT` when `--status` is combined with `--files` or `--revert-all`. - `NextActions` (array, optional): Ordered recovery steps, present only with `ErrorCode` on a parameter validation failure. Omitted from every other response, including successful apply, plain `--status`, and `--revert-all` runs. -- `Methods` (array): Per-method `{ Kind, Method, Reason, FilePath, InvocationCount, LifecycleNote, ReappliedFromSibling }` where `Kind` is `Patched`, `Skipped`, `Failed`, `Added`, `AlreadyActive`, or `Stale` on apply runs, and `Active`, `Added`, or `AddedField` on `--status` runs; empty on `--revert-all` runs. `AlreadyActive` means this file's source matched the last fully applied reload (a run with no Skipped or Failed outcomes), so the existing patch was left in place and the row carries the live `InvocationCount`. `Stale` means the method was deleted from the edited source while its patch is still installed: compiled callers keep running the patched body until `uloop compile`, `--revert-all`, or a later reload whose source restores the method to the compiled baseline clears it; a reload that declares the method with a different body replaces the patch instead of clearing it. Stale rows keep counting toward `ActivePatchTotal`, and the Message summary includes `Stale=N`. `InvocationCount` is meaningful on `Active` rows and on `AlreadyActive` and `Stale` apply rows (calls since the current patch was applied); it is `0` on other apply/revert outcomes. On `--status`, an `Active` row with `InvocationCount` 0 sets `Reason` to explain that the method has not run since the patch: finished calls do not re-run, the patched body takes effect on the next call, and how to retrigger an initialization-only path. When the edited source later declares a different signature, that `Active` row's `Reason` instead explains it is superseded by a new declaration of that signature and is no longer the entry point for new calls (superseded wins over the never-invoked sentence). Added-member rows always show InvocationCount 0 — added-member calls are not instrumented, and the row's Reason says so. `AddedField` rows list a live added field as `Type.field` with an empty `Reason`; they are not method patches. `LifecycleNote` is set when a patched method is a Unity one-shot lifecycle message (`private void Awake`/`Start`/`OnEnable`/`OnDisable`/`OnDestroy` on a `MonoBehaviour`), or when every compiled call path into the patched method (callers of callers are followed a few levels within the compiled assemblies) starts at such a message; empty otherwise — it does not change `Kind`. On an `Added` row whose method is a Unity message, `LifecycleNote` instead says whether the engine will reach it: a forwarded message (`Start`, `Update`, the collision/trigger/mouse messages, and the rest listed in [scope-and-limits.md](scope-and-limits.md)) carries the note that a hot-reload proxy component delivers it to live instances while Play Mode runs, that the proxy is rebuilt only when a later reload changes which messages the type adds or their signatures, that execution order relative to other components is not guaranteed, and that it is gone on any compile or domain reload (only an added `Start` row also says that it runs once on each existing instance when the proxy attaches, and again when the proxy is rebuilt); a message this feature leaves to the compiler (`Awake`, `OnEnable`, `OnDisable`, `OnDestroy`, the editor-only messages, and any non-void message) carries the note that the engine does not invoke it until `uloop compile`, and the run adds one `Warnings` line naming every such message together. `Added` rows carry the added member's signature and file; their `InvocationCount` is always `0` (added-member calls are not instrumented). `ReappliedFromSibling` is `true` on every apply row, whatever its `Kind`, that belongs to a sibling file the run pulled in to re-apply changes from earlier reloads rather than to a file passed in `--files`; it is `false` on the other rows and on every `--status` and `--revert-all` row. Message's re-applied count covers only the `Patched` and `Added` rows among them. Example `--status` row: `{ "Kind": "Added", "Method": "Ns.Host.NewHelper(System.Int32)", "Reason": "Added-member calls are not instrumented, so InvocationCount is always 0 for this row.", "FilePath": "Assets/Scripts/Host.cs", "InvocationCount": 0, "LifecycleNote": "", "ReappliedFromSibling": false }` +- `Methods` (array): Per-method `{ Kind, Method, Reason, FilePath, InvocationCount, LifecycleNote, ReappliedFromSibling }` where `Kind` is `Patched`, `Skipped`, `Failed`, `Added`, `AlreadyActive`, or `Stale` on apply runs, and `Active`, `Added`, or `AddedField` on `--status` runs; empty on `--revert-all` runs. `AlreadyActive` means this file's source matched the last fully applied reload (a run with no Skipped or Failed outcomes), so the existing patch was left in place and the row carries the live `InvocationCount`. `Stale` means the method was deleted from the edited source while its patch is still installed: compiled callers keep running the patched body until `uloop compile`, `--revert-all`, or a later reload whose source restores the method to the compiled baseline clears it; a reload that declares the method with a different body replaces the patch instead of clearing it. Stale rows keep counting toward `ActivePatchTotal`, and the Message summary includes `Stale=N`. `InvocationCount` is meaningful on `Active` rows and on `AlreadyActive` and `Stale` apply rows (calls since the current patch was applied); it is `0` on other apply/revert outcomes. On `--status`, an `Active` row with `InvocationCount` 0 sets `Reason` to explain that the method has not run since the patch: finished calls do not re-run, the patched body takes effect on the next call, and how to retrigger an initialization-only path. When the edited source later declares a different signature, that `Active` row's `Reason` instead explains it is superseded by a new declaration of that signature and is no longer the entry point for new calls (superseded wins over the never-invoked sentence). Added-member rows always show InvocationCount 0 — added-member calls are not instrumented, and the row's Reason says so. `AddedField` rows list a live added field as `Type.field` with an empty `Reason`; they are not method patches. `LifecycleNote` is set when a patched method is a Unity one-shot lifecycle message (`private void Awake`/`Start`/`OnEnable`/`OnDisable`/`OnDestroy` on a `MonoBehaviour`), or when every compiled call path into the patched method (callers of callers are followed a few levels within the compiled assemblies) starts at such a message; empty otherwise — it does not change `Kind`. On an `Added` row whose method is a Unity message, `LifecycleNote` instead says whether the engine will reach it: a forwarded message (`Start`, `Update`, the collision/trigger/mouse messages, and the rest listed in [scope-and-limits.md](scope-and-limits.md)) carries the note that a hot-reload proxy component delivers it to live instances while Play Mode runs, that the proxy is rebuilt only when a later reload changes which messages the type adds or their signatures, that execution order relative to other components is not guaranteed, and that it is gone on any compile or domain reload (only an added `Start` row also says that it runs once on each existing instance when the proxy attaches, and again when the proxy is rebuilt); a message this feature leaves to the compiler (`Awake`, `OnEnable`, `OnDisable`, `OnDestroy`, the editor-only messages, and any non-void message) carries the note that the engine does not invoke it until `uloop compile`, and the run adds one `Warnings` line naming every such message together. `Added` rows carry the added member's signature and file; their `InvocationCount` is always `0` (added-member calls are not instrumented). `ReappliedFromSibling` is `true` on every apply row, whatever its `Kind`, that belongs to a sibling file the run pulled in to re-apply changes from earlier reloads rather than to a file passed in `--files`; it is `false` on the other rows and on every `--status` and `--revert-all` row. Message's re-applied count covers only the `Patched` and `Added` rows among them. `Method` spells parameter types as .NET metadata does, the same on every method row: a constructed generic as ``System.Collections.Generic.List`1``, a multidimensional array as `System.Int32[0...,0...]`, and a nested type with `+`. Example `--status` row: `{ "Kind": "Added", "Method": "Ns.Host.NewHelper(System.Int32)", "Reason": "Added-member calls are not instrumented, so InvocationCount is always 0 for this row.", "FilePath": "Assets/Scripts/Host.cs", "InvocationCount": 0, "LifecycleNote": "", "ReappliedFromSibling": false }` - `Warnings` (array): Non-fatal notes — one aggregated line listing the patched methods at risk of being already JIT-inlined into existing callers — those marked `[AggressiveInlining]`, plus (only when Code Optimization is Release) those with tiny pre-patch bodies — meaning the change may not show at those call sites, the pause-point interaction (see [pause-point-interaction.md](pause-point-interaction.md)), and the const drift, outside-body drift, and missing-baseline entries described in [scope-and-limits.md](scope-and-limits.md). Skipped outcomes are echoed here as `Skipped : `, or as one `Skipped N methods: ()` line per reason when several share it, so checking `Warnings` alone is enough to see that an edit was not applied. When a reload re-applies unchanged files so their patches bind to this run's shim, Warnings includes `Also re-applied N unchanged file(s) with active patches in assembly '...' so their patches bind to this reload's shim: ...`. When a pulled-in sibling fails in that reload, Warnings includes `'...' was pulled in to re-bind its active patches but this reload failed for it; see its rows for which patches changed and run uloop compile to clear the run.` instead of the re-applied line. When every row of that sibling was `Skipped` and none failed, Warnings instead includes `'...' was pulled in to re-bind its active patches, but every method there was Skipped this time; see its rows for the reasons. Any earlier patches there stay active until uloop compile clears the run.` When the reload stopped before re-applying anything at all, so that sibling has no rows, Warnings instead includes `'...' was pulled in to re-bind its active patches, but this reload stopped before re-applying them, so its active patches are unchanged. Fix the refused declaration and rerun, or run uloop compile to clear the run.` When the whole reload was refused, so that sibling's only rows are `Method` = `(file)` `Failed` rows repeating the refusal and none of its unchanged patches were reverted, Warnings instead includes `'...' was pulled in to re-bind its active patches, but the whole reload was refused before re-applying them, so its active patches are unchanged; its rows repeat the refusal reason. Fix that and rerun, or run uloop compile to clear the run.` When a sibling still has active patches but its source changed since they were applied, Warnings includes `'...' has active patches but its source changed since they were applied, so it was not re-applied; pass it to hot-reload to update it.` When a run carries two or more warnings and all of them are hot reload warnings, the Message ends with "A single 'uloop compile' clears all of them at once when you want them gone; none of them has to be cleared before you keep working." — it is the shortest recovery, not an obligation to compile immediately. Pause-point warnings carry their own recovery steps, so that line does not appear when they are present. Nor does it appear when an `IntroducedTypes` row is `Failed` or a warning says a declared type requires a compile, because that type does not exist until one. It is also left off when any `Methods` row is `Failed`, or when a `Methods` row of a file you passed, or of a sibling retried after an earlier Skip, is `Skipped`: that body is not running yet, so it needs a fix or a compile before you keep working. A `Skipped` row of a sibling pulled in only to re-bind its active patches does not leave it off, because the earlier patches there keep running. - `PatchedTotal` (number): Methods patched in this run - `AddedFields` (array): source-level names ("Type.field") of fields this reload added; their values live outside the compiled type until 'uloop compile'. Every run that adds fields also carries one warning stating that the values live outside the compiled assembly and last only until the next 'uloop compile' or domain reload; the warning names exactly the fields listed in AddedFields. An active added field declared with `[SerializeField]`, `[SerializeReference]`, or `[FormerlySerializedAs]` is also named, as `Namespace.Type.field` (nested types joined with `.`), in one `Added field(s) with a serialization attribute will not appear in the Inspector or serialize until 'uloop compile': ...` warning that points at [added-field-wiring.md](added-field-wiring.md). Only the run that first leaves the field active names it; a file that is Skipped or Failed names none of its fields, and a field is named again only after it stopped being active or after `--revert-all`. Pause-point `CapturedVariables` never includes these fields; `enable-pause-point` warns when the resolved type has any. From 1d41fcac6705335dc7aa6fa10d445b4b7988a9c9 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 29 Sep 2026 09:43:16 +0900 Subject: [PATCH 2/3] Leave callers with active patches out of the stale-signature warning Each group's signature-change gate covers only its own entries, so a compiled caller in another assembly was always named in the warning, even when this run (in an earlier or a later group) or an earlier run had patched it and it no longer runs the compiled body that calls the old signature. Groups run one assembly at a time, so no check at gate time can see what a later group patches. - The gate now records the uncovered call-site hits in structured form in a run-scoped HotReloadRunStaleSignatureWarnings instead of formatting text. BuildResult drops every caller whose (assembly, label) is active when the run ends, before de-duplicating the display by wire key, and keeps the warning text unchanged. Reading the state at the end also keeps a caller that a later group peeled back to its compiled body. - HotReloadActivePatchInfo carries the declaring assembly name, so a method with the same label in another assembly is not taken for the caller. - Known limits stay on the safe or documented side: a patched body that still calls the removed method is not detected (shims are not scanned), and callers inside lambdas or local functions keep compiler-generated names and stay listed. The return-type gate is unchanged and now pinned: a caller in another assembly still gates the change even when patched. Tests: 14 unit tests for the run-scoped filter, a coverage test that the collection keeps one hit per assembly-qualified caller, and 7 end-to-end cases (caller patched in an earlier or later group, host-only rerun with an active or unpatched caller, caller peeled later in the same run, active caller still calling the removed method, return-type change stays gated). Turning the filter off, ignoring the assembly, de-duplicating before filtering, or de-duplicating the collection by wire key each fails the expected tests. --- ...loadCrossAssemblyStaleSignatureE2ETests.cs | 423 ++++++++++++++++++ ...rossAssemblyStaleSignatureE2ETests.cs.meta | 11 + ...otReloadCrossAssemblyStaleSignatureHost.cs | 30 ++ ...oadCrossAssemblyStaleSignatureHost.cs.meta | 11 + .../HotReload/HotReloadGroupProcessorTests.cs | 10 +- ...HotReloadIntroducedTypeOutcomeSinkTests.cs | 2 +- .../HotReload/HotReloadOrchestratorTests.cs | 2 +- ...HotReloadRetainedTypePatchReverterTests.cs | 2 +- ...HotReloadRunStaleSignatureWarningsTests.cs | 369 +++++++++++++++ ...loadRunStaleSignatureWarningsTests.cs.meta | 11 + .../HotReloadSignatureChangeCoverageTests.cs | 90 ++-- .../HotReloadUnchangedPatchPeelTests.cs | 2 +- ...ReloadCrossAssemblyStaleSignatureCaller.cs | 23 + ...dCrossAssemblyStaleSignatureCaller.cs.meta | 11 + .../HotReload/HotReloadFileSinks.cs | 8 +- .../HotReload/HotReloadGroupProcessor.cs | 7 +- .../HotReload/HotReloadInputFileResolver.cs | 1 + .../HotReload/HotReloadRunAccumulator.cs | 8 + .../HotReloadRunStaleSignatureWarnings.cs | 110 +++++ ...HotReloadRunStaleSignatureWarnings.cs.meta | 11 + .../HotReloadSiblingRebindReporter.cs | 1 + .../HotReloadSignatureChangeCoverage.cs | 64 +-- .../HotReload/HotReloadSignatureChangeGate.cs | 26 +- .../HotReloadStaleSignatureCallSites.cs | 33 ++ .../HotReloadStaleSignatureCallSites.cs.meta | 11 + .../Patching/HotReloadActivePatchInfo.cs | 12 +- .../HotReload/Patching/HotReloadDomain.cs | 3 +- 27 files changed, 1176 insertions(+), 116 deletions(-) create mode 100644 Assets/Tests/Editor/HotReload/HotReloadCrossAssemblyStaleSignatureE2ETests.cs create mode 100644 Assets/Tests/Editor/HotReload/HotReloadCrossAssemblyStaleSignatureE2ETests.cs.meta create mode 100644 Assets/Tests/Editor/HotReload/HotReloadCrossAssemblyStaleSignatureHost.cs create mode 100644 Assets/Tests/Editor/HotReload/HotReloadCrossAssemblyStaleSignatureHost.cs.meta create mode 100644 Assets/Tests/Editor/HotReload/HotReloadRunStaleSignatureWarningsTests.cs create mode 100644 Assets/Tests/Editor/HotReload/HotReloadRunStaleSignatureWarningsTests.cs.meta create mode 100644 Assets/Tests/Editor/HotReloadCallSiteCrossAssembly/HotReloadCrossAssemblyStaleSignatureCaller.cs create mode 100644 Assets/Tests/Editor/HotReloadCallSiteCrossAssembly/HotReloadCrossAssemblyStaleSignatureCaller.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/HotReload/HotReloadRunStaleSignatureWarnings.cs create mode 100644 Packages/src/Editor/FirstPartyTools/HotReload/HotReloadRunStaleSignatureWarnings.cs.meta create mode 100644 Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStaleSignatureCallSites.cs create mode 100644 Packages/src/Editor/FirstPartyTools/HotReload/HotReloadStaleSignatureCallSites.cs.meta 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)); } } From 7f2611f8d0fbb5d372b8cee856f8ff52bed671c7 Mon Sep 17 00:00:00 2001 From: hatayama Date: Tue, 29 Sep 2026 09:43:30 +0900 Subject: [PATCH 3/3] Document which callers the stale-signature warning leaves out The hot reload docs said a return-type change applies once this or an earlier reload patched every compiled caller. A caller in another assembly still gates the change even after being patched, because its patch is compiled against the compiled assembly, where the old signature still exists; an end-to-end test now pins that. The SKILL.md summary and the gate paragraph now limit the "already patched" case to the same assembly. The paragraph on the warning for renamed, re-parameterized, or deleted methods now says a caller whose patch is active when the reload ends is left out, and names the limits: what the patched body calls is not checked, a copy the JIT inlined or a delegate created before the patch can still reach the old method, and a call inside a lambda or local function stays listed under its compiler-generated name. SKILL.md stays at 7,996 of 8,000 bytes. The generated copies under .claude and .agents are regenerated and match the source byte for byte. --- .agents/skills/uloop-hot-reload/SKILL.md | 6 +++--- .../references/scope-and-limits.md | 19 ++++++++++++++----- .claude/skills/uloop-hot-reload/SKILL.md | 6 +++--- .../references/scope-and-limits.md | 19 ++++++++++++++----- .../FirstPartyTools/HotReload/Skill/SKILL.md | 6 +++--- .../Skill/references/scope-and-limits.md | 19 ++++++++++++++----- 6 files changed, 51 insertions(+), 24 deletions(-) diff --git a/.agents/skills/uloop-hot-reload/SKILL.md b/.agents/skills/uloop-hot-reload/SKILL.md index ee922569ee..eb812d9224 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 2e58192e73..082f3b1e5a 100644 --- a/.agents/skills/uloop-hot-reload/references/scope-and-limits.md +++ b/.agents/skills/uloop-hot-reload/references/scope-and-limits.md @@ -177,25 +177,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 ee922569ee..eb812d9224 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 2e58192e73..082f3b1e5a 100644 --- a/.claude/skills/uloop-hot-reload/references/scope-and-limits.md +++ b/.claude/skills/uloop-hot-reload/references/scope-and-limits.md @@ -177,25 +177,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/Packages/src/Editor/FirstPartyTools/HotReload/Skill/SKILL.md b/Packages/src/Editor/FirstPartyTools/HotReload/Skill/SKILL.md index ee922569ee..eb812d9224 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 2e58192e73..082f3b1e5a 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 @@ -177,25 +177,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