Improve packagereference vs pacakges.config distinction detection

NuGetUtility.sln

@@ -40,6 +40,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "integration", "integration"
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ProjectWithReferenceContainingLicenseExpression", "integration\ProjectWithReferenceContainingLicenseExpression\ProjectWithReferenceContainingLicenseExpression.csproj", "{01704839-219A-4CE2-93F4-DB3CB8773DCE}"
 EndProject
+Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "EmptyCppProject", "tests\targets\EmptyCppProject\EmptyCppProject.vcxproj", "{9B107A91-87F9-420C-ADF8-732C77DAFB93}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -247,6 +249,22 @@ Global
 		{01704839-219A-4CE2-93F4-DB3CB8773DCE}.TestWindows|x64.Build.0 = TestWindows|Any CPU
 		{01704839-219A-4CE2-93F4-DB3CB8773DCE}.TestWindows|x86.ActiveCfg = TestWindows|Any CPU
 		{01704839-219A-4CE2-93F4-DB3CB8773DCE}.TestWindows|x86.Build.0 = TestWindows|Any CPU
+		{9B107A91-87F9-420C-ADF8-732C77DAFB93}.Debug|Any CPU.ActiveCfg = Debug|x64
+		{9B107A91-87F9-420C-ADF8-732C77DAFB93}.Debug|x64.ActiveCfg = Debug|x64
+		{9B107A91-87F9-420C-ADF8-732C77DAFB93}.Debug|x64.Build.0 = Debug|x64
+		{9B107A91-87F9-420C-ADF8-732C77DAFB93}.Debug|x86.ActiveCfg = Debug|Win32
+		{9B107A91-87F9-420C-ADF8-732C77DAFB93}.Debug|x86.Build.0 = Debug|Win32
+		{9B107A91-87F9-420C-ADF8-732C77DAFB93}.Release|Any CPU.ActiveCfg = Release|x64
+		{9B107A91-87F9-420C-ADF8-732C77DAFB93}.Release|x64.ActiveCfg = Release|x64
+		{9B107A91-87F9-420C-ADF8-732C77DAFB93}.Release|x64.Build.0 = Release|x64
+		{9B107A91-87F9-420C-ADF8-732C77DAFB93}.Release|x86.ActiveCfg = Release|Win32
+		{9B107A91-87F9-420C-ADF8-732C77DAFB93}.Release|x86.Build.0 = Release|Win32
+		{9B107A91-87F9-420C-ADF8-732C77DAFB93}.TestWindows|Any CPU.ActiveCfg = Debug|x64
+		{9B107A91-87F9-420C-ADF8-732C77DAFB93}.TestWindows|Any CPU.Build.0 = Debug|x64
+		{9B107A91-87F9-420C-ADF8-732C77DAFB93}.TestWindows|x64.ActiveCfg = Debug|x64
+		{9B107A91-87F9-420C-ADF8-732C77DAFB93}.TestWindows|x64.Build.0 = Debug|x64
+		{9B107A91-87F9-420C-ADF8-732C77DAFB93}.TestWindows|x86.ActiveCfg = Debug|Win32
+		{9B107A91-87F9-420C-ADF8-732C77DAFB93}.TestWindows|x86.Build.0 = Debug|Win32
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -264,6 +282,7 @@ Global
 		{FBA6622A-C9E3-4250-AB79-35F02CAD2419} = {D2AB2D00-1F48-487D-BFE0-99FDB4E071CC}
 		{DE079B9C-B6BA-4D53-8B83-03D3CBD4027F} = {D2AB2D00-1F48-487D-BFE0-99FDB4E071CC}
 		{01704839-219A-4CE2-93F4-DB3CB8773DCE} = {FFB2826C-17A3-4C18-8FFE-A34AB51592F7}
+		{9B107A91-87F9-420C-ADF8-732C77DAFB93} = {FA92392F-D895-4D1E-A5ED-E6DC3C08223E}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {70887D40-0182-4C32-BFA1-B5A02E405F11}

src/NuGetUtility/Extensions/ProjectExtensions.cs

