Properly test packages.config feature

src/NuGetUtility/Program.cs

@@ -99,9 +99,9 @@ private async Task<int> OnExecuteAsync(CancellationToken cancellationToken)
             IFileDownloader urlLicenseFileDownloader = GetFileDownloader(httpClient);
             IOutputFormatter output = GetOutputFormatter();
 
-            var msBuild = new MsBuildAbstraction();
+            MsBuildAbstraction msBuild = OperatingSystem.IsWindows() ? new WindowsMsBuildAbstraction() : new MsBuildAbstraction();
             var projectCollector = new ProjectsCollector(msBuild);
-            var projectReader = new ReferencedPackageReader(msBuild, new LockFileFactory());
+            var projectReader = new ReferencedPackageReader(msBuild, new LockFileFactory(), GetPackagesConfigReader());
             var validator = new LicenseValidator.LicenseValidator(licenseMappings,
                 allowedLicenses,
                 urlLicenseFileDownloader,
@@ -137,6 +137,10 @@ private async Task<int> OnExecuteAsync(CancellationToken cancellationToken)
             await output.Write(outputStream, results.OrderBy(r => r.PackageId).ThenBy(r => r.PackageVersion).ToList());
             return results.Count(r => r.ValidationErrors.Any());
         }
+
+        private static IPackagesConfigReader GetPackagesConfigReader() =>
+            OperatingSystem.IsWindows() ? new WindowsPackagesConfigReader() : new FailingPackagesConfigReader();
+
         private static IAsyncEnumerable<ReferencedPackageWithContext> GetPackageInfos(
             ProjectWithReferencedPackages projectWithReferences,
             IEnumerable<CustomPackageInformation> overridePackageInformation,

src/NuGetUtility/ReferencedPackagesReader/ReferencedPackageReader.cs

@@ -9,13 +9,16 @@ public class ReferencedPackageReader
     {
         private const string ProjectReferenceIdentifier = "project";
         private readonly ILockFileFactory _lockFileFactory;
+        private readonly IPackagesConfigReader _packagesConfigReader;
         private readonly IMsBuildAbstraction _msBuild;
 
         public ReferencedPackageReader(IMsBuildAbstraction msBuild,
-            ILockFileFactory lockFileFactory)
+            ILockFileFactory lockFileFactory,
+            IPackagesConfigReader packagesConfigReader)
         {
             _msBuild = msBuild;
             _lockFileFactory = lockFileFactory;
+            _packagesConfigReader = packagesConfigReader;
         }
 
         public IEnumerable<PackageIdentity> GetInstalledPackages(string projectPath, bool includeTransitive)
@@ -30,7 +33,7 @@ public IEnumerable<PackageIdentity> GetInstalledPackages(string projectPath, boo
             if (project.IsPackageReferenceProject())
                 return GetInstalledPackagesFromAssetsFile(includeTransitive, project);
 
-            return PackagesConfigReader.GetPackages(project.GetPackagesConfigPath());
+            return _packagesConfigReader.GetPackages(project);
         }
 
         private IEnumerable<PackageIdentity> GetInstalledPackagesFromAssetsFile(bool includeTransitive,

src/NuGetUtility/Wrapper/MsBuildWrapper/MsBuildAbstraction.cs

@@ -1,29 +1,21 @@
-using System.Management;
 using Microsoft.Build.Construction;
 using Microsoft.Build.Evaluation;
 using Microsoft.Build.Exceptions;
 using Microsoft.Build.Execution;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Locator;
-using NuGetUtility.Extensions;
 using NuGetUtility.Wrapper.NuGetWrapper.Versioning;
 
 namespace NuGetUtility.Wrapper.MsBuildWrapper
 {
     public class MsBuildAbstraction : IMsBuildAbstraction
     {
         private const string CollectPackageReferences = "CollectPackageReferences";
-        private readonly Dictionary<string, string> _project_props = new();
+        private readonly Dictionary<string, string> _globalProjectProperties = new();
 
         public MsBuildAbstraction()
         {
             RegisterMsBuildLocatorIfNeeded();
-
-            // to support VC-projects we need to workaround : https://github.com/3F/MvsSln/issues/1
-            // adding 'VCTargetsPath' to Project::GlobalProperties seem to be enough
-
-            if (GetBestVCTargetsPath() is string path)
-                _project_props.Add("VCTargetsPath", $"{path}\\");
         }
 
         public IEnumerable<PackageReference> GetPackageReferencesFromProjectForFramework(IProject project,
@@ -48,7 +40,7 @@ public IProject GetProject(string projectPath)
         {
             ProjectRootElement rootElement = TryGetProjectRootElement(projectPath);
 
-            var project = new Project(rootElement, _project_props, null);
+            var project = new Project(rootElement, _globalProjectProperties, null);
 
             return new ProjectWrapper(project);
         }
@@ -60,35 +52,6 @@ public IEnumerable<string> GetProjectsFromSolution(string inputPath)
             return sln.ProjectsInOrder.Select(p => p.AbsolutePath);
         }
 
-        private static string? GetBestVCTargetsPath()
-        {
-            var cpp_props = new List<FileInfo>();
-
-            foreach (string path in GetVisualStudioInstallPaths())
-                cpp_props.AddRange(new DirectoryInfo(path).GetFiles("Microsoft.Cpp.Default.props", SearchOption.AllDirectories));
-
-            // if multiple, assume most recent 'LastWriteTime' property is 'best'
-            return cpp_props.OrderBy(f => f.LastWriteTime).LastOrDefault()?.DirectoryName;
-        }
-
-        private static IEnumerable<string> GetVisualStudioInstallPaths()
-        {
-            // https://learn.microsoft.com/en-us/visualstudio/install/tools-for-managing-visual-studio-instances?view=vs-2022#using-windows-management-instrumentation-wmi
-
-            var result = new List<string>();
-
-            if (OperatingSystem.IsWindows())
-            {
-                var mmc = new ManagementClass("root/cimv2/vs:MSFT_VSInstance");
-
-                foreach (ManagementBaseObject? vs_instance in mmc.GetInstances())
-                    if (vs_instance["InstallLocation"] is string install_path)
-                        result.Add(install_path);
-            }
-
-            return result;
-        }
-
         private static void RegisterMsBuildLocatorIfNeeded()
         {
             if (!MSBuildLocator.IsRegistered)
@@ -108,5 +71,7 @@ private static ProjectRootElement TryGetProjectRootElement(string projectPath)
                 throw new MsBuildAbstractionException($"Failed to open project: {projectPath}", e);
             }
         }
+
+        protected void AddGlobalProjectProperty(string name, string value) => _globalProjectProperties.Add(name, value);
     }
 }

src/NuGetUtility/Wrapper/MsBuildWrapper/WindowsMsBuildAbstraction.cs

@@ -0,0 +1,45 @@
+using System.Management;
+using System.Runtime.Versioning;
+
+namespace NuGetUtility.Wrapper.MsBuildWrapper
+{
+    [SupportedOSPlatform("windows")]
+    public class WindowsMsBuildAbstraction : MsBuildAbstraction
+    {
+        public WindowsMsBuildAbstraction()
+        {
+            // to support VC-projects we need to workaround : https://github.com/3F/MvsSln/issues/1
+            // adding 'VCTargetsPath' to Project::GlobalProperties seem to be enough
+
+            if (GetBestVCTargetsPath() is string path)
+            {
+                AddGlobalProjectProperty("VCTargetsPath", $"{path}\\");
+            }
+        }
+
+        private static string? GetBestVCTargetsPath()
+        {
+            var cppProperties = new List<FileInfo>();
+
+            foreach (string path in GetVisualStudioInstallPaths())
+                cppProperties.AddRange(new DirectoryInfo(path).GetFiles("Microsoft.Cpp.Default.props", SearchOption.AllDirectories));
+
+            // if multiple, assume most recent 'LastWriteTime' property is 'best'
+            return cppProperties.OrderBy(f => f.LastWriteTime).LastOrDefault()?.DirectoryName;
+        }
+
+        private static IEnumerable<string> GetVisualStudioInstallPaths()
+        {
+            // https://learn.microsoft.com/en-us/visualstudio/install/tools-for-managing-visual-studio-instances?view=vs-2022#using-windows-management-instrumentation-wmi
+
+            var result = new List<string>();
+            var mmc = new ManagementClass("root/cimv2/vs:MSFT_VSInstance");
+
+            foreach (ManagementBaseObject? vs_instance in mmc.GetInstances())
+                if (vs_instance["InstallLocation"] is string install_path)
+                    result.Add(install_path);
+
+            return result;
+        }
+    }
+}

src/NuGetUtility/Wrapper/NuGetWrapper/Packaging/Core/FailingPackagesConfigReader.cs

@@ -0,0 +1,12 @@
+using NuGetUtility.Wrapper.MsBuildWrapper;
+
+namespace NuGetUtility.Wrapper.NuGetWrapper.Packaging.Core
+{
+    public class FailingPackagesConfigReader : IPackagesConfigReader
+    {
+        public IEnumerable<PackageIdentity> GetPackages(IProject project)
+        {
+            throw new PackagesConfigReaderException($"Invalid project structure detected. Currently packages.config projects are only supported on Windows (Project: {project.FullPath})");
+        }
+    }
+}

src/NuGetUtility/Wrapper/NuGetWrapper/Packaging/Core/IPackagesConfigReader.cs

@@ -0,0 +1,9 @@
+using NuGetUtility.Wrapper.MsBuildWrapper;
+
+namespace NuGetUtility.Wrapper.NuGetWrapper.Packaging.Core
+{
+    public interface IPackagesConfigReader
+    {
+        IEnumerable<PackageIdentity> GetPackages(IProject project);
+    }
+}

src/NuGetUtility/Wrapper/NuGetWrapper/Packaging/Core/PackagesConfigReaderException.cs

@@ -0,0 +1,7 @@
+namespace NuGetUtility.Wrapper.NuGetWrapper.Packaging.Core
+{
+    public class PackagesConfigReaderException : Exception
+    {
+        public PackagesConfigReaderException(string? message) : base(message) { }
+    }
+}

src/NuGetUtility/Wrapper/NuGetWrapper/Packaging/Core/PackageConfigReader.cs → src/NuGetUtility/Wrapper/NuGetWrapper/Packaging/Core/WindowsPackagesConfigReader.cs

@@ -1,13 +1,15 @@
-using System.Xml.Linq;
+using System.Xml.Linq;
+using NuGetUtility.Extensions;
+using NuGetUtility.Wrapper.MsBuildWrapper;
 using NuGetUtility.Wrapper.NuGetWrapper.Versioning;
 
 namespace NuGetUtility.Wrapper.NuGetWrapper.Packaging.Core
 {
-    public static class PackagesConfigReader
+    public class WindowsPackagesConfigReader : IPackagesConfigReader
     {
-        public static IEnumerable<PackageIdentity> GetPackages(string path)
+        public IEnumerable<PackageIdentity> GetPackages(IProject project)
         {
-            var document = XDocument.Load(path);
+            var document = XDocument.Load(project.GetPackagesConfigPath());
 
             var reader = new NuGet.Packaging.PackagesConfigReader(document);
 

tests/NuGetUtility.Test/Extensions/ProjectExtensionsTest.cs

@@ -65,7 +65,7 @@ public void IsPackageReferenceProject_Should_ReturnTrue_If_ProjectIsPackageRefer
             _project.GetRestoreStyleTag().Returns(restoreStyleTag);
             _project.GetEvaluatedIncludes().Returns(new List<string> { "not-packages.config" });
 
-            bool result = _project!.IsPackageReferenceProject();
+            bool result = _project.IsPackageReferenceProject();
 
             Assert.That(result, Is.True);
         }
@@ -86,9 +86,20 @@ public void IsPackageReferenceProject_Should_ReturnFalse_If_ProjectIsNotPackageR
             _project.GetRestoreStyleTag().Returns(restoreStyleTag);
             _project.GetEvaluatedIncludes().Returns(new List<string> { evaluatedInclude });
 
-            bool result = _project!.IsPackageReferenceProject();
+            bool result = _project.IsPackageReferenceProject();
 
             Assert.That(result, Is.False);
         }
+
+        [TestCase()]
+        public void GetPackagesConfigPath_Should_Return_CorrectPath()
+        {
+            string path = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString(), Guid.NewGuid().ToString());
+            _project.FullPath.Returns(path);
+
+            string result = _project.GetPackagesConfigPath();
+
+            Assert.That(result, Is.EqualTo(Path.Combine(Path.GetDirectoryName(path)!, "packages.config")));
+        }
     }
 }

