diff --git a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetPackageReferenceFrameworkAwareDetector.cs b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetPackageReferenceFrameworkAwareDetector.cs index 41b23a4b..c3e5a386 100644 --- a/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetPackageReferenceFrameworkAwareDetector.cs +++ b/src/Microsoft.ComponentDetection.Detectors/nuget/NuGetPackageReferenceFrameworkAwareDetector.cs @@ -62,6 +62,28 @@ private static string[] GetFrameworkReferences(LockFile lockFile, LockFileTarget return results.Distinct().ToArray(); } + private static bool IsADevelopmentDependency(LockFileTargetLibrary library, LockFile lockFile) + { + // a placeholder item is an empty file that doesn't exist with name _._ meant to indicate an empty folder in a nuget package, but also used by NuGet when a package's assets are excluded. + static bool IsAPlaceholderItem(LockFileItem item) => Path.GetFileName(item.Path).Equals(PackagingCoreConstants.EmptyFolder, StringComparison.OrdinalIgnoreCase); + + // All(IsAPlaceholderItem) checks if the collection is empty or all items are placeholders. + return library.RuntimeAssemblies.All(IsAPlaceholderItem) && + library.RuntimeTargets.All(IsAPlaceholderItem) && + library.ResourceAssemblies.All(IsAPlaceholderItem) && + library.NativeLibraries.All(IsAPlaceholderItem) && + library.ContentFiles.All(IsAPlaceholderItem) && + library.Build.All(IsAPlaceholderItem) && + library.BuildMultiTargeting.All(IsAPlaceholderItem) && + + // The SDK looks at the library for analyzers using the following hueristic: + // https://github.com/dotnet/sdk/blob/d7fe6e66d8f67dc93c5c294a75f42a2924889196/src/Tasks/Microsoft.NET.Build.Tasks/NuGetUtils.NuGet.cs#L43 + (!lockFile.GetLibrary(library.Name, library.Version)?.Files + .Any(file => file.StartsWith("analyzers", StringComparison.Ordinal) + && file.EndsWith(".dll", StringComparison.OrdinalIgnoreCase) + && !file.EndsWith(".resources.dll", StringComparison.OrdinalIgnoreCase)) ?? false); + } + protected override Task OnFileFoundAsync(ProcessRequest processRequest, IDictionary detectorArgs, CancellationToken cancellationToken = default) { try @@ -89,13 +111,15 @@ protected override Task OnFileFoundAsync(ProcessRequest processRequest, IDiction { var frameworkReferences = GetFrameworkReferences(lockFile, target); var frameworkPackages = FrameworkPackages.GetFrameworkPackages(target.TargetFramework, frameworkReferences); - bool IsAFrameworkPackage(string id, NuGetVersion version) => frameworkPackages.Any(fp => fp.IsAFrameworkComponent(id, version)); + bool IsFrameworkOrDevelopmentDependency(LockFileTargetLibrary library) => + frameworkPackages.Any(fp => fp.IsAFrameworkComponent(library.Name, library.Version)) || + IsADevelopmentDependency(library, lockFile); // This call to GetTargetLibrary is not guarded, because if this can't be resolved then something is fundamentally broken (e.g. an explicit dependency reference not being in the list of libraries) // issue: we treat top level dependencies for all targets as top level for each target, but some may not be top level for other targets, or may not even be present for other targets. foreach (var library in explicitReferencedDependencies.Select(x => target.GetTargetLibrary(x.Name)).Where(x => x != null)) { - this.NavigateAndRegister(target, explicitlyReferencedComponentIds, singleFileComponentRecorder, library, null, IsAFrameworkPackage); + this.NavigateAndRegister(target, explicitlyReferencedComponentIds, singleFileComponentRecorder, library, null, IsFrameworkOrDevelopmentDependency); } } @@ -117,7 +141,7 @@ private void NavigateAndRegister( ISingleFileComponentRecorder singleFileComponentRecorder, LockFileTargetLibrary library, string parentComponentId, - Func isAFrameworkComponent, + Func isDevelopmentDependency, HashSet visited = null) { if (library.Type == ProjectDependencyType) @@ -125,9 +149,6 @@ private void NavigateAndRegister( return; } - var isFrameworkComponent = isAFrameworkComponent(library.Name, library.Version); - var isDevelopmentDependency = IsADevelopmentDependency(library); - visited ??= []; var libraryComponent = new DetectedComponent(new NuGetComponent(library.Name, library.Version.ToNormalizedString())); @@ -137,7 +158,7 @@ private void NavigateAndRegister( libraryComponent, explicitlyReferencedComponentIds.Contains(libraryComponent.Component.Id), parentComponentId, - isDevelopmentDependency: isFrameworkComponent || isDevelopmentDependency, + isDevelopmentDependency: isDevelopmentDependency(library), targetFramework: target.TargetFramework?.GetShortFolderName()); foreach (var dependency in library.Dependencies) @@ -153,15 +174,9 @@ private void NavigateAndRegister( if (targetLibrary != null) { visited.Add(dependency.Id); - this.NavigateAndRegister(target, explicitlyReferencedComponentIds, singleFileComponentRecorder, targetLibrary, libraryComponent.Component.Id, isAFrameworkComponent, visited); + this.NavigateAndRegister(target, explicitlyReferencedComponentIds, singleFileComponentRecorder, targetLibrary, libraryComponent.Component.Id, isDevelopmentDependency, visited); } } - - // a placeholder item is an empty file that doesn't exist with name _._ meant to indicate an empty folder in a nuget package, but also used by NuGet when a package's assets are excluded. - bool IsAPlaceholderItem(LockFileItem item) => Path.GetFileName(item.Path).Equals(PackagingCoreConstants.EmptyFolder, StringComparison.OrdinalIgnoreCase); - - // A library is development dependency if all of the runtime assemblies and runtime targets are placeholders or empty (All returns true for empty). - bool IsADevelopmentDependency(LockFileTargetLibrary library) => library.RuntimeAssemblies.Concat(library.RuntimeTargets).All(IsAPlaceholderItem); } private void RegisterPackageDownloads(ISingleFileComponentRecorder singleFileComponentRecorder, LockFile lockFile) @@ -177,12 +192,13 @@ private void RegisterPackageDownloads(ISingleFileComponentRecorder singleFileCom var libraryComponent = new DetectedComponent(new NuGetComponent(packageDownload.Name, packageDownload.VersionRange.MinVersion.ToNormalizedString())); - // PackageDownload is always a development dependency since it's usage does not make it part of the application + // Conservatively assume that PackageDownloads are not develeopment dependencies even though usage will not effect any runtime behavior. + // Most often they are used for some runtime deployment -- runtime packs, host packs, AOT infrastructure, etc, so opt in treating them as non-development-dependencies. singleFileComponentRecorder.RegisterUsage( libraryComponent, isExplicitReferencedDependency: true, parentComponentId: null, - isDevelopmentDependency: true, + isDevelopmentDependency: false, targetFramework: framework.FrameworkName?.GetShortFolderName()); } } diff --git a/test/Microsoft.ComponentDetection.Detectors.Tests/nuget/NuGetPackageReferenceFrameworkAwareDetectorTests.cs b/test/Microsoft.ComponentDetection.Detectors.Tests/nuget/NuGetPackageReferenceFrameworkAwareDetectorTests.cs index 99cff08a..2171ce80 100644 --- a/test/Microsoft.ComponentDetection.Detectors.Tests/nuget/NuGetPackageReferenceFrameworkAwareDetectorTests.cs +++ b/test/Microsoft.ComponentDetection.Detectors.Tests/nuget/NuGetPackageReferenceFrameworkAwareDetectorTests.cs @@ -43,7 +43,7 @@ public async Task ScanDirectoryAsync_Base_2_2_VerificationAsync() detectedComponents.Should().HaveCount(22); var nonDevComponents = detectedComponents.Where(c => !componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).GetValueOrDefault()); - nonDevComponents.Should().HaveCount(3); + nonDevComponents.Should().HaveCount(17); foreach (var component in detectedComponents) { @@ -68,7 +68,7 @@ public async Task ScanDirectoryAsync_Base_2_2_additional_VerificationAsync() detectedComponents.Should().HaveCount(68); var nonDevComponents = detectedComponents.Where(c => !componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).GetValueOrDefault()); - nonDevComponents.Should().HaveCount(22); + nonDevComponents.Should().HaveCount(27); nonDevComponents.Select(x => x.Component).Cast().FirstOrDefault(x => x.Name.Contains("Polly")).Should().NotBeNull(); nonDevComponents.Select(x => x.Component).Cast().Count(x => x.Name.Contains("System.Composition")).Should().Be(5); @@ -90,9 +90,10 @@ public async Task ScanDirectoryAsync_ExcludedFrameworkComponent_2_2_Verification .WithFile(this.projectAssetsJsonFileName, osAgnostic) .ExecuteDetectorAsync(); - var developmentDependencies = componentRecorder.GetDetectedComponents().Where(c => componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).GetValueOrDefault()); - developmentDependencies.Should().HaveCountGreaterThan(5, "Ommitted framework assemblies are missing. There should be more than ten, but this is a gut check to make sure we have data."); - developmentDependencies.Should().Contain(c => c.Component.Id.StartsWith("Microsoft.NETCore.App "), "Microsoft.NETCore.App should be treated as a development dependency."); + var dependencies = componentRecorder.GetDetectedComponents(); + var developmentDependencies = dependencies.Where(c => componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).GetValueOrDefault()); + developmentDependencies.Should().HaveCount(5); + developmentDependencies.Should().Contain(c => c.Component.Id.StartsWith("Microsoft.NETCore.Platforms "), "Microsoft.NETCore.Platforms should be treated as a development dependency."); } [TestMethod] @@ -326,10 +327,10 @@ public async Task ScanDirectoryAsync_PackageDownload_VerificationAsync() .WithFile(this.projectAssetsJsonFileName, TestResources.project_assets_packageDownload) .ExecuteDetectorAsync(); - var developmentDependencies = componentRecorder.GetDetectedComponents().Where(c => componentRecorder.GetEffectiveDevDependencyValue(c.Component.Id).GetValueOrDefault()); - developmentDependencies.Should().HaveCount(3, "PackageDownload dev dependencies should exist."); - developmentDependencies.Select(c => c.Component).Should().AllBeOfType(); - developmentDependencies.Select(c => c.TargetFrameworks).Should().AllSatisfy(tfms => tfms.Should().BeEquivalentTo(["net8.0"])); + var dependencies = componentRecorder.GetDetectedComponents(); + dependencies.Should().HaveCount(3, "PackageDownload dependencies should exist."); + dependencies.Select(c => c.Component).Should().AllBeOfType(); + dependencies.Select(c => c.TargetFrameworks).Should().AllSatisfy(tfms => tfms.Should().BeEquivalentTo(["net8.0"])); } [TestMethod]