@@ -7,34 +7,16 @@ namespace NuGetUtility.Extensions
 {
     public static class ProjectExtensions
     {
-        private const string PackageReferenceValue = "PackageReference";
         private const string PackagesConfigFileName = "packages.config";
 
-        public static bool HasNuGetPackagesReferenced(this IProject project)
-        {
-            return (project.GetPackageReferenceCount() > 0) || project.HasPackagesConfigFile();
-        }
-
-        public static bool IsPackageReferenceProject(this IProject project)
-        {
-            return TargetIsEqualIfSet(project.GetNuGetStyleTag(), PackageReferenceValue) &&
-                   TargetIsEqualIfSet(project.GetRestoreStyleTag(), PackageReferenceValue) &&
-                   !project.HasPackagesConfigFile();
-        }
-
         public static string GetPackagesConfigPath(this IProject project)
         {
             return Path.Combine(Path.GetDirectoryName(project.FullPath) ?? string.Empty, PackagesConfigFileName);
         }
 
-        private static bool HasPackagesConfigFile(this IProject project)
+        public static bool HasPackagesConfigFile(this IProject project)
         {
             return project.GetEvaluatedIncludes().Any(include => include?.Equals(PackagesConfigFileName) ?? false);
         }
-
-        private static bool TargetIsEqualIfSet(string source, string target)
-        {
-            return string.IsNullOrEmpty(source) || source.Equals(target);
-        }
     }
 }

src/NuGetUtility/ReferencedPackagesReader/ReferencedPackageReader.cs

@@ -1,6 +1,7 @@
 // Licensed to the projects contributors.
 // The license conditions are provided in the LICENSE file located in the project root
 
+using System.Diagnostics.CodeAnalysis;
 using NuGetUtility.Extensions;
 using NuGetUtility.Wrapper.MsBuildWrapper;
 using NuGetUtility.Wrapper.NuGetWrapper.Packaging.Core;
@@ -28,23 +29,28 @@ public IEnumerable<PackageIdentity> GetInstalledPackages(string projectPath, boo
         {
             IProject project = _msBuild.GetProject(projectPath);
 
-            if (!project.HasNuGetPackagesReferenced() && !includeTransitive)
+            if (TryGetInstalledPackagesFromAssetsFile(includeTransitive, project, out IEnumerable<PackageIdentity>? dependencies))
             {
-                return Enumerable.Empty<PackageIdentity>();
+                return dependencies;
             }
 
-            if (project.IsPackageReferenceProject())
+            if (project.HasPackagesConfigFile())
             {
-                return GetInstalledPackagesFromAssetsFile(includeTransitive, project);
+                return _packagesConfigReader.GetPackages(project);
             }
 
-            return _packagesConfigReader.GetPackages(project);
+            return Array.Empty<PackageIdentity>();
         }
 
-        private IEnumerable<PackageIdentity> GetInstalledPackagesFromAssetsFile(bool includeTransitive,
-            IProject project)
+        private bool TryGetInstalledPackagesFromAssetsFile(bool includeTransitive,
+            IProject project,
+            [NotNullWhen(true)] out IEnumerable<PackageIdentity>? installedPackages)
         {
-            ILockFile assetsFile = LoadAssetsFile(project);
+            installedPackages = null;
+            if (!TryLoadAssetsFile(project, out ILockFile? assetsFile))
+            {
+                return false;
+            }
 
             var referencedLibraries = new HashSet<ILockFileLibrary>();
 
@@ -55,7 +61,8 @@ private IEnumerable<PackageIdentity> GetInstalledPackagesFromAssetsFile(bool inc
                 referencedLibraries.AddRange(referencedLibrariesForTarget);
             }
 
-            return referencedLibraries.Select(r => new PackageIdentity(r.Name, r.Version));
+            installedPackages = referencedLibraries.Select(r => new PackageIdentity(r.Name, r.Version));
+            return true;
         }
 
         private IEnumerable<ILockFileLibrary> GetReferencedLibrariesForTarget(IProject project,
@@ -102,18 +109,22 @@ private static ITargetFrameworkInformation GetTargetFrameworkInformation(ILockFi
             }
         }
 
-        private ILockFile LoadAssetsFile(IProject project)
+        private bool TryLoadAssetsFile(IProject project, [NotNullWhen(true)] out ILockFile? assetsFile)
         {
-            string assetsPath = project.GetAssetsPath();
-            ILockFile assetsFile = _lockFileFactory.GetFromFile(assetsPath);
+            if (!project.TryGetAssetsPath(out string assetsPath))
+            {
+                assetsFile = null;
+                return false;
+            }
+            assetsFile = _lockFileFactory.GetFromFile(assetsPath);
 
             if (!assetsFile.PackageSpec.IsValid() || !(assetsFile.Targets?.Any() ?? false))
             {
                 throw new ReferencedPackageReaderException(
                     $"Failed to validate project assets for project {project.FullPath}");
             }
 
-            return assetsFile;
+            return true;
         }
     }
 }

src/NuGetUtility/Wrapper/MsBuildWrapper/IProject.cs