tests/NuGetUtility.Test/ReferencedPackagesReader/ReferencedPackageReaderTest.cs

@@ -1,8 +1,8 @@
 using AutoFixture;
 using AutoFixture.AutoNSubstitute;
+using Microsoft.Build.Evaluation;
 using NSubstitute;
 using NuGetUtility.ReferencedPackagesReader;
-using NuGetUtility.Test.Helper.AutoFixture;
 using NuGetUtility.Test.Helper.ShuffelledEnumerable;
 using NuGetUtility.Wrapper.MsBuildWrapper;
 using NuGetUtility.Wrapper.NuGetWrapper.Frameworks;
@@ -26,6 +26,7 @@ public void SetUp()
             _projectMock = Substitute.For<IProject>();
             _lockFileMock = Substitute.For<ILockFile>();
             _packageSpecMock = Substitute.For<IPackageSpec>();
+            _packagesConfigReader = Substitute.For<IPackagesConfigReader>();
             _lockFileTargets = fixture.CreateMany<ILockFileTarget>(TargetFrameworkCount).ToArray();
             _lockFileLibraries = fixture.CreateMany<ILockFileLibrary>(50).ToArray();
             _packageSpecTargetFrameworks =
@@ -87,13 +88,14 @@ public void SetUp()
                 }
             }
 
