Adding MigrateWebSdkRule to the DefaultMigrationRuleSet (#4963)

Livar · Piotr Puszkiewicz · commit 8cec61c6f74c · 2016-12-08T09:23:49.000-08:00
* Adding MigrateWebSdkRule to the DefaultMigrationRuleSet and adding a E2E test to cover it.

* Do not migrate compile and EmbeddedResources for web application, because those are included in the Web Sdk already.

* Addressing code review comments
diff --git a/src/Microsoft.DotNet.ProjectJsonMigration/DefaultMigrationRuleSet.cs b/src/Microsoft.DotNet.ProjectJsonMigration/DefaultMigrationRuleSet.cs
@@ -25,7 +25,8 @@ internal class DefaultMigrationRuleSet : IMigrationRule
             new MigrateAssemblyInfoRule(),
             new RemoveDefaultsFromProjectRule(),
             new CleanOutputProjectRule(),
-            new SaveOutputProjectRule()
+            new SaveOutputProjectRule(),
+            new MigrateWebSdkRule()
         };
 
         public void Apply(MigrationSettings migrationSettings, MigrationRuleInputs migrationRuleInputs)
diff --git a/src/Microsoft.DotNet.ProjectJsonMigration/Rules/MigrateBuildOptionsRule.cs b/src/Microsoft.DotNet.ProjectJsonMigration/Rules/MigrateBuildOptionsRule.cs
@@ -196,6 +196,9 @@ public void Apply(MigrationSettings migrationSettings, MigrationRuleInputs migra
 
             var compilerOptions = projectContext.ProjectFile.GetCompilerOptions(null, null);
 
+            var project = migrationRuleInputs.DefaultProjectContext.ProjectFile;
+            var projectType = project.GetProjectType();
+
             // If we're in a configuration, we need to be careful not to overwrite values from BuildOptions
             // without a configuration
             if (_configurationBuildOptions == null)
@@ -207,7 +210,9 @@ public void Apply(MigrationSettings migrationSettings, MigrationRuleInputs migra
                     propertyGroup,
                     itemGroup,
                     _transformApplicator,
-                    migrationSettings.ProjectDirectory);
+                    migrationSettings.ProjectDirectory,
+                    projectType,
+                    csproj);
             }
             else
             {
@@ -217,7 +222,9 @@ public void Apply(MigrationSettings migrationSettings, MigrationRuleInputs migra
                     propertyGroup,
                     itemGroup,
                     _transformApplicator,
-                    migrationSettings.ProjectDirectory);
+                    migrationSettings.ProjectDirectory,
+                    projectType,
+                    csproj);
             }
         }
 