@@ -1,19 +1,15 @@
 // Licensed to the projects contributors.
 // The license conditions are provided in the LICENSE file located in the project root
 
+using System.Diagnostics.CodeAnalysis;
+
 namespace NuGetUtility.Wrapper.MsBuildWrapper
 {
     public interface IProject
     {
         public string FullPath { get; }
 
-        string GetAssetsPath();
-
-        string GetRestoreStyleTag();
-
-        string GetNuGetStyleTag();
-
-        int GetPackageReferenceCount();
+        bool TryGetAssetsPath([NotNullWhen(true)] out string assetsFile);
 
         IEnumerable<string> GetEvaluatedIncludes();
     }

src/NuGetUtility/Wrapper/MsBuildWrapper/MsBuildAbstraction.cs

@@ -14,7 +14,9 @@ namespace NuGetUtility.Wrapper.MsBuildWrapper
     public class MsBuildAbstraction : IMsBuildAbstraction
     {
         private const string CollectPackageReferences = "CollectPackageReferences";
-        private readonly Dictionary<string, string> _globalProjectProperties = new();
+        private ProjectCollection? _projects;
+
+        private ProjectCollection Projects => _projects ??= InitializeProjectCollection();
 
         public MsBuildAbstraction()
         {
@@ -48,9 +50,7 @@ public IProject GetProject(string projectPath)
             }
 #endif
 
-            ProjectRootElement rootElement = TryGetProjectRootElement(projectPath);
-
-            var project = new Project(rootElement, _globalProjectProperties, null);
+            Project project = Projects.LoadProject(projectPath);
 
             return new ProjectWrapper(project);
         }
@@ -82,6 +82,11 @@ private static ProjectRootElement TryGetProjectRootElement(string projectPath)
             }
         }
 
-        protected void AddGlobalProjectProperty(string name, string value) => _globalProjectProperties.Add(name, value);
+        private static ProjectCollection InitializeProjectCollection()
+        {
+            ProjectCollection collection = ProjectCollection.GlobalProjectCollection;
+            collection.UnloadAllProjects();
+            return collection;
+        }
     }
 }

src/NuGetUtility/Wrapper/MsBuildWrapper/ProjectWrapper.cs

@@ -1,16 +1,14 @@
 // Licensed to the projects contributors.
 // The license conditions are provided in the LICENSE file located in the project root
 
