Merge pull request #5200 from livarcocc/remove_condition_tfm_for_single_tfm

Remove condition tfm for single tfm

src/Microsoft.DotNet.ProjectJsonMigration/MigrationRuleInputs.cs

@@ -28,6 +28,8 @@ namespace Microsoft.DotNet.ProjectJsonMigration
             }
         }
 
+        public bool IsMultiTFM => ProjectContexts.Count() > 1;
+
         public MigrationRuleInputs(
             IEnumerable<ProjectContext> projectContexts, 
             ProjectRootElement outputMSBuildProject,

src/Microsoft.DotNet.ProjectJsonMigration/Rules/MigratePackageDependenciesAndToolsRule.cs

@@ -60,7 +60,10 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
             {
                 MigrationTrace.Instance.WriteLine(String.Format(LocalizableStrings.MigratingFramework, targetFramework.FrameworkName.GetShortFolderName()));
 
-                MigrateImports(migrationRuleInputs.CommonPropertyGroup, targetFramework);
+                MigrateImports(
+                    migrationRuleInputs.CommonPropertyGroup,
+                    targetFramework,
+                    migrationRuleInputs.IsMultiTFM);
 
                 MigrateDependencies(
                     project,
@@ -161,13 +164,14 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
 
         private void MigrateImports(
             ProjectPropertyGroupElement commonPropertyGroup,
-            TargetFrameworkInformation targetFramework)
+            TargetFrameworkInformation targetFramework,
+            bool isMultiTFM)
         {
             var transform = ImportsTransformation.Transform(targetFramework);
 
             if (transform != null)
             {
-                transform.Condition = targetFramework.FrameworkName.GetMSBuildCondition();
+                transform.Condition = isMultiTFM ? targetFramework.FrameworkName.GetMSBuildCondition() : null;
                 _transformApplicator.Execute(transform, commonPropertyGroup, mergeExisting: true);
             }
             else

src/Microsoft.DotNet.ProjectJsonMigration/Rules/MigrateScriptsRule.cs

@@ -28,16 +28,21 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
 
             foreach (var scriptSet in scripts)
             {
-                MigrateScriptSet(csproj, migrationRuleInputs.CommonPropertyGroup, scriptSet.Value, scriptSet.Key);
+                MigrateScriptSet(
+                    csproj,
+                    scriptSet.Value,
+                    scriptSet.Key,
+                    migrationRuleInputs.IsMultiTFM);
             }
         }
 
-        public ProjectTargetElement MigrateScriptSet(ProjectRootElement csproj,
+        public ProjectTargetElement MigrateScriptSet(
-            ProjectPropertyGroupElement propertyGroup,
+            ProjectRootElement csproj,
             IEnumerable<string> scriptCommands,
-            string scriptSetName)
+            string scriptSetName,
+            bool isMultiTFM)
         {
-            var target = CreateTarget(csproj, scriptSetName);
+            var target = CreateTarget(csproj, scriptSetName, isMultiTFM);
             foreach (var scriptCommand in scriptCommands)
             {
                 if (CommandIsNotNeededInMSBuild(scriptCommand))
@@ -94,7 +99,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
             return path.StartsWith("/") || path.Substring(1).StartsWith(":\\");
         }
 
-        private ProjectTargetElement CreateTarget(ProjectRootElement csproj, string scriptSetName)
+        private ProjectTargetElement CreateTarget(ProjectRootElement csproj, string scriptSetName, bool isMultiTFM)
         {
             var targetName = $"{scriptSetName[0].ToString().ToUpper()}{string.Concat(scriptSetName.Skip(1))}Script";
 
@@ -117,12 +122,19 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
                 target.AfterTargets = targetHookInfo.TargetName;
             }
 
-            // Run Scripts After each inner build
+            if (isMultiTFM)
-            target.Condition = " '$(IsCrossTargetingBuild)' != 'true' ";
+            {
+                ConditionTargetToRunScriptsAfterEachInnerBuild(target);
+            }
 
             return target;
         }
 
+        private void ConditionTargetToRunScriptsAfterEachInnerBuild(ProjectTargetElement target)
+        {
+            target.Condition = " '$(IsCrossTargetingBuild)' != 'true' ";
+        }
+
         private void AddExec(ProjectTargetElement target, string command)
         {
             var task = target.AddTask("Exec");

src/Microsoft.DotNet.ProjectJsonMigration/Rules/MigrateTFMRule.cs

@@ -34,20 +34,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
             CleanExistingProperties(csproj);
             CleanExistingPackageReferences(csproj);
 
-            if(migrationRuleInputs.ProjectContexts.Count() == 1)
+            if(migrationRuleInputs.IsMultiTFM)
-            {
-                _transformApplicator.Execute(
-                    FrameworkTransform.Transform(
-                        migrationRuleInputs.ProjectContexts.Single().TargetFramework),
-                    propertyGroup,
-                    mergeExisting: true);
-                _transformApplicator.Execute(
-                    FrameworkRuntimeIdentifiersTransform.Transform(
-                        migrationRuleInputs.ProjectContexts.Single()),
-                    propertyGroup,
-                    mergeExisting: true);
-            }
-            else
             {
                 _transformApplicator.Execute(
                     FrameworksTransform.Transform(
@@ -65,6 +52,19 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
                     propertyGroup,
                     mergeExisting: true);
             }
+            else
+            {
+                _transformApplicator.Execute(
+                    FrameworkTransform.Transform(
+                        migrationRuleInputs.ProjectContexts.Single().TargetFramework),
+                    propertyGroup,
+                    mergeExisting: true);
+                _transformApplicator.Execute(
+                    FrameworkRuntimeIdentifiersTransform.Transform(
+                        migrationRuleInputs.ProjectContexts.Single()),
+                    propertyGroup,
+                    mergeExisting: true);
+            }
         }
 
         private void CleanExistingProperties(ProjectRootElement csproj)

test/Microsoft.DotNet.ProjectJsonMigration.Tests/Rules/GivenThatIWantToMigratePackageDependencies.cs

@@ -15,7 +15,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
     public class GivenThatIWantToMigratePackageDependencies : PackageDependenciesTestBase
     {
         [Fact]
-        public void It_migrates_basic_PackageReference()
+        public void ItMigratesBasicPackageReference()
         {
             var mockProj = RunPackageDependenciesRuleOnPj(@"                
                 {
@@ -29,7 +29,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_migrates_type_build_to_PrivateAssets()
+        public void ItMigratesTypeBuildToPrivateAssets()
         {
             var mockProj = RunPackageDependenciesRuleOnPj(@"                
                 {
@@ -50,7 +50,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_migrates_suppress_parent_array_to_PrivateAssets()
+        public void ItMigratesSuppressParentArrayToPrivateAssets()
         {
             var mockProj = RunPackageDependenciesRuleOnPj(@"                
                 {
@@ -69,7 +69,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_migrates_suppress_parent_string_to_PrivateAssets()
+        public void ItMigratesSuppressParentStringToPrivateAssets()
         {
             var mockProj = RunPackageDependenciesRuleOnPj(@"                
                 {
@@ -88,7 +88,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_migrates_include_exclude_arrays_to_IncludeAssets()
+        public void ItMigratesIncludeExcludeArraysToIncludeAssets()
         {
             var mockProj = RunPackageDependenciesRuleOnPj(@"                
                 {
@@ -108,7 +108,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_migrates_include_string_to_IncludeAssets()
+        public void ItMigratesIncludeStringToIncludeAssets()
         {
             var mockProj = RunPackageDependenciesRuleOnPj(@"                
                 {
@@ -128,7 +128,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_migrates_include_exclude_overlapping_strings_to_IncludeAssets()
+        public void ItMigratesIncludeExcludeOverlappingStringsToIncludeAssets()
         {
             var mockProj = RunPackageDependenciesRuleOnPj(@"                
                 {
@@ -149,7 +149,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
 
 
         [Fact]
-        public void It_migrates_Tools()
+        public void ItMigratesTools()
         {
             var mockProj = RunPackageDependenciesRuleOnPj(@"                
                 {
@@ -163,7 +163,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_migrates_imports_per_framework()
+        public void ItMigratesImportsPerFramework()
         {
             var importPropertyName = "PackageTargetFallback";
 
@@ -201,7 +201,27 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_auto_add_desktop_references_during_migrate()
+        public void ItDoesNotAddConditionToPackageTargetFallBackWhenMigratingASingleTFM()
+        {
+            var importPropertyName = "PackageTargetFallback";
+
+            var mockProj = RunPackageDependenciesRuleOnPj(@"                
+                {
+                    ""frameworks"": {
+                        ""netcoreapp1.0"" : {
+                            ""imports"": [""netstandard1.3"", ""net451""]
+                        }
+                    }
+                }");
+
+            var imports = mockProj.Properties.Where(p => p.Name == importPropertyName);
+            imports.Should().HaveCount(1);
+
+            imports.Single().Condition.Should().BeEmpty();
+        }
+
+        [Fact]
+        public void ItAutoAddDesktopReferencesDuringMigrate()
         {
             var mockProj = RunPackageDependenciesRuleOnPj(@"                
                 {
@@ -234,7 +254,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_migrates_test_projects_to_have_test_sdk()
+        public void ItMigratesTestProjectsToHaveTestSdk()
         {
             var mockProj = RunPackageDependenciesRuleOnPj(@"
                 {
@@ -267,7 +287,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_migrates_test_projects_to_have_test_sdk_and_xunit_packagedependencies()
+        public void ItMigratesTestProjectsToHaveTestSdkAndXunitPackagedependencies()
         {
             var mockProj = RunPackageDependenciesRuleOnPj(@"
                 {
@@ -306,7 +326,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_migrates_test_projects_to_have_test_sdk_and_xunit_packagedependencies_overwrite_existing_packagedependencies()
+        public void ItMigratesTestProjectsToHaveTestSdkAndXunitPackagedependenciesOverwriteExistingPackagedependencies()
         {
             var mockProj = RunPackageDependenciesRuleOnPj(@"
                 {
@@ -348,7 +368,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_migrates_test_projects_to_have_test_sdk_and_mstest_packagedependencies()
+        public void ItMigratesTestProjectsToHaveTestSdkAndMstestPackagedependencies()
         {
             var mockProj = RunPackageDependenciesRuleOnPj(@"
                 {
@@ -408,7 +428,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
                 }
               }
             }")]
-        public void It_migrates_library_and_does_not_double_netstandard_ref(string pjContent)
+        public void ItMigratesLibraryAndDoesNotDoubleNetstandardRef(string pjContent)
         {
             var mockProj = RunPackageDependenciesRuleOnPj(pjContent);
 

test/Microsoft.DotNet.ProjectJsonMigration.Tests/Rules/GivenThatIWantToMigrateScripts.cs

@@ -15,6 +15,8 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
 {
     public class GivenThatIWantToMigrateScripts : TestBase
     {
+        private const bool IsMultiTFM = true;
+
         [Theory]
         [InlineData("compile:TargetFramework", "$(TargetFramework)")]
         [InlineData("publish:TargetFramework", "$(TargetFramework)")]
@@ -65,9 +67,9 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
 
             var target = scriptMigrationRule.MigrateScriptSet(
                 mockProj,
-                mockProj.AddPropertyGroup(),
                 commands,
-                scriptName);
+                scriptName,
+                IsMultiTFM);
 
             target.BeforeTargets.Should().Be(targetName);
         }
@@ -85,9 +87,9 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
 
             var target = scriptMigrationRule.MigrateScriptSet(
                 mockProj,
-                mockProj.AddPropertyGroup(),
                 commands,
-                scriptName);
+                scriptName,
+                IsMultiTFM);
 
             target.AfterTargets.Should().Be(targetName);
         }
@@ -107,9 +109,9 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
 
             var target = scriptMigrationRule.MigrateScriptSet(
                 mockProj,
-                mockProj.AddPropertyGroup(),
                 commands,
-                scriptName);
+                scriptName,
+                IsMultiTFM);
 
             foreach (var task in target.Tasks)
             {
@@ -143,9 +145,10 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
 
             var target = scriptMigrationRule.MigrateScriptSet(
                 mockProj,
-                mockProj.AddPropertyGroup(),
                 commands,
-                scriptName);
+                scriptName,
+                IsMultiTFM);
+
             target.Tasks.Count().Should().Be(commands.Length);
 
             foreach (var task in target.Tasks)
@@ -172,9 +175,9 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
 
             var target = scriptMigrationRule.MigrateScriptSet(
                 mockProj,
-                mockProj.AddPropertyGroup(),
                 commands,
-                "postpublish");
+                "postpublish",
+                IsMultiTFM);
             target.Tasks.Should().BeEmpty();
         }
 
@@ -186,7 +189,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void MigratingScriptsCreatesTargetWithIsCrossTargettingBuildNotEqualTrueCondition()
+        public void MigratingScriptsWithMultiTFMCreatesTargetWithIsCrossTargettingBuildNotEqualTrueCondition()
         {
             var scriptMigrationRule = new MigrateScriptsRule();
             ProjectRootElement mockProj = ProjectRootElement.Create();
@@ -195,12 +198,28 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
 
             var target = scriptMigrationRule.MigrateScriptSet(
                 mockProj,
-                mockProj.AddPropertyGroup(),
                 commands,
-                "prepublish");
+                "prepublish",
+                IsMultiTFM);
             target.Condition.Should().Be(" '$(IsCrossTargetingBuild)' != 'true' ");
         }
 
+        [Fact]
+        public void MigratingScriptsWithSingleTFMDoesNotCreateTargetWithIsCrossTargettingBuild()
+        {
+            var scriptMigrationRule = new MigrateScriptsRule();
+            ProjectRootElement mockProj = ProjectRootElement.Create();
+
+            var commands = new[] { "compile:FullTargetFramework", "compile:Configuration"};
+
+            var target = scriptMigrationRule.MigrateScriptSet(
+                mockProj,
+                commands,
+                "prepublish",
+                false);
+            target.Condition.Should().BeEmpty();
+        }
+
         [Fact]
         public void MigratingScriptsThrowsOnInvalidScriptSet()
         {
@@ -211,9 +230,9 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
 
             Action action = () => scriptMigrationRule.MigrateScriptSet(
                 mockProj,
-                mockProj.AddPropertyGroup(),
                 commands,
-                "invalidScriptSet");
+                "invalidScriptSet",
+                IsMultiTFM);
 
             action.ShouldThrow<MigrationException>()
                 .WithMessage("MIGRATE1019::Unsupported Script Event Hook: invalidScriptSet is an unsupported script event hook for project migration");

test/Microsoft.DotNet.ProjectJsonMigration.Tests/TemporaryProjectFileRuleRunner.cs

@@ -1,4 +1,5 @@
 using System.Collections.Generic;
+using System.Linq;
 using Microsoft.Build.Construction;
 using Microsoft.DotNet.ProjectJsonMigration.Rules;
 using Microsoft.DotNet.Internal.ProjectModel;
@@ -8,28 +9,48 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
 {
     internal class TemporaryProjectFileRuleRunner
     {
-        public static ProjectRootElement RunRules(IEnumerable<IMigrationRule> rules, string projectJson,
+        public static ProjectRootElement RunRules(
-            string testDirectory, ProjectRootElement xproj=null)
+            IEnumerable<IMigrationRule> rules,
+            string projectJson,
+            string testDirectory,
+            ProjectRootElement xproj=null)
         {
-            var projectContext = GenerateProjectContextFromString(testDirectory, projectJson);
+            var projectContexts = GenerateProjectContextsFromString(testDirectory, projectJson);
-            return RunMigrationRulesOnGeneratedProject(rules, projectContext, testDirectory, xproj);
+            return RunMigrationRulesOnGeneratedProject(rules, projectContexts, testDirectory, xproj);
         }
 
-        private static ProjectContext GenerateProjectContextFromString(string projectDirectory, string json)
+        private static IEnumerable<ProjectContext> GenerateProjectContextsFromString(
+            string projectDirectory,
+            string json)
         {
             var testPj = new ProjectJsonBuilder(null)
                 .FromStringBase(json)
                 .SaveToDisk(projectDirectory);
 
-            return ProjectContext.Create(testPj, FrameworkConstants.CommonFrameworks.NetCoreApp10);
+            var projectContexts = ProjectContext.CreateContextForEachFramework(projectDirectory);
+
+            if (projectContexts.Count() == 0)
+            {
+                projectContexts = new []
+                { 
+                    ProjectContext.Create(testPj, FrameworkConstants.CommonFrameworks.NetCoreApp10)
+                };
             }
 
-        private static ProjectRootElement RunMigrationRulesOnGeneratedProject(IEnumerable<IMigrationRule> rules,
+            return projectContexts;
-            ProjectContext projectContext, string testDirectory, ProjectRootElement xproj)
+        }
+
+        private static ProjectRootElement RunMigrationRulesOnGeneratedProject(
+            IEnumerable<IMigrationRule> rules,
+            IEnumerable<ProjectContext> projectContexts,
+            string testDirectory,
+            ProjectRootElement xproj)
         {
             var project = ProjectRootElement.Create();
             var testSettings = MigrationSettings.CreateMigrationSettingsTestHook(testDirectory, testDirectory, project);
-            var testInputs = new MigrationRuleInputs(new[] {projectContext}, project,
+            var testInputs = new MigrationRuleInputs(
+                projectContexts,
+                project,
                 project.AddItemGroup(),
                 project.AddPropertyGroup(),
                 xproj);