@@ -227,7 +234,9 @@ private void PerformConfigurationPropertyAndItemMappings(
             ProjectPropertyGroupElement propertyGroup,
             ProjectItemGroupElement itemGroup,
             ITransformApplicator transformApplicator,
-            string projectDirectory)
+            string projectDirectory,
+            ProjectType projectType,
+            ProjectRootElement csproj)
         {
             foreach (var transform in _propertyTransforms)
             {
@@ -243,7 +252,13 @@ private void PerformConfigurationPropertyAndItemMappings(
             foreach (var includeContextTransformExecute in _includeContextTransformExecutes)
             {
                 var nonConfigurationOutput = includeContextTransformExecute(compilerOptions, projectDirectory);
-                var configurationOutput = includeContextTransformExecute(configurationCompilerOptions, projectDirectory);
+                var configurationOutput =
+                    includeContextTransformExecute(configurationCompilerOptions, projectDirectory);
+
+                configurationOutput = RemoveDefaultCompileAndEmbeddedResourceForWebProjects(
+                    configurationOutput,
+                    projectType,
+                    csproj);
 
                 transformApplicator.Execute(configurationOutput, itemGroup, mergeExisting: true);
             }
@@ -283,7 +298,9 @@ private void PerformPropertyAndItemMappings(
             ProjectPropertyGroupElement propertyGroup, 
             ProjectItemGroupElement itemGroup,
             ITransformApplicator transformApplicator,
-            string projectDirectory)
+            string projectDirectory,
+            ProjectType projectType,
+            ProjectRootElement csproj)
         {
             foreach (var transform in _propertyTransforms)
             {
@@ -292,13 +309,40 @@ private void PerformPropertyAndItemMappings(
 
             foreach (var includeContextTransformExecute in _includeContextTransformExecutes)
             {
+                var transform = includeContextTransformExecute(compilerOptions, projectDirectory);
+                
+                transform = RemoveDefaultCompileAndEmbeddedResourceForWebProjects(
+                    transform,
+                    projectType,
+                    csproj);
+
                 transformApplicator.Execute(
-                    includeContextTransformExecute(compilerOptions, projectDirectory),
+                    transform,
                     itemGroup,
                     mergeExisting: true);
             }
         }
 
+        private IEnumerable<ProjectItemElement> RemoveDefaultCompileAndEmbeddedResourceForWebProjects(
+            IEnumerable<ProjectItemElement> transform,
+            ProjectType projectType,
+            ProjectRootElement csproj)
+        {
+            if(projectType == ProjectType.Web)
+            {
+                var itemsToRemove = transform.Where(p => 
+                        p != null && 
+                        p.Include.Contains("**\\*") &&
+                        (p.ItemType == "Compile" || p.ItemType == "EmbeddedResource"));
+
+                CleanExistingItems(csproj, new [] {"Compile", "EmbeddedResource"});
+
+                transform = transform.Where(p => !itemsToRemove.Contains(p));
+            }
+
+            return transform;
+        }
+
         private void CleanExistingProperties(ProjectRootElement csproj)
         {
             var existingPropertiesToRemove = new [] {"OutputType", "TargetExt"};
@@ -314,6 +358,19 @@ private void CleanExistingProperties(ProjectRootElement csproj)
             }
         }
 
+        private void CleanExistingItems(ProjectRootElement csproj, IEnumerable<string> itemsToRemove)
+        {
+            foreach (var itemName in itemsToRemove)
+            {
+                var items = csproj.Items.Where(i => i.ItemType == itemName);
+
+                foreach (var item in items)
+                {
+                    item.Parent.RemoveChild(item);
+                }
+            }
+        }
+
         private IncludeContext GetCompileIncludeContext(CommonCompilerOptions compilerOptions, string projectDirectory)
         {
             // Defaults from src/Microsoft.DotNet.ProjectModel/ProjectReader.cs #L596
diff --git a/test/Microsoft.DotNet.ProjectJsonMigration.Tests/Rules/GivenThatIWantToMigrateBuildOptions.cs b/test/Microsoft.DotNet.ProjectJsonMigration.Tests/Rules/GivenThatIWantToMigrateBuildOptions.cs
@@ -63,6 +63,27 @@ public void Migrating_empty_buildOptions_populates_only_Compile_and_EmbeddedReso
             mockProj.Items.First(i => i.ItemType == "EmbeddedResource").Exclude.Should().BeEmpty();
         }
 
+        [Fact]
+        public void Migrating_web_project_without_custom_sources_or_resources_does_not_emit_compile_and_embeddedResource()
+        {
+            var mockProj = RunBuildOptionsRuleOnPj(@"
+                {
+                    ""buildOptions"": {
+                        ""emitEntryPoint"": true
+                    },
+                    ""dependencies"": {
+                        ""Microsoft.AspNetCore.Mvc"" : {
+                            ""version"": ""1.0.0""
+                        }
+                    },
+                    ""frameworks"": {
+                        ""netcoreapp1.0"": {}
+                    }
+                }");
+
+            mockProj.Items.Count().Should().Be(0);
+        }
+
         [Fact]
         public void Migrating_EmitEntryPoint_true_populates_OutputType_field()
         {
diff --git a/test/dotnet-migrate.Tests/GivenThatIWantToMigrateTestApps.cs b/test/dotnet-migrate.Tests/GivenThatIWantToMigrateTestApps.cs
@@ -22,7 +22,7 @@ public class GivenThatIWantToMigrateTestApps : TestBase
         [InlineData("TestAppWithRuntimeOptions")]
         [InlineData("TestAppWithContents")]
         [InlineData("AppWithAssemblyInfo")]
-        public void It_migrates_apps(string projectName)
+        public void ItMigratesApps(string projectName)
         {
             var projectDirectory = TestAssetsManager.CreateTestInstance(projectName, identifier: projectName)
                                                     .WithLockFiles()
@@ -50,7 +50,7 @@ public void It_migrates_apps(string projectName)
         }
 
         [Fact]
-        public void It_migrates_signed_apps()
+        public void ItMigratesSignedApps()
         {
             var projectDirectory = TestAssetsManager.CreateTestInstance("TestAppWithSigning").WithLockFiles().Path;
 
@@ -74,7 +74,7 @@ public void It_migrates_signed_apps()
         }
 
         [Fact]
-        public void It_migrates_dotnet_new_console_with_identical_outputs()
+        public void ItMigratesDotnetNewConsoleWithIdenticalOutputs()
         {
             var testInstance = TestAssetsManager
                 .CreateTestInstance("ProjectJsonConsoleTemplate");
@@ -96,8 +96,8 @@ public void It_migrates_dotnet_new_console_with_identical_outputs()
             VerifyAllMSBuildOutputsRunnable(projectDirectory);
         }
 
-        [Fact(Skip="Final tools version missing.")]
-        public void It_migrates_old_dotnet_new_web_without_tools_with_outputs_containing_project_json_outputs()
+        [Fact]
+        public void ItMigratesOldDotnetNewWebWithoutToolsWithOutputsContainingProjectJsonOutputs()
         {
             var testInstance = TestAssetsManager
                 .CreateTestInstance("ProjectJsonWebTemplate")
@@ -108,10 +108,7 @@ public void It_migrates_old_dotnet_new_web_without_tools_with_outputs_containing
             var globalDirectory = Path.Combine(projectDirectory, "..");  
             var projectJsonFile = Path.Combine(projectDirectory, "project.json");  
               
-            WriteGlobalJson(globalDirectory);  
-            var projectJson = JObject.Parse(File.ReadAllText(projectJsonFile));  
-            projectJson.Remove("tools");  
-            File.WriteAllText(projectJsonFile, projectJson.ToString());  
+            WriteGlobalJson(globalDirectory);
 
             var outputComparisonData = GetComparisonData(projectDirectory);
 
@@ -126,9 +123,27 @@ public void It_migrates_old_dotnet_new_web_without_tools_with_outputs_containing
             outputsIdentical.Should().BeTrue();
         }
 
+        public void ItAddsMicrosoftNetWebSdkToTheSdkAttributeOfAWebApp()
+        {
+            var testInstance = TestAssetsManager
+                .CreateTestInstance("ProjectJsonWebTemplate")
+                .WithLockFiles();
+
+            var projectDirectory = testInstance.Path;
+
+            var globalDirectory = Path.Combine(projectDirectory, "..");  
+            var projectJsonFile = Path.Combine(projectDirectory, "project.json");  
+              
+            MigrateProject(new [] { projectDirectory });
+
+            var csProj = Path.Combine(projectDirectory, $"{new DirectoryInfo(projectDirectory).Name}.csproj");
+
+            File.ReadAllText(csProj).Should().Contain(@"Sdk=""Microsoft.NET.Sdk.Web""");
+        }
+
         [Theory]
         [InlineData("TestLibraryWithTwoFrameworks")]
-        public void It_migrates_projects_with_multiple_TFMs(string projectName)
+        public void ItMigratesProjectsWithMultipleTFMs(string projectName)
         {
             var projectDirectory =
                 TestAssetsManager.CreateTestInstance(projectName, identifier: projectName).WithLockFiles().Path;
@@ -150,7 +165,7 @@ public void It_migrates_projects_with_multiple_TFMs(string projectName)
         [InlineData("TestAppWithLibrary/TestLibrary")]
         [InlineData("TestLibraryWithAnalyzer")]
         [InlineData("PJTestLibraryWithConfiguration")]
-        public void It_migrates_a_library(string projectName)
+        public void ItMigratesALibrary(string projectName)
         {
             var projectDirectory =
                 TestAssetsManager.CreateTestInstance(projectName, identifier: projectName).WithLockFiles().Path;
@@ -174,7 +189,7 @@ public void It_migrates_a_library(string projectName)
         [InlineData("ProjectC", "ProjectC,ProjectD,ProjectE")]
         [InlineData("ProjectD", "ProjectD")]
         [InlineData("ProjectE", "ProjectE")]
-        public void It_migrates_root_project_and_references(string projectName, string expectedProjects)
+        public void ItMigratesRootProjectAndReferences(string projectName, string expectedProjects)
         {
             var projectDirectory =
                 TestAssetsManager.CreateTestInstance("TestAppDependencyGraph", callingMethod: $"{projectName}.RefsTest").Path;
@@ -192,7 +207,7 @@ public void It_migrates_root_project_and_references(string projectName, string e
         [InlineData("ProjectC")]
         [InlineData("ProjectD")]
         [InlineData("ProjectE")]
-        public void It_migrates_root_project_and_skips_references(string projectName)
+        public void ItMigratesRootProjectAndSkipsReferences(string projectName)
         {
             var projectDirectory =
                 TestAssetsManager.CreateTestInstance("TestAppDependencyGraph", callingMethod: $"{projectName}.SkipRefsTest").Path;
@@ -205,7 +220,7 @@ public void It_migrates_root_project_and_skips_references(string projectName)
          [Theory]
          [InlineData(true)]
          [InlineData(false)]
-         public void It_migrates_all_projects_in_given_directory(bool skipRefs)
+         public void ItMigratesAllProjectsInGivenDirectory(bool skipRefs)
          {
             var projectDirectory = TestAssetsManager.CreateTestInstance("TestAppDependencyGraph", callingMethod: $"MigrateDirectory.SkipRefs.{skipRefs}").Path;
 
@@ -224,7 +239,7 @@ public void It_migrates_all_projects_in_given_directory(bool skipRefs)
          }
 
          [Fact]
-         public void It_migrates_given_project_json()
+         public void ItMigratesGivenProjectJson()
          {
             var projectDirectory = TestAssetsManager.CreateTestInstance("TestAppDependencyGraph").Path;
 
@@ -239,7 +254,7 @@ public void It_migrates_given_project_json()
 
          [Fact]
          // regression test for https://github.com/dotnet/cli/issues/4269
-         public void It_migrates_and_builds_P2P_references()
+         public void ItMigratesAndBuildsP2PReferences()
          {
             var assetsDir = TestAssetsManager.CreateTestInstance("TestAppDependencyGraph").WithLockFiles().Path;
 
@@ -269,7 +284,7 @@ public void It_migrates_and_builds_P2P_references()
          [Theory]
          [InlineData("src", "ProjectH")]
          [InlineData("src with spaces", "ProjectJ")]
-         public void It_migrates_and_builds_projects_in_global_json(string path, string projectName)
+         public void ItMigratesAndBuildsProjectsInGlobalJson(string path, string projectName)
          {
             var assetsDir = TestAssetsManager.CreateTestInstance(Path.Combine("TestAppDependencyGraph", "ProjectsWithGlobalJson"), 
                                                                  callingMethod: $"ProjectsWithGlobalJson.{projectName}")
@@ -306,7 +321,7 @@ public void It_migrates_and_builds_projects_in_global_json(string path, string p
         [Theory]
         [InlineData(true)]
         [InlineData(false)]
-        public void Migration_outputs_error_when_no_projects_found(bool useGlobalJson)
+        public void MigrationOutputsErrorWhenNoProjectsFound(bool useGlobalJson)
         {
             var projectDirectory = TestAssetsManager.CreateTestDirectory("Migration_outputs_error_when_no_projects_found");
 
@@ -353,7 +368,7 @@ public void Migration_outputs_error_when_no_projects_found(bool useGlobalJson)
         }
 
         [Fact]
-        public void It_migrates_and_publishes_projects_with_runtimes()
+        public void ItMigratesAndPublishesProjectsWithRuntimes()
         {
             var projectName = "PJTestAppSimple";
             var projectDirectory = TestAssetsManager
@@ -369,7 +384,7 @@ public void It_migrates_and_publishes_projects_with_runtimes()
         [WindowsOnlyTheory]
         [InlineData("DesktopTestProjects", "AutoAddDesktopReferencesDuringMigrate", true)]
         [InlineData("TestProjects", "PJTestAppSimple", false)]
-        public void It_auto_add_desktop_references_during_migrate(string testGroup, string projectName, bool isDesktopApp)
+        public void ItAutoAddDesktopReferencesDuringMigrate(string testGroup, string projectName, bool isDesktopApp)
         {
             var runtime = DotnetLegacyRuntimeIdentifiers.InferLegacyRestoreRuntimeIdentifier();
             var testAssetManager = GetTestGroupTestAssetsManager(testGroup);
@@ -384,7 +399,7 @@ public void It_auto_add_desktop_references_during_migrate(string testGroup, stri
         }
 
         [Fact]
-        public void It_builds_a_migrated_app_with_a_indirect_dependency()
+        public void ItBuildsAMigratedAppWithAnIndirectDependency()
         {
             const string projectName = "ProjectA";
             var solutionDirectory =
@@ -399,7 +414,7 @@ public void It_builds_a_migrated_app_with_a_indirect_dependency()
         }
 
         [Fact]
-        public void It_migrates_project_with_output_name()
+        public void ItMigratesProjectWithOutputName()
         {
             string projectName = "AppWithOutputAssemblyName";
             string expectedOutputName = "MyApp";
@@ -426,18 +441,18 @@ public void It_migrates_project_with_output_name()
         [Theory]
         [InlineData("LibraryWithoutNetStandardLibRef")]
         [InlineData("LibraryWithNetStandardLibRef")]
-        public void It_migrates_and_builds_library(string projectName)
+        public void ItMigratesAndBuildsLibrary(string projectName)
         {
             var projectDirectory = TestAssetsManager.CreateTestInstance(projectName,
-                callingMethod: $"{nameof(It_migrates_and_builds_library)}-projectName").Path;
+                callingMethod: $"{nameof(ItMigratesAndBuildsLibrary)}-projectName").Path;
 
             MigrateProject(projectDirectory);
             Restore(projectDirectory, projectName);
             BuildMSBuild(projectDirectory, projectName);
         }
 
         [Fact]
-        public void It_fails_gracefully_when_migrating_app_with_missing_dependency()
+        public void ItFailsGracefullyWhenMigratingAppWithMissingDependency()
         {
             string projectName = "MigrateAppWithMissingDep";
             var projectDirectory = Path.Combine(GetTestGroupTestAssetsManager("NonRestoredTestProjects").CreateTestInstance(projectName).Path, "MyApp");