+using System.Diagnostics.CodeAnalysis;
 using Microsoft.Build.Evaluation;
 
 namespace NuGetUtility.Wrapper.MsBuildWrapper
 {
     internal class ProjectWrapper : IProject
     {
-        private const string PackageReferenceTypeTag = "PackageReference";
         private const string ProjectAssetsFile = "ProjectAssetsFile";
-        private const string RestoreStyleTag = "RestoreProjectStyle";
-        private const string NugetStyleTag = "NuGetProjectStyle";
 
         private readonly Project _project;
 
@@ -19,31 +17,21 @@ public ProjectWrapper(Project project)
             _project = project;
         }
 
-        public string GetAssetsPath()
+        public bool TryGetAssetsPath([NotNullWhen(true)] out string assetsFile)
         {
-            string assetsFile = _project.GetPropertyValue(ProjectAssetsFile);
+            assetsFile = _project.GetPropertyValue(ProjectAssetsFile);
+            if (string.IsNullOrEmpty(assetsFile))
+            {
+                return false;
+            }
+
             if (!File.Exists(assetsFile))
             {
                 throw new MsBuildAbstractionException(
                     $"Failed to get the project assets file for project {_project.FullPath} ({assetsFile})");
             }
 
-            return assetsFile;
-        }
-
-        public string GetRestoreStyleTag()
-        {
-            return _project.GetPropertyValue(RestoreStyleTag);
-        }
-
-        public string GetNuGetStyleTag()
-        {
-            return _project.GetPropertyValue(NugetStyleTag);
-        }
-
-        public int GetPackageReferenceCount()
-        {
-            return _project.GetItems(PackageReferenceTypeTag).Count;
+            return true;
         }
 
         public IEnumerable<string> GetEvaluatedIncludes()

tests/NuGetUtility.Test/Extensions/ProjectExtensionsTest.cs

@@ -18,83 +18,7 @@ public void SetUp()
 
         private IProject _project = null!;
 
-        [Test]
-        public void HasNugetPackagesReferenced_Should_ReturnTrue_If_PackageReferenceCountIsMoreThanZero(
-            [Values(1, 50, 999)] int referenceCount)
-        {
-            _project.GetPackageReferenceCount().Returns(referenceCount);
-
-            bool result = _project.HasNuGetPackagesReferenced();
-
-            Assert.That(result, Is.True);
-        }
-
-        [Test]
-        public void HasNugetPackagesReferences_Should_ReturnTrue_If_ProjectHasPackagesConfigFileReferenced()
-        {
-            _project.GetPackageReferenceCount().Returns(0);
-            _project.GetEvaluatedIncludes().Returns(new List<string> { "packages.config" });
-
-            bool result = _project!.HasNuGetPackagesReferenced();
-
-            Assert.That(result, Is.True);
-        }
-
-        [Test]
-        public void
-            HasNugetPackagesReferenced_Should_ReturnFalse_If_PackageReferenceCountIsZeroOrLess_And_ProjectHasNoPackagesConfigFileReferenced(
-                [Values(-9999, -50, 0)] int referenceCount)
-        {
-            _project.GetPackageReferenceCount().Returns(referenceCount);
-            _project.GetEvaluatedIncludes().Returns(Enumerable.Empty<string>());
-
-            bool result = _project!.HasNuGetPackagesReferenced();
-
-            Assert.That(result, Is.False);
-        }
-
-        [TestCase(null, null)]
-        [TestCase("", "")]
-        [TestCase(null, "PackageReference")]
-        [TestCase("", "PackageReference")]
-        [TestCase("PackageReference", null)]
-        [TestCase("PackageReference", "")]
-        [TestCase("PackageReference", "PackageReference")]
-        public void IsPackageReferenceProject_Should_ReturnTrue_If_ProjectIsPackageReferenceProject(
-            string nugetStyleTag,
-            string restoreStyleTag)
-        {
-            _project.GetNuGetStyleTag().Returns(nugetStyleTag);
-            _project.GetRestoreStyleTag().Returns(restoreStyleTag);
-            _project.GetEvaluatedIncludes().Returns(new List<string> { "not-packages.config" });
-
-            bool result = _project.IsPackageReferenceProject();
-
-            Assert.That(result, Is.True);
-        }
-
-        [TestCase("InvalidTag", "InvalidTag", "packages.config")]
-        [TestCase("InvalidTag", "PackageReference", "packages.config")]
-        [TestCase("InvalidTag", "InvalidTag", "not-packages.config")]
-        [TestCase("InvalidTag", "PackageReference", "not-packages.config")]
-        [TestCase("PackageReference", "InvalidTag", "packages.config")]
-        [TestCase("PackageReference", "PackageReference", "packages.config")]
-        [TestCase("PackageReference", "InvalidTag", "not-packages.config")]
-        public void IsPackageReferenceProject_Should_ReturnFalse_If_ProjectIsNotPackageReferenceProject(
-            string nugetStyleTag,
-            string restoreStyleTag,
-            string evaluatedInclude)
-        {
-            _project.GetNuGetStyleTag().Returns(nugetStyleTag);
-            _project.GetRestoreStyleTag().Returns(restoreStyleTag);
-            _project.GetEvaluatedIncludes().Returns(new List<string> { evaluatedInclude });
-
-            bool result = _project.IsPackageReferenceProject();
-
-            Assert.That(result, Is.False);
-        }
-
-        [TestCase()]
+        [TestCase]
         public void GetPackagesConfigPath_Should_Return_CorrectPath()
         {
             string path = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString(), Guid.NewGuid().ToString());
@@ -104,5 +28,20 @@ public void GetPackagesConfigPath_Should_Return_CorrectPath()
 
             Assert.That(result, Is.EqualTo(Path.Combine(Path.GetDirectoryName(path)!, "packages.config")));
         }
+
+        [TestCase(new string?[] { }, false)]
+        [TestCase(new string?[] { null }, false)]
+        [TestCase(new string?[] { null, "not-packages.config" }, false)]
+        [TestCase(new string?[] { "not-packages.config" }, false)]
+        [TestCase(new string?[] { "packages.config" }, true)]
+        [TestCase(new string?[] { null, "packages.config" }, true)]
+        [TestCase(new string?[] { "not-packages.config", "packages.config" }, true)]
+        [TestCase(new string?[] { null, "not-packages.config", "packages.config" }, true)]
+        public void HasPackagesConfigFile_Should_Return_Correct_Result(IEnumerable<string> evaluatedIncludes, bool expectation)
+        {
+            _project.GetEvaluatedIncludes().Returns(evaluatedIncludes);
+
+            Assert.That(_project.HasPackagesConfigFile(), Is.EqualTo(expectation));
+        }
     }
 }