Implemented IsApplicable for PackageReferenceAnalyzers (#390)

CHANGELOG.md

@@ -9,9 +9,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 ### Fixed
 - Fixed issue surfacing from floating package references. [#371](https://github.com/dotnet/upgrade-assistant/pull/371)
+- Microsoft.AspNetCore.Mvc.NewtonsoftJson package should no longer be added to .NET Framework projects [#290](https://github.com/dotnet/upgrade-assistant/pull/390)
 - Fixed issue to ignore disabled NuGet sources. [#396](https://github.com/dotnet/upgrade-assistant/issues/396)
 
-
 ## Version 0.2.217201 - 2021-03-23  ([Link](https://www.nuget.org/packages/upgrade-assistant/0.2.217201))
 ### Added
 - Include try-convert version in upgrade-assistant package #358

src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/Analyzers/NewtonsoftReferenceAnalyzer.cs

@@ -9,19 +9,25 @@ using Microsoft.Extensions.Logging;
 
 namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Analyzers
 {
+    /// <summary>
+    /// Increases backward compatibility by using the Newtonsoft Serializer for ASP.NET Core.
+    /// </summary>
     public class NewtonsoftReferenceAnalyzer : IPackageReferencesAnalyzer
     {
         private const string NewtonsoftPackageName = "Microsoft.AspNetCore.Mvc.NewtonsoftJson";
 
+        private readonly TargetFrameworkMoniker _netCore3Moniker = new TargetFrameworkMoniker("netcoreapp3.0");
         private readonly IPackageLoader _packageLoader;
         private readonly ILogger<NewtonsoftReferenceAnalyzer> _logger;
+        private readonly ITargetFrameworkMonikerComparer _tfmComparer;
 
         public string Name => "Newtonsoft.Json reference analyzer";
 
-        public NewtonsoftReferenceAnalyzer(IPackageLoader packageLoader, ILogger<NewtonsoftReferenceAnalyzer> logger)
+        public NewtonsoftReferenceAnalyzer(IPackageLoader packageLoader, ILogger<NewtonsoftReferenceAnalyzer> logger, ITargetFrameworkMonikerComparer tfmComparer)
         {
             _packageLoader = packageLoader ?? throw new ArgumentNullException(nameof(packageLoader));
             _logger = logger ?? throw new ArgumentNullException(nameof(logger));
+            _tfmComparer = tfmComparer ?? throw new ArgumentNullException(nameof(tfmComparer));
         }
 
         public async Task<PackageAnalysisState> AnalyzeAsync(IProject project, PackageAnalysisState state, CancellationToken token)
@@ -37,7 +43,9 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Analyzers
             }
 
             // This reference only needs added to ASP.NET Core exes
-            if (!(project.Components.HasFlag(ProjectComponents.AspNetCore) && project.OutputType == ProjectOutputType.Exe))
+            if (!(project.Components.HasFlag(ProjectComponents.AspNetCore)
+                && project.OutputType == ProjectOutputType.Exe
+                && !project.TargetFrameworks.Any(tfm => _tfmComparer.Compare(tfm, _netCore3Moniker) < 0)))
             {
                 return state;
             }

src/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages/IPackageReferencesAnalyzer.cs

@@ -17,6 +17,7 @@ namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages
         /// </summary>
         /// <param name="project">The project whose NuGet package references should be analyzed.</param>
         /// <param name="state">The current analysis state which will be updated and returned.</param>
+        /// <param name="token">The token used to gracefully cancel this request.</param>
         /// <returns>The analysis state object provided updated based on this analyzer's analysis.</returns>
         Task<PackageAnalysisState> AnalyzeAsync(IProject project, PackageAnalysisState state, CancellationToken token);
     }

tests/components/Microsoft.DotNet.UpgradeAssistant.MSBuild.Tests/MSBuildStepTestCollection.cs

@@ -13,6 +13,8 @@ namespace Microsoft.DotNet.UpgradeAssistant.MSBuild.Tests
     [CollectionDefinition(Name)]
     public class MSBuildStepTestCollection : ICollectionFixture<MSBuildRegistrationFixture>
     {
+        // by design this class must be implemented in every test project that uses the MSBuildRegistrationFixture
+        // https://github.com/xunit/xunit/issues/409
         public const string Name = "Package Step Tests";
     }
 }

tests/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Configuration.Tests/ConfigUpdaterSubStepTests.cs

@@ -3,10 +3,8 @@
 
 using System;
 using System.Collections.Generic;
-using System.Collections.Immutable;
 using System.IO;
 using System.Linq;
-using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
 using Autofac;

tests/steps/Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Tests/Analyzers/NewtonsoftReferenceAnalyzerTests.cs

@@ -0,0 +1,163 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Threading;
+using System.Threading.Tasks;
+using Autofac.Extras.Moq;
+using Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Analyzers;
+using Moq;
+using Xunit;
+
+namespace Microsoft.DotNet.UpgradeAssistant.Steps.Packages.Tests.Analyzers
+{
+    /// <summary>
+    /// Unit tests for the NewtonsoftReferenceAnalyzer.
+    /// </summary>
+    public class NewtonsoftReferenceAnalyzerTests
+    {
+        private const string NewtonsoftPackageName = "Microsoft.AspNetCore.Mvc.NewtonsoftJson";
+
+        /// <summary>
+        /// Validates that the analyzer will only be applied when TFM is not net48.
+        /// </summary>
+        /// <returns>a task.</returns>
+        [Fact]
+        public async Task AnalyzerIsApplicableToAspNetCoreWebProject()
+        {
+            // Arrange
+            using var mock = AutoMock.GetLoose();
+            var analyzer = mock.Create<NewtonsoftReferenceAnalyzer>();
+            var project = CreateProjectForWhichAnalyzerIsApplicable(mock);
+            var packageState = await CreatePackageAnalysisState(mock, project).ConfigureAwait(false);
+            var packageLoader = CreatePackageLoader(mock);
+
+            // Act
+            var actual = await analyzer.AnalyzeAsync(project.Object, packageState, default).ConfigureAwait(false);
+
+            // Assert
+            Assert.Contains(actual.PackagesToAdd, (package) => package.Name.Equals(NewtonsoftPackageName, System.StringComparison.Ordinal));
+            packageLoader.Verify(pl => pl.GetLatestVersionAsync(It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string[]>(), It.IsAny<CancellationToken>()),
+                Times.Once());
+        }
+
+        /// <summary>
+        /// Validates that the analyzer will not be applied to net48 TFM.
+        /// </summary>
+        /// <returns>a task.</returns>
+        [Fact]
+        public async Task AnalyzerIsNotApplicableToNetFrameworkProjects()
+        {
+            // Arrange
+            using var mock = AutoMock.GetLoose();
+
+            var analyzer = mock.Create<NewtonsoftReferenceAnalyzer>();
+            var project = CreateProjectForWhichAnalyzerIsApplicable(mock);
+            var packageState = await CreatePackageAnalysisState(mock, project).ConfigureAwait(false);
+            var packageLoader = CreatePackageLoader(mock);
+
+            // shift project attributes so that it is not applicable
+            project.Setup(p => p.TargetFrameworks).Returns(new[] { new TargetFrameworkMoniker("net472") });
+            var comparer = mock.Mock<ITargetFrameworkMonikerComparer>();
+            comparer.Setup(comparer => comparer.Compare(It.IsAny<TargetFrameworkMoniker>(), It.IsAny<TargetFrameworkMoniker>()))
+                .Returns(-1);
+
+            // Act
+            var actual = await analyzer.AnalyzeAsync(project.Object, packageState, default).ConfigureAwait(false);
+
+            // Assert
+            Assert.DoesNotContain(actual.PackagesToAdd, (package) => package.Name.Equals(NewtonsoftPackageName, System.StringComparison.Ordinal));
+            packageLoader.Verify(pl => pl.GetLatestVersionAsync(It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string[]>(), It.IsAny<CancellationToken>()),
+                Times.Never());
+        }
+
+        /// <summary>
+        /// Validates that the analyzer will not be applied to library projects.
+        /// </summary>
+        /// <returns>a task.</returns>
+        [Fact]
+        public async Task AnalyzerIsNotApplicableToLibraryProjects()
+        {
+            // Arrange
+            using var mock = AutoMock.GetLoose();
+
+            var analyzer = mock.Create<NewtonsoftReferenceAnalyzer>();
+            var project = CreateProjectForWhichAnalyzerIsApplicable(mock);
+            var packageState = await CreatePackageAnalysisState(mock, project).ConfigureAwait(false);
+            var packageLoader = CreatePackageLoader(mock);
+
+            // shift project attributes so that it is not applicable
+            project.Setup(p => p.OutputType).Returns(ProjectOutputType.Library);
+
+            // Act
+            var actual = await analyzer.AnalyzeAsync(project.Object, packageState, default).ConfigureAwait(false);
+
+            // Assert
+            Assert.DoesNotContain(actual.PackagesToAdd, (package) => package.Name.Equals(NewtonsoftPackageName, System.StringComparison.Ordinal));
+            packageLoader.Verify(pl => pl.GetLatestVersionAsync(It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string[]>(), It.IsAny<CancellationToken>()),
+                Times.Never());
+        }
+
+        /// <summary>
+        /// Validates that the analyzer will not be applied to net48 TFM.
+        /// </summary>
+        /// <returns>a task.</returns>
+        [Fact]
+        public async Task AnalyzerIsOnlyApplicableToAspNetProjects()
+        {
+            // Arrange
+            using var mock = AutoMock.GetLoose();
+
+            var analyzer = mock.Create<NewtonsoftReferenceAnalyzer>();
+            var project = CreateProjectForWhichAnalyzerIsApplicable(mock);
+            var packageState = await CreatePackageAnalysisState(mock, project).ConfigureAwait(false);
+            var packageLoader = CreatePackageLoader(mock);
+
+            // shift project attributes so that it is not applicable
+            project.Setup(p => p.Components).Returns(ProjectComponents.Wpf);
+
+            // Act
+            var actual = await analyzer.AnalyzeAsync(project.Object, packageState, default).ConfigureAwait(false);
+
+            // Assert
+            Assert.DoesNotContain(actual.PackagesToAdd, (package) => package.Name.Equals(NewtonsoftPackageName, System.StringComparison.Ordinal));
+            packageLoader.Verify(pl => pl.GetLatestVersionAsync(It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string[]>(), It.IsAny<CancellationToken>()),
+                Times.Never());
+        }
+
+        private static async Task<PackageAnalysisState> CreatePackageAnalysisState(AutoMock mock, Mock<IProject> project)
+        {
+            var context = mock.Mock<IUpgradeContext>();
+            context.Setup(c => c.CurrentProject).Returns(project.Object);
+            var restorer = mock.Mock<IPackageRestorer>();
+            restorer.Setup(r => r.RestorePackagesAsync(
+                It.IsAny<IUpgradeContext>(),
+                It.IsAny<IProject>(),
+                It.IsAny<CancellationToken>())).Returns(Task.FromResult(true));
+
+            return await PackageAnalysisState.CreateAsync(context.Object, restorer.Object, CancellationToken.None).ConfigureAwait(true);
+        }
+
+        private static Mock<IProject> CreateProjectForWhichAnalyzerIsApplicable(AutoMock mock)
+        {
+            var project = mock.Mock<IProject>();
+            var nugetReferences = mock.Mock<INuGetReferences>();
+            nugetReferences.Setup(n => n.IsTransitivelyAvailable(It.IsAny<string>()))
+                .Returns(false);
+
+            project.Setup(p => p.TargetFrameworks).Returns(new[] { new TargetFrameworkMoniker("net5.0") });
+            project.Setup(p => p.Components).Returns(ProjectComponents.AspNetCore);
+            project.Setup(p => p.OutputType).Returns(ProjectOutputType.Exe);
+            project.Setup(p => p.NuGetReferences).Returns(nugetReferences.Object);
+
+            return project;
+        }
+
+        private static Mock<IPackageLoader> CreatePackageLoader(AutoMock mock)
+        {
+            var packageLoader = mock.Mock<IPackageLoader>();
+            packageLoader.Setup(pl => pl.GetLatestVersionAsync(It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<string[]>(), It.IsAny<CancellationToken>()))
+                .Returns(Task.FromResult((NuGetReference?)new NuGetReference(NewtonsoftPackageName, "122.0.0")));
+            return packageLoader;
+        }
+    }
+}

tests/tool/Integration.Tests/IntegrationScenarios/AspNetSample/csharp/Upgraded/TemplateMvc.csproj

@@ -31,8 +31,8 @@
     <PackageReference Include="Microsoft.DotNet.UpgradeAssistant.Extensions.Default.Analyzers" Version="1.0.0">
       <PrivateAssets>all</PrivateAssets>
     </PackageReference>
-    <PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="5.0.4" />
     <PackageReference Include="Antlr4" Version="4.6.6" />
+    <PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="5.0.4" />
   </ItemGroup>
   <ItemGroup>
     <PackageReference Include="bootstrap" Version="3.4.1" />