diff --git a/Assets/Tests/Editor/ThirdPartyToolMigrationAsmdefTextEditorTests.cs b/Assets/Tests/Editor/ThirdPartyToolMigrationAsmdefTextEditorTests.cs new file mode 100644 index 000000000..c9ac97c08 --- /dev/null +++ b/Assets/Tests/Editor/ThirdPartyToolMigrationAsmdefTextEditorTests.cs @@ -0,0 +1,177 @@ +using System; +using System.Text; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Domain; +using io.github.hatayama.UnityCliLoop.Infrastructure; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Test fixture that verifies surgical asmdef references edits preserve surrounding bytes. + /// + public sealed class ThirdPartyToolMigrationAsmdefTextEditorTests + { + [Test] + public void ReplaceReferencesArray_WhenSourceUsesCrlfAndFourSpaceIndent_PreservesBytesOutsideArray() + { + // Verifies CRLF/4-space asmdef text keeps every byte outside the references array. + string source = + "{\r\n" + + " \"name\": \"MyCompany.Tools.Editor\",\r\n" + + " \"references\": [\r\n" + + " \"uLoopMCP.Editor\"\r\n" + + " ],\r\n" + + " \"includePlatforms\": [\r\n" + + " \"Editor\"\r\n" + + " ]\r\n" + + "}\r\n"; + string expected = + "{\r\n" + + " \"name\": \"MyCompany.Tools.Editor\",\r\n" + + " \"references\": [\r\n" + + " \"GUID:fc3fd32eddbee40e39c2d76dc184957b\"\r\n" + + " ],\r\n" + + " \"includePlatforms\": [\r\n" + + " \"Editor\"\r\n" + + " ]\r\n" + + "}\r\n"; + + ThirdPartyToolMigrationAsmdefTextEditor.AsmdefReferencesEditResult result = + ThirdPartyToolMigrationAsmdefTextEditor.ReplaceReferencesArray( + source, + new[] { "GUID:fc3fd32eddbee40e39c2d76dc184957b" }); + + Assert.That(result.Replaced, Is.True); + Assert.That(result.Content, Is.EqualTo(expected)); + } + + [Test] + public void ReplaceReferencesArray_WhenSourceUsesTwoSpaceIndentAndLf_MatchesSourceStyle() + { + // Verifies 2-space LF asmdefs render migrated elements with matching indentation. + string source = + "{\n" + + " \"name\": \"MyCompany.Tools.Editor\",\n" + + " \"references\": [\n" + + " \"Old\"\n" + + " ]\n" + + "}\n"; + + ThirdPartyToolMigrationAsmdefTextEditor.AsmdefReferencesEditResult result = + ThirdPartyToolMigrationAsmdefTextEditor.ReplaceReferencesArray( + source, + new[] { "New1", "New2" }); + + Assert.That(result.Replaced, Is.True); + Assert.That(result.Content, Does.Contain(" \"references\": [\n \"New1\",\n \"New2\"\n ]")); + Assert.That(result.Content, Does.Not.Contain("\r\n")); + } + + [Test] + public void ReplaceReferencesArray_WhenArrayIsSingleLine_KeepsSingleLineFormat() + { + // Verifies a single-line references array stays single-line after replacement. + string source = "{\"name\":\"MyCompany.Tools.Editor\",\"references\": [\"Old\"]}"; + + ThirdPartyToolMigrationAsmdefTextEditor.AsmdefReferencesEditResult result = + ThirdPartyToolMigrationAsmdefTextEditor.ReplaceReferencesArray( + source, + new[] { "New1", "New2" }); + + Assert.That(result.Replaced, Is.True); + Assert.That(result.Content, Is.EqualTo( + "{\"name\":\"MyCompany.Tools.Editor\",\"references\": [\"New1\", \"New2\"]}")); + } + + [Test] + public void ReplaceReferencesArray_WhenStringValueEqualsReferences_IgnoresDecoyValue() + { + // Verifies decoy string values and escaped text do not steal the references array match. + string source = + "{\n" + + " \"name\": \"references\",\n" + + " \"note\": \"\\\"references\\\":\",\n" + + " \"references\": [\"Old\"]\n" + + "}\n"; + + ThirdPartyToolMigrationAsmdefTextEditor.AsmdefReferencesEditResult result = + ThirdPartyToolMigrationAsmdefTextEditor.ReplaceReferencesArray( + source, + new[] { "New" }); + + Assert.That(result.Replaced, Is.True); + Assert.That(result.Content, Does.Contain("\"name\": \"references\"")); + Assert.That(result.Content, Does.Contain("\"note\": \"\\\"references\\\":\"")); + Assert.That(result.Content, Does.Contain("\"references\": [\"New\"]")); + } + + [Test] + public void ReplaceReferencesArray_WhenReferencesPropertyIsMissing_ReturnsNotReplaced() + { + // Verifies asmdefs without a references property report NotReplaced. + string source = + "{\n" + + " \"name\": \"MyCompany.Tools.Editor\"\n" + + "}\n"; + + ThirdPartyToolMigrationAsmdefTextEditor.AsmdefReferencesEditResult result = + ThirdPartyToolMigrationAsmdefTextEditor.ReplaceReferencesArray( + source, + new[] { "New" }); + + Assert.That(result.Replaced, Is.False); + Assert.That(result.Content, Is.EqualTo(string.Empty)); + } + + [Test] + public void MigrateAsmdefSource_WhenReferencesPropertyIsMissing_FallsBackToReserialization() + { + // Verifies missing references still gains required entries through full re-serialization. + string source = + "{\n" + + " \"name\": \"MyCompany.Tools.Editor\"\n" + + "}"; + + ThirdPartyToolMigrationContentResult result = + ThirdPartyToolMigrationRules.MigrateAsmdefSource( + source, + hasLegacyCSharpSource: true, + requiresToolContractsReference: false, + requiresApplicationReference: false, + requiresDomainReference: false, + requiresFirstPartyScreenshotReference: false); + + Assert.That(result.Changed, Is.True); + Assert.That(result.Content, Does.Contain("\"references\": [")); + Assert.That(result.Content, Does.Contain("GUID:fc3fd32eddbee40e39c2d76dc184957b")); + } + + [Test] + public void MigrateAsmdefSource_WhenNothingChanges_ReturnsSourceUnchanged() + { + // Verifies the early-return path leaves the original asmdef text untouched. + string source = + "{\r\n" + + " \"name\": \"MyCompany.Tools.Editor\",\r\n" + + " \"references\": [\r\n" + + " \"GUID:fc3fd32eddbee40e39c2d76dc184957b\"\r\n" + + " ]\r\n" + + "}\r\n"; + + ThirdPartyToolMigrationContentResult result = + ThirdPartyToolMigrationRules.MigrateAsmdefSource( + source, + hasLegacyCSharpSource: false, + requiresToolContractsReference: false, + requiresApplicationReference: false, + requiresDomainReference: false, + requiresFirstPartyScreenshotReference: false); + + Assert.That(result.Changed, Is.False); + Assert.That(result.Content, Is.EqualTo(source)); + Assert.That(Encoding.UTF8.GetBytes(result.Content), Is.EqualTo(Encoding.UTF8.GetBytes(source))); + } + } +} diff --git a/Assets/Tests/Editor/ThirdPartyToolMigrationAsmdefTextEditorTests.cs.meta b/Assets/Tests/Editor/ThirdPartyToolMigrationAsmdefTextEditorTests.cs.meta new file mode 100644 index 000000000..914ff22fd --- /dev/null +++ b/Assets/Tests/Editor/ThirdPartyToolMigrationAsmdefTextEditorTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: e62e3d40a673d4e338e77cde3e7e3aa6 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/ThirdPartyToolMigrationCodeTextMaskBuilderTests.cs b/Assets/Tests/Editor/ThirdPartyToolMigrationCodeTextMaskBuilderTests.cs new file mode 100644 index 000000000..be4772093 --- /dev/null +++ b/Assets/Tests/Editor/ThirdPartyToolMigrationCodeTextMaskBuilderTests.cs @@ -0,0 +1,78 @@ +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Domain; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Test fixture that verifies migration code masking ignores preprocessor text and bounds char literals. + /// + public sealed class ThirdPartyToolMigrationCodeTextMaskBuilderTests + { + [Test] + public void CreateCodeCharacters_WhenRegionNameContainsApostrophe_KeepsFollowingCodeUnmasked() + { + // Verifies a #region title apostrophe does not mask the rest of the file as a char literal. + string source = "#region Bob's helpers\npublic class Tool {}\n"; + + bool[] codeCharacters = ThirdPartyToolMigrationCodeTextMaskBuilder.CreateCodeCharacters(source); + + int codeStartIndex = source.IndexOf("public class Tool"); + Assert.That(codeStartIndex, Is.GreaterThanOrEqualTo(0)); + Assert.That(AreAllCodeCharacters(codeCharacters, codeStartIndex, "public class Tool".Length), Is.True); + } + + [Test] + public void CreateCodeCharacters_WhenWarningContainsApostrophe_KeepsFollowingCodeUnmasked() + { + // Verifies a #warning apostrophe does not mask the following source as a char literal. + string source = "#warning Don't call this API\npublic class Tool {}\n"; + + bool[] codeCharacters = ThirdPartyToolMigrationCodeTextMaskBuilder.CreateCodeCharacters(source); + + int codeStartIndex = source.IndexOf("public class Tool"); + Assert.That(codeStartIndex, Is.GreaterThanOrEqualTo(0)); + Assert.That(AreAllCodeCharacters(codeCharacters, codeStartIndex, "public class Tool".Length), Is.True); + } + + [Test] + public void CreateCodeCharacters_WhenValidCharLiteral_MasksOnlyTheLiteral() + { + // Verifies a real char literal stays masked while the following statement remains code. + string source = "char c = 'a';\nint x = 1;\n"; + + bool[] codeCharacters = ThirdPartyToolMigrationCodeTextMaskBuilder.CreateCodeCharacters(source); + + int literalIndex = source.IndexOf("'a'"); + Assert.That(codeCharacters[literalIndex], Is.False); + Assert.That(codeCharacters[literalIndex + 1], Is.False); + Assert.That(codeCharacters[literalIndex + 2], Is.False); + Assert.That(codeCharacters[source.IndexOf("int x")], Is.True); + } + + [Test] + public void CreateCodeCharacters_WhenApostropheDoesNotCloseSoon_TreatsItAsCode() + { + // Verifies a long unmatched apostrophe is not treated as an open-ended char literal. + string source = "var name = Bob's helpers;\nint x = 1;\n"; + + bool[] codeCharacters = ThirdPartyToolMigrationCodeTextMaskBuilder.CreateCodeCharacters(source); + + Assert.That(codeCharacters[source.IndexOf("int x")], Is.True); + Assert.That(AreAllCodeCharacters(codeCharacters, source.IndexOf("helpers"), "helpers".Length), Is.True); + } + + private static bool AreAllCodeCharacters(bool[] codeCharacters, int startIndex, int length) + { + for (int index = startIndex; index < startIndex + length; index++) + { + if (!codeCharacters[index]) + { + return false; + } + } + + return true; + } + } +} diff --git a/Assets/Tests/Editor/ThirdPartyToolMigrationCodeTextMaskBuilderTests.cs.meta b/Assets/Tests/Editor/ThirdPartyToolMigrationCodeTextMaskBuilderTests.cs.meta new file mode 100644 index 000000000..6b3541205 --- /dev/null +++ b/Assets/Tests/Editor/ThirdPartyToolMigrationCodeTextMaskBuilderTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: ca68b733dc7204b64827610344755ef4 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/ThirdPartyToolMigrationFileWriterTests.cs b/Assets/Tests/Editor/ThirdPartyToolMigrationFileWriterTests.cs new file mode 100644 index 000000000..ab82be3df --- /dev/null +++ b/Assets/Tests/Editor/ThirdPartyToolMigrationFileWriterTests.cs @@ -0,0 +1,311 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Infrastructure; + +using static io.github.hatayama.UnityCliLoop.Infrastructure.ThirdPartyToolMigrationFileServiceConstants; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Test fixture that verifies batch migration writes commit atomically or roll back. + /// + public sealed class ThirdPartyToolMigrationFileWriterTests + { + [Test] + public void WriteBatch_WhenAllTargetsAreWritable_CommitsEveryFileAndLeavesNoSidecars() + { + // Verifies a mixed existing/new batch commits all targets and leaves no sidecar files. + string tempDirectory = CreateTempDirectory(); + try + { + string existingFile1 = Path.Combine(tempDirectory, "ExistingOne.cs"); + string existingFile2 = Path.Combine(tempDirectory, "ExistingTwo.cs"); + string newFile = Path.Combine(tempDirectory, "NewFile.cs"); + File.WriteAllText(existingFile1, "original-one"); + File.WriteAllText(existingFile2, "original-two"); + + List changes = new() + { + new MigrationFileChange(existingFile1, "migrated-one"), + new MigrationFileChange(existingFile2, "migrated-two"), + new MigrationFileChange(newFile, "migrated-new") + }; + + ThirdPartyToolMigrationFileWriter.WriteBatch(changes); + + Assert.That(File.ReadAllText(existingFile1), Is.EqualTo("migrated-one")); + Assert.That(File.ReadAllText(existingFile2), Is.EqualTo("migrated-two")); + Assert.That(File.ReadAllText(newFile), Is.EqualTo("migrated-new")); + Assert.That(CountSidecarFiles(tempDirectory), Is.EqualTo(0)); + } + finally + { + Directory.Delete(tempDirectory, recursive: true); + } + } + + [Test] + public void WriteBatch_WhenPrepareFails_LeavesAllTargetFilesUntouched() + { + // Verifies a prepare failure leaves every target untouched and removes temp sidecars. + string tempDirectory = CreateTempDirectory(); + try + { + string existingFile = Path.Combine(tempDirectory, "Existing.cs"); + File.WriteAllText(existingFile, "original"); + string missingDirectoryFile = Path.Combine( + tempDirectory, + "missing-directory", + "Missing.cs"); + + List changes = new() + { + new MigrationFileChange(existingFile, "migrated"), + new MigrationFileChange(missingDirectoryFile, "never-written") + }; + + Assert.Throws( + () => ThirdPartyToolMigrationFileWriter.WriteBatch(changes)); + + Assert.That(File.ReadAllText(existingFile), Is.EqualTo("original")); + Assert.That(CountSidecarFiles(tempDirectory), Is.EqualTo(0)); + } + finally + { + Directory.Delete(tempDirectory, recursive: true); + } + } + + [Test] + public void WriteBatch_WhenCommitFails_RestoresCommittedFilesFromBackups() + { + // Verifies a mid-batch commit failure restores already committed files from backups. + string tempDirectory = CreateTempDirectory(); + try + { + string file1 = Path.Combine(tempDirectory, "FileOne.cs"); + string file2 = Path.Combine(tempDirectory, "FileTwo.cs"); + string file3 = Path.Combine(tempDirectory, "FileThree.cs"); + File.WriteAllText(file1, "original-one"); + File.WriteAllText(file3, "original-three"); + Directory.CreateDirectory(file2); + + List changes = new() + { + new MigrationFileChange(file1, "migrated-one"), + new MigrationFileChange(file2, "migrated-two"), + new MigrationFileChange(file3, "migrated-three") + }; + + Assert.Throws( + () => ThirdPartyToolMigrationFileWriter.WriteBatch(changes)); + + Assert.That(File.ReadAllText(file1), Is.EqualTo("original-one")); + Assert.That(File.ReadAllText(file3), Is.EqualTo("original-three")); + Assert.That(CountSidecarFiles(tempDirectory), Is.EqualTo(0)); + } + finally + { + Directory.Delete(tempDirectory, recursive: true); + } + } + + [Test] + public void WriteBatch_WhenCommitFails_DeletesNewlyCreatedFiles() + { + // Verifies rollback removes newly created targets that had no backup sidecar. + string tempDirectory = CreateTempDirectory(); + try + { + string newFile = Path.Combine(tempDirectory, "NewFile.cs"); + string failingTarget = Path.Combine(tempDirectory, "FailingTarget.cs"); + Directory.CreateDirectory(failingTarget); + + List changes = new() + { + new MigrationFileChange(newFile, "migrated-new"), + new MigrationFileChange(failingTarget, "never-committed") + }; + + Assert.Throws( + () => ThirdPartyToolMigrationFileWriter.WriteBatch(changes)); + + Assert.That(File.Exists(newFile), Is.False); + Assert.That(CountSidecarFiles(tempDirectory), Is.EqualTo(0)); + } + finally + { + Directory.Delete(tempDirectory, recursive: true); + } + } + + [Test] + public async Task WriteBatchAsync_WhenBatchExceedsYieldSize_CommitsEveryFile() + { + // Verifies the async prepare yield path still commits every file in a large batch. + string tempDirectory = CreateTempDirectory(); + try + { + List changes = new(); + for (int index = 0; index < PreviewYieldBatchSize + 8; index++) + { + string filePath = Path.Combine(tempDirectory, $"File{index:D2}.cs"); + File.WriteAllText(filePath, $"original-{index}"); + changes.Add(new MigrationFileChange(filePath, $"migrated-{index}")); + } + + await ThirdPartyToolMigrationFileWriter.WriteBatchAsync(changes); + + for (int index = 0; index < changes.Count; index++) + { + Assert.That( + File.ReadAllText(changes[index].FilePath), + Is.EqualTo($"migrated-{index}")); + } + + Assert.That(CountSidecarFiles(tempDirectory), Is.EqualTo(0)); + } + finally + { + Directory.Delete(tempDirectory, recursive: true); + } + } + + [Test] + public void WriteBatch_WhenTargetHasUtf8Bom_PreservesBomBytes() + { + // Verifies a UTF-8 BOM target keeps EF BB BF after a batch write. + string tempDirectory = CreateTempDirectory(); + try + { + string filePath = Path.Combine(tempDirectory, "BomFile.cs"); + File.WriteAllText(filePath, "original", new UTF8Encoding(encoderShouldEmitUTF8Identifier: true)); + + ThirdPartyToolMigrationFileWriter.WriteBatch( + new List + { + new MigrationFileChange(filePath, "migrated") + }); + + byte[] bytes = File.ReadAllBytes(filePath); + Assert.That(bytes[0], Is.EqualTo(0xEF)); + Assert.That(bytes[1], Is.EqualTo(0xBB)); + Assert.That(bytes[2], Is.EqualTo(0xBF)); + Assert.That(Encoding.UTF8.GetString(bytes, 3, bytes.Length - 3), Is.EqualTo("migrated")); + } + finally + { + Directory.Delete(tempDirectory, recursive: true); + } + } + + [Test] + public void WriteBatch_WhenTargetIsUtf16LittleEndian_PreservesEncoding() + { + // Verifies a UTF-16 LE BOM target is rewritten as UTF-16 LE with the new content. + string tempDirectory = CreateTempDirectory(); + try + { + string filePath = Path.Combine(tempDirectory, "Utf16File.cs"); + Encoding utf16 = new UnicodeEncoding(bigEndian: false, byteOrderMark: true); + File.WriteAllText(filePath, "original", utf16); + + ThirdPartyToolMigrationFileWriter.WriteBatch( + new List + { + new MigrationFileChange(filePath, "migrated") + }); + + byte[] bytes = File.ReadAllBytes(filePath); + Assert.That(bytes[0], Is.EqualTo(0xFF)); + Assert.That(bytes[1], Is.EqualTo(0xFE)); + Assert.That(utf16.GetString(bytes, 2, bytes.Length - 2), Is.EqualTo("migrated")); + } + finally + { + Directory.Delete(tempDirectory, recursive: true); + } + } + + [Test] + public void WriteBatch_WhenTargetHasNoBom_DoesNotAddBom() + { + // Verifies a BOM-less UTF-8 target does not gain EF BB BF after a batch write. + string tempDirectory = CreateTempDirectory(); + try + { + string filePath = Path.Combine(tempDirectory, "NoBomFile.cs"); + File.WriteAllText(filePath, "original", new UTF8Encoding(encoderShouldEmitUTF8Identifier: false)); + + ThirdPartyToolMigrationFileWriter.WriteBatch( + new List + { + new MigrationFileChange(filePath, "migrated") + }); + + byte[] bytes = File.ReadAllBytes(filePath); + Assert.That(bytes.Length, Is.GreaterThanOrEqualTo(1)); + Assert.That(bytes[0], Is.Not.EqualTo(0xEF)); + Assert.That(Encoding.UTF8.GetString(bytes), Is.EqualTo("migrated")); + } + finally + { + Directory.Delete(tempDirectory, recursive: true); + } + } + + [Test] + public void WriteBatch_WhenContentUsesCrlf_PreservesCrlfBytes() + { + // Verifies CRLF content bytes survive the batch write without lone LF rewriting. + string tempDirectory = CreateTempDirectory(); + try + { + string filePath = Path.Combine(tempDirectory, "CrlfFile.cs"); + File.WriteAllText(filePath, "original\r\nline", new UTF8Encoding(false)); + string migratedContent = "migrated\r\nline\r\n"; + + ThirdPartyToolMigrationFileWriter.WriteBatch( + new List + { + new MigrationFileChange(filePath, migratedContent) + }); + + byte[] bytes = File.ReadAllBytes(filePath); + string decoded = Encoding.UTF8.GetString(bytes); + Assert.That(decoded, Is.EqualTo(migratedContent)); + Assert.That(decoded.Contains("\r\n"), Is.True); + Assert.That(decoded.Replace("\r\n", string.Empty).Contains("\n"), Is.False); + } + finally + { + Directory.Delete(tempDirectory, recursive: true); + } + } + + private static string CreateTempDirectory() + { + string tempDirectory = Path.Combine( + Path.GetTempPath(), + "UnityCliLoopMigrationWriterTests", + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(tempDirectory); + return tempDirectory; + } + + private static int CountSidecarFiles(string directory) + { + return Directory.EnumerateFiles(directory, "*", SearchOption.AllDirectories) + .Count(filePath => + filePath.EndsWith(".tmp", StringComparison.OrdinalIgnoreCase) || + filePath.EndsWith(".bak", StringComparison.OrdinalIgnoreCase)); + } + } +} diff --git a/Assets/Tests/Editor/ThirdPartyToolMigrationFileWriterTests.cs.meta b/Assets/Tests/Editor/ThirdPartyToolMigrationFileWriterTests.cs.meta new file mode 100644 index 000000000..e1c031c44 --- /dev/null +++ b/Assets/Tests/Editor/ThirdPartyToolMigrationFileWriterTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 1e2826ae14e624b0d81d3bf76a647d5b +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/ThirdPartyToolMigrationLocalTypeShadowingTests.cs b/Assets/Tests/Editor/ThirdPartyToolMigrationLocalTypeShadowingTests.cs new file mode 100644 index 000000000..f9488f413 --- /dev/null +++ b/Assets/Tests/Editor/ThirdPartyToolMigrationLocalTypeShadowingTests.cs @@ -0,0 +1,135 @@ +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Domain; +using io.github.hatayama.UnityCliLoop.Infrastructure; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Test fixture that verifies local type shadowing is not rewritten by contract/domain/toolinfo renames. + /// + public sealed class ThirdPartyToolMigrationLocalTypeShadowingTests + { + [Test] + public void MigrateCSharpSource_WhenProjectDefinesSecuritySettings_KeepsProjectTypeUsages() + { + // Verifies a project-owned SecuritySettings type is not rewritten to the V3 security setting type. + string source = @"using io.github.hatayama.uLoopMCP; + +public sealed class SecuritySettings +{ + public bool Enabled { get; set; } +} + +public sealed class SettingsFactory +{ + public SecuritySettings Create() + { + return new SecuritySettings(); + } +}"; + + ThirdPartyToolMigrationContentResult result = + ThirdPartyToolMigrationRules.MigrateCSharpSource(source); + + Assert.That(result.Content, Does.Contain("public sealed class SecuritySettings")); + Assert.That(result.Content, Does.Contain("public SecuritySettings Create()")); + Assert.That(result.Content, Does.Contain("return new SecuritySettings();")); + Assert.That(result.Content, Does.Not.Contain("UnityCliLoopSecuritySetting")); + } + + [Test] + public void MigrateCSharpSource_WhenProjectDefinesServiceResult_KeepsProjectTypeUsages() + { + // Verifies a project-owned ServiceResult type is not rewritten to the ToolContracts type. + string source = @"using io.github.hatayama.uLoopMCP; + +public sealed class ServiceResult +{ + public bool Ok { get; set; } +} + +public sealed class ResultFactory +{ + public ServiceResult Create() + { + return new ServiceResult(); + } +}"; + + ThirdPartyToolMigrationContentResult result = + ThirdPartyToolMigrationRules.MigrateCSharpSource(source); + + Assert.That(result.Content, Does.Contain("public sealed class ServiceResult")); + Assert.That(result.Content, Does.Contain("public ServiceResult Create()")); + Assert.That(result.Content, Does.Contain("return new ServiceResult();")); + Assert.That( + result.Content, + Does.Not.Contain("io.github.hatayama.UnityCliLoop.ToolContracts.ServiceResult")); + } + + [Test] + public void MigrateCSharpSource_WhenProjectDefinesToolInfo_KeepsProjectTypeUsages() + { + // Verifies a project-owned ToolInfo type is not rewritten to the ToolContracts type. + string source = @"using io.github.hatayama.uLoopMCP; + +public sealed class ToolInfo +{ + public string Name { get; set; } +} + +public sealed class ToolInfoFactory +{ + public ToolInfo Create() + { + return new ToolInfo(); + } +}"; + + ThirdPartyToolMigrationContentResult result = + ThirdPartyToolMigrationRules.MigrateCSharpSource(source); + + Assert.That(result.Content, Does.Contain("public sealed class ToolInfo")); + Assert.That(result.Content, Does.Contain("public ToolInfo Create()")); + Assert.That(result.Content, Does.Contain("return new ToolInfo();")); + Assert.That( + result.Content, + Does.Not.Contain("io.github.hatayama.UnityCliLoop.ToolContracts.ToolInfo")); + } + + [Test] + public void MigrateCSharpSourceForLegacyAssembly_WhenAssemblyDeclaresSecuritySettings_KeepsProjectTypeUsages() + { + // Verifies sibling-file ownership of SecuritySettings also blocks bare reference rewrites. + string source = @"using io.github.hatayama.uLoopMCP; + +public sealed class SettingsFactory +{ + public SecuritySettings Create() + { + return new SecuritySettings(); + } +}"; + + ThirdPartyToolMigrationContentResult result = + ThirdPartyToolMigrationRules.MigrateCSharpSourceForLegacyAssembly( + source, + hasLegacyAssemblySource: true, + hasAssemblyScopedCurrentToolContractsUsing: false, + hasAssemblyScopedCurrentApplicationUsing: false, + hasAssemblyScopedCurrentDomainUsing: false, + hasAssemblyScopedCurrentFirstPartyToolsUsing: false, + legacyAssemblyAliases: System.Array.Empty(), + legacyAssemblyToolInfoAliases: System.Array.Empty(), + currentApplicationAssemblyAliases: System.Array.Empty(), + currentDomainAssemblyAliases: System.Array.Empty(), + currentFirstPartyToolsAssemblyAliases: System.Array.Empty(), + assemblyDeclaredTypeNames: new[] { "SecuritySettings" }); + + Assert.That(result.Content, Does.Contain("public SecuritySettings Create()")); + Assert.That(result.Content, Does.Contain("return new SecuritySettings();")); + Assert.That(result.Content, Does.Not.Contain("UnityCliLoopSecuritySetting")); + } + } +} diff --git a/Assets/Tests/Editor/ThirdPartyToolMigrationLocalTypeShadowingTests.cs.meta b/Assets/Tests/Editor/ThirdPartyToolMigrationLocalTypeShadowingTests.cs.meta new file mode 100644 index 000000000..93dbeadf7 --- /dev/null +++ b/Assets/Tests/Editor/ThirdPartyToolMigrationLocalTypeShadowingTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: e8b72c13a78b849ab9ab7388b1658646 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/ThirdPartyToolMigrationProjectFileInventoryTests.cs b/Assets/Tests/Editor/ThirdPartyToolMigrationProjectFileInventoryTests.cs new file mode 100644 index 000000000..09cae48f8 --- /dev/null +++ b/Assets/Tests/Editor/ThirdPartyToolMigrationProjectFileInventoryTests.cs @@ -0,0 +1,69 @@ +using System; +using System.IO; +using System.Runtime.InteropServices; + +using NUnit.Framework; + +using io.github.hatayama.UnityCliLoop.Infrastructure; + +namespace io.github.hatayama.UnityCliLoop.Tests.Editor +{ + /// + /// Verifies migration project file inventory symlink/junction cycle guards. + /// + public sealed class ThirdPartyToolMigrationProjectFileInventoryTests + { + [Test] + public void Create_WhenAssetsContainsSelfReferentialSymbolicLink_CompletesWithoutLooping() + { + // Verifies a real directory symlink cycle is skipped and the inventory walk completes. + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + { + Assert.Ignore("Directory symlink creation requires elevated privileges on Windows."); + } + + string projectRoot = CreateProjectRoot(); + try + { + string assetsDirectory = Path.Combine(projectRoot, "Assets"); + string vendorDirectory = Path.Combine(assetsDirectory, "VendorTools"); + string cycleLinkDirectory = Path.Combine(vendorDirectory, "Cycle"); + Directory.CreateDirectory(vendorDirectory); + string includedPath = Path.Combine(vendorDirectory, "IncludedTool.cs"); + File.WriteAllText(includedPath, "public sealed class IncludedTool {}"); + CreateDirectorySymbolicLink(cycleLinkDirectory, vendorDirectory); + + ProjectFileInventory inventory = ProjectFileInventory.Create(projectRoot); + + Assert.That(inventory.CSharpFilePaths, Does.Contain(includedPath)); + Assert.That( + inventory.CSharpFilePaths, + Has.None.Matches(path => path.IndexOf("Cycle", StringComparison.Ordinal) >= 0)); + } + finally + { + Directory.Delete(projectRoot, recursive: true); + } + } + + private static string CreateProjectRoot() + { + string projectRoot = Path.Combine( + Path.GetTempPath(), + "UnityCliLoopMigrationInventoryTests", + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(Path.Combine(projectRoot, "Assets")); + Directory.CreateDirectory(Path.Combine(projectRoot, "ProjectSettings")); + return projectRoot; + } + + private static void CreateDirectorySymbolicLink(string linkPath, string targetPath) + { + int result = Symlink(targetPath, linkPath); + Assert.That(result, Is.EqualTo(0), $"symlink failed for {linkPath} -> {targetPath}"); + } + + [DllImport("libc", EntryPoint = "symlink", SetLastError = true)] + private static extern int Symlink(string targetPath, string linkPath); + } +} diff --git a/Assets/Tests/Editor/ThirdPartyToolMigrationProjectFileInventoryTests.cs.meta b/Assets/Tests/Editor/ThirdPartyToolMigrationProjectFileInventoryTests.cs.meta new file mode 100644 index 000000000..a053c1989 --- /dev/null +++ b/Assets/Tests/Editor/ThirdPartyToolMigrationProjectFileInventoryTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 25c7741ecc6534e7aa0beb64ecbabcc5 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Assets/Tests/Editor/ThirdPartyToolMigrationRulesTests.cs b/Assets/Tests/Editor/ThirdPartyToolMigrationRulesTests.cs index 0fe67abad..ec4db6d06 100644 --- a/Assets/Tests/Editor/ThirdPartyToolMigrationRulesTests.cs +++ b/Assets/Tests/Editor/ThirdPartyToolMigrationRulesTests.cs @@ -4522,5 +4522,26 @@ public void GetExcludedDirectoryNames_IncludesUnityGeneratedDirectories() Assert.That(names.Contains("Temp"), Is.True); Assert.That(names.Contains(".git"), Is.True); } + + [Test] + public void IsExcludedDirectoryName_WhenDirectoryEndsWithTilde_ReturnsTrue() + { + // Verifies Unity trailing-~ folders such as Samples~ are excluded from migration scans. + Assert.That(ThirdPartyToolMigrationRules.IsExcludedDirectoryName("Samples~"), Is.True); + } + + [Test] + public void IsExcludedDirectoryName_WhenDirectoryStartsWithDot_ReturnsTrue() + { + // Verifies hidden/dot folders are excluded even when not listed by exact name. + Assert.That(ThirdPartyToolMigrationRules.IsExcludedDirectoryName(".hidden"), Is.True); + } + + [Test] + public void IsExcludedDirectoryName_WhenDirectoryIsOrdinaryAssetFolder_ReturnsFalse() + { + // Verifies normal Assets subfolders remain eligible for migration scanning. + Assert.That(ThirdPartyToolMigrationRules.IsExcludedDirectoryName("VendorTools"), Is.False); + } } } diff --git a/Assets/Tests/Editor/ThirdPartyToolMigrationWizardWindowTests.cs b/Assets/Tests/Editor/ThirdPartyToolMigrationWizardWindowTests.cs index 4729e438f..a4c4e1897 100644 --- a/Assets/Tests/Editor/ThirdPartyToolMigrationWizardWindowTests.cs +++ b/Assets/Tests/Editor/ThirdPartyToolMigrationWizardWindowTests.cs @@ -16,21 +16,78 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor /// public sealed class ThirdPartyToolMigrationWizardWindowTests { - [TestCase(false, true, false)] - [TestCase(true, false, false)] - [TestCase(true, true, true)] - public void ShouldStartInitialRefresh_ReturnsExpectedValue( - bool shouldRefreshAfterCreateGui, - bool shouldAutoScanThirdPartyToolMigration, - bool expected) + [Test] + public void ConsumeShouldStartInitialRefresh_WhenPreparedForAutoScan_ReturnsTrueWithoutSessionFlag() + { + // Verifies auto-open windows start scanning from the serialized refresh flag alone. + ThirdPartyToolMigrationWizardWindow window = + ScriptableObject.CreateInstance(); + try + { + ThirdPartyToolMigrationWizardWindow.PrepareForOpen( + window, + "Unity CLI Loop Migration", + new Rect(12f, 34f, 360f, 220f), + shouldRefreshAfterCreateGui: true); + + bool shouldStartInitialRefresh = window.ConsumeShouldStartInitialRefresh(); + + Assert.That(shouldStartInitialRefresh, Is.True); + } + finally + { + Object.DestroyImmediate(window); + } + } + + [Test] + public void ConsumeShouldStartInitialRefresh_WhenConsumedTwice_ReturnsFalseOnSecondCall() + { + // Verifies the initial-refresh flag is one-shot so CreateGUI cannot double-scan. + ThirdPartyToolMigrationWizardWindow window = + ScriptableObject.CreateInstance(); + try + { + ThirdPartyToolMigrationWizardWindow.PrepareForOpen( + window, + "Unity CLI Loop Migration", + new Rect(12f, 34f, 360f, 220f), + shouldRefreshAfterCreateGui: true); + + bool first = window.ConsumeShouldStartInitialRefresh(); + bool second = window.ConsumeShouldStartInitialRefresh(); + + Assert.That(first, Is.True); + Assert.That(second, Is.False); + } + finally + { + Object.DestroyImmediate(window); + } + } + + [Test] + public void ConsumeShouldStartInitialRefresh_WhenManualOpen_ReturnsFalse() { - // Verifies that automatic scanning requires both auto-open intent and the session scan flag. - bool shouldStartInitialRefresh = - ThirdPartyToolMigrationWizardWindow.ShouldStartInitialRefresh( - shouldRefreshAfterCreateGui, - shouldAutoScanThirdPartyToolMigration); + // Verifies manually opened windows wait for an explicit Check instead of auto-scanning. + ThirdPartyToolMigrationWizardWindow window = + ScriptableObject.CreateInstance(); + try + { + ThirdPartyToolMigrationWizardWindow.PrepareForOpen( + window, + "Unity CLI Loop Migration", + new Rect(12f, 34f, 360f, 220f), + shouldRefreshAfterCreateGui: false); - Assert.That(shouldStartInitialRefresh, Is.EqualTo(expected)); + bool shouldStartInitialRefresh = window.ConsumeShouldStartInitialRefresh(); + + Assert.That(shouldStartInitialRefresh, Is.False); + } + finally + { + Object.DestroyImmediate(window); + } } [TestCase(false, false, false)] @@ -94,24 +151,116 @@ public async Task RunAutoScanAsync_WhenScanThrows_LogsExceptionAndConsumesState( Assert.That(loggedException, Is.SameAs(expectedException)); } - [TestCase( - 1, - "1 file needs V3 C# source structure migration.\n" + - "The Unity Console is showing errors because this file still uses the old custom tool API.\n\n" + - "Click Migrate to update it automatically. The errors should disappear after migration.")] - [TestCase( - 3, - "3 files need V3 C# source structure migration.\n" + - "The Unity Console is showing errors because these files still use the old custom tool API.\n\n" + - "Click Migrate to update them automatically. The errors should disappear after migration.")] - public void GetMigrationStatusText_WhenTargetsExist_ReturnsFileCount( - int fileCount, - string expectedText) - { - // Verifies that the migration wizard summarizes detected V2 custom tool files. - string text = ThirdPartyToolMigrationWizardWindow.GetMigrationStatusText(fileCount); - - Assert.That(text, Is.EqualTo(expectedText)); + [Test] + public void GetMigrationStatusText_WhenTargetsExist_IncludesRelativeFilePaths() + { + // Verifies the migration status lists project-relative target paths after the summary. + string projectRoot = System.IO.Path.Combine( + System.IO.Path.GetTempPath(), + "UnityCliLoopMigrationStatusRoot", + System.Guid.NewGuid().ToString("N")); + System.IO.Directory.CreateDirectory(projectRoot); + try + { + string[] filePaths = + { + System.IO.Path.Combine(projectRoot, "Assets", "Vendor", "One.cs"), + System.IO.Path.Combine(projectRoot, "Assets", "Vendor", "Two.cs") + }; + + string text = ThirdPartyToolMigrationWizardWindow.GetMigrationStatusText( + filePaths, + projectRoot); + + Assert.That(text, Does.StartWith( + "2 files need V3 C# source structure migration.\n" + + "The Unity Console is showing errors because these files still use the old custom tool API.\n\n" + + "Click Migrate to update them automatically. The errors should disappear after migration.\n\n" + + "Files:\n")); + Assert.That(text, Does.Contain("Assets/Vendor/One.cs")); + Assert.That(text, Does.Contain("Assets/Vendor/Two.cs")); + Assert.That(text, Does.Not.Contain("\\")); + } + finally + { + System.IO.Directory.Delete(projectRoot, recursive: true); + } + } + + [Test] + public void NormalizeDisplayPathSeparators_WhenPathsUseBackslashes_NormalizesToForwardSlashes() + { + // Verifies displayed migration paths always use forward slashes on every platform. + string normalized = ThirdPartyToolMigrationWizardStateRules.NormalizeDisplayPathSeparators( + "Assets\\Vendor\\Tool.cs"); + + Assert.That(normalized, Is.EqualTo("Assets/Vendor/Tool.cs")); + } + + [Test] + public void FormatMigrationTargetPathsForStatus_WhenPathCountExceedsLimit_AppendsOverflowSummary() + { + // Verifies long target lists truncate with a +N more files suffix. + string projectRoot = System.IO.Path.Combine( + System.IO.Path.GetTempPath(), + "UnityCliLoopMigrationOverflowRoot", + System.Guid.NewGuid().ToString("N")); + System.IO.Directory.CreateDirectory(projectRoot); + try + { + int totalCount = ThirdPartyToolMigrationWizardStateRules.MaxMigrationTargetPathsInStatus + 3; + string[] filePaths = new string[totalCount]; + for (int index = 0; index < totalCount; index++) + { + filePaths[index] = System.IO.Path.Combine(projectRoot, "Assets", $"File{index:D3}.cs"); + } + + string text = ThirdPartyToolMigrationWizardText.FormatMigrationTargetPathsForStatus( + filePaths, + projectRoot); + + Assert.That(text, Does.Contain("Assets/File000.cs")); + Assert.That( + text, + Does.Contain( + $"Assets/File{(ThirdPartyToolMigrationWizardStateRules.MaxMigrationTargetPathsInStatus - 1):D3}.cs")); + Assert.That(text, Does.Contain("+3 more files")); + Assert.That(text, Does.Not.Contain($"Assets/File{ThirdPartyToolMigrationWizardStateRules.MaxMigrationTargetPathsInStatus:D3}.cs")); + } + finally + { + System.IO.Directory.Delete(projectRoot, recursive: true); + } + } + + [Test] + public void ConfirmMigrationApply_WhenDialogIsCanceled_ReturnsFalse() + { + // Verifies Cancel leaves migration unstarted by returning false from the confirm helper. + bool confirmed = ThirdPartyToolMigrationWizardWindow.ConfirmMigrationApply( + 2, + (title, message, ok, cancel) => + { + Assert.That(title, Is.EqualTo(ThirdPartyToolMigrationWizardText.MigrationConfirmDialogTitle)); + Assert.That(message, Does.Contain("2 files will be rewritten in place.")); + Assert.That(message, Does.Contain("VCS recommended")); + Assert.That(ok, Is.EqualTo(ThirdPartyToolMigrationWizardText.MigrationConfirmDialogOkText)); + Assert.That(cancel, Is.EqualTo(ThirdPartyToolMigrationWizardText.MigrationConfirmDialogCancelText)); + return false; + }); + + Assert.That(confirmed, Is.False); + } + + [Test] + public void ConfirmMigrationApply_WhenDialogIsAccepted_ReturnsTrue() + { + // Verifies Migrate confirmation allows the apply path to continue. + bool confirmed = ThirdPartyToolMigrationWizardWindow.ConfirmMigrationApply( + 1, + (_, _, _, _) => true); + + Assert.That(confirmed, Is.True); } [Test] diff --git a/Packages/src/Editor/Domain/ThirdPartyToolMigrationCSharpRules.cs b/Packages/src/Editor/Domain/ThirdPartyToolMigrationCSharpRules.cs index 2c5f33778..2d3ec58e4 100644 --- a/Packages/src/Editor/Domain/ThirdPartyToolMigrationCSharpRules.cs +++ b/Packages/src/Editor/Domain/ThirdPartyToolMigrationCSharpRules.cs @@ -371,10 +371,12 @@ public void ApplyContractRenames() _migratedContent = ReplaceLegacyDomainTypeNamesInCode( _migratedContent, _legacyNamespaceAliases, + _assemblyDeclaredTypeNames, ref _replacementCount); _migratedContent = ReplaceLegacyContractTypeNamesInCode( _migratedContent, _legacyNamespaceAliases, + _assemblyDeclaredTypeNames, ref _replacementCount); ApplyCSharpReplacementRules(); } @@ -396,6 +398,7 @@ public void ApplyRegistrarRenames() ApplyRegistrarReplacementRules(); _migratedContent = ReplaceLegacyToolInfoTypeReferencesInCode( _migratedContent, + _assemblyDeclaredTypeNames, ref _replacementCount); } diff --git a/Packages/src/Editor/Domain/ThirdPartyToolMigrationCodeTextMaskBuilder.cs b/Packages/src/Editor/Domain/ThirdPartyToolMigrationCodeTextMaskBuilder.cs index e4dcb8166..67765e7e5 100644 --- a/Packages/src/Editor/Domain/ThirdPartyToolMigrationCodeTextMaskBuilder.cs +++ b/Packages/src/Editor/Domain/ThirdPartyToolMigrationCodeTextMaskBuilder.cs @@ -62,6 +62,16 @@ public static bool[] CreateCodeCharacters(string source) int index = 0; while (index < source.Length) { + if (IsPreprocessorDirectiveAt(source, index)) + { + // Preprocessor text is not C# code; skipping the whole directive line prevents + // apostrophes in titles like #region Bob's helpers from opening a char literal. + int directiveEndIndex = FindLineCommentEndIndex(source, index); + MarkRangeAsIgnored(codeCharacters, index, directiveEndIndex); + index = directiveEndIndex; + continue; + } + if (IsInterpolatedRawStringStart(source, index)) { index = MarkInterpolatedRawStringAsIgnored(codeCharacters, source, index); @@ -244,7 +254,10 @@ public static int FindRawStringEndIndex(string source, int quoteIndex) public static int FindCharLiteralEndIndex(string source, int quoteIndex) { int index = quoteIndex + 1; - while (index < source.Length) + // Longest valid C# char literal content is \Uxxxxxxxx (10 chars). An unmatched apostrophe + // beyond that bound is ordinary code, not an open-ended char literal that masks the file. + int maxClosingQuoteIndex = quoteIndex + 1 + MaxCharLiteralContentLength; + while (index <= maxClosingQuoteIndex && index < source.Length) { if (source[index] == '\\') { @@ -260,7 +273,32 @@ public static int FindCharLiteralEndIndex(string source, int quoteIndex) index++; } - return source.Length; + return quoteIndex; + } + + public static bool IsPreprocessorDirectiveAt(string source, int index) + { + Debug.Assert(source != null, "source must not be null"); + Debug.Assert(index >= 0, "index must not be negative"); + + if (index >= source.Length || source[index] != '#') + { + return false; + } + + int lineStartIndex = index; + while (lineStartIndex > 0 && source[lineStartIndex - 1] != '\n') + { + lineStartIndex--; + } + + int cursor = lineStartIndex; + while (cursor < index && (source[cursor] == ' ' || source[cursor] == '\t')) + { + cursor++; + } + + return cursor == index; } public static void MarkRangeAsIgnored(bool[] codeCharacters, int startIndex, int endIndex) diff --git a/Packages/src/Editor/Domain/ThirdPartyToolMigrationRuleCatalog.cs b/Packages/src/Editor/Domain/ThirdPartyToolMigrationRuleCatalog.cs index 2f80b4047..632be3cff 100644 --- a/Packages/src/Editor/Domain/ThirdPartyToolMigrationRuleCatalog.cs +++ b/Packages/src/Editor/Domain/ThirdPartyToolMigrationRuleCatalog.cs @@ -56,6 +56,8 @@ public static class ThirdPartyToolMigrationRuleCatalog public const string CurrentConstantsTypeName = "UnityCliLoopConstants"; public const string CurrentEditorFrameWaitTimeoutMemberName = "EDITOR_FRAME_WAIT_TIMEOUT_MS"; public const int MinimumRawStringDelimiterQuoteCount = 3; + // Longest valid C# char literal content is \Uxxxxxxxx; beyond that an apostrophe is ordinary code. + public const int MaxCharLiteralContentLength = 10; public static readonly string[] ExcludedDirectoryNames = { ".git", diff --git a/Packages/src/Editor/Domain/ThirdPartyToolMigrationSourceDetectionRules.cs b/Packages/src/Editor/Domain/ThirdPartyToolMigrationSourceDetectionRules.cs index 47740e503..ff6eec78d 100644 --- a/Packages/src/Editor/Domain/ThirdPartyToolMigrationSourceDetectionRules.cs +++ b/Packages/src/Editor/Domain/ThirdPartyToolMigrationSourceDetectionRules.cs @@ -270,6 +270,13 @@ public static bool IsExcludedDirectoryName(string directoryName) { Debug.Assert(!string.IsNullOrEmpty(directoryName), "directoryName must not be null or empty"); + // Unity ignores trailing-~ folders (Samples~) and hidden/dot folders; keep them out of scans. + if (directoryName.StartsWith(".", StringComparison.Ordinal) || + directoryName.EndsWith("~", StringComparison.Ordinal)) + { + return true; + } + return ExcludedDirectoryNames.Any( excludedDirectoryName => string.Equals( excludedDirectoryName, diff --git a/Packages/src/Editor/Domain/ThirdPartyToolMigrationTypeReplacementRules.cs b/Packages/src/Editor/Domain/ThirdPartyToolMigrationTypeReplacementRules.cs index 1aa07fb40..97533cace 100644 --- a/Packages/src/Editor/Domain/ThirdPartyToolMigrationTypeReplacementRules.cs +++ b/Packages/src/Editor/Domain/ThirdPartyToolMigrationTypeReplacementRules.cs @@ -110,14 +110,19 @@ public static string ReplaceUnqualifiedLegacyRegistrarReferencesInCode( public static string ReplaceLegacyContractTypeNamesInCode( string source, string[] aliases, + string[] assemblyDeclaredTypeNames, ref int replacementCount) { Debug.Assert(source != null, "source must not be null"); Debug.Assert(aliases != null, "aliases must not be null"); + Debug.Assert(assemblyDeclaredTypeNames != null, "assemblyDeclaredTypeNames must not be null"); string migratedContent = source; foreach (TypeReplacementRule rule in ToolContractTypeReplacementRules) { + bool hasProtectedTypeDeclaration = DeclaresLocalType(migratedContent, rule.LegacyName) || + assemblyDeclaredTypeNames.Contains(rule.LegacyName); + Regex fullyQualifiedRegex = new( $@"(?:(?:global::)?{Regex.Escape(LegacyNamespace)}\.){Regex.Escape(rule.LegacyName)}\b", RegexOptions.Compiled); @@ -145,7 +150,8 @@ public static string ReplaceLegacyContractTypeNamesInCode( migratedContent = ReplaceRegexInCode( migratedContent, unqualifiedRegex, - match => ShouldMigrateLegacyTypeReference(migratedContent, rule.LegacyName, match.Index) + match => !hasProtectedTypeDeclaration && + ShouldMigrateLegacyTypeReference(migratedContent, rule.LegacyName, match.Index) ? rule.CurrentName : match.Value, ref replacementCount); @@ -157,14 +163,19 @@ public static string ReplaceLegacyContractTypeNamesInCode( public static string ReplaceLegacyDomainTypeNamesInCode( string source, string[] aliases, + string[] assemblyDeclaredTypeNames, ref int replacementCount) { Debug.Assert(source != null, "source must not be null"); Debug.Assert(aliases != null, "aliases must not be null"); + Debug.Assert(assemblyDeclaredTypeNames != null, "assemblyDeclaredTypeNames must not be null"); string migratedContent = source; foreach (TypeReplacementRule rule in DomainTypeReplacementRules) { + bool hasProtectedTypeDeclaration = DeclaresLocalType(migratedContent, rule.LegacyName) || + assemblyDeclaredTypeNames.Contains(rule.LegacyName); + Regex fullyQualifiedRegex = new( $@"(?:(?:global::)?{Regex.Escape(LegacyNamespace)}\.){Regex.Escape(rule.LegacyName)}\b", RegexOptions.Compiled); @@ -192,7 +203,8 @@ public static string ReplaceLegacyDomainTypeNamesInCode( migratedContent = ReplaceRegexInCode( migratedContent, unqualifiedRegex, - match => ShouldMigrateLegacyTypeReference(migratedContent, rule.LegacyName, match.Index) + match => !hasProtectedTypeDeclaration && + ShouldMigrateLegacyTypeReference(migratedContent, rule.LegacyName, match.Index) ? $"{CurrentNamespace}.{rule.CurrentName}" : match.Value, ref replacementCount); @@ -477,14 +489,21 @@ public static string ReplaceCurrentDomainContractTypeNameInCode( ref replacementCount); } - public static string ReplaceLegacyToolInfoTypeReferencesInCode(string source, ref int replacementCount) + public static string ReplaceLegacyToolInfoTypeReferencesInCode( + string source, + string[] assemblyDeclaredTypeNames, + ref int replacementCount) { Debug.Assert(source != null, "source must not be null"); + Debug.Assert(assemblyDeclaredTypeNames != null, "assemblyDeclaredTypeNames must not be null"); + bool hasProtectedTypeDeclaration = DeclaresLocalType(source, "ToolInfo") || + assemblyDeclaredTypeNames.Contains("ToolInfo"); return ReplaceRegexInCode( source, UnqualifiedToolInfoRegex, - match => ShouldMigrateLegacyToolInfoTypeReference(source, match.Index) + match => !hasProtectedTypeDeclaration && + ShouldMigrateLegacyToolInfoTypeReference(source, match.Index) ? $"{CurrentNamespace}.ToolInfo" : match.Value, ref replacementCount); diff --git a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAsmdefRules.cs b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAsmdefRules.cs index 5e526acf4..83142431c 100644 --- a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAsmdefRules.cs +++ b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAsmdefRules.cs @@ -51,14 +51,28 @@ internal static ThirdPartyToolMigrationContentResult MigrateAsmdefSource( Array.Empty()); } - JArray migratedReferences = CreateReferencesArray(migrationResult.References); - asmdef["references"] = migratedReferences; + ThirdPartyToolMigrationAsmdefTextEditor.AsmdefReferencesEditResult editResult = + ThirdPartyToolMigrationAsmdefTextEditor.ReplaceReferencesArray(source, migrationResult.References); + // The surgical edit needs an existing "references" array to anchor on; when the asmdef has + // none, there is no original formatting to preserve, so full re-serialization is acceptable. + string migratedSource = editResult.Replaced + ? editResult.Content + : CreateReserializedAsmdefSource(asmdef, migrationResult.References); return new ThirdPartyToolMigrationContentResult( - asmdef.ToString(Formatting.Indented), + migratedSource, migrationResult.ReplacementCount, Array.Empty()); } + private static string CreateReserializedAsmdefSource(JObject asmdef, string[] references) + { + Debug.Assert(asmdef != null, "asmdef must not be null"); + Debug.Assert(references != null, "references must not be null"); + + asmdef["references"] = CreateReferencesArray(references); + return asmdef.ToString(Formatting.Indented); + } + private static string[] ReadReferenceValues(JArray references) { Debug.Assert(references != null, "references must not be null"); diff --git a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAsmdefTextEditor.cs b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAsmdefTextEditor.cs new file mode 100644 index 000000000..943fcc1c1 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAsmdefTextEditor.cs @@ -0,0 +1,291 @@ +using System; +using System.Diagnostics; +using System.Text; + +using Newtonsoft.Json; + +namespace io.github.hatayama.UnityCliLoop.Infrastructure +{ + /// + /// Replaces only the top-level "references" array inside asmdef JSON text so every byte + /// outside that array (indentation, line endings, trailing newline, other properties) + /// is preserved exactly. + /// + internal static class ThirdPartyToolMigrationAsmdefTextEditor + { + internal static AsmdefReferencesEditResult ReplaceReferencesArray( + string source, + string[] references) + { + Debug.Assert(source != null, "source must not be null"); + Debug.Assert(references != null, "references must not be null"); + Debug.Assert(references.Length > 0, "references must not be empty"); + + ReferencesArraySpan span = FindTopLevelReferencesArray(source); + if (!span.Found) + { + return AsmdefReferencesEditResult.NotReplaced; + } + + string renderedArray = RenderReferencesArray(source, span, references); + string content = source.Substring(0, span.ArrayStartIndex) + + renderedArray + + source.Substring(span.ArrayEndIndexExclusive); + return AsmdefReferencesEditResult.ReplacedWith(content); + } + + private static ReferencesArraySpan FindTopLevelReferencesArray(string source) + { + int depth = 0; + int index = 0; + while (index < source.Length) + { + char current = source[index]; + if (current == '"') + { + int stringStartIndex = index; + index = SkipJsonString(source, index); + if (depth != 1) + { + continue; + } + + // A depth-1 string is a property name only when the next + // non-whitespace character is a colon. + int colonIndex = SkipWhitespace(source, index); + if (colonIndex >= source.Length || source[colonIndex] != ':') + { + continue; + } + + string propertyName = source.Substring( + stringStartIndex + 1, + index - stringStartIndex - 2); + int valueStartIndex = SkipWhitespace(source, colonIndex + 1); + if (!string.Equals(propertyName, "references", StringComparison.Ordinal) || + valueStartIndex >= source.Length || + source[valueStartIndex] != '[') + { + continue; + } + + int arrayEndIndexExclusive = SkipJsonArray(source, valueStartIndex); + return ReferencesArraySpan.FoundAt( + stringStartIndex, + valueStartIndex, + arrayEndIndexExclusive); + } + + if (current == '{' || current == '[') + { + depth++; + } + else if (current == '}' || current == ']') + { + depth--; + } + + index++; + } + + return ReferencesArraySpan.NotFound; + } + + // Returns the index just after the closing quote. + private static int SkipJsonString(string source, int openQuoteIndex) + { + int index = openQuoteIndex + 1; + while (index < source.Length) + { + char current = source[index]; + if (current == '\\') + { + index += 2; + continue; + } + + if (current == '"') + { + return index + 1; + } + + index++; + } + + return source.Length; + } + + // Returns the index just after the matching closing bracket. + private static int SkipJsonArray(string source, int openBracketIndex) + { + int bracketDepth = 0; + int index = openBracketIndex; + while (index < source.Length) + { + char current = source[index]; + if (current == '"') + { + index = SkipJsonString(source, index); + continue; + } + + if (current == '[') + { + bracketDepth++; + } + else if (current == ']') + { + bracketDepth--; + if (bracketDepth == 0) + { + return index + 1; + } + } + + index++; + } + + return source.Length; + } + + private static int SkipWhitespace(string source, int startIndex) + { + int index = startIndex; + while (index < source.Length && char.IsWhiteSpace(source[index])) + { + index++; + } + + return index; + } + + private static string RenderReferencesArray( + string source, + ReferencesArraySpan span, + string[] references) + { + string originalArrayText = source.Substring( + span.ArrayStartIndex, + span.ArrayEndIndexExclusive - span.ArrayStartIndex); + if (originalArrayText.IndexOf('\n') < 0) + { + return RenderSingleLineArray(references); + } + + // The file's own newline flavor and the property line's indent decide the layout; + // Environment.NewLine would silently rewrite CRLF files on macOS and vice versa. + string newline = source.IndexOf("\r\n", StringComparison.Ordinal) >= 0 ? "\r\n" : "\n"; + string propertyIndent = GetLineIndent(source, span.PropertyNameIndex); + string indentUnit = propertyIndent.Length > 0 ? propertyIndent : " "; + string elementIndent = propertyIndent + indentUnit; + StringBuilder builder = new(); + builder.Append('['); + for (int index = 0; index < references.Length; index++) + { + builder.Append(newline); + builder.Append(elementIndent); + builder.Append(JsonConvert.ToString(references[index])); + if (index < references.Length - 1) + { + builder.Append(','); + } + } + + builder.Append(newline); + builder.Append(propertyIndent); + builder.Append(']'); + return builder.ToString(); + } + + private static string RenderSingleLineArray(string[] references) + { + StringBuilder builder = new(); + builder.Append('['); + for (int index = 0; index < references.Length; index++) + { + builder.Append(JsonConvert.ToString(references[index])); + if (index < references.Length - 1) + { + builder.Append(", "); + } + } + + builder.Append(']'); + return builder.ToString(); + } + + // Returns the leading whitespace of the property's line, or empty when the + // property does not start its own line. + private static string GetLineIndent(string source, int propertyNameIndex) + { + Debug.Assert(propertyNameIndex > 0, "propertyNameIndex must be inside the object"); + + int lineStartIndex = source.LastIndexOf('\n', propertyNameIndex - 1) + 1; + int index = lineStartIndex; + while (index < propertyNameIndex && (source[index] == ' ' || source[index] == '\t')) + { + index++; + } + + if (index != propertyNameIndex) + { + return string.Empty; + } + + return source.Substring(lineStartIndex, index - lineStartIndex); + } + + internal readonly struct AsmdefReferencesEditResult + { + public static AsmdefReferencesEditResult NotReplaced => new(false, string.Empty); + + public static AsmdefReferencesEditResult ReplacedWith(string content) + { + return new AsmdefReferencesEditResult(true, content); + } + + private AsmdefReferencesEditResult(bool replaced, string content) + { + Replaced = replaced; + Content = content; + } + + public bool Replaced { get; } + public string Content { get; } + } + + private readonly struct ReferencesArraySpan + { + public static ReferencesArraySpan NotFound => new(false, 0, 0, 0); + + public static ReferencesArraySpan FoundAt( + int propertyNameIndex, + int arrayStartIndex, + int arrayEndIndexExclusive) + { + return new ReferencesArraySpan( + true, + propertyNameIndex, + arrayStartIndex, + arrayEndIndexExclusive); + } + + private ReferencesArraySpan( + bool found, + int propertyNameIndex, + int arrayStartIndex, + int arrayEndIndexExclusive) + { + Found = found; + PropertyNameIndex = propertyNameIndex; + ArrayStartIndex = arrayStartIndex; + ArrayEndIndexExclusive = arrayEndIndexExclusive; + } + + public bool Found { get; } + public int PropertyNameIndex { get; } + public int ArrayStartIndex { get; } + public int ArrayEndIndexExclusive { get; } + } + } +} diff --git a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAsmdefTextEditor.cs.meta b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAsmdefTextEditor.cs.meta new file mode 100644 index 000000000..2bd7a40f5 --- /dev/null +++ b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAsmdefTextEditor.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 0cca2e10f7002407c963bc4f3966b7e8 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileService.cs b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileService.cs index 582fb815e..a9fc880b1 100644 --- a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileService.cs +++ b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileService.cs @@ -92,10 +92,7 @@ public ThirdPartyToolMigrationResult ApplyMigration(string projectRoot) string normalizedProjectRoot = NormalizeProjectRoot(projectRoot); MigrationPlan plan = GetCurrentMigrationPlan(normalizedProjectRoot); InvalidatePreviewCache(); - foreach (MigrationFileChange change in plan.Changes) - { - ThirdPartyToolMigrationFileWriter.Write(change.FilePath, change.Content); - } + ThirdPartyToolMigrationFileWriter.WriteBatch(plan.Changes); return new ThirdPartyToolMigrationResult( plan.ChangedFilePaths.Count, @@ -120,15 +117,7 @@ public async Task ApplyMigrationAsync( } InvalidatePreviewCache(); - for (int index = 0; index < plan.Changes.Count; index++) - { - MigrationFileChange change = plan.Changes[index]; - ThirdPartyToolMigrationFileWriter.Write(change.FilePath, change.Content); - if ((index + 1) % ThirdPartyToolMigrationFileServiceConstants.PreviewYieldBatchSize == 0) - { - await Task.Yield(); - } - } + await ThirdPartyToolMigrationFileWriter.WriteBatchAsync(plan.Changes); return new ThirdPartyToolMigrationResult( plan.ChangedFilePaths.Count, diff --git a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileWriter.cs b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileWriter.cs index 9cd2875ec..d6f9e92b7 100644 --- a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileWriter.cs +++ b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileWriter.cs @@ -2,30 +2,219 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; +using System.Text; +using System.Threading.Tasks; namespace io.github.hatayama.UnityCliLoop.Infrastructure { /// - /// Writes migrated files atomically through temporary sidecar files. + /// Writes a migration plan as one batch transaction: every temp sidecar is written first, + /// then all targets are committed via rename/replace while their backups are kept, and a + /// mid-batch commit failure rolls the already committed files back from those backups. /// internal static class ThirdPartyToolMigrationFileWriter { - internal static void Write(string filePath, string content) + private const string TempSidecarExtension = ".tmp"; + private const string BackupSidecarExtension = ".bak"; + + internal static void WriteBatch(IReadOnlyList changes) { - Debug.Assert(!string.IsNullOrEmpty(filePath), "filePath must not be null or empty"); - Debug.Assert(content != null, "content must not be null"); + Debug.Assert(changes != null, "changes must not be null"); + + List preparedWrites = PrepareAll(changes); + CommitAll(preparedWrites); + } + + internal static async Task WriteBatchAsync(IReadOnlyList changes) + { + Debug.Assert(changes != null, "changes must not be null"); + + List preparedWrites = await PrepareAllAsync(changes); + // Commit is rename-only and fast; it must run without yields so no editor callback + // (asset refresh, domain reload) can observe a half-committed batch. + CommitAll(preparedWrites); + } + + private static List PrepareAll(IReadOnlyList changes) + { + List preparedWrites = new(changes.Count); + bool prepared = false; + try + { + foreach (MigrationFileChange change in changes) + { + Prepare(preparedWrites, change); + } + + prepared = true; + return preparedWrites; + } + finally + { + if (!prepared) + { + // A prepare failure has not touched any target file yet; deleting the temp + // sidecars returns the project to its exact pre-apply state. + DeleteTempSidecars(preparedWrites, 0); + } + } + } + + private static async Task> PrepareAllAsync( + IReadOnlyList changes) + { + List preparedWrites = new(changes.Count); + bool prepared = false; + try + { + for (int index = 0; index < changes.Count; index++) + { + Prepare(preparedWrites, changes[index]); + if ((index + 1) % ThirdPartyToolMigrationFileServiceConstants.PreviewYieldBatchSize == 0) + { + await Task.Yield(); + } + } + + prepared = true; + return preparedWrites; + } + finally + { + if (!prepared) + { + DeleteTempSidecars(preparedWrites, 0); + } + } + } + + private static void Prepare(List preparedWrites, MigrationFileChange change) + { + string tempFilePath = CreateUniqueSidecarPath(change.FilePath, TempSidecarExtension); + // Register before WriteAllText so a mid-write IOException can still clean up a partial .tmp. + preparedWrites.Add(new PreparedWrite(change.FilePath, tempFilePath)); + // The plan carries decoded strings only, so the original file's BOM/encoding must be + // re-detected here and reapplied; otherwise every migrated file collapses to BOM-less UTF-8. + Encoding encoding = ThirdPartyToolMigrationFileAccess.Exists(change.FilePath) + ? ThirdPartyToolMigrationFileAccess.DetectEncodingFromBom(change.FilePath) + : new UTF8Encoding(false); + ThirdPartyToolMigrationFileAccess.WriteAllTextWithEncoding(tempFilePath, change.Content, encoding); + } + + private static void CommitAll(List preparedWrites) + { + List committedWrites = new(preparedWrites.Count); + bool committed = false; + try + { + foreach (PreparedWrite preparedWrite in preparedWrites) + { + committedWrites.Add(Commit(preparedWrite)); + } + + committed = true; + } + finally + { + if (committed) + { + DeleteBackupSidecars(committedWrites); + } + else + { + // The commit exception is propagating right now; rollback and cleanup must be + // best-effort so they never replace that original exception. + RollbackCommitted(committedWrites); + DeleteTempSidecars(preparedWrites, committedWrites.Count); + } + } + } - string tempFilePath = CreateUniqueSidecarPath(filePath, ".tmp"); - ThirdPartyToolMigrationFileAccess.WriteAllText(tempFilePath, content); - if (!ThirdPartyToolMigrationFileAccess.Exists(filePath)) + private static CommittedWrite Commit(PreparedWrite preparedWrite) + { + if (!ThirdPartyToolMigrationFileAccess.Exists(preparedWrite.TargetFilePath)) + { + ThirdPartyToolMigrationFileAccess.Move( + preparedWrite.TempFilePath, + preparedWrite.TargetFilePath); + return new CommittedWrite(preparedWrite.TargetFilePath, null); + } + + string backupFilePath = CreateUniqueSidecarPath( + preparedWrite.TargetFilePath, + BackupSidecarExtension); + ThirdPartyToolMigrationFileAccess.Replace( + preparedWrite.TempFilePath, + preparedWrite.TargetFilePath, + backupFilePath); + return new CommittedWrite(preparedWrite.TargetFilePath, backupFilePath); + } + + private static void RollbackCommitted(List committedWrites) + { + for (int index = committedWrites.Count - 1; index >= 0; index--) { - ThirdPartyToolMigrationFileAccess.Move(tempFilePath, filePath); - return; + CommittedWrite committedWrite = committedWrites[index]; + try + { + ThirdPartyToolMigrationFileAccess.Delete(committedWrite.TargetFilePath); + if (committedWrite.BackupFilePath != null) + { + ThirdPartyToolMigrationFileAccess.Move( + committedWrite.BackupFilePath, + committedWrite.TargetFilePath); + } + } + catch (Exception restoreException) + { + // Keep restoring the remaining files; a file that cannot be restored keeps + // its .bak on disk so the user can recover it manually. + UnityEngine.Debug.LogError( + $"[uloop] Migration rollback failed for '{committedWrite.TargetFilePath}': " + + $"{restoreException.Message}" + + (committedWrite.BackupFilePath == null + ? string.Empty + : $" Backup kept at '{committedWrite.BackupFilePath}'.")); + } } + } - string backupFilePath = CreateUniqueSidecarPath(filePath, ".bak"); - ThirdPartyToolMigrationFileAccess.Replace(tempFilePath, filePath, backupFilePath); - ThirdPartyToolMigrationFileAccess.Delete(backupFilePath); + private static void DeleteTempSidecars(List preparedWrites, int firstIndex) + { + for (int index = firstIndex; index < preparedWrites.Count; index++) + { + TryDeleteSidecar(preparedWrites[index].TempFilePath); + } + } + + private static void DeleteBackupSidecars(List committedWrites) + { + foreach (CommittedWrite committedWrite in committedWrites) + { + if (committedWrite.BackupFilePath != null) + { + TryDeleteSidecar(committedWrite.BackupFilePath); + } + } + } + + private static void TryDeleteSidecar(string sidecarFilePath) + { + try + { + if (ThirdPartyToolMigrationFileAccess.Exists(sidecarFilePath)) + { + ThirdPartyToolMigrationFileAccess.Delete(sidecarFilePath); + } + } + catch (Exception deleteException) + { + // Sidecar cleanup must never fail the migration itself; a stray sidecar is + // visible in the project window and harmless to compilation. + UnityEngine.Debug.LogError( + $"[uloop] Failed to delete migration sidecar '{sidecarFilePath}': " + + $"{deleteException.Message}"); + } } internal static string CreateUniqueSidecarPath(string filePath, string extension) @@ -43,6 +232,31 @@ internal static string CreateUniqueSidecarPath(string filePath, string extension return sidecarPath; } + + private readonly struct PreparedWrite + { + public PreparedWrite(string targetFilePath, string tempFilePath) + { + TargetFilePath = targetFilePath; + TempFilePath = tempFilePath; + } + + public string TargetFilePath { get; } + public string TempFilePath { get; } + } + + private readonly struct CommittedWrite + { + public CommittedWrite(string targetFilePath, string backupFilePath) + { + TargetFilePath = targetFilePath; + BackupFilePath = backupFilePath; + } + + public string TargetFilePath { get; } + // Null when the target file did not exist before the commit (plain move, no backup). + public string BackupFilePath { get; } + } } /// @@ -57,12 +271,63 @@ internal static string ReadAllText(string filePath) return File.ReadAllText(GetFileSystemPath(filePath)); } - internal static void WriteAllText(string filePath, string content) + internal static Encoding DetectEncodingFromBom(string filePath) + { + Debug.Assert(!string.IsNullOrEmpty(filePath), "filePath must not be null or empty"); + + byte[] bom = new byte[4]; + int readCount = 0; + using (FileStream stream = File.OpenRead(GetFileSystemPath(filePath))) + { + // FileStream.Read may return fewer bytes than requested; read until EOF or 4 bytes. + while (readCount < bom.Length) + { + int read = stream.Read(bom, readCount, bom.Length - readCount); + if (read == 0) + { + break; + } + + readCount += read; + } + } + + // UTF-32 BOMs must be checked before UTF-16 because they share the same leading bytes. + if (readCount >= 4 && bom[0] == 0xFF && bom[1] == 0xFE && bom[2] == 0x00 && bom[3] == 0x00) + { + return new UTF32Encoding(bigEndian: false, byteOrderMark: true); + } + + if (readCount >= 4 && bom[0] == 0x00 && bom[1] == 0x00 && bom[2] == 0xFE && bom[3] == 0xFF) + { + return new UTF32Encoding(bigEndian: true, byteOrderMark: true); + } + + if (readCount >= 3 && bom[0] == 0xEF && bom[1] == 0xBB && bom[2] == 0xBF) + { + return new UTF8Encoding(encoderShouldEmitUTF8Identifier: true); + } + + if (readCount >= 2 && bom[0] == 0xFF && bom[1] == 0xFE) + { + return new UnicodeEncoding(bigEndian: false, byteOrderMark: true); + } + + if (readCount >= 2 && bom[0] == 0xFE && bom[1] == 0xFF) + { + return new UnicodeEncoding(bigEndian: true, byteOrderMark: true); + } + + return new UTF8Encoding(encoderShouldEmitUTF8Identifier: false); + } + + internal static void WriteAllTextWithEncoding(string filePath, string content, Encoding encoding) { Debug.Assert(!string.IsNullOrEmpty(filePath), "filePath must not be null or empty"); Debug.Assert(content != null, "content must not be null"); + Debug.Assert(encoding != null, "encoding must not be null"); - File.WriteAllText(GetFileSystemPath(filePath), content); + File.WriteAllText(GetFileSystemPath(filePath), content, encoding); } internal static IEnumerable ReadLines(string filePath) diff --git a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationProjectFileInventory.cs b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationProjectFileInventory.cs index 13b49e0f6..04e81d880 100644 --- a/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationProjectFileInventory.cs +++ b/Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationProjectFileInventory.cs @@ -227,10 +227,32 @@ internal static bool ShouldExcludeDirectory(string projectRoot, string directory return true; } + // why not follow symlinks/junctions: cycle prevention takes priority over scanning + // through linked package trees that some developers keep under Assets. + if (!Directory.Exists(directoryPath)) + { + // Dangling reparse points report Exists=false; skip before reading Attributes. + return true; + } + + if (IsReparsePointDirectory(directoryPath)) + { + return true; + } + string directoryName = Path.GetFileName(directoryPath); return ThirdPartyToolMigrationRules.IsExcludedDirectoryName(directoryName); } + private static bool IsReparsePointDirectory(string directoryPath) + { + Debug.Assert(!string.IsNullOrEmpty(directoryPath), "directoryPath must not be null or empty"); + Debug.Assert(Directory.Exists(directoryPath), "directoryPath must exist before reading attributes"); + + DirectoryInfo directoryInfo = new DirectoryInfo(directoryPath); + return (directoryInfo.Attributes & FileAttributes.ReparsePoint) != 0; + } + private static bool IsProjectRootPackagesDirectory(string projectRoot, string directoryPath) { Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); diff --git a/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardStateRules.cs b/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardStateRules.cs index e9cceb98e..1076ce24d 100644 --- a/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardStateRules.cs +++ b/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardStateRules.cs @@ -10,12 +10,38 @@ namespace io.github.hatayama.UnityCliLoop.Presentation internal static class ThirdPartyToolMigrationWizardStateRules { internal const int MigrationProgressUiUpdateIntervalMilliseconds = 100; + internal const int MaxMigrationTargetPathsInStatus = 50; - internal static bool ShouldStartInitialRefresh( - bool shouldRefreshAfterCreateGui, - bool shouldAutoScanThirdPartyToolMigration) + internal static int GetMigrationTargetPathsOverflowCount(int totalPathCount, int maxPaths) { - return shouldRefreshAfterCreateGui && shouldAutoScanThirdPartyToolMigration; + Debug.Assert(totalPathCount >= 0, "totalPathCount must not be negative"); + Debug.Assert(maxPaths > 0, "maxPaths must be positive"); + + if (totalPathCount <= maxPaths) + { + return 0; + } + + return totalPathCount - maxPaths; + } + + internal static string NormalizeDisplayPathSeparators(string path) + { + Debug.Assert(path != null, "path must not be null"); + + // Status text must stay Windows-safe: never leave backslashes in displayed relative paths. + return path.Replace('\\', '/'); + } + + internal static string ToProjectRelativeDisplayPath(string filePath, string projectRoot) + { + Debug.Assert(!string.IsNullOrEmpty(filePath), "filePath must not be null or empty"); + Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); + + string relativePath = System.IO.Path.GetRelativePath( + System.IO.Path.GetFullPath(projectRoot), + System.IO.Path.GetFullPath(filePath)); + return NormalizeDisplayPathSeparators(relativePath); } internal static bool ShouldReportMigrationProgress( diff --git a/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardText.cs b/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardText.cs index e60fdf4e6..da1c3d710 100644 --- a/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardText.cs +++ b/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardText.cs @@ -29,6 +29,9 @@ internal static class ThirdPartyToolMigrationWizardText "- After editing, summarize changed files, remaining candidates, and any commands I should verify manually."; internal const string MigrationNotCheckedText = "C# source migration status has not been checked."; internal const string NoMigrationTargetsText = "No C# source structure migration is needed."; + internal const string MigrationConfirmDialogTitle = "Migrate C# Sources?"; + internal const string MigrationConfirmDialogOkText = "Migrate"; + internal const string MigrationConfirmDialogCancelText = "Cancel"; private const string MigrationCheckingText = "Scanning C# source files for V3 custom tool API migration..."; private const string MigrationApplyingText = "Migrating C# source files to V3 custom tool APIs..."; private const string MigrationButtonReadyText = "Migrate"; @@ -39,11 +42,14 @@ internal static class ThirdPartyToolMigrationWizardText private const string RemoveMigrationSkillButtonText = "Remove Migration Skill"; private const string UpdatingMigrationSkillButtonText = "Updating..."; private const string CopyMigrationSkillPromptButtonText = "Copy AI Prompt"; + private const string MigrationTargetFilesHeading = "Files:"; - internal static string GetMigrationStatusText(int fileCount) + internal static string GetMigrationStatusText(string[] filePaths, string projectRoot) { - Debug.Assert(fileCount >= 0, "fileCount must not be negative"); + Debug.Assert(filePaths != null, "filePaths must not be null"); + Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); + int fileCount = filePaths.Length; string noun = fileCount == 1 ? "file" : "files"; string verb = fileCount == 1 ? "needs" : "need"; string subject = fileCount == 1 ? "this file still uses" : "these files still use"; @@ -52,7 +58,49 @@ internal static string GetMigrationStatusText(int fileCount) return $"{fileCount} {noun} {verb} V3 C# source structure migration.\n" + $"The Unity Console is showing errors because {subject} the old custom tool API.\n\n" + $"Click Migrate to update {objectPronoun} automatically. " + - "The errors should disappear after migration."; + "The errors should disappear after migration.\n\n" + + FormatMigrationTargetPathsForStatus(filePaths, projectRoot); + } + + internal static string GetMigrationConfirmDialogMessage(int fileCount) + { + Debug.Assert(fileCount >= 0, "fileCount must not be negative"); + + string noun = fileCount == 1 ? "file" : "files"; + return $"{fileCount} {noun} will be rewritten in place.\n\n" + + "Commit or back up your project first (VCS recommended)."; + } + + internal static string FormatMigrationTargetPathsForStatus(string[] filePaths, string projectRoot) + { + Debug.Assert(filePaths != null, "filePaths must not be null"); + Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); + + int maxPaths = ThirdPartyToolMigrationWizardStateRules.MaxMigrationTargetPathsInStatus; + int overflowCount = ThirdPartyToolMigrationWizardStateRules.GetMigrationTargetPathsOverflowCount( + filePaths.Length, + maxPaths); + int displayCount = filePaths.Length - overflowCount; + System.Text.StringBuilder builder = new(); + builder.Append(MigrationTargetFilesHeading); + for (int index = 0; index < displayCount; index++) + { + builder.Append('\n'); + builder.Append( + ThirdPartyToolMigrationWizardStateRules.ToProjectRelativeDisplayPath( + filePaths[index], + projectRoot)); + } + + if (overflowCount > 0) + { + builder.Append('\n'); + builder.Append('+'); + builder.Append(overflowCount); + builder.Append(" more files"); + } + + return builder.ToString(); } internal static string GetMigrationProgressText( diff --git a/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardView.cs b/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardView.cs index 256e0da94..68ead776f 100644 --- a/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardView.cs +++ b/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardView.cs @@ -133,10 +133,13 @@ internal void ShowNotCheckedState(bool isMigrating) _refreshButton.SetEnabled(true); } - internal void ShowMigrationTargetsState(int fileCount, bool isMigrating) + internal void ShowMigrationTargetsState(string[] filePaths, string projectRoot, bool isMigrating) { + Debug.Assert(filePaths != null, "filePaths must not be null"); + Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); + _migrationStatusTextField.SetValueWithoutNotify( - ThirdPartyToolMigrationWizardText.GetMigrationStatusText(fileCount)); + ThirdPartyToolMigrationWizardText.GetMigrationStatusText(filePaths, projectRoot)); ViewDataBinder.SetVisible(_migrationProgressBar, false); ViewDataBinder.SetVisible(_migrationButtonRow, true); _migrateButton.SetEnabled(!isMigrating); diff --git a/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWindow.cs b/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWindow.cs index 4e24b28f9..dc9a8142f 100644 --- a/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWindow.cs +++ b/Packages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWindow.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; @@ -33,7 +34,9 @@ public class ThirdPartyToolMigrationWizardWindow : EditorWindow private bool _isUpdatingMigrationSkill; private SkillsTarget _migrationSkillTarget = SkillsTarget.Claude; private SkillInstallState _migrationSkillInstallState = SkillInstallState.Missing; + private string[] _pendingMigrationFilePaths = Array.Empty(); private CancellationTokenSource _migrationOperationCts; + private CancellationTokenSource _migrationSkillOperationCts; private SkillSetupUseCase _skillSetupUseCase; private ThirdPartyToolMigrationUseCase _thirdPartyToolMigrationUseCase; private ThirdPartyToolMigrationWizardView _view; @@ -148,15 +151,6 @@ private static void LogAutoScanException(System.Exception ex) Debug.LogException(ex); } - internal static bool ShouldStartInitialRefresh( - bool shouldRefreshAfterCreateGui, - bool shouldAutoScanThirdPartyToolMigration) - { - return ThirdPartyToolMigrationWizardStateRules.ShouldStartInitialRefresh( - shouldRefreshAfterCreateGui, - shouldAutoScanThirdPartyToolMigration); - } - internal static bool ShouldOpenWindowAfterAutoScan( bool hasMigrationTargets, bool isCancellationRequested) @@ -192,9 +186,23 @@ internal static Rect WithContentHeight(Rect currentRect, float contentHeight, Ve frameSize); } - internal static string GetMigrationStatusText(int fileCount) + internal static string GetMigrationStatusText(string[] filePaths, string projectRoot) + { + return ThirdPartyToolMigrationWizardText.GetMigrationStatusText(filePaths, projectRoot); + } + + internal static bool ConfirmMigrationApply( + int fileCount, + Func displayDialog) { - return ThirdPartyToolMigrationWizardText.GetMigrationStatusText(fileCount); + Debug.Assert(displayDialog != null, "displayDialog must not be null"); + Debug.Assert(fileCount >= 0, "fileCount must not be negative"); + + return displayDialog( + ThirdPartyToolMigrationWizardText.MigrationConfirmDialogTitle, + ThirdPartyToolMigrationWizardText.GetMigrationConfirmDialogMessage(fileCount), + ThirdPartyToolMigrationWizardText.MigrationConfirmDialogOkText, + ThirdPartyToolMigrationWizardText.MigrationConfirmDialogCancelText); } internal static string GetMigrationProgressText( @@ -396,6 +404,7 @@ private void OnDisable() { _resizer?.Pause(); CancelMigrationOperation(); + CancelMigrationSkillOperation(); } private void ShowInitialState(bool shouldStartInitialRefresh) @@ -427,33 +436,56 @@ private async void RefreshUI() string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); System.IProgress progress = CreateProgressReporter(ct); - ThirdPartyToolMigrationPreview preview = - await Task.Run(async () => + ThirdPartyToolMigrationPreview preview; + try + { + preview = await Task.Run(async () => await _thirdPartyToolMigrationUseCase.PreviewMigrationAsync(projectRoot, progress, ct)); - if (ct.IsCancellationRequested) + await MainThreadSwitcher.SwitchToMainThread(); + } + catch (System.OperationCanceledException) + { + // Cancellation comes from window close or a superseding operation; that owner drives the UI. + return; + } + catch (System.Exception ex) { + // Without this async-void boundary the exception hits the sync context, the window + // stays on "Scanning..." forever, and the operation CTS leaks. + Debug.LogException(ex); + if (IsMigrationOperationActive(ct)) + { + CompleteMigrationOperation(ct); + ShowNotCheckedState(); + } + return; } - await MainThreadSwitcher.SwitchToMainThread(); if (ct.IsCancellationRequested) { return; } CompleteMigrationOperation(ct); - if (!preview.HasTargets) { ShowNoMigrationTargetsState(); return; } - ShowMigrationTargetsState(preview.FileCount); + ShowMigrationTargetsState(preview.FilePaths); } private async void HandleMigrateThirdPartyTools() { + if (!ConfirmMigrationApply( + _pendingMigrationFilePaths.Length, + (title, message, ok, cancel) => EditorUtility.DisplayDialog(title, message, ok, cancel))) + { + return; + } + CancellationToken ct = BeginMigrationOperation(); string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); ThirdPartyToolMigrationResult result = default; @@ -492,6 +524,19 @@ private async void HandleMigrateThirdPartyTools() isMigrationCompletionPending = false; } + catch (System.OperationCanceledException) + { + // The finally block already skips the interrupted-refresh when the token is canceled. + return; + } + catch (System.Exception ex) + { + // PR1 makes a mid-batch apply failure a designed path (rollback, then throw). Log it and + // return; the finally block sees the still-pending completion and rescans, so the UI + // reflects the rolled-back files. + Debug.LogException(ex); + return; + } finally { _isMigrating = false; @@ -533,9 +578,13 @@ private void ShowNotCheckedState() ScheduleResizeToContent(); } - private void ShowMigrationTargetsState(int fileCount) + private void ShowMigrationTargetsState(string[] filePaths) { - _view.ShowMigrationTargetsState(fileCount, _isMigrating); + Debug.Assert(filePaths != null, "filePaths must not be null"); + + _pendingMigrationFilePaths = filePaths; + string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); + _view.ShowMigrationTargetsState(filePaths, projectRoot, _isMigrating); ScheduleResizeToContent(); } @@ -579,6 +628,7 @@ private void HandleMigrationSkillTargetChanged(SkillsTarget target) private async void HandleToggleMigrationSkill() { + CancellationToken ct = BeginMigrationSkillOperation(); string projectRoot = UnityCliLoopPathResolver.GetProjectRoot(); SkillSetupTargetInfo target = CreateMigrationSkillTargetInfo(_migrationSkillTarget); SkillInstallState currentInstallState = @@ -600,21 +650,41 @@ await _skillSetupUseCase.RemoveV3MigrationSkillFilesAsync( projectRoot, targets, GroupMigrationSkillUnderUnityCliLoop, - CancellationToken.None); - return; + ct); } - - await _skillSetupUseCase.InstallV3MigrationSkillFilesAsync( - projectRoot, - targets, - GroupMigrationSkillUnderUnityCliLoop, - CancellationToken.None); + else + { + await _skillSetupUseCase.InstallV3MigrationSkillFilesAsync( + projectRoot, + targets, + GroupMigrationSkillUnderUnityCliLoop, + ct); + } + } + catch (System.OperationCanceledException) + { + // The window is closing or a newer toggle superseded this one; do not touch its UI. + return; + } + catch (System.Exception ex) + { + // Fall through so the tail refresh shows the real on-disk install state after a failure. + Debug.LogException(ex); } finally { _isUpdatingMigrationSkill = false; - RefreshMigrationSkillState(); } + + // The use case may complete off the main thread; UI Toolkit access below requires it. + await MainThreadSwitcher.SwitchToMainThread(); + if (!IsMigrationSkillOperationActive(ct)) + { + return; + } + + CompleteMigrationSkillOperation(ct); + RefreshMigrationSkillState(); } private static SkillSetupTargetInfo CreateMigrationSkillTargetInfo(SkillsTarget target) @@ -657,7 +727,27 @@ private void CancelMigrationOperation() _migrationOperationCts = null; } - private bool ConsumeShouldStartInitialRefresh() + private CancellationToken BeginMigrationSkillOperation() + { + CancelMigrationSkillOperation(); + CancellationTokenSource cts = new CancellationTokenSource(); + _migrationSkillOperationCts = cts; + return cts.Token; + } + + private void CancelMigrationSkillOperation() + { + if (_migrationSkillOperationCts == null) + { + return; + } + + _migrationSkillOperationCts.Cancel(); + _migrationSkillOperationCts.Dispose(); + _migrationSkillOperationCts = null; + } + + internal bool ConsumeShouldStartInitialRefresh() { if (!_shouldRefreshAfterCreateGui) { @@ -665,9 +755,7 @@ private bool ConsumeShouldStartInitialRefresh() } _shouldRefreshAfterCreateGui = false; - bool shouldAutoScanThirdPartyToolMigration = - GetSessionFlagsRepository().ConsumeShouldAutoScanThirdPartyToolMigration(); - return ShouldStartInitialRefresh(true, shouldAutoScanThirdPartyToolMigration); + return true; } private void TryStartInitialRefresh() @@ -697,5 +785,21 @@ private bool IsMigrationOperationActive(CancellationToken ct) { return _migrationOperationCts != null && _migrationOperationCts.Token.Equals(ct); } + + private void CompleteMigrationSkillOperation(CancellationToken ct) + { + if (!IsMigrationSkillOperationActive(ct)) + { + return; + } + + _migrationSkillOperationCts.Dispose(); + _migrationSkillOperationCts = null; + } + + private bool IsMigrationSkillOperationActive(CancellationToken ct) + { + return _migrationSkillOperationCts != null && _migrationSkillOperationCts.Token.Equals(ct); + } } } diff --git a/Packages/src/TemporarySkills~/v3-cli-invocation-migration/Skill/SKILL.md b/Packages/src/TemporarySkills~/v3-cli-invocation-migration/Skill/SKILL.md index 3dfa7c198..bfad39dbe 100644 --- a/Packages/src/TemporarySkills~/v3-cli-invocation-migration/Skill/SKILL.md +++ b/Packages/src/TemporarySkills~/v3-cli-invocation-migration/Skill/SKILL.md @@ -5,6 +5,8 @@ description: Migrate only uloop V2 CLI option syntax in agent skills, Markdown, # V3 CLI Invocation Migration +Agent-facing CLI migration candidates live in this skill. Installed-skill cleanup names stay in `SkillTargetInstaller` / `skills.go` `deprecatedSkillNames`. + Use this skill to update V2-era `uloop` CLI option syntax to V3 syntax in agent-facing docs and automation. @@ -37,8 +39,9 @@ Prefer `rg` for searches when available. If `rg` is unavailable, use the best av - For third-party tools, check the tool's current schema or documentation before deciding whether `false` means removal, a `--no-*` flag, or a renamed option. -- For first-party tools, use the reference table. `compile` and `run-tests` - have special renamed negative flags. +- For first-party tools, use the reference table. `compile`, `run-tests`, + `get-hierarchy`, `record-input`, and `replay-input` have special renamed + negative flags. - A valid edit is limited to replacing, adding, or removing a `uloop` option token and the boolean value attached to that option in the same invocation. - Preserve surrounding Markdown, shell, and PowerShell formatting. Do not @@ -63,6 +66,8 @@ Prefer `rg` for searches when available. If `rg` is unavailable, use the best av - `uloop` command lines and examples. - Boolean-looking CLI options: `--* true`, `--*=true`, `--* false`, `--*=false`. - First-party renamed options from the reference, including bare flags. -- Removed commands: `get-project-info` and `get-version`. Report these as - out-of-scope command migration candidates unless the user explicitly asked - to migrate removed commands. +- Removed or renamed commands: `get-project-info`, `get-version`, + `unity-search`, `execute-menu-item`, `get-menu-items`, + `get-unity-search-providers`, `get-provider-details`, and `capture-window` + (renamed to `screenshot`). Report these as out-of-scope command migration + candidates unless the user explicitly asked to migrate removed commands. diff --git a/Packages/src/TemporarySkills~/v3-cli-invocation-migration/Skill/references/first-party-v2-to-v3.md b/Packages/src/TemporarySkills~/v3-cli-invocation-migration/Skill/references/first-party-v2-to-v3.md index 21e00d48c..382e9f7c7 100644 --- a/Packages/src/TemporarySkills~/v3-cli-invocation-migration/Skill/references/first-party-v2-to-v3.md +++ b/Packages/src/TemporarySkills~/v3-cli-invocation-migration/Skill/references/first-party-v2-to-v3.md @@ -1,5 +1,7 @@ # First-Party V2 to V3 CLI Migration +Agent-facing CLI migration candidates live here. Installed-skill cleanup names stay in `SkillTargetInstaller` / `skills.go` `deprecatedSkillNames`. + Use this reference as the canonical option migration map. Search results are only candidates. Edit a match only after the surrounding context proves it is a V2 `uloop` invocation. @@ -21,8 +23,10 @@ Prefer `rg` when available, but any repository search tool is acceptable. - Search `uloop` first and inspect command examples, shell scripts, PowerShell scripts, and agent skills. - Search boolean-looking CLI syntax: `--` plus nearby `true` or `false`, including `--flag true`, `--flag=false`, and inline Markdown command examples. - Search renamed first-party option names: `wait-for-domain-reload`, `reload-external-scene-changes`, `force-recompile`, `save-before-run`, `show-overlay`, `include-components`, `include-inactive`, and `compile-only`. -- Search removed first-party commands only to report them as out-of-scope - command migration candidates: `get-project-info` and `get-version`. +- Search removed or renamed first-party commands only to report them as + out-of-scope command migration candidates: `get-project-info`, `get-version`, + `unity-search`, `execute-menu-item`, `get-menu-items`, + `get-unity-search-providers`, `get-provider-details`, and `capture-window`. - Skip generated installed skill copies under `.agents`, `.claude`, `.codex`, `.cursor`, `.gemini`, `.windsurf`, `.agent`, or equivalent target folders unless the user explicitly asks to migrate installed copies. ## Boolean Argument Rules @@ -30,9 +34,13 @@ Prefer `rg` when available, but any repository search tool is acceptable. | V2 form | V3 form | | --- | --- | | `--flag true` | `--flag` when the V3 option is a positive default-false boolean | +| `--flag=true` | `--flag` when the V3 option is a positive default-false boolean | | `--flag=false` | remove the option when the V3 default is already false | +| `--flag false` | remove the option when the V3 default is already false | | `--flag true` | remove the option when the V3 default is already true | +| `--flag=true` | remove the option when the V3 default is already true | | `--flag false` | use the V3 negative option when the V3 default is true | +| `--flag=false` | use the V3 negative option when the V3 default is true | For third-party tools, inspect the current tool schema or docs before choosing the replacement. Do not infer third-party negative flags from first-party conventions. @@ -65,3 +73,9 @@ For third-party tools, inspect the current tool schema or docs before choosing t | --- | --- | | `uloop get-project-info` | Report only unless the user explicitly asks for removed command migration. Do not guess from the command name alone. | | `uloop get-version` | Report only unless the user explicitly asks for removed command migration. Do not guess from the command name alone. | +| `uloop unity-search` | Report only unless the user explicitly asks for removed command migration. Do not guess from the command name alone. | +| `uloop execute-menu-item` | Report only unless the user explicitly asks for removed command migration. Do not guess from the command name alone. | +| `uloop get-menu-items` | Report only unless the user explicitly asks for removed command migration. Do not guess from the command name alone. | +| `uloop get-unity-search-providers` | Report only unless the user explicitly asks for removed command migration. Do not guess from the command name alone. | +| `uloop get-provider-details` | Report only unless the user explicitly asks for removed command migration. Do not guess from the command name alone. | +| `uloop capture-window` | Renamed to `uloop screenshot`. Report only unless the user explicitly asks for removed command migration. Do not guess from the command name alone. | diff --git a/cli/dispatcher/internal/dispatcher/skills_test.go b/cli/dispatcher/internal/dispatcher/skills_test.go index 05b587d58..a74f93444 100644 --- a/cli/dispatcher/internal/dispatcher/skills_test.go +++ b/cli/dispatcher/internal/dispatcher/skills_test.go @@ -1105,6 +1105,154 @@ func TestRunV3MigrationSkillUninstallRemovesAlternateLayout(t *testing.T) { } } +// Tests that an already-installed V3 migration skill still removes the alternate layout. +func TestInstallV3MigrationSkillForTargetRemovesAlternateLayoutWhenInstalled(t *testing.T) { + projectRoot := t.TempDir() + skillContent := `--- +name: v3-cli-invocation-migration +--- + +# temporary migration +` + writeV3MigrationSkillFixture(t, projectRoot, skillContent) + skills, err := collectV3MigrationSkillDefinition(projectRoot) + if err != nil { + t.Fatalf("collectV3MigrationSkillDefinition failed: %v", err) + } + target := targetConfigs["codex"] + baseDir, err := getSkillsBaseDir(projectRoot, target, false) + if err != nil { + t.Fatalf("getSkillsBaseDir failed: %v", err) + } + flatDir := getPreferredSkillDir(baseDir, v3MigrationSkillName, false) + groupedDir := getPreferredSkillDir(baseDir, v3MigrationSkillName, true) + if err := syncSkillDirectory(skills[0].sourceDirectory, flatDir); err != nil { + t.Fatalf("seed flat install failed: %v", err) + } + writeSkillFile(t, groupedDir, skillContent) + + result, err := installV3MigrationSkillForTarget(projectRoot, target, skills, false, false) + if err != nil { + t.Fatalf("installV3MigrationSkillForTarget failed: %v", err) + } + if result.installed != 0 || result.updated != 0 || result.skipped != 1 { + t.Fatalf("install result mismatch: %#v", result) + } + if _, err := os.Stat(filepath.Join(flatDir, "SKILL.md")); err != nil { + t.Fatalf("preferred flat skill should remain: %v", err) + } + if _, err := os.Stat(groupedDir); !os.IsNotExist(err) { + t.Fatalf("alternate grouped skill should be removed: %v", err) + } +} + +// Tests that an outdated V3 migration skill is re-synced and counted as updated. +func TestInstallV3MigrationSkillForTargetUpdatesOutdatedSkill(t *testing.T) { + projectRoot := t.TempDir() + writeV3MigrationSkillFixture(t, projectRoot, `--- +name: v3-cli-invocation-migration +--- + +# temporary migration v1 +`) + skills, err := collectV3MigrationSkillDefinition(projectRoot) + if err != nil { + t.Fatalf("collectV3MigrationSkillDefinition failed: %v", err) + } + target := targetConfigs["codex"] + baseDir, err := getSkillsBaseDir(projectRoot, target, false) + if err != nil { + t.Fatalf("getSkillsBaseDir failed: %v", err) + } + flatDir := getPreferredSkillDir(baseDir, v3MigrationSkillName, false) + if err := syncSkillDirectory(skills[0].sourceDirectory, flatDir); err != nil { + t.Fatalf("seed flat install failed: %v", err) + } + + updatedContent := `--- +name: v3-cli-invocation-migration +--- + +# temporary migration v2 +` + writeV3MigrationSkillFixture(t, projectRoot, updatedContent) + skills, err = collectV3MigrationSkillDefinition(projectRoot) + if err != nil { + t.Fatalf("collectV3MigrationSkillDefinition failed after update: %v", err) + } + + result, err := installV3MigrationSkillForTarget(projectRoot, target, skills, false, false) + if err != nil { + t.Fatalf("installV3MigrationSkillForTarget failed: %v", err) + } + if result.installed != 0 || result.updated != 1 || result.skipped != 0 { + t.Fatalf("install result mismatch: %#v", result) + } + installedContent, err := os.ReadFile(filepath.Join(flatDir, "SKILL.md")) + if err != nil { + t.Fatalf("read installed skill failed: %v", err) + } + if !strings.Contains(string(installedContent), "temporary migration v2") { + t.Fatalf("outdated skill should be re-synced: %s", installedContent) + } +} + +// Tests that uninstalling a grouped V3 migration skill removes an empty managed parent directory. +func TestRunV3MigrationSkillUninstallRemovesEmptyGroupedParent(t *testing.T) { + projectRoot := t.TempDir() + target := targetConfigs["codex"] + baseDir, err := getSkillsBaseDir(projectRoot, target, false) + if err != nil { + t.Fatalf("getSkillsBaseDir failed: %v", err) + } + groupedDir := getPreferredSkillDir(baseDir, v3MigrationSkillName, true) + managedParent := filepath.Join(baseDir, managedSkillsDir) + writeSkillFile(t, groupedDir, "---\nname: v3-cli-invocation-migration\n---\n") + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + options := skillCommandOptions{targets: []skillTarget{target}} + + code := runV3MigrationSkillUninstall(projectRoot, options, stdout, stderr) + + if code != 0 { + t.Fatalf("uninstall should succeed: code=%d stderr=%s", code, stderr.String()) + } + if _, err := os.Stat(groupedDir); !os.IsNotExist(err) { + t.Fatalf("grouped migration skill should be removed: %v", err) + } + if _, err := os.Stat(managedParent); !os.IsNotExist(err) { + t.Fatalf("empty managed parent should be removed: %v", err) + } +} + +// Tests that install-v3-migration without targets prints guidance instead of installing. +func TestRunV3MigrationSkillsSubcommandWithoutTargetsPrintsGuidance(t *testing.T) { + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + + code := runV3MigrationSkillsSubcommand( + "install-v3-migration", + t.TempDir(), + skillCommandOptions{}, + stdout, + stderr, + ) + + if code != 0 { + t.Fatalf("guidance path should succeed: code=%d stderr=%s", code, stderr.String()) + } + output := stdout.String() + if !strings.Contains(output, "Please specify at least one target for 'install-v3-migration'") { + t.Fatalf("guidance should name the subcommand: %s", output) + } + for _, id := range allSkillTargetIDs { + flag := "--" + id + if !strings.Contains(output, flag) { + t.Fatalf("target guidance missing %s:\n%s", flag, output) + } + } +} + // writeTestSkill seeds a SKILL.md fixture at projectRoot/relativeDir via the // shared clitest.WriteSkillFile helper, which owns the CRLF normalization. func writeTestSkill(t *testing.T, projectRoot string, relativeDir string, content string) { @@ -1112,6 +1260,30 @@ func writeTestSkill(t *testing.T, projectRoot string, relativeDir string, conten clitest.WriteSkillFile(t, projectRoot, relativeDir, "SKILL.md", content) } +func writeV3MigrationSkillFixture(t *testing.T, projectRoot string, skillContent string) { + t.Helper() + writePackageRootMarker(t, projectRoot) + writeTestSkill(t, projectRoot, "Packages/src/TemporarySkills~/v3-cli-invocation-migration/Skill", skillContent) + referenceDir := filepath.Join( + projectRoot, + "Packages", + "src", + "TemporarySkills~", + "v3-cli-invocation-migration", + "Skill", + "references", + ) + if err := os.MkdirAll(referenceDir, 0o755); err != nil { + t.Fatalf("failed to create reference dir: %v", err) + } + if err := os.WriteFile( + filepath.Join(referenceDir, "first-party-v2-to-v3.md"), + []byte("# reference\n"), + 0o644); err != nil { + t.Fatalf("failed to write reference: %v", err) + } +} + func writeManifest(t *testing.T, projectRoot string, content string) { t.Helper() manifestDir := filepath.Join(projectRoot, "Packages") diff --git a/cli/dispatcher/internal/dispatcher/skills_v3_migration.go b/cli/dispatcher/internal/dispatcher/skills_v3_migration.go index 628024e96..c15cd9853 100644 --- a/cli/dispatcher/internal/dispatcher/skills_v3_migration.go +++ b/cli/dispatcher/internal/dispatcher/skills_v3_migration.go @@ -70,17 +70,19 @@ func installV3MigrationSkillForTarget(projectRoot string, target skillTarget, sk return skillInstallResult{}, err } destinationDir := getPreferredSkillDir(baseDir, skill.name, grouped) - if status == "installed" { - result.skipped++ - continue - } - if err := syncSkillDirectory(skill.sourceDirectory, destinationDir); err != nil { - return skillInstallResult{}, err + if status != "installed" { + if err := syncSkillDirectory(skill.sourceDirectory, destinationDir); err != nil { + return skillInstallResult{}, err + } } alternateDir := getPreferredSkillDir(baseDir, skill.name, !grouped) if err := os.RemoveAll(alternateDir); err != nil { return skillInstallResult{}, err } + if status == "installed" { + result.skipped++ + continue + } if status == "outdated" { result.updated++ continue