-            _uut = new ReferencedPackageReader(_msBuild, _lockFileFactory);
+            _uut = new ReferencedPackageReader(_msBuild, _lockFileFactory, _packagesConfigReader);
         }
 
         private const int TargetFrameworkCount = 5;
         private ReferencedPackageReader _uut = null!;
         private IMsBuildAbstraction _msBuild = null!;
         private ILockFileFactory _lockFileFactory = null!;
+        private IPackagesConfigReader _packagesConfigReader = null!;
         private string _projectPath = null!;
         private string _assetsFilePath = null!;
         private IProject _projectMock = null!;
@@ -227,5 +229,34 @@ public void
 
             Assert.That(result.Count(), Is.EqualTo(0));
         }
+
+        [Test]
+        public void GetInstalledPackages_Should_Use_PackageGonfigReader_If_ProjectIsPackageConfigProject(
+            [Values] bool includeTransitive)
+        {
+            _packageSpecMock.IsValid().Returns(false);
+            _projectMock.FullPath.Returns(_projectPath);
+            _projectMock.GetEvaluatedIncludes().Returns(new List<string> { "packages.config" });
+
+            _ = _uut.GetInstalledPackages(_projectPath, includeTransitive);
+
+            _packagesConfigReader.Received(1).GetPackages(Arg.Any<IProject>());
+            _packagesConfigReader.Received(1).GetPackages(_projectMock);
+        }
+
+        [Test]
+        public void GetInstalledPackages_Should_ReturnPackagesReturnedBy_PackageGonfigReader_If_ProjectIsPackageConfigProject(
+            [Values] bool includeTransitive)
+        {
+            _packageSpecMock.IsValid().Returns(false);
+            _projectMock.FullPath.Returns(_projectPath);
+            _projectMock.GetEvaluatedIncludes().Returns(new List<string> { "packages.config" });
+            PackageIdentity[] expectedPackages = _packageReferencesFromProjectForFramework.First().Value.Select(l => new PackageIdentity(l.PackageName, l.Version!)).ToArray();
+            _packagesConfigReader.GetPackages(Arg.Any<IProject>()).Returns(expectedPackages);
+
+            IEnumerable<PackageIdentity> packages = _uut.GetInstalledPackages(_projectPath, includeTransitive);
+
+            CollectionAssert.AreEqual(expectedPackages, packages);
+        }
     }
 }

