diff --git a/Assets/Tests/Editor/HotReload/HotReloadAddedMemberAccessE2ETests.cs b/Assets/Tests/Editor/HotReload/HotReloadAddedMemberAccessE2ETests.cs index d8cbeb93af..be43967cd4 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadAddedMemberAccessE2ETests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadAddedMemberAccessE2ETests.cs @@ -14,9 +14,11 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload { /// - /// End-to-end EditMode coverage for added private members that compiled-member accessor - /// delegates cannot reach: a static property the compiled type lacks, and ref/out arguments. - /// The reload must emit them and the patched runtime must return their values. + /// End-to-end EditMode coverage for private member use in reloaded bodies: added private + /// members that compiled-member accessor delegates cannot reach (a static property the + /// compiled type lacks, ref/out arguments), nameof of a compiled private field, and a property + /// pattern on a compiled private field. The reload must apply them and the patched runtime + /// must return their values. /// public class HotReloadAddedMemberAccessE2ETests { @@ -93,6 +95,79 @@ public async Task Run_AddedMethodsUsingRefOutAndAddedStaticProperty_ReturnTheirV Assert.That(new HotReloadCrossFileAddedMemberHost().Scaled(3), Is.EqualTo(211), FormatOutcomes(result)); } + /// + /// What: an added static method that names a compiled instance field with nameof is added + /// rather than failing its shim compile, and the patched caller returns the name's length. + /// + [Test] + public async Task Run_AddedStaticMethodNamingInstanceFieldWithNameof_IsAddedAndReturnsTheNameLength() + { + string source = ReadFixture(HostFileName); + source = ReplaceInSource( + source, + HostScaledBodyAnchor, + " return AddedStoredNameLength() + factor;\n"); + source = ReplaceInSource( + source, + HostValueAnchor, + " private static int AddedStoredNameLength()\n {\n" + + " return nameof(_stored).Length;\n }\n\n" + + HostValueAnchor); + + string hostPath = FixturePath(HostFileName); + HotReloadOrchestratorResult result = await HotReloadCompositionRoot.Services.Orchestrator.RunAsync( + new[] { hostPath }, + contentPathOverride: null, + CancellationToken.None, + new Dictionary + { + [hostPath] = HotReloadTestSourceWriter.WriteEditedSource( + "AddedStaticNameofHost.cs", + source) + }); + + AssertNoKind(result, HotReloadMethodOutcomeKind.Failed); + AssertNoKind(result, HotReloadMethodOutcomeKind.Skipped); + AssertKind(result, HotReloadMethodOutcomeKind.Patched, "Scaled"); + AssertKind(result, HotReloadMethodOutcomeKind.Added, "AddedStoredNameLength"); + // "_stored".Length + 3 = 7 + 3. + Assert.That(new HotReloadCrossFileAddedMemberHost().Scaled(3), Is.EqualTo(10), FormatOutcomes(result)); + } + + /// + /// What: a compiled method whose new body matches property patterns against a compiled + /// private field is patched, and the patched call tells the matching pattern from the + /// non-matching one. + /// + [Test] + public async Task Run_PropertyPatternOnCompiledPrivateField_IsPatchedAndEvaluatesThePatterns() + { + string source = ReadFixture(HostFileName); + source = ReplaceInSource( + source, + HostScaledBodyAnchor, + " return (this is { _stored: 0 } ? 100 : 0) + (this is { _stored: 1 } ? 10 : 0) + factor;\n"); + + string hostPath = FixturePath(HostFileName); + HotReloadOrchestratorResult result = await HotReloadCompositionRoot.Services.Orchestrator.RunAsync( + new[] { hostPath }, + contentPathOverride: null, + CancellationToken.None, + new Dictionary + { + [hostPath] = HotReloadTestSourceWriter.WriteEditedSource( + "PropertyPatternPrivateFieldHost.cs", + source) + }); + + AssertNoKind(result, HotReloadMethodOutcomeKind.Failed); + AssertNoKind(result, HotReloadMethodOutcomeKind.Skipped); + AssertKind(result, HotReloadMethodOutcomeKind.Patched, "Scaled"); + // A new instance's _stored is 0, so only the first pattern matches: 100 + 0 + 3. A shim + // that read the wrong value, or matched both or neither, returns something else. + Assert.That(new HotReloadCrossFileAddedMemberHost().Scaled(3), Is.EqualTo(103), FormatOutcomes(result)); + } + private static void AssertNoKind(HotReloadOrchestratorResult result, HotReloadMethodOutcomeKind kind) { foreach (HotReloadMethodOutcome outcome in result.Methods) diff --git a/Assets/Tests/Editor/HotReload/TransformWorkerAddedMemberTests.cs b/Assets/Tests/Editor/HotReload/TransformWorkerAddedMemberTests.cs index b3e0dd92b2..94fab69642 100644 --- a/Assets/Tests/Editor/HotReload/TransformWorkerAddedMemberTests.cs +++ b/Assets/Tests/Editor/HotReload/TransformWorkerAddedMemberTests.cs @@ -474,6 +474,215 @@ public async Task Rewrite_NameofAddedMethod_FoldsToStringLiteral() Assert.That(slice, Does.Not.Contain("nameof(")); } + /// + /// What: nameof of a compiled instance field inside an added static method folds to a + /// string literal instead of naming an instance parameter the static shim does not have. + /// + [Test] + public async Task Rewrite_NameofInstanceFieldInAddedStaticMethod_FoldsToStringLiteral() + { + string onDisk = File.ReadAllText(ResolveHostPath()); + string edited = WithHostMembers( + onDisk, + "private static int AddedNameLength()\n {\n return nameof(_privateSeed).Length;\n }"); + string sourcePath = WriteEdited("NameofInstanceFieldInAddedStatic.cs", edited); + + TransformWorkerClientResult result = await RunWorkerOnSourceAsync( + sourcePath, + HostProjectRelativePath, + snapshotSource: onDisk); + Assert.That(result.Success, Is.True, result.ErrorMessage); + + TransformWorkerEntryDto added = FindEntry(result, "AddedNameLength"); + Assert.That(added, Is.Not.Null, "Skipped=" + FormatSkipped(result.Output.skipped)); + string slice = SliceShimMethod(result.Output.shimSource, added.shimMethodName); + Assert.That(slice, Does.Contain("\"_privateSeed\"")); + Assert.That(slice, Does.Not.Contain("nameof(")); + Assert.That(slice, Does.Not.Contain("__uloopInstance")); + } + + /// + /// What: nameof of a compiled instance field, method group, and property inside an edited + /// existing static method folds to string literals, so its shim names no instance parameter. + /// + [Test] + public async Task Rewrite_NameofInstanceMembersInExistingStaticMethod_FoldsToStringLiterals() + { + string onDisk = File.ReadAllText(ResolveHostPath()); + string edited = onDisk.Replace( + " private static int PrivateStaticSeven()\n {\n return 7;\n }", + " private static int PrivateStaticSeven()\n {\n" + + " return nameof(PublicSeed).Length + nameof(ExistingValue).Length" + + " + nameof(ExistingGetter).Length;\n" + + " }", + StringComparison.Ordinal); + Assert.That(edited, Is.Not.EqualTo(onDisk)); + string sourcePath = WriteEdited("NameofInstanceMembersInExistingStatic.cs", edited); + + TransformWorkerClientResult result = await RunWorkerOnSourceAsync( + sourcePath, + HostProjectRelativePath, + snapshotSource: onDisk); + Assert.That(result.Success, Is.True, result.ErrorMessage); + + TransformWorkerEntryDto existing = FindEntry(result, "PrivateStaticSeven"); + Assert.That(existing, Is.Not.Null, "Skipped=" + FormatSkipped(result.Output.skipped)); + Assert.That(existing.patchKind, Is.Not.EqualTo(HotReloadConstants.PatchKindAddedMethod)); + string slice = SliceShimMethod(result.Output.shimSource, existing.shimMethodName); + Assert.That(slice, Does.Contain("\"PublicSeed\"")); + Assert.That(slice, Does.Contain("\"ExistingValue\"")); + Assert.That(slice, Does.Contain("\"ExistingGetter\"")); + Assert.That(slice, Does.Not.Contain("nameof(")); + Assert.That(slice, Does.Not.Contain("__uloopInstance")); + } + + /// + /// What: nameof of a bare instance field, a this-qualified field, and a parameter inside an + /// instance method folds to string literals. + /// + [Test] + public async Task Rewrite_NameofInInstanceMethod_FoldsToStringLiterals() + { + string onDisk = File.ReadAllText(ResolveHostPath()); + string edited = onDisk.Replace( + " public int ExistingCaller(int value)\n {\n return value;\n }", + " public int ExistingCaller(int value)\n {\n" + + " return nameof(_privateSeed).Length + nameof(this.PublicSeed).Length" + + " + nameof(value).Length + value;\n" + + " }", + StringComparison.Ordinal); + string sourcePath = WriteEdited("NameofInInstanceMethod.cs", edited); + + TransformWorkerClientResult result = await RunWorkerOnSourceAsync( + sourcePath, + HostProjectRelativePath, + snapshotSource: onDisk); + Assert.That(result.Success, Is.True, result.ErrorMessage); + + TransformWorkerEntryDto caller = FindEntry(result, nameof(HotReloadAddedMemberHost.ExistingCaller)); + Assert.That(caller, Is.Not.Null, "Skipped=" + FormatSkipped(result.Output.skipped)); + string slice = SliceShimMethod(result.Output.shimSource, caller.shimMethodName); + Assert.That(slice, Does.Contain("\"_privateSeed\"")); + Assert.That(slice, Does.Contain("\"PublicSeed\"")); + Assert.That(slice, Does.Contain("\"value\"")); + Assert.That(slice, Does.Not.Contain("nameof(")); + } + + /// + /// What: nameof of a name that does not bind stays a nameof expression in the shim instead + /// of folding to a name the original code never compiled with. + /// + [Test] + public async Task Rewrite_NameofUnboundName_IsNotFolded() + { + string onDisk = File.ReadAllText(ResolveHostPath()); + string edited = onDisk.Replace( + " public int ExistingCaller(int value)\n {\n return value;\n }", + " public int ExistingCaller(int value)\n {\n return nameof(NoSuchName).Length + value;\n }", + StringComparison.Ordinal); + string sourcePath = WriteEdited("NameofUnboundName.cs", edited); + + TransformWorkerClientResult result = await RunWorkerOnSourceAsync( + sourcePath, + HostProjectRelativePath, + snapshotSource: onDisk); + Assert.That(result.Success, Is.True, result.ErrorMessage); + + TransformWorkerEntryDto caller = FindEntry(result, nameof(HotReloadAddedMemberHost.ExistingCaller)); + Assert.That(caller, Is.Not.Null, "Skipped=" + FormatSkipped(result.Output.skipped)); + string slice = SliceShimMethod(result.Output.shimSource, caller.shimMethodName); + Assert.That(slice, Does.Contain("nameof(NoSuchName)")); + } + + /// + /// What: nameof of a generic type whose type argument does not bind stays a nameof + /// expression in the shim even though the outer type name itself binds. + /// + [Test] + public async Task Rewrite_NameofWithUnboundTypeArgument_IsNotFolded() + { + string onDisk = File.ReadAllText(ResolveHostPath()); + string edited = onDisk.Replace( + " public int ExistingCaller(int value)\n {\n return value;\n }", + " public int ExistingCaller(int value)\n {\n" + + " return nameof(System.Collections.Generic.List).Length + value;\n" + + " }", + StringComparison.Ordinal); + string sourcePath = WriteEdited("NameofUnboundTypeArgument.cs", edited); + + TransformWorkerClientResult result = await RunWorkerOnSourceAsync( + sourcePath, + HostProjectRelativePath, + snapshotSource: onDisk); + Assert.That(result.Success, Is.True, result.ErrorMessage); + + TransformWorkerEntryDto caller = FindEntry(result, nameof(HotReloadAddedMemberHost.ExistingCaller)); + Assert.That(caller, Is.Not.Null, "Skipped=" + FormatSkipped(result.Output.skipped)); + string slice = SliceShimMethod(result.Output.shimSource, caller.shimMethodName); + Assert.That(slice, Does.Contain("nameof(")); + Assert.That(slice, Does.Not.Contain("\"List\"")); + } + + /// + /// What: nameof of an added method that is skipped, and so never registered as added, + /// still folds to a string literal instead of naming a member the compiled type lacks. + /// + [Test] + public async Task Rewrite_NameofAddedMethodThatIsNotApplied_FoldsToStringLiteral() + { + string onDisk = File.ReadAllText(ResolveHostPath()); + string edited = WithHostMembers( + onDisk, + "public T AddedGenericProbe(T value)\n {\n return value;\n }"); + edited = edited.Replace( + " public int ExistingCaller(int value)\n {\n return value;\n }", + " public int ExistingCaller(int value)\n {\n" + + " return nameof(AddedGenericProbe).Length + value;\n" + + " }", + StringComparison.Ordinal); + string sourcePath = WriteEdited("NameofAddedMethodNotApplied.cs", edited); + + TransformWorkerClientResult result = await RunWorkerOnSourceAsync( + sourcePath, + HostProjectRelativePath, + snapshotSource: onDisk); + Assert.That(result.Success, Is.True, result.ErrorMessage); + + TransformWorkerEntryDto caller = FindEntry(result, nameof(HotReloadAddedMemberHost.ExistingCaller)); + Assert.That(caller, Is.Not.Null, "Skipped=" + FormatSkipped(result.Output.skipped)); + Assert.That(FindEntry(result, "AddedGenericProbe"), Is.Null); + string slice = SliceShimMethod(result.Output.shimSource, caller.shimMethodName); + Assert.That(slice, Does.Contain("\"AddedGenericProbe\"")); + Assert.That(slice, Does.Not.Contain("nameof(")); + } + + /// + /// What: a property pattern that names a compiled member of the target type keeps the + /// bare member name in the shim, since a pattern's member name cannot take a receiver. + /// + [Test] + public async Task Rewrite_PropertyPatternNamingCompiledMember_KeepsTheBareName() + { + string onDisk = File.ReadAllText(ResolveHostPath()); + string edited = onDisk.Replace( + " public int ExistingCaller(int value)\n {\n return value;\n }", + " public int ExistingCaller(int value)\n {\n return Inner is { PublicSeed: 3 } ? 1 : value;\n }", + StringComparison.Ordinal); + string sourcePath = WriteEdited("PropertyPatternCompiledMember.cs", edited); + + TransformWorkerClientResult result = await RunWorkerOnSourceAsync( + sourcePath, + HostProjectRelativePath, + snapshotSource: onDisk); + Assert.That(result.Success, Is.True, result.ErrorMessage); + + TransformWorkerEntryDto caller = FindEntry(result, nameof(HotReloadAddedMemberHost.ExistingCaller)); + Assert.That(caller, Is.Not.Null, "Skipped=" + FormatSkipped(result.Output.skipped)); + string slice = SliceShimMethod(result.Output.shimSource, caller.shimMethodName); + Assert.That(slice, Does.Contain("PublicSeed:")); + Assert.That(slice, Does.Not.Contain("__uloopInstance.PublicSeed")); + } + /// /// What: added virtual, override, generic, and method-group-capturing methods are skipped /// with the documented reasons; the captured added instance method itself still emits. diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/HarmonyAccessorShimRewrite.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/HarmonyAccessorShimRewrite.cs index 9d35fe1158..26dd76be8b 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/HarmonyAccessorShimRewrite.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/HarmonyAccessorShimRewrite.cs @@ -300,4 +300,13 @@ internal static bool IsObjectOrCollectionInitializerMemberName(SimpleNameSyntax return assignment.Parent is InitializerExpressionSyntax; } + + // `x is { Member: 1 }` names a member of the matched value: the name is not an expression, + // so it cannot take a receiver. + internal static bool IsSubpatternMemberName(SimpleNameSyntax node) + { + return node.Parent is NameColonSyntax nameColon + && nameColon.Name == node + && nameColon.Parent is SubpatternSyntax; + } } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/NameofRules.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/NameofRules.cs index 8252eed776..61603c07dc 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/NameofRules.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/NameofRules.cs @@ -40,4 +40,24 @@ public static bool IsInsideNameofArgument(SyntaxNode node) return false; } + + // Returns the name a bound nameof evaluates to, or null when the nameof must not be folded. + public static string FindBoundNameofValueOrNull( + InvocationExpressionSyntax nameofInvocation, + SemanticModel semanticModel) + { + // Why the error check before the constant: an operand that does not bind, or binds only + // in part, can still yield a constant, and that value need not be the name the code + // would compile with. + foreach (Diagnostic diagnostic in semanticModel.GetDiagnostics(nameofInvocation.Span)) + { + if (diagnostic.Severity == DiagnosticSeverity.Error) + { + return null; + } + } + + Optional constant = semanticModel.GetConstantValue(nameofInvocation); + return constant.HasValue ? constant.Value as string : null; + } } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimBodyRewriter.cs b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimBodyRewriter.cs index 46209b44df..ff3872c016 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimBodyRewriter.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/TransformWorker~/ShimBodyRewriter.cs @@ -145,6 +145,18 @@ public override SyntaxNode VisitInvocationExpression(InvocationExpressionSyntax return folded; } + // Why fold instead of rewriting the operand: the shim is a static method of another + // type, so nothing needs the operand rebound there, and a rewritten operand + // (__uloopInstance.x) names a parameter a static method's shim does not have. + string boundName = NameofRules.FindBoundNameofValueOrNull(node, _semanticModel); + if (boundName != null) + { + return SyntaxFactory.LiteralExpression( + SyntaxKind.StringLiteralExpression, + SyntaxFactory.Literal(boundName)) + .WithTriviaFrom(node); + } + return base.VisitInvocationExpression(node); } @@ -506,6 +518,14 @@ private SyntaxNode VisitName(SimpleNameSyntax node, SyntaxNode original) return original; } + // Why only before the final qualification, not with the name-side checks at the top: + // returning there would also skip the accessor-read and added-field rewrites above, and + // a pattern naming a member the shim cannot reach could then compile and fail at run time. + if (HarmonyAccessorShimRewrite.IsSubpatternMemberName(node)) + { + return original; + } + return HarmonyAccessorShimRewrite.QualifyOwnedMemberAccess(node, ownership.isStatic, ownership.containingType); }