From 1842419c4e240f4fb1df0ce9f1b953bc4abc0832 Mon Sep 17 00:00:00 2001 From: "Brett V. Forsgren" Date: Fri, 12 Apr 2024 15:05:09 -0600 Subject: [PATCH] use MSBuild-evaluated property values for TFM --- .../Discover/DiscoveryWorkerTestBase.cs | 30 +++- .../DiscoveryWorkerTests.PackagesConfig.cs | 28 ++-- .../Discover/DiscoveryWorkerTests.Project.cs | 25 ++++ .../Utilities/MSBuildHelperTests.cs | 7 +- .../Utilities/SdkPackageUpdaterHelperTests.cs | 2 +- .../Discover/SdkProjectDiscovery.cs | 131 +++++++++--------- .../Updater/SdkPackageUpdater.cs | 7 +- .../Utilities/MSBuildHelper.cs | 118 ++++++---------- .../spec/dependabot/nuget/file_parser_spec.rb | 3 - 9 files changed, 184 insertions(+), 167 deletions(-) diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTestBase.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTestBase.cs index c1476d28d3..36e5807374 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTestBase.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTestBase.cs @@ -1,4 +1,5 @@ using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using System.Text.Json; using NuGetUpdater.Core.Discover; @@ -29,7 +30,7 @@ protected static async Task TestDiscoveryAsync( protected static void ValidateWorkspaceResult(ExpectedWorkspaceDiscoveryResult expectedResult, WorkspaceDiscoveryResult actualResult) { Assert.NotNull(actualResult); - Assert.Equal(expectedResult.FilePath, actualResult.FilePath); + Assert.Equal(expectedResult.FilePath.NormalizePathToUnix(), actualResult.FilePath.NormalizePathToUnix()); ValidateDirectoryPackagesProps(expectedResult.DirectoryPackagesProps, actualResult.DirectoryPackagesProps); ValidateResultWithDependencies(expectedResult.GlobalJson, actualResult.GlobalJson); ValidateResultWithDependencies(expectedResult.DotNetToolsJson, actualResult.DotNetToolsJson); @@ -50,7 +51,7 @@ void ValidateResultWithDependencies(ExpectedDependencyDiscoveryResult? expectedR Assert.NotNull(actualResult); } - Assert.Equal(expectedResult.FilePath, actualResult.FilePath); + Assert.Equal(expectedResult.FilePath.NormalizePathToUnix(), actualResult.FilePath.NormalizePathToUnix()); ValidateDependencies(expectedResult.Dependencies, actualResult.Dependencies); Assert.Equal(expectedResult.ExpectedDependencyCount ?? expectedResult.Dependencies.Length, actualResult.Dependencies.Length); } @@ -64,12 +65,12 @@ void ValidateProjectResults(ImmutableArray ex foreach (var expectedProject in expectedProjects) { - var actualProject = actualProjects.Single(p => p.FilePath == expectedProject.FilePath); + var actualProject = actualProjects.Single(p => p.FilePath.NormalizePathToUnix() == expectedProject.FilePath.NormalizePathToUnix()); - Assert.Equal(expectedProject.FilePath, actualProject.FilePath); - AssertEx.Equal(expectedProject.Properties, actualProject.Properties); + Assert.Equal(expectedProject.FilePath.NormalizePathToUnix(), actualProject.FilePath.NormalizePathToUnix()); + AssertEx.Equal(expectedProject.Properties, actualProject.Properties, PropertyComparer.Instance); AssertEx.Equal(expectedProject.TargetFrameworks, actualProject.TargetFrameworks); - AssertEx.Equal(expectedProject.ReferencedProjectPaths, actualProject.ReferencedProjectPaths); + AssertEx.Equal(expectedProject.ReferencedProjectPaths.Select(PathHelper.NormalizePathToUnix), actualProject.ReferencedProjectPaths.Select(PathHelper.NormalizePathToUnix)); ValidateDependencies(expectedProject.Dependencies, actualProject.Dependencies); Assert.Equal(expectedProject.ExpectedDependencyCount ?? expectedProject.Dependencies.Length, actualProject.Dependencies.Length); } @@ -114,4 +115,21 @@ protected static async Task RunDiscoveryAsync(TestFile var resultJson = await File.ReadAllTextAsync(resultPath); return JsonSerializer.Deserialize(resultJson, DiscoveryWorker.SerializerOptions)!; } + + internal class PropertyComparer : IEqualityComparer + { + public static PropertyComparer Instance { get; } = new(); + + public bool Equals(Property? x, Property? y) + { + return x?.Name == y?.Name && + x?.Value == y?.Value && + x?.SourceFilePath.NormalizePathToUnix() == y?.SourceFilePath.NormalizePathToUnix(); + } + + public int GetHashCode([DisallowNull] Property obj) + { + throw new NotImplementedException(); + } + } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTests.PackagesConfig.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTests.PackagesConfig.cs index 035da3a873..a0dcab83d5 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTests.PackagesConfig.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTests.PackagesConfig.cs @@ -25,11 +25,14 @@ await TestDiscoveryAsync( - + """), ("myproj.csproj", """ + + net46 + """) ], @@ -40,16 +43,21 @@ await TestDiscoveryAsync( new() { FilePath = "myproj.csproj", + Properties = [ + new("TargetFramework", "net46", "myproj.csproj"), + ], + TargetFrameworks = ["net46"], Dependencies = [ - new("Microsoft.CodeDom.Providers.DotNetCompilerPlatform", "1.0.0", DependencyType.PackagesConfig, TargetFrameworks: []), - new("Microsoft.Net.Compilers", "1.0.1", DependencyType.PackagesConfig, TargetFrameworks: []), - new("Microsoft.Web.Infrastructure", "1.0.0.0", DependencyType.PackagesConfig, TargetFrameworks: []), - new("Microsoft.Web.Xdt", "2.1.1", DependencyType.PackagesConfig, TargetFrameworks: []), - new("Newtonsoft.Json", "8.0.3", DependencyType.PackagesConfig, TargetFrameworks: []), - new("NuGet.Core", "2.11.1", DependencyType.PackagesConfig, TargetFrameworks: []), - new("NuGet.Server", "2.11.2", DependencyType.PackagesConfig, TargetFrameworks: []), - new("RouteMagic", "1.3", DependencyType.PackagesConfig, TargetFrameworks: []), - new("WebActivatorEx", "2.1.0", DependencyType.PackagesConfig, TargetFrameworks: []), + new("Microsoft.NETFramework.ReferenceAssemblies", "1.0.3", DependencyType.Unknown, TargetFrameworks: ["net46"], IsTransitive: true), + new("Microsoft.CodeDom.Providers.DotNetCompilerPlatform", "1.0.0", DependencyType.PackagesConfig, TargetFrameworks: ["net46"]), + new("Microsoft.Net.Compilers", "1.0.1", DependencyType.PackagesConfig, TargetFrameworks: ["net46"]), + new("Microsoft.Web.Infrastructure", "1.0.0.0", DependencyType.PackagesConfig, TargetFrameworks: ["net46"]), + new("Microsoft.Web.Xdt", "2.1.1", DependencyType.PackagesConfig, TargetFrameworks: ["net46"]), + new("Newtonsoft.Json", "8.0.3", DependencyType.PackagesConfig, TargetFrameworks: ["net46"]), + new("NuGet.Core", "2.11.1", DependencyType.PackagesConfig, TargetFrameworks: ["net46"]), + new("NuGet.Server", "2.11.2", DependencyType.PackagesConfig, TargetFrameworks: ["net46"]), + new("RouteMagic", "1.3", DependencyType.PackagesConfig, TargetFrameworks: ["net46"]), + new("WebActivatorEx", "2.1.0", DependencyType.PackagesConfig, TargetFrameworks: ["net46"]), ], } ], diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTests.Project.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTests.Project.cs index d4ddeeba1f..5dfa34eb78 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTests.Project.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Discover/DiscoveryWorkerTests.Project.cs @@ -376,5 +376,30 @@ await TestDiscoveryAsync( ], }); } + + [Fact] + + public async Task NoDependenciesReturnedIfNoTargetFrameworkCanBeResolved() + { + await TestDiscoveryAsync( + workspacePath: "", + files: [ + ("myproj.csproj", """ + + + $(SomeCommonTfmThatCannotBeResolved) + + + + + + """) + ], + expectedResult: new() + { + FilePath = "", + Projects = [] + }); + } } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Utilities/MSBuildHelperTests.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Utilities/MSBuildHelperTests.cs index 5b804f733f..9509d11ec2 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Utilities/MSBuildHelperTests.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Utilities/MSBuildHelperTests.cs @@ -110,15 +110,16 @@ public void ProjectPathsCanBeParsedFromSolutionFiles(string solutionContent, str [InlineData("netstandard2.0", "netstandard2.0", null)] [InlineData(" ; netstandard2.0 ; ", "netstandard2.0", null)] [InlineData("netstandard2.0 ; netstandard2.1 ; ", "netstandard2.0", "netstandard2.1")] - public void TfmsCanBeDeterminedFromProjectContents(string projectContents, string? expectedTfm1, string? expectedTfm2) + [InlineData("netstandard2.0v4.7.2", "netstandard2.0", null)] + [InlineData("$(PropertyThatCannotBeResolved)", null, null)] + public async Task TfmsCanBeDeterminedFromProjectContents(string projectContents, string? expectedTfm1, string? expectedTfm2) { var projectPath = Path.GetTempFileName(); try { File.WriteAllText(projectPath, projectContents); var expectedTfms = new[] { expectedTfm1, expectedTfm2 }.Where(tfm => tfm is not null).ToArray(); - var buildFile = ProjectBuildFile.Open(Path.GetDirectoryName(projectPath)!, projectPath); - var actualTfms = MSBuildHelper.GetTargetFrameworkMonikers(ImmutableArray.Create(buildFile)); + var (_buildFiles, actualTfms) = await MSBuildHelper.LoadBuildFilesAndTargetFrameworksAsync(Path.GetDirectoryName(projectPath)!, projectPath); AssertEx.Equal(expectedTfms, actualTfms); } finally diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Utilities/SdkPackageUpdaterHelperTests.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Utilities/SdkPackageUpdaterHelperTests.cs index 0aef2949da..f39bd61359 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Utilities/SdkPackageUpdaterHelperTests.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Utilities/SdkPackageUpdaterHelperTests.cs @@ -167,7 +167,7 @@ public async Task BuildFileEnumerationWithGlobalJsonWithComments() private static async Task LoadBuildFilesFromTemp(TemporaryDirectory temporaryDirectory, string relativeProjectPath) { - var buildFiles = await MSBuildHelper.LoadBuildFilesAsync(temporaryDirectory.DirectoryPath, $"{temporaryDirectory.DirectoryPath}/{relativeProjectPath}"); + var (buildFiles, _tfms) = await MSBuildHelper.LoadBuildFilesAndTargetFrameworksAsync(temporaryDirectory.DirectoryPath, $"{temporaryDirectory.DirectoryPath}/{relativeProjectPath}"); var buildFilePaths = buildFiles.Select(f => f.RelativePath.NormalizePathToUnix()).ToArray(); return buildFilePaths; } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/SdkProjectDiscovery.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/SdkProjectDiscovery.cs index 761b09da50..947966c82c 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/SdkProjectDiscovery.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Discover/SdkProjectDiscovery.cs @@ -9,86 +9,87 @@ internal static class SdkProjectDiscovery public static async Task> DiscoverAsync(string repoRootPath, string workspacePath, string projectPath, Logger logger) { // Determine which targets and props files contribute to the build. - var buildFiles = await MSBuildHelper.LoadBuildFilesAsync(repoRootPath, projectPath, includeSdkPropsAndTargets: true); + var (buildFiles, projectTargetFrameworks) = await MSBuildHelper.LoadBuildFilesAndTargetFrameworksAsync(repoRootPath, projectPath); + var tfms = projectTargetFrameworks.Order().ToImmutableArray(); // Get all the dependencies which are directly referenced from the project file or indirectly referenced from // targets and props files. var topLevelDependencies = MSBuildHelper.GetTopLevelPackageDependencyInfos(buildFiles); var results = ImmutableArray.CreateBuilder(); - foreach (var buildFile in buildFiles) + if (tfms.Length > 0) { - // Only include build files that exist beneath the RepoRootPath. - if (buildFile.IsOutsideBasePath) + foreach (var buildFile in buildFiles) { - continue; - } - - // The build file dependencies have the correct DependencyType and the TopLevelDependencies have the evaluated version. - // Combine them to have the set of dependencies that are directly referenced from the build file. - var fileDependencies = BuildFile.GetDependencies(buildFile) - .ToDictionary(d => d.Name, StringComparer.OrdinalIgnoreCase); - var sdkDependencies = fileDependencies.Values - .Where(d => d.Type == DependencyType.MSBuildSdk) - .ToImmutableArray(); - var indirectDependencies = topLevelDependencies - .Where(d => !fileDependencies.ContainsKey(d.Name)) - .ToImmutableArray(); - var directDependencies = topLevelDependencies - .Where(d => fileDependencies.ContainsKey(d.Name)) - .Select(d => + // Only include build files that exist beneath the RepoRootPath. + if (buildFile.IsOutsideBasePath) { - var dependency = fileDependencies[d.Name]; - return d with - { - Type = dependency.Type, - IsDirect = true - }; - }).ToImmutableArray(); - - if (buildFile.GetFileType() == ProjectBuildFileType.Project) - { - // Collect information that is specific to the project file. - var tfms = MSBuildHelper.GetTargetFrameworkMonikers(buildFiles) - .OrderBy(tfm => tfm) - .ToImmutableArray(); - var properties = MSBuildHelper.GetProperties(buildFiles).Values - .Where(p => !p.SourceFilePath.StartsWith("..")) - .OrderBy(p => p.Name) - .ToImmutableArray(); - var referencedProjectPaths = MSBuildHelper.GetProjectPathsFromProject(projectPath) - .Select(path => Path.GetRelativePath(workspacePath, path)) - .OrderBy(p => p) - .ToImmutableArray(); + continue; + } - // Get the complete set of dependencies including transitive dependencies. - var dependencies = indirectDependencies.Concat(directDependencies).ToImmutableArray(); - dependencies = dependencies - .Select(d => d with { TargetFrameworks = tfms }) + // The build file dependencies have the correct DependencyType and the TopLevelDependencies have the evaluated version. + // Combine them to have the set of dependencies that are directly referenced from the build file. + var fileDependencies = BuildFile.GetDependencies(buildFile) + .ToDictionary(d => d.Name, StringComparer.OrdinalIgnoreCase); + var sdkDependencies = fileDependencies.Values + .Where(d => d.Type == DependencyType.MSBuildSdk) .ToImmutableArray(); - var transitiveDependencies = await GetTransitiveDependencies(repoRootPath, projectPath, tfms, dependencies, logger); - ImmutableArray allDependencies = dependencies.Concat(transitiveDependencies).Concat(sdkDependencies) - .OrderBy(d => d.Name) + var indirectDependencies = topLevelDependencies + .Where(d => !fileDependencies.ContainsKey(d.Name)) .ToImmutableArray(); + var directDependencies = topLevelDependencies + .Where(d => fileDependencies.ContainsKey(d.Name)) + .Select(d => + { + var dependency = fileDependencies[d.Name]; + return d with + { + Type = dependency.Type, + IsDirect = true + }; + }).ToImmutableArray(); - results.Add(new() + if (buildFile.GetFileType() == ProjectBuildFileType.Project) { - FilePath = Path.GetRelativePath(workspacePath, buildFile.Path), - Properties = properties, - TargetFrameworks = tfms, - ReferencedProjectPaths = referencedProjectPaths, - Dependencies = allDependencies, - }); - } - else - { - results.Add(new() - { - FilePath = Path.GetRelativePath(workspacePath, buildFile.Path), - Dependencies = directDependencies.Concat(sdkDependencies) + // Collect information that is specific to the project file. + var properties = MSBuildHelper.GetProperties(buildFiles).Values + .Where(p => !p.SourceFilePath.StartsWith("..")) + .OrderBy(p => p.Name) + .ToImmutableArray(); + var referencedProjectPaths = MSBuildHelper.GetProjectPathsFromProject(projectPath) + .Select(path => Path.GetRelativePath(workspacePath, path)) + .OrderBy(p => p) + .ToImmutableArray(); + + // Get the complete set of dependencies including transitive dependencies. + var dependencies = indirectDependencies.Concat(directDependencies).ToImmutableArray(); + dependencies = dependencies + .Select(d => d with { TargetFrameworks = tfms }) + .ToImmutableArray(); + var transitiveDependencies = await GetTransitiveDependencies(repoRootPath, projectPath, tfms, dependencies, logger); + ImmutableArray allDependencies = dependencies.Concat(transitiveDependencies).Concat(sdkDependencies) .OrderBy(d => d.Name) - .ToImmutableArray(), - }); + .ToImmutableArray(); + + results.Add(new() + { + FilePath = Path.GetRelativePath(workspacePath, buildFile.Path), + Properties = properties, + TargetFrameworks = tfms, + ReferencedProjectPaths = referencedProjectPaths, + Dependencies = allDependencies, + }); + } + else + { + results.Add(new() + { + FilePath = Path.GetRelativePath(workspacePath, buildFile.Path), + Dependencies = directDependencies.Concat(sdkDependencies) + .OrderBy(d => d.Name) + .ToImmutableArray(), + }); + } } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/SdkPackageUpdater.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/SdkPackageUpdater.cs index 9dfaf7d302..2d572055ad 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/SdkPackageUpdater.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/SdkPackageUpdater.cs @@ -20,8 +20,7 @@ public static async Task UpdateDependencyAsync( // SDK-style project, modify the XML directly logger.Log(" Running for SDK-style project"); - var buildFiles = await MSBuildHelper.LoadBuildFilesAsync(repoRootPath, projectPath); - var tfms = MSBuildHelper.GetTargetFrameworkMonikers(buildFiles); + var (buildFiles, tfms) = await MSBuildHelper.LoadBuildFilesAndTargetFrameworksAsync(repoRootPath, projectPath); // Get the set of all top-level dependencies in the current project var topLevelDependencies = MSBuildHelper.GetTopLevelPackageDependencyInfos(buildFiles).ToArray(); @@ -226,10 +225,10 @@ private static async Task AddTransitiveDependencyAsync(string projectPath, strin logger.Log($" Adding [{dependencyName}/{newDependencyVersion}] as a top-level package reference."); // see https://learn.microsoft.com/nuget/consume-packages/install-use-packages-dotnet-cli - var (exitCode, _, _) = await ProcessEx.RunAsync("dotnet", $"add {projectPath} package {dependencyName} --version {newDependencyVersion}", workingDirectory: Path.GetDirectoryName(projectPath)); + var (exitCode, stdout, stderr) = await ProcessEx.RunAsync("dotnet", $"add {projectPath} package {dependencyName} --version {newDependencyVersion}", workingDirectory: Path.GetDirectoryName(projectPath)); if (exitCode != 0) { - logger.Log($" Transitive dependency [{dependencyName}/{newDependencyVersion}] was not added."); + logger.Log($" Transitive dependency [{dependencyName}/{newDependencyVersion}] was not added.\nSTDOUT:\n{stdout}\nSTDERR:\n{stderr}"); } } diff --git a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/MSBuildHelper.cs b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/MSBuildHelper.cs index bb01e30d85..d74ef51368 100644 --- a/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/MSBuildHelper.cs +++ b/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/MSBuildHelper.cs @@ -60,72 +60,6 @@ public static void RegisterMSBuild() } } - public static string[] GetTargetFrameworkMonikers(ImmutableArray buildFiles) - { - HashSet targetFrameworkValues = new(StringComparer.OrdinalIgnoreCase); - Dictionary propertyInfo = new(StringComparer.OrdinalIgnoreCase); - - foreach (var buildFile in buildFiles) - { - var projectRoot = CreateProjectRootElement(buildFile); - - foreach (var property in projectRoot.Properties) - { - if (property.Name.Equals("TargetFramework", StringComparison.OrdinalIgnoreCase) || - property.Name.Equals("TargetFrameworks", StringComparison.OrdinalIgnoreCase)) - { - if (buildFile.IsOutsideBasePath) - { - continue; - } - - foreach (var tfm in property.Value.Split(';', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)) - { - targetFrameworkValues.Add(tfm); - } - } - else if (property.Name.Equals("TargetFrameworkVersion", StringComparison.OrdinalIgnoreCase)) - { - if (buildFile.IsOutsideBasePath) - { - continue; - } - - // For packages.config projects that use TargetFrameworkVersion, we need to convert it to TargetFramework - targetFrameworkValues.Add($"net{property.Value.TrimStart('v').Replace(".", "")}"); - } - else - { - propertyInfo[property.Name] = new(property.Name, property.Value, buildFile.RelativePath); - } - } - } - - HashSet targetFrameworks = new(StringComparer.OrdinalIgnoreCase); - - foreach (var targetFrameworkValue in targetFrameworkValues) - { - var (resultType, _, tfms, _, errorMessage) = - GetEvaluatedValue(targetFrameworkValue, propertyInfo, propertiesToIgnore: ["TargetFramework", "TargetFrameworks"]); - if (resultType != EvaluationResultType.Success) - { - continue; - } - - if (string.IsNullOrEmpty(tfms)) - { - continue; - } - - foreach (var tfm in tfms.Split(';', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)) - { - targetFrameworks.Add(tfm); - } - } - - return targetFrameworks.ToArray(); - } - public static IEnumerable GetProjectPathsFromSolution(string solutionPath) { var solution = SolutionFile.Parse(solutionPath); @@ -569,7 +503,7 @@ internal static bool TryGetDirectoryPackagesPropsPath(string repoRootPath, strin return directoryPackagesPropsPath is not null; } - internal static async Task> LoadBuildFilesAsync(string repoRootPath, string projectPath, bool includeSdkPropsAndTargets = false) + internal static async Task<(ImmutableArray ProjectBuildFiles, string[] TargetFrameworks)> LoadBuildFilesAndTargetFrameworksAsync(string repoRootPath, string projectPath) { var buildFileList = new List { @@ -579,6 +513,7 @@ internal static async Task> LoadBuildFilesAsync // a global.json file might cause problems with the dotnet msbuild command; create a safe version temporarily TryGetGlobalJsonPath(repoRootPath, projectPath, out var globalJsonPath); var safeGlobalJsonName = $"{globalJsonPath}{Guid.NewGuid()}"; + HashSet targetFrameworks = new(StringComparer.OrdinalIgnoreCase); try { @@ -607,16 +542,51 @@ internal static async Task> LoadBuildFilesAsync // load the project even if it imports a file that doesn't exist (e.g. a file that's generated at restore // or build time). using var projectCollection = new ProjectCollection(); // do this in a one-off instance and don't pollute the global collection - var project = Project.FromFile(projectPath, new ProjectOptions + Project project = Project.FromFile(projectPath, new ProjectOptions { LoadSettings = ProjectLoadSettings.IgnoreMissingImports, ProjectCollection = projectCollection, }); buildFileList.AddRange(project.Imports.Select(i => i.ImportedProject.FullPath.NormalizePathToUnix())); + + // use the MSBuild-evaluated value so we don't have to try to manually parse XML + IEnumerable targetFrameworkProperties = project.Properties.Where(p => p.Name.Equals("TargetFramework", StringComparison.OrdinalIgnoreCase)).ToList(); + IEnumerable targetFrameworksProperties = project.Properties.Where(p => p.Name.Equals("TargetFrameworks", StringComparison.OrdinalIgnoreCase)).ToList(); + IEnumerable targetFrameworkVersionProperties = project.Properties.Where(p => p.Name.Equals("TargetFrameworkVersion", StringComparison.OrdinalIgnoreCase)).ToList(); + foreach (ProjectProperty tfm in targetFrameworkProperties) + { + if (!string.IsNullOrWhiteSpace(tfm.EvaluatedValue)) + { + targetFrameworks.Add(tfm.EvaluatedValue); + } + } + + foreach (ProjectProperty tfms in targetFrameworksProperties) + { + foreach (string tfmValue in tfms.EvaluatedValue.Split(';', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)) + { + targetFrameworks.Add(tfmValue); + } + } + + if (targetFrameworks.Count == 0) + { + // Only try this if we haven't been able to resolve anything yet. This is because deep in the SDK, a + // `TargetFramework` of `netstandard2.0` (eventually) gets turned into `v2.0` and we don't want to + // interpret that as a .NET Framework 2.0 project. + foreach (ProjectProperty tfvm in targetFrameworkVersionProperties) + { + // `v0.0` is an error case where no TFM could be evaluated + if (tfvm.EvaluatedValue != "v0.0") + { + targetFrameworks.Add($"net{tfvm.EvaluatedValue.TrimStart('v').Replace(".", "")}"); + } + } + } } catch (InvalidProjectFileException) { - return []; + return ([], []); } finally { @@ -627,16 +597,14 @@ internal static async Task> LoadBuildFilesAsync } var repoRootPathPrefix = repoRootPath.NormalizePathToUnix() + "/"; - var buildFiles = includeSdkPropsAndTargets - ? buildFileList.Distinct() - : buildFileList - .Where(f => f.StartsWith(repoRootPathPrefix, StringComparison.OrdinalIgnoreCase)) - .Distinct(); + var buildFiles = buildFileList + .Where(f => f.StartsWith(repoRootPathPrefix, StringComparison.OrdinalIgnoreCase)) + .Distinct(); var result = buildFiles .Where(File.Exists) .Select(path => ProjectBuildFile.Open(repoRootPath, path)) .ToImmutableArray(); - return result; + return (result, targetFrameworks.ToArray()); } [GeneratedRegex("^\\s*NuGetData::Package=(?[^,]+), Version=(?.+)$")] diff --git a/nuget/spec/dependabot/nuget/file_parser_spec.rb b/nuget/spec/dependabot/nuget/file_parser_spec.rb index 2ae0ef504d..783f685e4b 100644 --- a/nuget/spec/dependabot/nuget/file_parser_spec.rb +++ b/nuget/spec/dependabot/nuget/file_parser_spec.rb @@ -701,9 +701,6 @@ it "does not return the `.csproj` with an unresolvable TFM" do expect(dependencies.length).to eq(0) - expect(Dependabot.logger).to have_received(:warn).with( - "Excluding project file 'my.csproj' due to unresolvable target framework" - ) end end