Preserve legacy behavior when loading sln with duplicate project ids.

global.json

@@ -1,6 +1,6 @@
 {
   "sdk": {
-    "version": "9.0.102",
+    "version": "9.0.200",
     "rollForward": "patch",
     "allowPrerelease": false
   },

src/Microsoft.VisualStudio.SolutionPersistence/Serializer/SlnV12/SlnFileV12Serializer.Reader.cs

@@ -69,6 +69,11 @@ internal ValueTask<SolutionModel> ParseAsync(ISolutionSerializer serializer, str
             // Some property bags need to be loaded after all projects have been resolved.
             List<(SolutionItemModel, SolutionPropertyBag)> delayLoadProperties = [];
 
+            // Keep track of projects with duplicate ids so that we can
+            // duplicate the configuration after all projects have been loaded.
+            // This preserves the orginial buggy behavior of the solution parser.
+            List<(Guid NewId, SolutionProjectModel DuplicateProject)> fixedProjectIds = [];
+
             try
             {
                 using (solutionModel.SuspendProjectValidation())
@@ -87,7 +92,7 @@ internal ValueTask<SolutionModel> ParseAsync(ISolutionSerializer serializer, str
                             case LineType.Project:
                                 this.TarnishIf(inProject);
                                 inProject = true;
-                                currentProject = this.ReadProjectInfo(solutionModel, ref tokenizer);
+                                currentProject = this.ReadProjectInfo(solutionModel, ref tokenizer, fixedProjectIds);
                                 break;
 
                             case LineType.EndProject:
@@ -200,6 +205,17 @@ internal ValueTask<SolutionModel> ParseAsync(ISolutionSerializer serializer, str
                         this.TarnishIf(!item.AddSlnProperties(properties));
                     }
 
+                    foreach ((Guid newId, SolutionProjectModel duplicateProject) in fixedProjectIds)
+                    {
+                        // The newly generated id will not have configurations, so make a
+                        // copy of the original project configurations.
+                        // This preserves the orginial buggy behavior of the solution parser.
+                        if (solutionModel.FindItemById(newId) is SolutionProjectModel project)
+                        {
+                            project.ProjectConfigurationRules = duplicateProject.ProjectConfigurationRules;
+                        }
+                    }
+
                     VisualStudioProperties vsProperties = solutionModel.VisualStudioProperties;
                     vsProperties.OpenWith = CommentToOpenWithVS(openWithVsVersion.AsSpan());
                     vsProperties.MinimumVersion = minVsVersion;
@@ -218,6 +234,9 @@ internal ValueTask<SolutionModel> ParseAsync(ISolutionSerializer serializer, str
 
             return new ValueTask<SolutionModel>(solutionModel);
 
+            // Adds the property bag to the project or folder.
+            // Handles special cases for the sln parser.
+            // Returns false if there was an error reading the properties and the solution file should be considered tarnished.
             static bool AddProjectProperties(
                 SolutionItemModel? currentProject,
                 SolutionPropertyBag? currentPropertyBag,
@@ -498,7 +517,7 @@ private bool ReadLine(out StringTokenizer lineScanner)
             return checkOnly ? null : new SolutionPropertyBag(sectionName.ToString(), scope);
         }
 
-        private SolutionItemModel ReadProjectInfo(SolutionModel solution, ref StringTokenizer tokenizer)
+        private SolutionItemModel ReadProjectInfo(SolutionModel solution, ref StringTokenizer tokenizer, List<(Guid NewId, SolutionProjectModel DuplicateProject)> fixedProjectIds)
         {
             // Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "App1", "App1\App1.csproj", "{B0D4AB54-EB86-4C88-A2A4-C55D0C200244}"
             //         ^  <- this is tokenizer pos.
@@ -527,14 +546,16 @@ private SolutionItemModel ReadProjectInfo(SolutionModel solution, ref StringToke
             this.SolutionAssert(!projectUniqueId.IsEmpty, Errors.MissingProjectId);
             this.TarnishIf(!Guid.TryParse(projectUniqueId.ToString(), out Guid projectId));
 
+            SolutionItemModel? duplicateItem = solution.FindItemById(projectId);
+
             if (projectTypeId == ProjectTypeTable.SolutionFolder)
             {
 #pragma warning disable CS0618 // Type or member is obsolete (Temporaily create a potentially invalid solution folder until nested projects is interpreted.)
                 SolutionFolderModel folder = solution.CreateSlnFolder(name: displayName.ToString());
 #pragma warning restore CS0618 // Type or member is obsolete
 
                 // Solution folders with duplicate ids should not error when reading sln files to preserve legacy behavior.
-                if (solution.FindItemById(projectId) is not null)
+                if (duplicateItem is not null)
                 {
                     projectId = Guid.NewGuid();
                     this.tarnished = true;
@@ -545,16 +566,41 @@ private SolutionItemModel ReadProjectInfo(SolutionModel solution, ref StringToke
             }
             else
             {
+                string path = PathExtensions.ConvertBackslashToModel(relativePath.ToString());
+
+                // This should error, or remove any configuration associated with the duplicate id.
+                // However some old parsers would just ignore the duplicate id and continue.
+                if (duplicateItem is SolutionFolderModel duplicateFolder)
+                {
+                    duplicateFolder.Id = Guid.NewGuid();
+                    this.tarnished = true;
+                }
+                else if (duplicateItem is SolutionProjectModel duplicateProject)
+                {
+                    projectId = CreateNewProjectId(solution, path);
+                    this.tarnished = true;
+
+                    // Record the new project id so it's configuration can be duplicated.
+                    fixedProjectIds.Add((projectId, duplicateProject));
+                }
+
 #pragma warning disable CS0618 // Type or member is obsolete (Temporaily create a potentially invalid solution folder until nested projects is interpreted.)
                 SolutionProjectModel project = solution.AddSlnProject(
-                    filePath: PathExtensions.ConvertBackslashToModel(relativePath.ToString()),
+                    filePath: path,
                     projectTypeId: projectTypeId,
                     folder: null);
 #pragma warning restore CS0618 // Type or member is obsolete
                 project.Id = projectId;
                 project.DisplayName = displayName.ToString();
                 return project;
             }
+
+            // If creating a new project id, go ahead and try to use the slnx default id.
+            static Guid CreateNewProjectId(SolutionModel solution, string path)
+            {
+                Guid id = DefaultIdGenerator.CreateIdFrom(path);
+                return solution.FindItemById(id) is null ? id : Guid.NewGuid();
+            }
         }
 
         // Condition that would mark solution file as "tarnished"

test/Microsoft.VisualStudio.SolutionPersistence.Tests/Serialization/Format.cs

@@ -14,12 +14,6 @@ public class Format
     [Fact]
     public async Task ConvertASCIItoUTF8Async()
     {
-        if (IsMono)
-        {
-            // Mono is not supported.
-            return;
-        }
-
         const string asciiFolderName = "directory123";
         const string utf8FolderName = "répertoire123";
 

test/Microsoft.VisualStudio.SolutionPersistence.Tests/Serialization/IdGenerator.cs

@@ -77,12 +77,6 @@ public void CheckGeneratedIds()
     [Fact]
     public async Task CheckModelIds()
     {
-        if (IsMono)
-        {
-            // Mono is not supported.
-            return;
-        }
-
         SolutionModel solution = await SolutionSerializers.SlnXml.OpenAsync(SlnAssets.XmlSlnxMany.Stream, CancellationToken.None);
 
         SolutionProjectModel? urlProject = solution.FindProject("http://localhost:8080");

test/Microsoft.VisualStudio.SolutionPersistence.Tests/Serialization/InvalidSolutions.cs

@@ -29,12 +29,6 @@ public Task InvalidXmlSlnxAsync()
     [Fact]
     public async Task InvalidSlnxAsync()
     {
-        if (IsMono)
-        {
-            // Mono is not supported.
-            return;
-        }
-
         ResourceStream wrongRoot = SlnAssets.LoadResource("Invalid/WrongRoot.slnx");
         string wrongRootFile = wrongRoot.SaveResourceToTempFile();
 
@@ -55,12 +49,6 @@ public async Task InvalidSlnxAsync()
     [Fact]
     public async Task InvalidSlnAsync()
     {
-        if (IsMono)
-        {
-            // Mono is not supported.
-            return;
-        }
-
         ResourceStream invalidSln = SlnAssets.LoadResource("Invalid/Invalid.sln");
         string invalidSlnFile = invalidSln.SaveResourceToTempFile();
 
@@ -239,4 +227,96 @@ public async Task DuplicateFolderIdSlnxAsync()
         Assert.Equal(3, ex.Line);
         Assert.Equal(4, ex.Column);
     }
+
+    /// <summary>
+    /// Ensure the slnx parser does fail if project id collides with folder id.
+    /// Expected behavior is the folder id is changed to a new value.
+    /// </summary>
+    [Fact]
+    public async Task DuplicateItemIdSlnAsync()
+    {
+        ResourceStream duplicateId = SlnAssets.LoadResource("Invalid/DuplicateItemId.sln");
+        SolutionModel solution = await SolutionSerializers.SlnFileV12.OpenAsync(duplicateId.Stream, CancellationToken.None);
+        Assert.NotNull(solution.SerializerExtension);
+        Assert.True(solution.SerializerExtension.Tarnished);
+
+        SolutionProjectModel? projectModel = solution.FindProject("DuplicateProjectId.csproj");
+        Assert.NotNull(projectModel);
+        Assert.Equal(projectModel.Id, new Guid("11111111-1111-1111-1111-111111111111"));
+
+        SolutionFolderModel? folderModel = solution.FindFolder("/DuplicateFolderId/");
+        Assert.NotNull(folderModel);
+        Assert.NotEqual(folderModel.Id, new Guid("11111111-1111-1111-1111-111111111111"));
+    }
+
+    /// <summary>
+    /// Recreate legacy bug behavior when duplicate project id guids are found in .sln files.
+    /// Use corrupt configuration and change second project id to new value.
+    /// </summary>
+    [Fact]
+    public async Task DuplicateProjectIdSlnAsync()
+    {
+        ResourceStream duplicateId = SlnAssets.LoadResource("Invalid/DuplicateProjectId.sln");
+        SolutionModel solution = await SolutionSerializers.SlnFileV12.OpenAsync(duplicateId.Stream, CancellationToken.None);
+        Assert.NotNull(solution.SerializerExtension);
+        Assert.True(solution.SerializerExtension.Tarnished);
+
+        SolutionProjectModel? project = solution.FindProject("DuplicateProjectId.csproj");
+        Assert.NotNull(project);
+        Assert.Equal(project.Id, new Guid("8BADBEEF-1111-2222-3333-444444444444"));
+
+        SolutionProjectModel? project2 = solution.FindProject("DuplicateProjectIdOpposite.csproj");
+        Assert.NotNull(project2);
+        Assert.NotEqual(project2.Id, new Guid("8BADBEEF-2222-3333-4444-555555555555"));
+
+        (string? buildType, string? platform, bool build, bool deploy) = project.GetProjectConfiguration(BuildTypeNames.Debug, PlatformNames.AnySpaceCPU);
+
+        (string? buildType2, string? platform2, bool build2, bool deploy2) = project2.GetProjectConfiguration(BuildTypeNames.Debug, PlatformNames.AnySpaceCPU);
+
+        // The configuration from the "opposite" project should be used by both as it appears last in the file.
+        Assert.Equal(BuildTypeNames.Release, buildType);
+        Assert.Equal(PlatformNames.AnySpaceCPU, platform);
+        Assert.True(build);
+        Assert.False(deploy);
+
+        // The configuration from the "opposite" project should be used by both as it appears last in the file.
+        Assert.Equal(BuildTypeNames.Release, buildType2);
+        Assert.Equal(PlatformNames.AnySpaceCPU, platform2);
+        Assert.True(build2);
+        Assert.False(deploy2);
+
+        // Compare the new model to the 'After' sln (Change the new Guid to FFFF to match).
+        project2.Id = new Guid("FFFFFFFF-FFFF-FFFF-FFFF-FFFFFFFFFFFF");
+        ResourceStream duplicateIdAfter = SlnAssets.LoadResource("DuplicateProjectId-After.sln");
+        AssertSolutionsAreEqual(duplicateIdAfter.ToLines(), await solution.ToLinesAsync(SolutionSerializers.SlnFileV12));
+    }
+
+    /// <summary>
+    /// Ensure the slnx parser does fail on duplicate project guids.
+    /// </summary>
+    [Fact]
+    public async Task DuplicateProjectIdSlnxAsync()
+    {
+        ResourceStream duplicateId = SlnAssets.LoadResource("Invalid/DuplicateProjectId.slnx");
+        SolutionException ex = await Assert.ThrowsAsync<SolutionException>(
+            async () => _ = await SolutionSerializers.SlnXml.OpenAsync(duplicateId.Stream, CancellationToken.None));
+
+        Assert.StartsWith(string.Format(Errors.DuplicateItemRef_Args2, "8badbeef-1111-2222-3333-444444444444", nameof(SolutionProjectModel)), ex.Message);
+        Assert.Equal(5, ex.Line);
+        Assert.Equal(6, ex.Column);
+    }
+
+    /// <summary>
+    /// Checks that an error is thrown if the solution contains an invalid project type id.
+    /// </summary>
+    [Fact]
+    public async Task InvalidProjectTypeSlnAsync()
+    {
+        ResourceStream invalidProjectType = SlnAssets.LoadResource("Invalid/InvalidProjectType.sln");
+        SolutionException ex = await Assert.ThrowsAsync<SolutionException>(
+            async () => _ = await SolutionSerializers.SlnFileV12.OpenAsync(invalidProjectType.Stream, CancellationToken.None));
+        Assert.StartsWith(Errors.InvalidProjectType, ex.Message);
+        Assert.Equal(5, ex.Line);
+        Assert.Null(ex.Column);
+    }
 }

test/Microsoft.VisualStudio.SolutionPersistence.Tests/Serialization/LegacyValues.cs

@@ -40,12 +40,6 @@ public async Task TrimVisualStudioPropertiesAsync()
     [Fact]
     public async Task IgnoreDisplayNameSlnx()
     {
-        if (IsMono)
-        {
-            // Mono is not supported.
-            return;
-        }
-
         await ValidateModifiedSolutionAsync(CreateModifiedModel, SlnAssets.XmlSlnxSingleNativeProject, SlnAssets.XmlSlnxSingleNativeProject);
 
         static void CreateModifiedModel(SolutionModel solution)
@@ -64,12 +58,6 @@ static void CreateModifiedModel(SolutionModel solution)
     [Fact]
     public async Task IgnoreDisplayNameSln()
     {
-        if (IsMono)
-        {
-            // Mono is not supported.
-            return;
-        }
-
         await ValidateModifiedSolutionAsync(CreateModifiedModel, SlnAssets.ClassicSlnSingleNativeProject, SlnAssets.ClassicSlnSingleNativeProject, SolutionSerializers.SlnFileV12);
 
         static void CreateModifiedModel(SolutionModel solution)

test/Microsoft.VisualStudio.SolutionPersistence.Tests/Serialization/MakeSlnx.MakeSlnxFixture.cs

@@ -24,7 +24,10 @@ public class MakeSlnxFixture
         /// </summary>
         public MakeSlnxFixture()
         {
-            string outputDirectory = Path.Combine(Path.GetTempPath(), OutputDirectory);
+            // Split output based on the .NET version (so we can run tests in parallel)
+            string netVersion = "net" + Environment.Version.ToString(3);
+
+            string outputDirectory = Path.Combine(Path.GetTempPath(), OutputDirectory, netVersion);
             if (Directory.Exists(outputDirectory))
             {
                 Directory.Delete(outputDirectory, true);