From 952bbad08bb22e62c90645bf2a447aa6de5255eb Mon Sep 17 00:00:00 2001 From: Robert Dailey Date: Fri, 18 Jul 2025 14:51:23 -0500 Subject: [PATCH] feat: Add support for NuGet Central Package Management Add support for Directory.Packages.props files with ManagePackageVersionsCentrally. Handles both PackageVersion and GlobalPackageReference elements to resolve package versions centrally, preventing "Version '' for package is not valid" errors. Enhanced to check Directory.Build.props as fallback location for ManagePackageVersionsCentrally property, following MSBuild conventions. Includes comprehensive integration tests covering: - Basic central package management with PackageVersion elements - GlobalPackageReference support for analyzer packages - Disabled central management scenarios - Directory.Build.props fallback behavior Fixes #39 --- .gitignore | 18 +- .../CentralPackages/CentralPackages.csproj | 13 ++ .../CentralPackages/Class1.cs | 6 + .../CentralPackages/Directory.Packages.props | 9 + .../CentralPackagesBuildProps.csproj | 13 ++ .../CentralPackagesBuildProps/Class1.cs | 6 + .../Directory.Build.props | 5 + .../Directory.Packages.props | 6 + .../CentralPackagesDisabled.csproj | 13 ++ .../CentralPackagesDisabled/Class1.cs | 6 + .../Directory.Packages.props | 9 + .../CentralPackagesGlobal.csproj | 13 ++ .../CentralPackagesGlobal/Class1.cs | 6 + .../Directory.Packages.props | 9 + .../CentralPackages.Default.verified.txt | 5 + ...ralPackagesBuildProps.Default.verified.txt | 5 + ...ntralPackagesDisabled.Default.verified.txt | 5 + ...CentralPackagesGlobal.Default.verified.txt | 5 + src/Snitch.Tests/ProgramTests.cs | 64 ++++++ src/Snitch/Analysis/CentralPackageManager.cs | 198 ++++++++++++++++++ src/Snitch/Analysis/ProjectBuilder.cs | 40 +++- 21 files changed, 450 insertions(+), 4 deletions(-) create mode 100644 src/Snitch.Tests.Fixtures/CentralPackages/CentralPackages.csproj create mode 100644 src/Snitch.Tests.Fixtures/CentralPackages/Class1.cs create mode 100644 src/Snitch.Tests.Fixtures/CentralPackages/Directory.Packages.props create mode 100644 src/Snitch.Tests.Fixtures/CentralPackagesBuildProps/CentralPackagesBuildProps.csproj create mode 100644 src/Snitch.Tests.Fixtures/CentralPackagesBuildProps/Class1.cs create mode 100644 src/Snitch.Tests.Fixtures/CentralPackagesBuildProps/Directory.Build.props create mode 100644 src/Snitch.Tests.Fixtures/CentralPackagesBuildProps/Directory.Packages.props create mode 100644 src/Snitch.Tests.Fixtures/CentralPackagesDisabled/CentralPackagesDisabled.csproj create mode 100644 src/Snitch.Tests.Fixtures/CentralPackagesDisabled/Class1.cs create mode 100644 src/Snitch.Tests.Fixtures/CentralPackagesDisabled/Directory.Packages.props create mode 100644 src/Snitch.Tests.Fixtures/CentralPackagesGlobal/CentralPackagesGlobal.csproj create mode 100644 src/Snitch.Tests.Fixtures/CentralPackagesGlobal/Class1.cs create mode 100644 src/Snitch.Tests.Fixtures/CentralPackagesGlobal/Directory.Packages.props create mode 100644 src/Snitch.Tests/Expectations/CentralPackages.Default.verified.txt create mode 100644 src/Snitch.Tests/Expectations/CentralPackagesBuildProps.Default.verified.txt create mode 100644 src/Snitch.Tests/Expectations/CentralPackagesDisabled.Default.verified.txt create mode 100644 src/Snitch.Tests/Expectations/CentralPackagesGlobal.Default.verified.txt create mode 100644 src/Snitch/Analysis/CentralPackageManager.cs diff --git a/.gitignore b/.gitignore index 828acaf..0a801bf 100644 --- a/.gitignore +++ b/.gitignore @@ -88,4 +88,20 @@ packages Thumbs.db # Rider -.idea/ \ No newline at end of file +.idea/ + +# Verify framework test artifacts +*.received.txt +*.received.* + +# .NET test artifacts +*.trx +*.coverage +*.coveragexml + +# Temporary files +test-* +temp-* + +# Test results directory +TestResults/ diff --git a/src/Snitch.Tests.Fixtures/CentralPackages/CentralPackages.csproj b/src/Snitch.Tests.Fixtures/CentralPackages/CentralPackages.csproj new file mode 100644 index 0000000..28de610 --- /dev/null +++ b/src/Snitch.Tests.Fixtures/CentralPackages/CentralPackages.csproj @@ -0,0 +1,13 @@ + + + + netstandard2.0 + latest + + + + + + + + \ No newline at end of file diff --git a/src/Snitch.Tests.Fixtures/CentralPackages/Class1.cs b/src/Snitch.Tests.Fixtures/CentralPackages/Class1.cs new file mode 100644 index 0000000..68fc679 --- /dev/null +++ b/src/Snitch.Tests.Fixtures/CentralPackages/Class1.cs @@ -0,0 +1,6 @@ +namespace CentralPackages +{ + public class Class1 + { + } +} \ No newline at end of file diff --git a/src/Snitch.Tests.Fixtures/CentralPackages/Directory.Packages.props b/src/Snitch.Tests.Fixtures/CentralPackages/Directory.Packages.props new file mode 100644 index 0000000..6bced27 --- /dev/null +++ b/src/Snitch.Tests.Fixtures/CentralPackages/Directory.Packages.props @@ -0,0 +1,9 @@ + + + true + + + + + + \ No newline at end of file diff --git a/src/Snitch.Tests.Fixtures/CentralPackagesBuildProps/CentralPackagesBuildProps.csproj b/src/Snitch.Tests.Fixtures/CentralPackagesBuildProps/CentralPackagesBuildProps.csproj new file mode 100644 index 0000000..28de610 --- /dev/null +++ b/src/Snitch.Tests.Fixtures/CentralPackagesBuildProps/CentralPackagesBuildProps.csproj @@ -0,0 +1,13 @@ + + + + netstandard2.0 + latest + + + + + + + + \ No newline at end of file diff --git a/src/Snitch.Tests.Fixtures/CentralPackagesBuildProps/Class1.cs b/src/Snitch.Tests.Fixtures/CentralPackagesBuildProps/Class1.cs new file mode 100644 index 0000000..0d0a20a --- /dev/null +++ b/src/Snitch.Tests.Fixtures/CentralPackagesBuildProps/Class1.cs @@ -0,0 +1,6 @@ +namespace CentralPackagesBuildProps +{ + public class Class1 + { + } +} \ No newline at end of file diff --git a/src/Snitch.Tests.Fixtures/CentralPackagesBuildProps/Directory.Build.props b/src/Snitch.Tests.Fixtures/CentralPackagesBuildProps/Directory.Build.props new file mode 100644 index 0000000..9a7757c --- /dev/null +++ b/src/Snitch.Tests.Fixtures/CentralPackagesBuildProps/Directory.Build.props @@ -0,0 +1,5 @@ + + + true + + \ No newline at end of file diff --git a/src/Snitch.Tests.Fixtures/CentralPackagesBuildProps/Directory.Packages.props b/src/Snitch.Tests.Fixtures/CentralPackagesBuildProps/Directory.Packages.props new file mode 100644 index 0000000..47fbd67 --- /dev/null +++ b/src/Snitch.Tests.Fixtures/CentralPackagesBuildProps/Directory.Packages.props @@ -0,0 +1,6 @@ + + + + + + \ No newline at end of file diff --git a/src/Snitch.Tests.Fixtures/CentralPackagesDisabled/CentralPackagesDisabled.csproj b/src/Snitch.Tests.Fixtures/CentralPackagesDisabled/CentralPackagesDisabled.csproj new file mode 100644 index 0000000..4ea2c21 --- /dev/null +++ b/src/Snitch.Tests.Fixtures/CentralPackagesDisabled/CentralPackagesDisabled.csproj @@ -0,0 +1,13 @@ + + + + netstandard2.0 + latest + + + + + + + + \ No newline at end of file diff --git a/src/Snitch.Tests.Fixtures/CentralPackagesDisabled/Class1.cs b/src/Snitch.Tests.Fixtures/CentralPackagesDisabled/Class1.cs new file mode 100644 index 0000000..357e719 --- /dev/null +++ b/src/Snitch.Tests.Fixtures/CentralPackagesDisabled/Class1.cs @@ -0,0 +1,6 @@ +namespace CentralPackagesDisabled +{ + public class Class1 + { + } +} \ No newline at end of file diff --git a/src/Snitch.Tests.Fixtures/CentralPackagesDisabled/Directory.Packages.props b/src/Snitch.Tests.Fixtures/CentralPackagesDisabled/Directory.Packages.props new file mode 100644 index 0000000..3a414a5 --- /dev/null +++ b/src/Snitch.Tests.Fixtures/CentralPackagesDisabled/Directory.Packages.props @@ -0,0 +1,9 @@ + + + false + + + + + + \ No newline at end of file diff --git a/src/Snitch.Tests.Fixtures/CentralPackagesGlobal/CentralPackagesGlobal.csproj b/src/Snitch.Tests.Fixtures/CentralPackagesGlobal/CentralPackagesGlobal.csproj new file mode 100644 index 0000000..267679a --- /dev/null +++ b/src/Snitch.Tests.Fixtures/CentralPackagesGlobal/CentralPackagesGlobal.csproj @@ -0,0 +1,13 @@ + + + + netstandard2.0 + latest + + + + + + + + \ No newline at end of file diff --git a/src/Snitch.Tests.Fixtures/CentralPackagesGlobal/Class1.cs b/src/Snitch.Tests.Fixtures/CentralPackagesGlobal/Class1.cs new file mode 100644 index 0000000..192d358 --- /dev/null +++ b/src/Snitch.Tests.Fixtures/CentralPackagesGlobal/Class1.cs @@ -0,0 +1,6 @@ +namespace CentralPackagesGlobal +{ + public class Class1 + { + } +} \ No newline at end of file diff --git a/src/Snitch.Tests.Fixtures/CentralPackagesGlobal/Directory.Packages.props b/src/Snitch.Tests.Fixtures/CentralPackagesGlobal/Directory.Packages.props new file mode 100644 index 0000000..474dc8c --- /dev/null +++ b/src/Snitch.Tests.Fixtures/CentralPackagesGlobal/Directory.Packages.props @@ -0,0 +1,9 @@ + + + true + + + + + + \ No newline at end of file diff --git a/src/Snitch.Tests/Expectations/CentralPackages.Default.verified.txt b/src/Snitch.Tests/Expectations/CentralPackages.Default.verified.txt new file mode 100644 index 0000000..d6245ed --- /dev/null +++ b/src/Snitch.Tests/Expectations/CentralPackages.Default.verified.txt @@ -0,0 +1,5 @@ +Analyzing... +Analyzing CentralPackages.csproj +Analyzing CentralPackages... + +Everything looks good! \ No newline at end of file diff --git a/src/Snitch.Tests/Expectations/CentralPackagesBuildProps.Default.verified.txt b/src/Snitch.Tests/Expectations/CentralPackagesBuildProps.Default.verified.txt new file mode 100644 index 0000000..ba55078 --- /dev/null +++ b/src/Snitch.Tests/Expectations/CentralPackagesBuildProps.Default.verified.txt @@ -0,0 +1,5 @@ +Analyzing... +Analyzing CentralPackagesBuildProps.csproj +Analyzing CentralPackagesBuildProps... + +Everything looks good! \ No newline at end of file diff --git a/src/Snitch.Tests/Expectations/CentralPackagesDisabled.Default.verified.txt b/src/Snitch.Tests/Expectations/CentralPackagesDisabled.Default.verified.txt new file mode 100644 index 0000000..a1ea245 --- /dev/null +++ b/src/Snitch.Tests/Expectations/CentralPackagesDisabled.Default.verified.txt @@ -0,0 +1,5 @@ +Analyzing... +Analyzing CentralPackagesDisabled.csproj +Analyzing CentralPackagesDisabled... + +Everything looks good! \ No newline at end of file diff --git a/src/Snitch.Tests/Expectations/CentralPackagesGlobal.Default.verified.txt b/src/Snitch.Tests/Expectations/CentralPackagesGlobal.Default.verified.txt new file mode 100644 index 0000000..1fa2014 --- /dev/null +++ b/src/Snitch.Tests/Expectations/CentralPackagesGlobal.Default.verified.txt @@ -0,0 +1,5 @@ +Analyzing... +Analyzing CentralPackagesGlobal.csproj +Analyzing CentralPackagesGlobal... + +Everything looks good! \ No newline at end of file diff --git a/src/Snitch.Tests/ProgramTests.cs b/src/Snitch.Tests/ProgramTests.cs index f264bcd..2344ffe 100644 --- a/src/Snitch.Tests/ProgramTests.cs +++ b/src/Snitch.Tests/ProgramTests.cs @@ -222,5 +222,69 @@ public async Task Should_Return_Expected_Result_For_FSharp_Not_Specifying_Framew exitCode.ShouldBe(0); await Verifier.Verify(output); } + + [Fact] + [Expectation("CentralPackages", "Default")] + public async Task Should_Return_Expected_Result_For_CentralPackages() + { + // Given + var fixture = new Fixture(); + var project = Fixture.GetPath("CentralPackages/CentralPackages.csproj"); + + // When + var (exitCode, output) = await Fixture.Run(project); + + // Then + exitCode.ShouldBe(0); + await Verifier.Verify(output); + } + + [Fact] + [Expectation("CentralPackagesGlobal", "Default")] + public async Task Should_Return_Expected_Result_For_CentralPackages_With_GlobalPackageReference() + { + // Given + var fixture = new Fixture(); + var project = Fixture.GetPath("CentralPackagesGlobal/CentralPackagesGlobal.csproj"); + + // When + var (exitCode, output) = await Fixture.Run(project); + + // Then + exitCode.ShouldBe(0); + await Verifier.Verify(output); + } + + [Fact] + [Expectation("CentralPackagesDisabled", "Default")] + public async Task Should_Return_Expected_Result_For_CentralPackages_When_Disabled() + { + // Given + var fixture = new Fixture(); + var project = Fixture.GetPath("CentralPackagesDisabled/CentralPackagesDisabled.csproj"); + + // When + var (exitCode, output) = await Fixture.Run(project); + + // Then + exitCode.ShouldBe(0); + await Verifier.Verify(output); + } + + [Fact] + [Expectation("CentralPackagesBuildProps", "Default")] + public async Task Should_Return_Expected_Result_For_CentralPackages_With_BuildProps() + { + // Given + var fixture = new Fixture(); + var project = Fixture.GetPath("CentralPackagesBuildProps/CentralPackagesBuildProps.csproj"); + + // When + var (exitCode, output) = await Fixture.Run(project); + + // Then + exitCode.ShouldBe(0); + await Verifier.Verify(output); + } } } diff --git a/src/Snitch/Analysis/CentralPackageManager.cs b/src/Snitch/Analysis/CentralPackageManager.cs new file mode 100644 index 0000000..cab2533 --- /dev/null +++ b/src/Snitch/Analysis/CentralPackageManager.cs @@ -0,0 +1,198 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Xml.Linq; + +namespace Snitch.Analysis +{ + internal sealed class CentralPackageManager + { + private readonly Dictionary _packageVersions; + private readonly Dictionary _globalPackageReferences; + private readonly bool _isCentralManagementEnabled; + + public bool IsCentralManagementEnabled => _isCentralManagementEnabled; + + public CentralPackageManager(string projectPath) + { + if (projectPath == null) + { + throw new ArgumentNullException(nameof(projectPath)); + } + + _packageVersions = new Dictionary(StringComparer.OrdinalIgnoreCase); + _globalPackageReferences = new Dictionary(StringComparer.OrdinalIgnoreCase); + + var directoryPackagesPath = FindDirectoryPackagesProps(projectPath); + if (!string.IsNullOrEmpty(directoryPackagesPath)) + { + _isCentralManagementEnabled = LoadDirectoryPackages(directoryPackagesPath); + } + } + + public string? GetPackageVersion(string packageName) + { + if (_packageVersions.TryGetValue(packageName, out var version)) + { + return version; + } + + return null; + } + + public string? GetGlobalPackageVersion(string packageName) + { + if (_globalPackageReferences.TryGetValue(packageName, out var version)) + { + return version; + } + + return null; + } + + public IEnumerable GetGlobalPackages() + { + return _globalPackageReferences + .Where(kvp => !string.IsNullOrEmpty(kvp.Value)) + .Select(kvp => new Package(kvp.Key, kvp.Value, null)); + } + + public bool HasPackageVersion(string packageName) + { + return _packageVersions.ContainsKey(packageName); + } + + public bool HasGlobalPackageReference(string packageName) + { + return _globalPackageReferences.ContainsKey(packageName); + } + + private string? FindDirectoryPackagesProps(string projectPath) + { + var currentDirectory = Path.GetDirectoryName(projectPath); + + while (currentDirectory != null) + { + var directoryPackagesPath = Path.Combine(currentDirectory, "Directory.Packages.props"); + if (File.Exists(directoryPackagesPath)) + { + return directoryPackagesPath; + } + + var parentDirectory = Directory.GetParent(currentDirectory); + if (parentDirectory == null) + { + break; + } + + currentDirectory = parentDirectory.FullName; + } + + return null; + } + + private bool LoadDirectoryPackages(string directoryPackagesPath) + { + try + { + var document = XDocument.Load(directoryPackagesPath); + var root = document.Root; + + if (root == null) + { + return false; + } + + var isCentralManagementEnabled = IsCentralManagementEnabledInFile(root); + + if (!isCentralManagementEnabled) + { + var directoryBuildPropsPath = Path.Combine(Path.GetDirectoryName(directoryPackagesPath)!, "Directory.Build.props"); + if (File.Exists(directoryBuildPropsPath)) + { + isCentralManagementEnabled = IsCentralManagementEnabledInPropsFile(directoryBuildPropsPath); + } + } + + if (!isCentralManagementEnabled) + { + return false; + } + + var itemGroups = root.Elements("ItemGroup"); + foreach (var itemGroup in itemGroups) + { + var packageVersions = itemGroup.Elements("PackageVersion"); + foreach (var packageVersion in packageVersions) + { + var include = packageVersion.Attribute("Include")?.Value; + var version = packageVersion.Attribute("Version")?.Value; + + if (!string.IsNullOrEmpty(include) && !string.IsNullOrEmpty(version)) + { + _packageVersions[include] = version; + } + } + + var globalPackageReferences = itemGroup.Elements("GlobalPackageReference"); + foreach (var globalPackageReference in globalPackageReferences) + { + var include = globalPackageReference.Attribute("Include")?.Value; + var version = globalPackageReference.Attribute("Version")?.Value; + + if (!string.IsNullOrEmpty(include) && !string.IsNullOrEmpty(version)) + { + _globalPackageReferences[include] = version; + } + } + } + + return isCentralManagementEnabled; + } + catch (Exception) + { + // If we can't parse the file, just continue without central management + return false; + } + } + + private static bool IsCentralManagementEnabledInFile(XElement root) + { + var propertyGroups = root.Elements("PropertyGroup"); + foreach (var propertyGroup in propertyGroups) + { + var centralManagementElement = propertyGroup.Element("ManagePackageVersionsCentrally"); + if (centralManagementElement != null && + bool.TryParse(centralManagementElement.Value, out var isCentralManagement) && + isCentralManagement) + { + return true; + } + } + + return false; + } + + private static bool IsCentralManagementEnabledInPropsFile(string propsFilePath) + { + try + { + var document = XDocument.Load(propsFilePath); + var root = document.Root; + + if (root == null) + { + return false; + } + + return IsCentralManagementEnabledInFile(root); + } + catch (Exception) + { + // If we can't parse the file, just continue without central management + return false; + } + } + } +} \ No newline at end of file diff --git a/src/Snitch/Analysis/ProjectBuilder.cs b/src/Snitch/Analysis/ProjectBuilder.cs index dbd30c3..8e15f74 100644 --- a/src/Snitch/Analysis/ProjectBuilder.cs +++ b/src/Snitch/Analysis/ProjectBuilder.cs @@ -98,13 +98,17 @@ private Project Build( // Add the project to the built list. built.Add(Path.GetFileName(path), project); + // Initialize Central Package Manager + var centralPackageManager = new CentralPackageManager(path); + // Get the package references. foreach (var packageReference in result.PackageReferences) { - if (packageReference.Value.TryGetValue("Version", out var version)) - { - var privateAssets = packageReference.Value.GetValueOrDefault("PrivateAssets"); + var privateAssets = packageReference.Value.GetValueOrDefault("PrivateAssets"); + var version = ResolvePackageVersion(packageReference, centralPackageManager); + if (!string.IsNullOrEmpty(version)) + { project.Packages.Add(new Package(packageReference.Key, version, privateAssets)); } } @@ -166,5 +170,35 @@ private Project Build( return results.FirstOrDefault(); } + + private static string? ResolvePackageVersion( + KeyValuePair> packageReference, + CentralPackageManager centralPackageManager) + { + // Try to get version from PackageReference first + if (packageReference.Value.TryGetValue("Version", out var version) && !string.IsNullOrEmpty(version)) + { + return version; + } + + // If no version and central management is enabled, try to get from Directory.Packages.props + if (centralPackageManager.IsCentralManagementEnabled) + { + var centralVersion = centralPackageManager.GetPackageVersion(packageReference.Key); + if (!string.IsNullOrEmpty(centralVersion)) + { + return centralVersion; + } + + // Check if this is a GlobalPackageReference (these might be processed as PackageReference without version) + var globalVersion = centralPackageManager.GetGlobalPackageVersion(packageReference.Key); + if (!string.IsNullOrEmpty(globalVersion)) + { + return globalVersion; + } + } + + return null; + } } } \ No newline at end of file