tests/NuGetUtility.Test/ReferencedPackagesReader/ReferencedPackagesReaderIntegrationTest.cs

@@ -1,3 +1,4 @@
+using Microsoft.Build.Evaluation;
 using NuGetUtility.ReferencedPackagesReader;
 using NuGetUtility.Wrapper.MsBuildWrapper;
 using NuGetUtility.Wrapper.NuGetWrapper.Packaging.Core;
@@ -11,7 +12,10 @@ public class ReferencedPackagesReaderIntegrationTest
         [SetUp]
         public void SetUp()
         {
-            _uut = new ReferencedPackageReader(new MsBuildAbstraction(), new LockFileFactory());
+            IMsBuildAbstraction msBuildAbstraction = OperatingSystem.IsWindows() ? new WindowsMsBuildAbstraction() : new MsBuildAbstraction();
+            IPackagesConfigReader packagesConfigReader = OperatingSystem.IsWindows() ? new WindowsPackagesConfigReader() : new FailingPackagesConfigReader();
+
+            _uut = new ReferencedPackageReader(msBuildAbstraction, new LockFileFactory(), packagesConfigReader);
         }
 
         private ReferencedPackageReader? _uut;
@@ -73,5 +77,15 @@ public void GetInstalledPackagesShould_ReturnPackagesForPackagesConfigProject()
 
             Assert.That(result.Count, Is.EqualTo(1));
         }
+
+        [Test]
+        [Platform(Exclude = "Win")]
+        public void GetInstalledPackagesShould_ThrowError()
+        {
+            string path = Path.GetFullPath("../../../../targets/PackagesConfigProject/PackagesConfigProject.csproj");
+
+            PackagesConfigReaderException? exception = Assert.Throws<PackagesConfigReaderException>(() => _uut!.GetInstalledPackages(path, false));
+            Assert.That(exception?.Message, Is.EqualTo($"Invalid project structure detected. Currently packages.config projects are only supported on Windows (Project: {path})"));
+        }
     }
 }