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 86a71ae1f0..73a2262311 100644 --- a/.agents/skills/uloop-hot-reload/references/scope-and-limits.md +++ b/.agents/skills/uloop-hot-reload/references/scope-and-limits.md @@ -175,7 +175,10 @@ baseline is available (next paragraph), other outside-body drift — existing-fi initializers, attributes, and other declaration edits — is reported as a `Warnings` entry as well (handled added members and reported removed members are excluded from this generic warning); without a baseline it stays silent. Either way, use -`uloop compile` for such edits. +`uloop compile` for such edits. The comparison is against the source the loaded +assembly was compiled from, so an earlier edit outside method bodies keeps this +warning on every reload until `uloop compile`. Comment-only edits (line, block, and +XML documentation comments) do not count. ## Signature changes: return type, rename, parameters 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 86a71ae1f0..73a2262311 100644 --- a/.claude/skills/uloop-hot-reload/references/scope-and-limits.md +++ b/.claude/skills/uloop-hot-reload/references/scope-and-limits.md @@ -175,7 +175,10 @@ baseline is available (next paragraph), other outside-body drift — existing-fi initializers, attributes, and other declaration edits — is reported as a `Warnings` entry as well (handled added members and reported removed members are excluded from this generic warning); without a baseline it stays silent. Either way, use -`uloop compile` for such edits. +`uloop compile` for such edits. The comparison is against the source the loaded +assembly was compiled from, so an earlier edit outside method bodies keeps this +warning on every reload until `uloop compile`. Comment-only edits (line, block, and +XML documentation comments) do not count. ## Signature changes: return type, rename, parameters diff --git a/Assets/Tests/Editor/HotReload/TransformWorkerAddedFieldTests.cs b/Assets/Tests/Editor/HotReload/TransformWorkerAddedFieldTests.cs index 5d28a0710b..e5908764c4 100644 --- a/Assets/Tests/Editor/HotReload/TransformWorkerAddedFieldTests.cs +++ b/Assets/Tests/Editor/HotReload/TransformWorkerAddedFieldTests.cs @@ -58,7 +58,7 @@ public class TransformWorkerAddedFieldTests // Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/OutsideMethodBodyDriftChecker.cs. // That constant lives in the Unity-ignored worker process and is not visible here. private const string OutsideMethodBodyDriftWarningFormat = - "Edits outside method bodies in {0} (fields, initializers, or attributes) are not applied by hot reload; run uloop compile to pick them up."; + "Edits outside method bodies in {0} (fields, initializers, or attributes) since the last compile are not applied by hot reload; run uloop compile to pick them up."; private const string FieldKindChangeProjectRelativePath = "Assets/Tests/Editor/HotReload/HotReloadAddedMemberHost.cs"; @@ -1470,7 +1470,7 @@ public async Task Drift_ExistingInitializerEditWithAddedField_StillWarns() Assert.That(foundDrift, Is.True, "Existing field initializer edits must still warn."); AssertHasDeclarationDriftWarning( result, - "Edits outside method bodies in AddedFieldWithInitializerDrift.cs (field initializer: PublicSeed) are not applied by hot reload; run uloop compile to pick them up."); + "Edits outside method bodies in AddedFieldWithInitializerDrift.cs (field initializer: PublicSeed) since the last compile are not applied by hot reload; run uloop compile to pick them up."); Assert.That(result.Output.hasAddedFieldRewrites, Is.True); } @@ -1496,7 +1496,7 @@ public async Task Drift_ExistingInitializerEdit_NamesDeclaration() Is.EqualTo( new[] { - "Edits outside method bodies in NamedInitializerDrift.cs (field initializer: PublicSeed) are not applied by hot reload; run uloop compile to pick them up." + "Edits outside method bodies in NamedInitializerDrift.cs (field initializer: PublicSeed) since the last compile are not applied by hot reload; run uloop compile to pick them up." })); } @@ -1525,7 +1525,7 @@ public async Task Drift_FieldDeclarationOrderSwap_EmitsFileOnlyWarning() Is.EqualTo( new[] { - "Edits outside method bodies in FieldOrderSwapDrift.cs (fields, initializers, or attributes) are not applied by hot reload; run uloop compile to pick them up." + "Edits outside method bodies in FieldOrderSwapDrift.cs (fields, initializers, or attributes) since the last compile are not applied by hot reload; run uloop compile to pick them up." })); } @@ -1552,7 +1552,7 @@ public async Task Drift_MultiDeclaratorAttributeEdit_NamesEverySibling() Is.EqualTo( new[] { - "Edits outside method bodies in MultiDeclaratorAttributeDrift.cs (field attributes: PairAlpha, PairBeta) are not applied by hot reload; run uloop compile to pick them up." + "Edits outside method bodies in MultiDeclaratorAttributeDrift.cs (field attributes: PairAlpha, PairBeta) since the last compile are not applied by hot reload; run uloop compile to pick them up." })); } @@ -1578,7 +1578,7 @@ public async Task Drift_MultiDeclaratorInitializerEdit_NamesOnlyChangedVariable( Is.EqualTo( new[] { - "Edits outside method bodies in MultiDeclaratorInitializerDrift.cs (field initializer: PairAlpha) are not applied by hot reload; run uloop compile to pick them up." + "Edits outside method bodies in MultiDeclaratorInitializerDrift.cs (field initializer: PairAlpha) since the last compile are not applied by hot reload; run uloop compile to pick them up." })); } @@ -1604,7 +1604,7 @@ public async Task Drift_MultiDeclaratorRegroup_FailsOpenToFileOnlyWarning() Is.EqualTo( new[] { - "Edits outside method bodies in MultiDeclaratorRegroupDrift.cs (fields, initializers, or attributes) are not applied by hot reload; run uloop compile to pick them up." + "Edits outside method bodies in MultiDeclaratorRegroupDrift.cs (fields, initializers, or attributes) since the last compile are not applied by hot reload; run uloop compile to pick them up." })); } @@ -1634,10 +1634,72 @@ public async Task Drift_DuplicateFieldSyntaxKey_FailsOpenToFileOnlyWarning() Is.EqualTo( new[] { - "Edits outside method bodies in DupFieldKeyFailOpen.cs (fields, initializers, or attributes) are not applied by hot reload; run uloop compile to pick them up." + "Edits outside method bodies in DupFieldKeyFailOpen.cs (fields, initializers, or attributes) since the last compile are not applied by hot reload; run uloop compile to pick them up." })); } + /// + /// What: when a duplicate field syntax key sends the drift check to the whole-tree + /// fail-open compare, an edit that only adds a comment still emits no warning. + /// + [Test] + public async Task Drift_DuplicateFieldSyntaxKeyWithCommentOnlyEdit_DoesNotWarn() + { + string onDisk = File.ReadAllText(ResolveHostPath()); + // Why the duplicate repeats a compiled field: a name the compiled type lacks counts as + // an added field of the edited source and is stripped from that tree only, so the + // compare would warn without any comment. + string snapshotSource = onDisk.Replace( + " public int PublicSeed = 3;", + " public int PublicSeed = 3;\n public int PublicSeed = 3;", + StringComparison.Ordinal); + string edited = snapshotSource.Replace( + " public int PublicSeed = 3;\n public int PublicSeed = 3;", + " // comment added by the test\n public int PublicSeed = 3;\n public int PublicSeed = 3;", + StringComparison.Ordinal); + Assert.That(edited, Is.Not.EqualTo(snapshotSource), "Precondition: the edit must add the comment."); + TransformWorkerClientResult result = await RunWorkerOnSourceAsync( + WriteEdited("DupFieldKeyCommentOnly.cs", edited), + HostProjectRelativePath, + snapshotSource: snapshotSource); + Assert.That(result.Success, Is.True, result.ErrorMessage); + Assert.That( + result.Output.files[0].declarationDriftWarnings, + Is.Empty, + string.Join("\n", result.Output.files[0].declarationDriftWarnings ?? Array.Empty())); + } + + /// + /// What: adding only a line comment above the attribute list of an existing field emits no + /// outside-method-body warning. + /// + [Test] + public async Task Drift_LineCommentOnlyEditAboveAFieldAttribute_DoesNotWarn() + { + string onDisk = File.ReadAllText(ResolveHostPath()); + // Why the snapshot carries the attribute too: no field of the compiled host has one, and + // the comment must be the only difference between the snapshot and the edited source. + string snapshotSource = onDisk.Replace( + " public int PublicSeed = 3;", + " [SerializeField]\n public int PublicSeed = 3;", + StringComparison.Ordinal); + string edited = snapshotSource.Replace( + " [SerializeField]\n public int PublicSeed = 3;", + " // comment added by the test\n [SerializeField]\n public int PublicSeed = 3;", + StringComparison.Ordinal); + Assert.That(snapshotSource, Is.Not.EqualTo(onDisk), "Precondition: the snapshot must add the attribute."); + Assert.That(edited, Is.Not.EqualTo(snapshotSource), "Precondition: the edit must add the comment."); + TransformWorkerClientResult result = await RunWorkerOnSourceAsync( + WriteEdited("FieldAttributeCommentOnly.cs", edited), + HostProjectRelativePath, + snapshotSource: snapshotSource); + Assert.That(result.Success, Is.True, result.ErrorMessage); + Assert.That( + result.Output.files[0].declarationDriftWarnings, + Is.Empty, + string.Join("\n", result.Output.files[0].declarationDriftWarnings ?? Array.Empty())); + } + /// /// What: a readonly added field can still be read through GetOrInit. /// @@ -1922,7 +1984,7 @@ public async Task Drift_PropertyKindChangeWithInitializerEdit_StillEmitsNamedOut new[] { "Compiled property 'io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload.HotReloadFieldKindChangeFixture.Hp' was removed or redeclared as a different member kind in the edited source; the compiled member stays until 'uloop compile'.", - "Edits outside method bodies in PropertyKindChangeWithInitializer.cs (field initializer: PublicSeed) are not applied by hot reload; run uloop compile to pick them up." + "Edits outside method bodies in PropertyKindChangeWithInitializer.cs (field initializer: PublicSeed) since the last compile are not applied by hot reload; run uloop compile to pick them up." })); } @@ -1953,7 +2015,7 @@ public async Task Drift_EventKindChangeWithInitializerEdit_StillEmitsNamedOutsid new[] { "Compiled event 'io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload.HotReloadFieldKindChangeFixture.ScoreChanged' was removed or redeclared as a different member kind in the edited source; the compiled member stays until 'uloop compile'.", - "Edits outside method bodies in EventKindChangeWithInitializer.cs (field initializer: PublicSeed) are not applied by hot reload; run uloop compile to pick them up." + "Edits outside method bodies in EventKindChangeWithInitializer.cs (field initializer: PublicSeed) since the last compile are not applied by hot reload; run uloop compile to pick them up." })); } diff --git a/Assets/Tests/Editor/HotReload/TransformWorkerClientTests.cs b/Assets/Tests/Editor/HotReload/TransformWorkerClientTests.cs index 205bc33827..a2e36dce3b 100644 --- a/Assets/Tests/Editor/HotReload/TransformWorkerClientTests.cs +++ b/Assets/Tests/Editor/HotReload/TransformWorkerClientTests.cs @@ -1065,6 +1065,103 @@ public async Task Run_WithSnapshotFieldInitializerChanged_EmitsOutsideMethodBody Has.Some.Contain("Edits outside method bodies in HotReloadE2EFixtures.cs")); } + /// + /// What: adding only an XML documentation comment above an existing method emits neither + /// the file-level nor a named outside-method-body warning. + /// + [Test] + public async Task Run_WithXmlDocCommentOnlyEditOnAMethod_DoesNotEmitOutsideMethodBodyWarning() + { + const string fileName = "XmlDocCommentOnMethodDrift.cs"; + TransformWorkerClientResult result = await RunWorkerOnEditedE2ECopyAsync( + fileName, + editedSource => ReplaceUniqueFragment( + editedSource, + " public int VisibleSibling()", + " /// Documentation added by the test.\n public int VisibleSibling()")); + + AssertDoesNotContainOutsideMethodBodyDriftWarning(result, fileName); + Assert.That(result.Output.files[0].declarationDriftWarnings, Is.Empty); + } + + /// + /// What: adding only a block comment inside a field initializer emits neither the + /// file-level nor a named outside-method-body warning. + /// + [Test] + public async Task Run_WithBlockCommentOnlyEditInsideAFieldInitializer_DoesNotEmitOutsideMethodBodyWarning() + { + const string fileName = "BlockCommentInFieldInitializerDrift.cs"; + TransformWorkerClientResult result = await RunWorkerOnEditedE2ECopyAsync( + fileName, + editedSource => ReplaceUniqueFragment( + editedSource, + "private int _secret = 10;", + "private int _secret = /* comment added by the test */ 10;")); + + AssertDoesNotContainOutsideMethodBodyDriftWarning(result, fileName); + Assert.That(result.Output.files[0].declarationDriftWarnings, Is.Empty); + } + + /// + /// What: adding only a line comment on its own line above a field emits neither the + /// file-level nor a named outside-method-body warning. + /// + [Test] + public async Task Run_WithLineCommentOnlyEditAboveAField_DoesNotEmitOutsideMethodBodyWarning() + { + const string fileName = "LineCommentAboveFieldDrift.cs"; + TransformWorkerClientResult result = await RunWorkerOnEditedE2ECopyAsync( + fileName, + editedSource => ReplaceUniqueFragment( + editedSource, + " private int _secret = 10;", + " // comment added by the test\n private int _secret = 10;")); + + AssertDoesNotContainOutsideMethodBodyDriftWarning(result, fileName); + Assert.That(result.Output.files[0].declarationDriftWarnings, Is.Empty); + } + + /// + /// What: adding only a block comment between a field's type and its name emits neither the + /// file-level nor a named outside-method-body warning. + /// + [Test] + public async Task Run_WithBlockCommentOnlyEditAfterAFieldType_DoesNotEmitOutsideMethodBodyWarning() + { + const string fileName = "BlockCommentAfterFieldTypeDrift.cs"; + // Why after the type: a comment on the same line belongs to the token before it, so + // this one becomes part of the field's type rather than of its modifiers. + TransformWorkerClientResult result = await RunWorkerOnEditedE2ECopyAsync( + fileName, + editedSource => ReplaceUniqueFragment( + editedSource, + "private int _secret = 10;", + "private int /* comment added by the test */ _secret = 10;")); + + AssertDoesNotContainOutsideMethodBodyDriftWarning(result, fileName); + Assert.That(result.Output.files[0].declarationDriftWarnings, Is.Empty); + } + + /// + /// What: rewording only the XML documentation comment of the type emits neither the + /// file-level nor a named outside-method-body warning. + /// + [Test] + public async Task Run_WithXmlDocCommentOnlyEditOnTheType_DoesNotEmitOutsideMethodBodyWarning() + { + const string fileName = "XmlDocCommentOnTypeDrift.cs"; + TransformWorkerClientResult result = await RunWorkerOnEditedE2ECopyAsync( + fileName, + editedSource => ReplaceUniqueFragment( + editedSource, + "Compiled fixture whose on-disk source path", + "Prebuilt fixture whose on-disk source path")); + + AssertDoesNotContainOutsideMethodBodyDriftWarning(result, fileName); + Assert.That(result.Output.files[0].declarationDriftWarnings, Is.Empty); + } + /// /// What: editing only a const value emits the dedicated const-drift warning and does not /// also emit the generic outside-method-body warning. @@ -1289,7 +1386,7 @@ public async Task Run_WithNonConstFieldInitializerEdit_StillEmitsOutsideBodyWarn Assert.That( result.Output.files[0].declarationDriftWarnings, Does.Contain( - "Edits outside method bodies in NonConstFieldInitializerDrift.cs (field initializer: _secret) are not applied by hot reload; run uloop compile to pick them up.")); + "Edits outside method bodies in NonConstFieldInitializerDrift.cs (field initializer: _secret) since the last compile are not applied by hot reload; run uloop compile to pick them up.")); } /// @@ -2057,7 +2154,7 @@ public async Task Run_WithOnEnableOrOnDisable_EmitsDirectNoteThatTogglesEnabled( // Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/OutsideMethodBodyDriftChecker.cs. // That constant lives in the Unity-ignored worker process and is not visible here. private const string OutsideMethodBodyDriftWarningFormat = - "Edits outside method bodies in {0} (fields, initializers, or attributes) are not applied by hot reload; run uloop compile to pick them up."; + "Edits outside method bodies in {0} (fields, initializers, or attributes) since the last compile are not applied by hot reload; run uloop compile to pick them up."; /// /// What: editing one instance constructor reports that .ctor as Skipped and omits an @@ -2412,7 +2509,7 @@ public async Task Run_UnsupportedMemberKind_CtorInitializerEdit_EmitsOutsideBody Assert.That( result.Output.files[0].declarationDriftWarnings, Does.Contain( - "Edits outside method bodies in UnsupportedKindCtorInitializerDrift.cs (constructor: .ctor) are not applied by hot reload; run uloop compile to pick them up.")); + "Edits outside method bodies in UnsupportedKindCtorInitializerDrift.cs (constructor: .ctor) since the last compile are not applied by hot reload; run uloop compile to pick them up.")); } /// @@ -2431,7 +2528,7 @@ public async Task Run_UnsupportedMemberKind_OperatorAttributeEdit_EmitsOutsideBo Assert.That( result.Output.files[0].declarationDriftWarnings, Does.Contain( - "Edits outside method bodies in UnsupportedKindOperatorAttributeDrift.cs (operator: +) are not applied by hot reload; run uloop compile to pick them up.")); + "Edits outside method bodies in UnsupportedKindOperatorAttributeDrift.cs (operator: +) since the last compile are not applied by hot reload; run uloop compile to pick them up.")); } /// @@ -2450,7 +2547,7 @@ public async Task Run_UnsupportedMemberKind_ConversionAttributeEdit_EmitsOutside Assert.That( result.Output.files[0].declarationDriftWarnings, Does.Contain( - "Edits outside method bodies in UnsupportedKindConversionAttributeDrift.cs (conversion: implicit->int) are not applied by hot reload; run uloop compile to pick them up.")); + "Edits outside method bodies in UnsupportedKindConversionAttributeDrift.cs (conversion: implicit->int) since the last compile are not applied by hot reload; run uloop compile to pick them up.")); } /// @@ -2469,7 +2566,7 @@ public async Task Run_UnsupportedMemberKind_EventAttributeEdit_EmitsOutsideBodyW Assert.That( result.Output.files[0].declarationDriftWarnings, Does.Contain( - "Edits outside method bodies in UnsupportedKindEventAttributeDrift.cs (event: Edited) are not applied by hot reload; run uloop compile to pick them up.")); + "Edits outside method bodies in UnsupportedKindEventAttributeDrift.cs (event: Edited) since the last compile are not applied by hot reload; run uloop compile to pick them up.")); } /// @@ -2714,6 +2811,19 @@ private static async Task RunWorkerOnEditedE2ECopyA return result; } + // Why the uniqueness check: Replace edits every occurrence, and a comment that also landed + // somewhere else would reach a comparison other than the one the test is about. + private static string ReplaceUniqueFragment(string source, string fragment, string replacement) + { + int index = source.IndexOf(fragment, StringComparison.Ordinal); + Assert.That(index, Is.GreaterThanOrEqualTo(0), "Precondition: the fixture must hold " + fragment); + Assert.That( + source.LastIndexOf(fragment, StringComparison.Ordinal), + Is.EqualTo(index), + "Precondition: the fixture must hold " + fragment + " exactly once."); + return source.Replace(fragment, replacement, StringComparison.Ordinal); + } + private static void AssertPatchedComputeWithPrivate(TransformWorkerClientResult result) { bool foundCompute = false; 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 86a71ae1f0..73a2262311 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 @@ -175,7 +175,10 @@ baseline is available (next paragraph), other outside-body drift — existing-fi initializers, attributes, and other declaration edits — is reported as a `Warnings` entry as well (handled added members and reported removed members are excluded from this generic warning); without a baseline it stays silent. Either way, use -`uloop compile` for such edits. +`uloop compile` for such edits. The comparison is against the source the loaded +assembly was compiled from, so an earlier edit outside method bodies keeps this +warning on every reload until `uloop compile`. Comment-only edits (line, block, and +XML documentation comments) do not count. ## Signature changes: return type, rename, parameters diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/OutsideMethodBodyDriftChecker.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/OutsideMethodBodyDriftChecker.cs index 273d15b87e..37e48b9d16 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/OutsideMethodBodyDriftChecker.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/OutsideMethodBodyDriftChecker.cs @@ -18,10 +18,10 @@ internal static class OutsideMethodBodyDriftChecker { internal const string OutsideMethodBodyDriftWarningFormat = - "Edits outside method bodies in {0} (fields, initializers, or attributes) are not applied by hot reload; run uloop compile to pick them up."; + "Edits outside method bodies in {0} (fields, initializers, or attributes) since the last compile are not applied by hot reload; run uloop compile to pick them up."; internal const string OutsideMethodBodyNamedDriftWarningFormat = - "Edits outside method bodies in {0} ({1}) are not applied by hot reload; run uloop compile to pick them up."; + "Edits outside method bodies in {0} ({1}) since the last compile are not applied by hot reload; run uloop compile to pick them up."; internal static void AppendOutsideMethodBodyDriftWarningIfNeeded( CompilationUnitSyntax snapshotRoot, diff --git a/docs/hot-reload.md b/docs/hot-reload.md index 2453fb83b8..7e9cd1a642 100644 --- a/docs/hot-reload.md +++ b/docs/hot-reload.md @@ -68,6 +68,9 @@ Cecil looks up the assemblies a publicized copy refers to in the directories of assembly's compile references first, then in those of every assembly it references transitively. When a verified snapshot marks a currently patched method as unchanged, the orchestrator reverts that patch to the compiled IL instead of re-emitting a shim. +The outside-method-body warning compares against the same snapshot, the source of the last +compile, so a declaration edit made since then keeps the warning on every reload until +`uloop compile`; comment-only differences do not count. ## Spike Findings @@ -436,7 +439,8 @@ Wire details: already hold active patches are re-applied so they bind to the newest shim. Shim compile errors caused by references to members that are still missing are reported with that hint, and changed `const` values (including enum members) are compared against the compiled target assembly and reported as - a response warning; other outside-body edits stay silent. + a response warning; other outside-body edits are reported as a response warning when a + verified source baseline is available, and stay silent without one. - A Unity message added to an existing `MonoBehaviour` is delivered by a generated proxy component that hot reload attaches to each live instance while Play Mode runs, because Unity's own message discovery only sees the compiled class. `Start`, `Update`,