Expose SDK as an ICollection to enable extention (#642)

* Expose SDK as an ICollection to enable extention

* Fix Tests

* PR Feedback

* Change IsSDK implementation

* Fix Integration tests

* More fixes for tests

* Change to ICollection from List

* PR Feedback

* Remove usage of list in the Remove method

* Remove isSDK

* Reverting the IsSdk removal

* Modifying contains calls

* Add some basic tests for SdkCollection

* Fix tests

* Fix tests

* Missed setting

* PR Feedback

* Add some more tests

src/common/Microsoft.DotNet.UpgradeAssistant.Abstractions/IProjectFile.cs

@@ -9,7 +9,7 @@ namespace Microsoft.DotNet.UpgradeAssistant
 {
     public interface IProjectFile
     {
-        string Sdk { get; }
+        ICollection<string> Sdk { get; }
 
         public bool IsSdk { get; }
 

src/components/Microsoft.DotNet.UpgradeAssistant.MSBuild/ComponentIdentifier.cs

@@ -36,7 +36,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.MSBuild
         private static ProjectComponents GetSDKProjectComponents(IProject project, IProjectFile file)
         {
             var components = ProjectComponents.None;
-            if (file.Sdk.Equals(MSBuildConstants.WebSdk, StringComparison.OrdinalIgnoreCase))
+            if (file.Sdk.Contains(MSBuildConstants.WebSdk))
             {
                 components |= ProjectComponents.AspNetCore;
             }
@@ -53,7 +53,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.MSBuild
                 components |= ProjectComponents.WindowsDesktop;
             }
 
-            if (file.Sdk.Equals(MSBuildConstants.DesktopSdk, StringComparison.OrdinalIgnoreCase))
+            if (file.Sdk.Contains(MSBuildConstants.DesktopSdk))
             {
                 components |= ProjectComponents.WindowsDesktop;
             }

src/components/Microsoft.DotNet.UpgradeAssistant.MSBuild/MSBuildProject.File.cs

@@ -38,26 +38,12 @@ namespace Microsoft.DotNet.UpgradeAssistant.MSBuild
             }
         }
 
-        public string Sdk
-        {
-            get
-            {
-                var sdk = ProjectRoot.Sdk;
-
-                if (sdk is null)
-                {
-                    throw new ArgumentOutOfRangeException("Should check IsSdk property first");
-                }
-
-                return sdk;
-            }
-        }
-
-        public bool IsSdk =>
-            ProjectRoot.Sdk is not null && ProjectRoot.Sdk.Contains(MSBuildConstants.DefaultSDK, StringComparison.OrdinalIgnoreCase);
+        public bool IsSdk => Sdk.Any();
 
         public ICollection<string> Imports => new ImportsCollection(ProjectRoot);
 
+        public ICollection<string> Sdk => new SdkCollection(ProjectRoot);
+
         public void SetTFM(TargetFrameworkMoniker tfm) => new TargetFrameworkMonikerCollection(this, _comparer).SetTargetFramework(tfm);
 
         public void AddPackages(IEnumerable<NuGetReference> references)

src/components/Microsoft.DotNet.UpgradeAssistant.MSBuild/MSBuildProject.cs

@@ -81,7 +81,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.MSBuild
 
         private ProjectOutputType GetDefaultOutputType()
         {
-            if (IsSdk && MSBuildConstants.SDKsWithExeDefaultOutputType.Contains(Sdk, StringComparer.OrdinalIgnoreCase))
+            if (Sdk.Any(p => MSBuildConstants.SDKsWithExeDefaultOutputType.Contains(p, StringComparer.OrdinalIgnoreCase)))
             {
                 return ProjectOutputType.Exe;
             }

src/components/Microsoft.DotNet.UpgradeAssistant.MSBuild/SdkCollection.cs

@@ -0,0 +1,79 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using Microsoft.Build.Construction;
+
+namespace Microsoft.DotNet.UpgradeAssistant.MSBuild
+{
+    public class SdkCollection : ICollection<string>
+    {
+        private readonly ProjectRootElement _projectRoot;
+
+        public SdkCollection(ProjectRootElement projectRoot)
+        {
+            _projectRoot = projectRoot;
+        }
+
+        private string[] SplitSdks() => _projectRoot.Sdk.Split(";", StringSplitOptions.RemoveEmptyEntries);
+
+        public bool Contains(string item)
+        {
+            return SplitSdks().Contains(item, StringComparer.OrdinalIgnoreCase);
+        }
+
+        public void Add(string item)
+        {
+            if (!Contains(item))
+            {
+                _projectRoot.Sdk = string.Concat(_projectRoot.Sdk, ";", item);
+            }
+        }
+
+        public void Clear()
+        {
+            _projectRoot.Sdk = string.Empty;
+        }
+
+        public int Count
+        {
+            get
+            {
+                return SplitSdks().Length;
+            }
+        }
+
+        public bool IsReadOnly
+        {
+            get { return false; }
+        }
+
+        public bool Remove(string item)
+        {
+            if (Contains(item))
+            {
+                var sdkList = SplitSdks().Where(p => p != item);
+                _projectRoot.Sdk = string.Join(";", sdkList);
+                return true;
+            }
+
+            return false;
+        }
+
+        public void CopyTo(string[] array, int arrayIndex)
+        {
+            ((ICollection<string>)this).CopyTo(array, arrayIndex);
+        }
+
+        public IEnumerator<string> GetEnumerator()
+        {
+             return SplitSdks().Cast<string>().GetEnumerator();
+        }
+
+        IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
+    }
+}

src/extensions/web/Microsoft.DotNet.UpgradeAssistant.Extensions.Web/WebSdkCleanupAnalyzer.cs

@@ -1,6 +1,5 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
-
 using System;
 using System.Linq;
 using System.Threading;
@@ -37,7 +36,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Extensions.Web
             // Check SDK directly (instead of using project.Components) since having the FrameworkReference counts as
             // having the AspNetCore component and this analyzer is specifically insterested in cases where both the SDK
             // and the framework reference are present.
-            if (!projectRoot.IsSdk || !projectRoot.Sdk.Equals(WebSdk, StringComparison.OrdinalIgnoreCase))
+            if (!projectRoot.Sdk.Contains(WebSdk))
             {
                 return Task.CompletedTask;
             }

tests/components/Microsoft.DotNet.UpgradeAssistant.MSBuild.Tests/ComponentIdentifierTests.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
 using Autofac.Extras.Moq;
@@ -27,7 +28,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.MSBuild.Tests
 
             var projectFile = mock.Mock<IProjectFile>();
             projectFile.Setup(f => f.IsSdk).Returns(true);
-            projectFile.Setup(f => f.Sdk).Returns(string.Empty);
+            projectFile.Setup(f => f.Sdk).Returns(Array.Empty<string>());
             projectFile.Setup(f => f.GetPropertyValue(It.IsAny<string>())).Returns(string.Empty);
             projectFile.Setup(f => f.GetPropertyValue(propertyName)).Returns(value);
 
@@ -45,18 +46,18 @@ namespace Microsoft.DotNet.UpgradeAssistant.MSBuild.Tests
             Assert.Equal(expected, components);
         }
 
-        [InlineData("", ProjectComponents.None)]
-        [InlineData("Microsoft.NET.Sdk.Web", ProjectComponents.AspNetCore)]
-        [InlineData("Microsoft.NET.Sdk.Desktop", ProjectComponents.WindowsDesktop)]
+        [InlineData(new[] { "" }, ProjectComponents.None)]
+        [InlineData(new[] { "Microsoft.NET.Sdk.Web" }, ProjectComponents.AspNetCore)]
+        [InlineData(new[] { "Microsoft.NET.Sdk.Desktop" }, ProjectComponents.WindowsDesktop)]
         [Theory]
-        public async Task SdkTypesAsync(string sdk, ProjectComponents expected)
+        public async Task SdkTypesAsync(string[] sdk, ProjectComponents expected)
         {
             // Arrange
             using var mock = AutoMock.GetLoose();
 
             var projectFile = mock.Mock<IProjectFile>();
             projectFile.Setup(f => f.IsSdk).Returns(true);
-            projectFile.Setup(f => f.Sdk).Returns(sdk);
+            projectFile.Setup(f => f.Sdk).Returns(new HashSet<string>(sdk, StringComparer.OrdinalIgnoreCase));
             projectFile.Setup(f => f.GetPropertyValue(It.IsAny<string>())).Returns(string.Empty);
 
             var project = mock.Mock<IProject>();
@@ -96,7 +97,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.MSBuild.Tests
 
             var projectFile = mock.Mock<IProjectFile>();
             projectFile.Setup(f => f.IsSdk).Returns(true);
-            projectFile.Setup(f => f.Sdk).Returns(string.Empty);
+            projectFile.Setup(f => f.Sdk).Returns(Array.Empty<string>());
             projectFile.Setup(f => f.GetPropertyValue(It.IsAny<string>())).Returns(string.Empty);
 
             var project = mock.Mock<IProject>();
@@ -135,7 +136,6 @@ namespace Microsoft.DotNet.UpgradeAssistant.MSBuild.Tests
             using var mock = AutoMock.GetLoose();
 
             var projectFile = mock.Mock<IProjectFile>();
-            projectFile.Setup(f => f.IsSdk).Returns(false);
 
             var project = mock.Mock<IProject>();
             project.Setup(p => p.GetFile()).Returns(projectFile.Object);

tests/components/Microsoft.DotNet.UpgradeAssistant.MSBuild.Tests/SdkCollectionTests.cs

@@ -0,0 +1,102 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Autofac.Extras.Moq;
+using Microsoft.Build.Construction;
+using Xunit;
+
+namespace Microsoft.DotNet.UpgradeAssistant.MSBuild.Tests
+{
+    [Collection(MSBuildStepTestCollection.Name)]
+    public class SdkCollectionTests
+    {
+        private const string DefaultSDK = "Microsoft.NET.Sdk";
+        private const string WebSDK = "Microsoft.NET.Sdk.Web";
+
+        [Fact]
+        public void SdkCollectionEmptyTest()
+        {
+            // Arrange
+            using var mock = AutoMock.GetLoose();
+
+            var projectRoot = ProjectRootElement.Create();
+            var sdkCollection = new SdkCollection(projectRoot);
+
+            // Assert
+            Assert.Empty(sdkCollection);
+        }
+
+        [InlineData(DefaultSDK)]
+        [Theory]
+        public void SdkCollectionContainsTest(string sdk)
+        {
+            // Arrange
+            using var mock = AutoMock.GetLoose();
+
+            var projectRoot = ProjectRootElement.Create();
+            projectRoot.Sdk = sdk;
+            var sdkCollection = new SdkCollection(projectRoot);
+
+            // Assert
+            Assert.Contains(sdk, sdkCollection);
+        }
+
+        [InlineData(WebSDK)]
+        [Theory]
+        public void SdkCollectionNotContainsTest(string sdk)
+        {
+            // Arrange
+            using var mock = AutoMock.GetLoose();
+
+            var projectRoot = ProjectRootElement.Create();
+            projectRoot.Sdk = DefaultSDK;
+            var sdkCollection = new SdkCollection(projectRoot);
+
+            // Assert
+            Assert.DoesNotContain(sdk, sdkCollection);
+        }
+
+        [InlineData(WebSDK, new string[] { DefaultSDK, WebSDK })]
+        [Theory]
+        public void SdkCollectionAddTest(string sdk, string[] expected)
+        {
+            // Arrange
+            using var mock = AutoMock.GetLoose();
+
+            var projectRoot = ProjectRootElement.Create();
+            projectRoot.Sdk = DefaultSDK;
+
+            var sdkCollection = new SdkCollection(projectRoot);
+
+            // Act
+            sdkCollection.Add(sdk);
+
+            // Assert
+            Assert.Equal(
+                sdkCollection,
+                expected);
+        }
+
+        [InlineData(WebSDK, new string[] { DefaultSDK }, true)]
+        [InlineData("System.NET.Sdk.Test", new string[] { DefaultSDK, WebSDK }, false)]
+        [Theory]
+        public void SdkCollectionRemoveTest(string sdkToRemove, string[] expected, bool expectedStatus)
+        {
+            // Arrange
+            using var mock = AutoMock.GetLoose();
+
+            var projectRoot = ProjectRootElement.Create();
+            projectRoot.Sdk = string.Concat(DefaultSDK, ";", WebSDK);
+
+            var sdkCollection = new SdkCollection(projectRoot);
+
+            // Act and Assert
+            Assert.Equal(expectedStatus, sdkCollection.Remove(sdkToRemove));
+
+            // Assert
+            Assert.Equal(
+                sdkCollection,
+                expected);
+        }
+    }
+}

tests/extensions/web/Microsoft.DotNet.UpgradeAssistant.Extensions.Web.Tests/WebSdkCleanupAnalyzerTests.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.IO;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
@@ -35,16 +36,16 @@ namespace Microsoft.DotNet.UpgradeAssistant.Extensions.Web.Tests
         }
 
         [Theory]
-        [InlineData("Microsoft.NET.Sdk.Web", new[] { "Microsoft.AspNetCore.App" }, new[] { "Microsoft.AspNetCore.App" })] // Vanilla positive case
-        [InlineData("Microsoft.NET.Sdk.Web", new[] { "X", "Y", "Microsoft.AspNetCore.App", "Z" }, new[] { "Microsoft.AspNetCore.App" })] // Multiple references
-        [InlineData("Microsoft.NET.Sdk.Web", new[] { "Microsoft.AspNetCore.App", "Microsoft.AspNetCore.App" }, new[] { "Microsoft.AspNetCore.App" })] // Duplicate references
-        [InlineData("microsoft.NET.SDK.Web", new[] { "microsoft.ASPNetCore.App" }, new[] { "microsoft.ASPNetCore.App" })] // Ununusual case
-        [InlineData("Microsoft.NET.Sdk", new[] { "Microsoft.AspNetCore.App" }, new string[0])] // Wrong SDK
-        [InlineData("Microsoft.NET.Sdk.Web", new[] { "A", "B", "C" }, new string[0])] // Wrong framework reference
+        [InlineData(new[] { "Microsoft.NET.Sdk.Web" }, new[] { "Microsoft.AspNetCore.App" }, new[] { "Microsoft.AspNetCore.App" })] // Vanilla positive case
+        [InlineData(new[] { "Microsoft.NET.Sdk.Web" }, new[] { "X", "Y", "Microsoft.AspNetCore.App", "Z" }, new[] { "Microsoft.AspNetCore.App" })] // Multiple references
+        [InlineData(new[] { "Microsoft.NET.Sdk.Web" }, new[] { "Microsoft.AspNetCore.App", "Microsoft.AspNetCore.App" }, new[] { "Microsoft.AspNetCore.App" })] // Duplicate references
+        [InlineData(new[] { "microsoft.NET.SDK.Web" }, new[] { "microsoft.ASPNetCore.App" }, new[] { "microsoft.ASPNetCore.App" })] // Ununusual case
+        [InlineData(new[] { "Microsoft.NET.Sdk" }, new[] { "Microsoft.AspNetCore.App" }, new string[0])] // Wrong SDK
+        [InlineData(new[] { "Microsoft.NET.Sdk.Web" }, new[] { "A", "B", "C" }, new string[0])] // Wrong framework reference
         [InlineData(null, new[] { "Microsoft.AspNetCore.App" }, new string[0])] // No SDK
-        [InlineData("Microsoft.NET.Sdk.Web", new string[0], new string[0])] // No framework reference
-        [InlineData("Microsoft.NET.Sdk.Web", null, new string[0])] // Null framework references
-        public async Task AnalyzeAsyncTests(string? sdk, string[]? frameworkReferences, string[] expectedReferencesToRemove)
+        [InlineData(new[] { "Microsoft.NET.Sdk.Web" }, new string[0], new string[0])] // No framework reference
+        [InlineData(new[] { "Microsoft.NET.Sdk.Web" }, null, new string[0])] // Null framework references
+        public async Task AnalyzeAsyncTests(string[]? sdk, string[]? frameworkReferences, string[] expectedReferencesToRemove)
         {
             // Arrange
             using var mock = AutoMock.GetLoose();
@@ -86,17 +87,17 @@ namespace Microsoft.DotNet.UpgradeAssistant.Extensions.Web.Tests
             await Assert.ThrowsAsync<InvalidOperationException>(() => analyzer.AnalyzeAsync(null!, state.Object, CancellationToken.None)).ConfigureAwait(true);
         }
 
-        private static IProject GetMockProjectAndPackageState(AutoMock mock, string? sdk = null, IEnumerable<Reference>? frameworkReferences = null)
+        private static IProject GetMockProjectAndPackageState(AutoMock mock, string[]? sdk = null, IEnumerable<Reference>? frameworkReferences = null)
         {
             var projectRoot = mock.Mock<IProjectFile>();
             projectRoot.Setup(r => r.IsSdk).Returns(sdk is not null);
             if (sdk is not null)
             {
-                projectRoot.Setup(r => r.Sdk).Returns(sdk);
+                projectRoot.Setup(r => r.Sdk).Returns(new HashSet<string>(sdk, StringComparer.OrdinalIgnoreCase));
             }
             else
             {
-                projectRoot.Setup(r => r.Sdk).Throws<ArgumentOutOfRangeException>();
+                projectRoot.Setup(r => r.Sdk).Returns(Array.Empty<string>());
             }
 
             var project = mock.Mock<IProject>();