From a9a1b9130320333256dcadbcc3a50fd5dacac590 Mon Sep 17 00:00:00 2001 From: Sunanda Balasubramanian Date: Thu, 10 Jun 2021 15:47:03 -0700 Subject: [PATCH] Make Package Analyzers TFM aware (#608) * Make Package Analyzers TFM aware * Fix tests * Add TFM to IDependencyAnalysisState * Remove unnecessary imports * Remove unnecessary imports * PR Feedback * Fix tests --- .../Dependencies/IDependencyAnalysisState.cs | 4 ++++ .../NewtonsoftReferenceAnalyzer.cs | 4 ++-- .../AnalyzePackageStatus.cs | 13 +++++++++++-- .../Analyzers/PackageMapReferenceAnalyzer.cs | 6 ++---- .../TargetCompatibilityReferenceAnalyzer.cs | 10 +++++----- .../Analyzers/UpgradeAssistantReferenceAnalyzer.cs | 2 +- .../Analyzers/WindowsCompatReferenceAnalyzer.cs | 4 ++-- .../DependencyAnalysisState.cs | 6 +++++- .../DependencyAnalyzerRunner.cs | 4 ++-- .../IDependencyAnalyzerRunner.cs | 3 ++- .../PackageUpdaterStep.cs | 5 +++-- .../NewtonsoftReferenceAnalyzerTests.cs | 3 +++ 12 files changed, 42 insertions(+), 22 deletions(-) diff --git a/src/common/Microsoft.DotNet.UpgradeAssistant.Abstractions/Dependencies/IDependencyAnalysisState.cs b/src/common/Microsoft.DotNet.UpgradeAssistant.Abstractions/Dependencies/IDependencyAnalysisState.cs index 9150b378..9d53454b 100644 --- a/src/common/Microsoft.DotNet.UpgradeAssistant.Abstractions/Dependencies/IDependencyAnalysisState.cs +++ b/src/common/Microsoft.DotNet.UpgradeAssistant.Abstractions/Dependencies/IDependencyAnalysisState.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; + namespace Microsoft.DotNet.UpgradeAssistant.Dependencies { public interface IDependencyAnalysisState @@ -16,5 +18,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Dependencies bool AreChangesRecommended { get; } BuildBreakRisk Risk { get; } + + IReadOnlyCollection TargetFrameworks { get; } } } diff --git a/src/extensions/web/Microsoft.DotNet.UpgradeAssistant.Extensions.Web/NewtonsoftReferenceAnalyzer.cs b/src/extensions/web/Microsoft.DotNet.UpgradeAssistant.Extensions.Web/NewtonsoftReferenceAnalyzer.cs index a083ab99..f742aa69 100644 --- a/src/extensions/web/Microsoft.DotNet.UpgradeAssistant.Extensions.Web/NewtonsoftReferenceAnalyzer.cs +++ b/src/extensions/web/Microsoft.DotNet.UpgradeAssistant.Extensions.Web/NewtonsoftReferenceAnalyzer.cs @@ -47,7 +47,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Extensions.Web // This reference only needs added to ASP.NET Core exes if (!(components.HasFlag(ProjectComponents.AspNetCore) && project.OutputType == ProjectOutputType.Exe - && !project.TargetFrameworks.Any(tfm => _tfmComparer.Compare(tfm, TargetFrameworkMoniker.NetCoreApp30) < 0))) + && !state.TargetFrameworks.Any(tfm => _tfmComparer.Compare(tfm, TargetFrameworkMoniker.NetCoreApp30) < 0))) { return; } @@ -60,7 +60,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Extensions.Web if (!state.Packages.Any(r => NewtonsoftPackageName.Equals(r.Name, StringComparison.OrdinalIgnoreCase))) { - var newtonsoftPackage = await _packageLoader.GetLatestVersionAsync(NewtonsoftPackageName, project.TargetFrameworks, false, token).ConfigureAwait(false); + var newtonsoftPackage = await _packageLoader.GetLatestVersionAsync(NewtonsoftPackageName, state.TargetFrameworks, false, token).ConfigureAwait(false); if (newtonsoftPackage is not null) { diff --git a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/AnalyzePackageStatus.cs b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/AnalyzePackageStatus.cs index fa2856d9..be7a6965 100644 --- a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/AnalyzePackageStatus.cs +++ b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/AnalyzePackageStatus.cs @@ -11,14 +11,17 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages public class AnalyzePackageStatus : IAnalyzeResultProvider { private readonly IDependencyAnalyzerRunner _packageAnalyzer; + private readonly ITargetFrameworkSelector _tfmSelector; private IDependencyAnalysisState? _analysisState; private ILogger Logger { get; } public AnalyzePackageStatus(IDependencyAnalyzerRunner packageAnalyzer, + ITargetFrameworkSelector tfmSelector, ILogger logger) { Logger = logger; + _tfmSelector = tfmSelector; _packageAnalyzer = packageAnalyzer ?? throw new ArgumentNullException(nameof(packageAnalyzer)); _analysisState = null; } @@ -35,9 +38,15 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages foreach (var project in projects) { + var targetTfm = await _tfmSelector.SelectTargetFrameworkAsync(project, token).ConfigureAwait(false); + var targetframeworks = new TargetFrameworkMoniker[] + { + targetTfm + }; + try { - _analysisState = await _packageAnalyzer.AnalyzeAsync(context, project, token).ConfigureAwait(false); + _analysisState = await _packageAnalyzer.AnalyzeAsync(context, project, targetframeworks, token).ConfigureAwait(false); if (!_analysisState.IsValid) { Logger.LogError($"Package analysis failed"); @@ -50,7 +59,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages Logger.LogCritical(exc, "Unexpected exception analyzing package references for: {ProjectPath}", context.CurrentProject.Required().FileInfo); } - Logger.LogInformation("Package Analysis for {ProjectPath}", project.FileInfo.Name); + Logger.LogInformation("Package Analysis for {ProjectPath} for the target TFM of {TargetTFM}", project.FileInfo.Name, targetTfm); if (_analysisState is null || !_analysisState.AreChangesRecommended) { diff --git a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/PackageMapReferenceAnalyzer.cs b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/PackageMapReferenceAnalyzer.cs index 8d1afe8a..6bc4a029 100644 --- a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/PackageMapReferenceAnalyzer.cs +++ b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/PackageMapReferenceAnalyzer.cs @@ -50,10 +50,8 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Analyzers throw new ArgumentNullException(nameof(state)); } - var currentTFM = project.TargetFrameworks; - // Get package maps as an array here so that they're only loaded once (as opposed to each iteration through the loop) - var packageMaps = currentTFM.Any(c => c.IsFramework) ? _packageMaps.Where(x => x.NetCorePackagesWorkOnNetFx).ToArray() : _packageMaps; + var packageMaps = state.TargetFrameworks.Any(c => c.IsFramework) ? _packageMaps.Where(x => x.NetCorePackagesWorkOnNetFx).ToArray() : _packageMaps; foreach (var packageReference in state.Packages) { @@ -108,7 +106,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Analyzers var packageToAdd = newPackage; if (packageToAdd.HasWildcardVersion) { - var reference = await _packageLoader.GetLatestVersionAsync(packageToAdd.Name, project.TargetFrameworks, false, token).ConfigureAwait(false); + var reference = await _packageLoader.GetLatestVersionAsync(packageToAdd.Name, state.TargetFrameworks, false, token).ConfigureAwait(false); if (reference is not null) { diff --git a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/TargetCompatibilityReferenceAnalyzer.cs b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/TargetCompatibilityReferenceAnalyzer.cs index c188cde8..e690d83c 100644 --- a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/TargetCompatibilityReferenceAnalyzer.cs +++ b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/TargetCompatibilityReferenceAnalyzer.cs @@ -43,20 +43,20 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Analyzers foreach (var packageReference in state.Packages) { // If the package doesn't target the right framework but a newer version does, mark it for removal and the newer version for addition - if (await _packageLoader.DoesPackageSupportTargetFrameworksAsync(packageReference, project.TargetFrameworks, token).ConfigureAwait(false)) + if (await _packageLoader.DoesPackageSupportTargetFrameworksAsync(packageReference, state.TargetFrameworks, token).ConfigureAwait(false)) { - _logger.LogDebug("Package {NuGetPackage} will work on {TargetFramework}", packageReference, project.TargetFrameworks); + _logger.LogDebug("Package {NuGetPackage} will work on {TargetFramework}", packageReference, state.TargetFrameworks); continue; } else { // If the package won't work on the target Framework, check newer versions of the package - var newerVersions = await _packageLoader.GetNewerVersionsAsync(packageReference, project.TargetFrameworks, true, token).ConfigureAwait(false); + var newerVersions = await _packageLoader.GetNewerVersionsAsync(packageReference, state.TargetFrameworks, true, token).ConfigureAwait(false); var updatedReference = newerVersions.FirstOrDefault(); if (updatedReference == null) { - _logger.LogWarning("No version of {PackageName} found that supports {TargetFramework}; leaving unchanged", packageReference.Name, project.TargetFrameworks); + _logger.LogWarning("No version of {PackageName} found that supports {TargetFramework}; leaving unchanged", packageReference.Name, state.TargetFrameworks); } else { @@ -70,7 +70,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Analyzers if (updatedReference.IsPrerelease) { - _logger.LogWarning("Package {NuGetPackage} has been upgraded to a prerelease version ({NewVersion}) because no released version supports target(s) {TFM}", packageReference.Name, updatedReference.Version, string.Join(", ", project.TargetFrameworks)); + _logger.LogWarning("Package {NuGetPackage} has been upgraded to a prerelease version ({NewVersion}) because no released version supports target(s) {TFM}", packageReference.Name, updatedReference.Version, string.Join(", ", state.TargetFrameworks)); } state.Packages.Remove(packageReference); diff --git a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/UpgradeAssistantReferenceAnalyzer.cs b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/UpgradeAssistantReferenceAnalyzer.cs index 492325e3..5ba17c43 100644 --- a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/UpgradeAssistantReferenceAnalyzer.cs +++ b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/UpgradeAssistantReferenceAnalyzer.cs @@ -46,7 +46,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Analyzers // If the project doesn't include a reference to the analyzer package, mark it for addition if (!state.Packages.Any(r => AnalyzerPackageName.Equals(r.Name, StringComparison.OrdinalIgnoreCase))) { - var analyzerPackage = await _packageLoader.GetLatestVersionAsync(AnalyzerPackageName, project.TargetFrameworks, true, token).ConfigureAwait(false); + var analyzerPackage = await _packageLoader.GetLatestVersionAsync(AnalyzerPackageName, state.TargetFrameworks, true, token).ConfigureAwait(false); if (analyzerPackage is not null) { diff --git a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/WindowsCompatReferenceAnalyzer.cs b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/WindowsCompatReferenceAnalyzer.cs index 257e1d3c..4caac767 100644 --- a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/WindowsCompatReferenceAnalyzer.cs +++ b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/WindowsCompatReferenceAnalyzer.cs @@ -42,7 +42,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Analyzers throw new ArgumentNullException(nameof(state)); } - if (!project.TargetFrameworks.Any(tfm => tfm.IsWindows)) + if (!state.TargetFrameworks.Any(tfm => tfm.IsWindows)) { return; } @@ -53,7 +53,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Analyzers return; } - var latestVersion = await _loader.GetLatestVersionAsync(PackageName, project.TargetFrameworks, false, token).ConfigureAwait(false); + var latestVersion = await _loader.GetLatestVersionAsync(PackageName, state.TargetFrameworks, false, token).ConfigureAwait(false); if (latestVersion is null) { diff --git a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/DependencyAnalysisState.cs b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/DependencyAnalysisState.cs index 74b60473..57fd3932 100644 --- a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/DependencyAnalysisState.cs +++ b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/DependencyAnalysisState.cs @@ -2,13 +2,14 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using Microsoft.DotNet.UpgradeAssistant.Dependencies; namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages { public sealed class DependencyAnalysisState : IDependencyAnalysisState { - public DependencyAnalysisState(IProject project, INuGetReferences nugetReferences) + public DependencyAnalysisState(IProject project, INuGetReferences nugetReferences, IReadOnlyCollection targetFrameworks) { if (project is null) { @@ -24,6 +25,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages Packages = new DependencyCollection(initial: nugetReferences.PackageReferences, setRisk: SetRisk); References = new DependencyCollection(project.References, SetRisk); IsValid = true; + TargetFrameworks = targetFrameworks; void SetRisk(BuildBreakRisk risk) { Risk = (BuildBreakRisk)Math.Max((int)Risk, (int)risk); @@ -47,5 +49,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages public bool AreChangesRecommended => FrameworkReferences.HasChanges || Packages.HasChanges || References.HasChanges; public bool IsValid { get; set; } + + public IReadOnlyCollection TargetFrameworks { get; } } } diff --git a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/DependencyAnalyzerRunner.cs b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/DependencyAnalyzerRunner.cs index e38a4290..4f6c5b3f 100644 --- a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/DependencyAnalyzerRunner.cs +++ b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/DependencyAnalyzerRunner.cs @@ -26,7 +26,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages _logger = logger; } - public async Task AnalyzeAsync(IUpgradeContext context, IProject? projectRoot, CancellationToken token) + public async Task AnalyzeAsync(IUpgradeContext context, IProject? projectRoot, IReadOnlyCollection targetframeworks, CancellationToken token) { if (projectRoot is null) { @@ -35,7 +35,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages } await _packageRestorer.RestorePackagesAsync(context, projectRoot, token).ConfigureAwait(false); - var analysisState = new DependencyAnalysisState(projectRoot, projectRoot.NuGetReferences); + var analysisState = new DependencyAnalysisState(projectRoot, projectRoot.NuGetReferences, targetframeworks); // Iterate through all package references in the project file foreach (var analyzer in _packageAnalyzers) diff --git a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/IDependencyAnalyzerRunner.cs b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/IDependencyAnalyzerRunner.cs index 5a8769bb..efe9a737 100644 --- a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/IDependencyAnalyzerRunner.cs +++ b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/IDependencyAnalyzerRunner.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; using Microsoft.DotNet.UpgradeAssistant.Dependencies; @@ -9,6 +10,6 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages { public interface IDependencyAnalyzerRunner { - Task AnalyzeAsync(IUpgradeContext context, IProject? projectRoot, CancellationToken token); + Task AnalyzeAsync(IUpgradeContext context, IProject? projectRoot, IReadOnlyCollection targetframeworks, CancellationToken token); } } diff --git a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/PackageUpdaterStep.cs b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/PackageUpdaterStep.cs index 10019e1f..56408e3c 100644 --- a/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/PackageUpdaterStep.cs +++ b/src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/PackageUpdaterStep.cs @@ -77,7 +77,8 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages try { - _analysisState = await _packageAnalyzer.AnalyzeAsync(context, context.CurrentProject, token).ConfigureAwait(false); + var currentProject = context.CurrentProject.Required(); + _analysisState = await _packageAnalyzer.AnalyzeAsync(context, currentProject, currentProject.TargetFrameworks, token).ConfigureAwait(false); if (!_analysisState.IsValid) { return new UpgradeStepInitializeResult(UpgradeStepStatus.Failed, $"Package analysis failed", BuildBreakRisk.Unknown); @@ -155,7 +156,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages count++; Logger.LogDebug("Re-running analysis to check whether additional changes are needed"); - _analysisState = await _packageAnalyzer.AnalyzeAsync(context, context.CurrentProject, token).ConfigureAwait(false); + _analysisState = await _packageAnalyzer.AnalyzeAsync(context, project, project.TargetFrameworks, token).ConfigureAwait(false); if (!_analysisState.IsValid) { return new UpgradeStepApplyResult(UpgradeStepStatus.Failed, "Package analysis failed"); diff --git a/tests/extensions/web/Microsoft.DotNet.UpgradeAssistant.Extensions.Web.Tests/NewtonsoftReferenceAnalyzerTests.cs b/tests/extensions/web/Microsoft.DotNet.UpgradeAssistant.Extensions.Web.Tests/NewtonsoftReferenceAnalyzerTests.cs index 10985d6d..b936cf2c 100644 --- a/tests/extensions/web/Microsoft.DotNet.UpgradeAssistant.Extensions.Web.Tests/NewtonsoftReferenceAnalyzerTests.cs +++ b/tests/extensions/web/Microsoft.DotNet.UpgradeAssistant.Extensions.Web.Tests/NewtonsoftReferenceAnalyzerTests.cs @@ -37,6 +37,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Extensions.Web.Tests var packageState = new Mock(); packageState.Setup(p => p.Packages).Returns(packages.Object); + packageState.Setup(p => p.TargetFrameworks).Returns(project.Object.TargetFrameworks); var packageLoader = CreatePackageLoader(mock); @@ -70,6 +71,8 @@ namespace Microsoft.DotNet.UpgradeAssistant.Extensions.Web.Tests comparer.Setup(comparer => comparer.Compare(It.IsAny(), It.IsAny())) .Returns(-1); + packageState.Setup(p => p.TargetFrameworks).Returns(project.Object.TargetFrameworks); + // Act await analyzer.AnalyzeAsync(project.Object, packageState.Object, default).ConfigureAwait(false);