allow the same package with multiple versions

This corresponds to a project with multiple TFMs where the same package is imported in each case, but with a different version each time.

Author: brettfo

src/Microsoft.ComponentDetection.Detectors/nuget/NuGetMSBuildBinaryLogComponentDetector.cs

@@ -58,7 +58,7 @@ public NuGetMSBuildBinaryLogComponentDetector(
 
     public override int Version { get; } = 1;
 
-    private static void ProcessResolvedPackageReference(Dictionary<string, HashSet<string>> topLevelDependencies, Dictionary<string, Dictionary<string, string>> projectResolvedDependencies, NamedNode node)
+    private static void ProcessResolvedPackageReference(Dictionary<string, HashSet<string>> topLevelDependencies, Dictionary<string, Dictionary<string, HashSet<string>>> projectResolvedDependencies, NamedNode node)
     {
         var doRemoveOperation = node is RemoveItem;
         var doAddOperation = node is AddItem;
@@ -110,14 +110,20 @@ private static void ProcessResolvedPackageReference(Dictionary<string, HashSet<s
                                 projectResolvedDependencies[project.ProjectFile] = projectDependencies;
                             }
 
+                            if (!projectDependencies.TryGetValue(packageName, out var packageVersions))
+                            {
+                                packageVersions = new(StringComparer.OrdinalIgnoreCase);
+                                projectDependencies[packageName] = packageVersions;
+                            }
+
                             if (doRemoveOperation)
                             {
-                                projectDependencies.Remove(packageName);
+                                packageVersions.Remove(packageVersion);
                             }
 
                             if (doAddOperation)
                             {
-                                projectDependencies[packageName] = packageVersion;
+                                packageVersions.Add(packageVersion);
                             }
 
                             project = project.GetNearestParent<Project>();
@@ -183,7 +189,7 @@ private void ProcessBinLog(Build buildRoot, ISingleFileComponentRecorder compone
     {
         // maps a project path to a set of resolved dependencies
         var projectTopLevelDependencies = new Dictionary<string, HashSet<string>>(StringComparer.OrdinalIgnoreCase);
-        var projectResolvedDependencies = new Dictionary<string, Dictionary<string, string>>(StringComparer.OrdinalIgnoreCase);
+        var projectResolvedDependencies = new Dictionary<string, Dictionary<string, HashSet<string>>>(StringComparer.OrdinalIgnoreCase);
         buildRoot.VisitAllChildren<BaseNode>(node =>
         {
             switch (node)
@@ -198,7 +204,7 @@ private void ProcessBinLog(Build buildRoot, ISingleFileComponentRecorder compone
 
         // dependencies were resolved per project, we need to re-arrange them to be per package/version
         var projectsPerPackage = new Dictionary<string, HashSet<string>>(StringComparer.OrdinalIgnoreCase);
-        foreach (var projectPath in projectResolvedDependencies.Keys)
+        foreach (var projectPath in projectResolvedDependencies.Keys.OrderBy(p => p))
         {
             if (Path.GetExtension(projectPath).Equals(".sln", StringComparison.OrdinalIgnoreCase))
             {
@@ -207,16 +213,19 @@ private void ProcessBinLog(Build buildRoot, ISingleFileComponentRecorder compone
             }
 
             var projectDependencies = projectResolvedDependencies[projectPath];
-            foreach (var (packageName, packageVersion) in projectDependencies)
+            foreach (var (packageName, packageVersions) in projectDependencies.OrderBy(p => p.Key))
             {
-                var key = $"{packageName}/{packageVersion}";
-                if (!projectsPerPackage.TryGetValue(key, out var projectPaths))
+                foreach (var packageVersion in packageVersions.OrderBy(v => v))
                 {
-                    projectPaths = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
-                    projectsPerPackage[key] = projectPaths;
-                }
+                    var key = $"{packageName}/{packageVersion}";
+                    if (!projectsPerPackage.TryGetValue(key, out var projectPaths))
+                    {
+                        projectPaths = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+                        projectsPerPackage[key] = projectPaths;
+                    }
 
-                projectPaths.Add(projectPath);
+                    projectPaths.Add(projectPath);
+                }
             }
         }
 

test/Microsoft.ComponentDetection.Detectors.Tests/MSBuildTestUtilities.cs

@@ -58,6 +58,7 @@ public static class MSBuildTestUtilities
     public static async Task<Stream> GetBinLogStreamFromFileContentsAsync(
         string defaultFilePath,
         string defaultFileContents,
+        string targetName = null,
         (string FileName, string Contents)[] additionalFiles = null,
         (string Name, string Version, string TargetFramework, string AdditionalMetadataXml)[] mockedPackages = null)
     {
@@ -79,7 +80,8 @@ public static async Task<Stream> GetBinLogStreamFromFileContentsAsync(
         await MockNuGetPackagesInDirectoryAsync(tempDir, mockedPackages);
 
         // generate the binlog
-        var (exitCode, stdOut, stdErr) = await RunProcessAsync("dotnet", $"build \"{fullDefaultFilePath}\" /t:GenerateBuildDependencyFile /bl:msbuild.binlog", workingDirectory: tempDir.DirectoryPath);
+        targetName ??= "GenerateBuildDependencyFile";
+        var (exitCode, stdOut, stdErr) = await RunProcessAsync("dotnet", $"build \"{fullDefaultFilePath}\" /t:{targetName} /bl:msbuild.binlog", workingDirectory: tempDir.DirectoryPath);
         exitCode.Should().Be(0, $"STDOUT:\n{stdOut}\n\nSTDERR:\n{stdErr}");
 
         // copy it to memory so the temporary directory can be cleaned up

test/Microsoft.ComponentDetection.Detectors.Tests/NuGetMSBuildBinaryLogComponentDetectorTests.cs

@@ -214,6 +214,50 @@ public async Task PackagesReportedFromSeparateProjectsDoNotOverlap()
         solutionComponents.Should().BeEmpty();
     }
 
+    [TestMethod]
+    public async Task PackagesAreReportedWhenConditionedOnTargetFramework()
+    {
+        // This test simulates a project with multiple TFMs where the same package is imported in each, but with a
+        // different version.  To avoid building the entire project, we can fake what MSBuild does by resolving
+        // packages with each TFM.  I manually verified that a "real" invocation of MSBuild with the "Build" target
+        // produces the same shape of the binlog as this test generates.
+
+        // The end result is that _all_ packages are reported, regardless of the TFM invokation they came from, and in
+        // this case that is good, because we really only care about what packages were used in the build and what
+        // project file they came from.
+        var (scanResult, componentRecorder) = await this.ExecuteDetectorAndGetBinLogAsync(
+            projectContents: $@"
+                <Project Sdk=""Microsoft.NET.Sdk"">
+                  <PropertyGroup>
+                    <TargetFrameworks>netstandard2.0;{MSBuildTestUtilities.TestTargetFramework}</TargetFrameworks>
+                  </PropertyGroup>
+                  <ItemGroup>
+                    <PackageReference Include=""Some.Package"" Version=""1.2.3"" Condition=""'$(TargetFramework)' == 'netstandard2.0'"" />
+                    <PackageReference Include=""Some.Package"" Version=""4.5.6"" Condition=""'$(TargetFramework)' == '{MSBuildTestUtilities.TestTargetFramework}'"" />
+                  </ItemGroup>
+                  <Target Name=""TEST_GenerateBuildDependencyFileForTargetFrameworks"">
+                    <MSBuild Projects=""$(MSBuildThisFile)"" Properties=""TargetFramework=netstandard2.0"" Targets=""GenerateBuildDependencyFile"" />
+                    <MSBuild Projects=""$(MSBuildThisFile)"" Properties=""TargetFramework={MSBuildTestUtilities.TestTargetFramework}"" Targets=""GenerateBuildDependencyFile"" />
+                  </Target>
+                </Project>
+                ",
+            targetName: "TEST_GenerateBuildDependencyFileForTargetFrameworks",
+            mockedPackages: new[]
+                {
+                    ("NETStandard.Library", "2.0.3", "netstandard2.0", "<dependencies />"),
+                    ("Some.Package", "1.2.3", "netstandard2.0", null),
+                    ("Some.Package", "4.5.6", MSBuildTestUtilities.TestTargetFramework, null),
+                }
+            );
+
+        var detectedComponents = componentRecorder.GetDetectedComponents();
+
+        var packages = detectedComponents
+            .Where(d => d.FilePaths.Any(p => p.Replace("\\", "/").EndsWith("/project.csproj")))
+            .Select(d => d.Component).Cast<NuGetComponent>().OrderBy(c => c.Name).ThenBy(c => c.Version).Select(c => $"{c.Name}/{c.Version}");
+        packages.Should().Equal("NETStandard.Library/2.0.3", "Some.Package/1.2.3", "Some.Package/4.5.6");
+    }
+
     [TestMethod]
     public async Task PackagesImplicitlyAddedBySdkDuringPublishAreAdded()
     {
@@ -428,10 +472,11 @@ public async Task PackagesImplicitlyAddedBySdkDuringPublishAreAdded()
 
     private async Task<(IndividualDetectorScanResult ScanResult, IComponentRecorder ComponentRecorder)> ExecuteDetectorAndGetBinLogAsync(
         string projectContents,
+        string targetName = null,
         (string FileName, string Content)[] additionalFiles = null,
         (string Name, string Version, string TargetFramework, string DependenciesXml)[] mockedPackages = null)
     {
-        using var binLogStream = await MSBuildTestUtilities.GetBinLogStreamFromFileContentsAsync("project.csproj", projectContents, additionalFiles: additionalFiles, mockedPackages: mockedPackages);
+        using var binLogStream = await MSBuildTestUtilities.GetBinLogStreamFromFileContentsAsync("project.csproj", projectContents, targetName: targetName, additionalFiles: additionalFiles, mockedPackages: mockedPackages);
         var (scanResult, componentRecorder) = await this.DetectorTestUtility
             .WithFile("msbuild.binlog", binLogStream)
             .ExecuteDetectorAsync();