Merge pull request #5174 from livarcocc/no_transitive_dependency_migration

Stopped doing hoisting of transitive project dependencies and references

TestAssets/NonRestoredTestProjects/MigrateAppWithMissingDep/MyApp/Program.cs

@@ -1,10 +0,0 @@
-namespace ConsoleApplication
-{
-    public class Program
-    {
-        public static void Main(string[] args)
-        {
-        }
-    }
-}
-

TestAssets/NonRestoredTestProjects/MigrateAppWithMissingDep/MyApp/project.json

@@ -1,20 +0,0 @@
-{
-  "version": "1.0.0-*",
-  "buildOptions": {
-    "emitEntryPoint": true
-  },
-
-  "dependencies": {
-    "Microsoft.NETCore.App": {
-      "type": "platform",
-      "version": "1.0.1"
-    },
-    "MyLib": "1.0.0-*"
-  },
-
-  "frameworks": {
-    "netcoreapp1.0": {
-      "imports": "dnxcore50"
-    }
-  }
-}

TestAssets/NonRestoredTestProjects/MigrateAppWithMissingDep/MyLib/NotUsedButDoNotRemove.cs

@@ -1,5 +0,0 @@
-// This file needs to be here as this error does not repro if the MyLib folder doesn't exist
-// Since git does not keep track of folders and files only, this folder needs to contain any file.
-namespace MyLib
-{
-}

src/Microsoft.DotNet.ProjectJsonMigration/ProjectDependency.cs

@@ -7,13 +7,11 @@ namespace Microsoft.DotNet.ProjectJsonMigration
     {
         public string Name { get; }
         public string ProjectFilePath { get; }
-        public bool Hoisted { get; }
 
-        public ProjectDependency(string name, string projectFilePath, bool hoisted)
+        public ProjectDependency(string name, string projectFilePath)
         {
             Name = name;
             ProjectFilePath = System.IO.Path.GetFullPath(projectFilePath);
-            Hoisted = hoisted;
         }
 
         public override bool Equals(object obj)

src/Microsoft.DotNet.ProjectJsonMigration/ProjectDependencyFinder.cs

@@ -80,34 +80,23 @@ namespace Microsoft.DotNet.ProjectJsonMigration
                     projectContext.ProjectFile,
                     framework,
                     preResolvedProjects,
-                    solutionFile,
+                    solutionFile
-                    HoistDependenciesThatAreNotDirectDependencies(projectToResolve, project)
                 );
-                projects.AddRange(dependencies);
                 allDependencies.UnionWith(dependencies);
             }
 
             return allDependencies;
         }
 
-        private bool HoistDependenciesThatAreNotDirectDependencies(
-            ProjectDependency originalProject,
-            ProjectDependency dependenciesOwner)
-        {
-            return originalProject != dependenciesOwner;
-        }
-
         public IEnumerable<ProjectDependency> ResolveDirectProjectDependenciesForFramework(
             Project project, 
             NuGetFramework framework, 
             IEnumerable<string> preResolvedProjects=null,
-            SlnFile solutionFile = null,
+            SlnFile solutionFile = null)
-            bool hoistedDependencies = false)
         {
             preResolvedProjects = preResolvedProjects ?? new HashSet<string>();
 
-            var possibleProjectDependencies = 
+            var possibleProjectDependencies = FindPossibleProjectDependencies(solutionFile, project.ProjectFilePath);
-                FindPossibleProjectDependencies(solutionFile, project.ProjectFilePath, hoistedDependencies);
 
             var projectDependencies = new List<ProjectDependency>();
 
@@ -233,8 +222,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration
 
         private Dictionary<string, ProjectDependency> FindPossibleProjectDependencies(
             SlnFile slnFile,
-            string projectJsonFilePath,
+            string projectJsonFilePath)
-            bool hoistedDependencies = false)
         {
             var projectRootDirectory = GetRootFromProjectJson(projectJsonFilePath);
 
@@ -249,7 +237,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration
 
             var projects = new Dictionary<string, ProjectDependency>(StringComparer.Ordinal);
 
-            foreach (var project in GetPotentialProjects(projectSearchPaths, hoistedDependencies))
+            foreach (var project in GetPotentialProjects(projectSearchPaths))
             {
                 if (projects.ContainsKey(project.Name))
                 {
@@ -311,8 +299,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration
         /// Create the list of potential projects from the search paths.
         /// </summary>
         private static List<ProjectDependency> GetPotentialProjects(
-            IEnumerable<string> searchPaths,
+            IEnumerable<string> searchPaths)
-            bool hoistedDependencies = false)
         {
             var projects = new List<ProjectDependency>();
 
@@ -338,8 +325,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration
                     // Check if we've already added this, just in case it was pre-loaded into the cache
                     var project = new ProjectDependency(
                         projectDirectory.Name,
-                        projectFilePath,
+                        projectFilePath);
-                        hoistedDependencies);
 
                     projects.Add(project);
                 }

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

@@ -113,16 +113,13 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
             ProjectRootElement outputMSBuildProject)
         {
             var projectDependencies = _projectDependencyFinder.ResolveAllProjectDependenciesForFramework(
-                    new ProjectDependency(project.Name, project.ProjectFilePath, false),
+                    new ProjectDependency(project.Name, project.ProjectFilePath),
                     framework,
                     migratedXProjDependencyNames,
                     solutionFile);
 
             var projectDependencyTransformResults = 
-                projectDependencies.Select(p => 
+                projectDependencies.Select(p => ProjectDependencyTransform.Transform(p));
-                    p.Hoisted ? 
-                        HoistedDependencyTransform.Transform(p) :
-                        ProjectDependencyTransform.Transform(p));
             
             if (projectDependencyTransformResults.Any())
             {
@@ -131,56 +128,6 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
                     projectDependencyTransformResults, 
                     framework);
             }
-
-            HoistFrameworkAssembliesForProjectDependencies(projectDependencies, outputMSBuildProject);
-        }
-
-        private void HoistFrameworkAssembliesForProjectDependencies(
-            IEnumerable<ProjectDependency> projectDependencies,
-            ProjectRootElement outputMSBuildProject)
-        {
-            foreach (var projectDependency in projectDependencies)
-            {
-                HoistFrameworkAssembliesForDesktopFrameworks(projectDependency, outputMSBuildProject);
-            }
-        }
-
-        private void HoistFrameworkAssembliesForDesktopFrameworks(
-            ProjectDependency projectDependency,
-            ProjectRootElement outputMSBuildProject)
-        {
-            var targetFrameworks = ProjectReader
-                    .GetProject(projectDependency.ProjectFilePath)
-                    .GetTargetFrameworks().Where(p => !p.FrameworkName.IsPackageBased);
-
-            foreach (var targetFramework in targetFrameworks)
-            {
-                HoistFrameworkAssemblies(targetFramework, outputMSBuildProject);
-            }
-        }
-
-        private void HoistFrameworkAssemblies(
-            TargetFrameworkInformation targetFramework,
-            ProjectRootElement outputMSBuildProject)
-        {
-            var frameworkAssemblies = targetFramework.Dependencies.Where(d =>
-                    d.LibraryRange.TypeConstraint == LibraryDependencyTarget.Reference);
-            if(frameworkAssemblies.Any())
-            {
-                var condition = targetFramework.FrameworkName.GetMSBuildCondition();
-                var itemGroup =
-                    outputMSBuildProject.ItemGroups.FirstOrDefault(i => i.Condition == condition) ??
-                    outputMSBuildProject.AddItemGroup();
-                itemGroup.Condition = condition;
-
-                foreach (var frameworkAssembly in frameworkAssemblies)
-                {
-                    _transformApplicator.Execute(
-                        FrameworkDependencyTransform.Transform(frameworkAssembly),
-                        itemGroup,
-                        mergeExisting: true);
-                }
-            }
         }
 
         private void AddProjectDependenciesToNewItemGroup(
@@ -202,10 +149,6 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
         private AddItemTransform<ProjectDependency> ProjectDependencyTransform => 
             GetProjectDependencyTransfrom();
 
-        private AddItemTransform<ProjectDependency> HoistedDependencyTransform =>
-            GetProjectDependencyTransfrom()
-            .WithMetadata("FromP2P", "true");
-
         private Func<AddItemTransform<ProjectDependency>> GetProjectDependencyTransfrom => 
             () => new AddItemTransform<ProjectDependency>(
                 "ProjectReference",
@@ -225,13 +168,5 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
             path => path,
             path => "",
             path => true);
-
-        private AddItemTransform<ProjectLibraryDependency> FrameworkDependencyTransform =>
-            new AddItemTransform<ProjectLibraryDependency>(
-                "Reference",
-                dep => dep.Name,
-                dep => "",
-                dep => true)
-            .WithMetadata("FromP2P", "true");
     }
 }

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

@@ -18,7 +18,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
     public class GivenThatIWantToMigrateProjectDependencies : TestBase
     {
         [Fact]
-        public void Project_dependencies_are_migrated_to_ProjectReference()
+        public void ProjectDependenciesAreMigratedToProjectReference()
         {
             var solutionDirectory =
                 TestAssetsManager.CreateTestInstance("TestAppWithLibrary", callingMethod: "p").Path;
@@ -41,7 +41,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_does_not_migrate_a_dependency_with_target_package_that_has_a_matching_project_as_a_ProjectReference()
+        public void ItDoesNotMigrateADependencyWithTargetPackageThatHasAMatchingProjectAsAProjectReference()
         {
             var testAssetsManager = GetTestGroupTestAssetsManager("NonRestoredTestProjects");
             var solutionDirectory =
@@ -62,7 +62,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void TFM_specific_Project_dependencies_are_migrated_to_ProjectReference_under_condition_ItemGroup()
+        public void TFMSpecificProjectDependenciesAreMigratedToProjectReferenceUnderConditionItemGroup()
         {
             var solutionDirectory =
                 TestAssetsManager.CreateTestInstance("TestAppWithLibraryUnderTFM", callingMethod: "p").Path;
@@ -85,7 +85,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_throws_when_project_dependency_is_unresolved()
+        public void ItThrowsWhenProjectDependencyIsUnresolved()
         {
             // No Lock file => unresolved
             var solutionDirectory =
@@ -108,7 +108,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         [Theory]
         [InlineData(@"some/path/to.cSproj", new [] { @"some/path/to.cSproj" })]
         [InlineData(@"to.CSPROJ",new [] { @"to.CSPROJ" })]
-        public void It_migrates_csproj_ProjectReference_in_xproj(string projectReference, string[] expectedMigratedReferences)
+        public void ItMigratesCsprojProjectReferenceInXproj(string projectReference, string[] expectedMigratedReferences)
         {
             var xproj = ProjectRootElement.Create();
             xproj.AddItem("ProjectReference", projectReference);
@@ -137,7 +137,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_migrates_csproj_ProjectReference_in_xproj_including_condition_on_ProjectReference()
+        public void ItMigratesCsprojProjectReferenceInXprojIncludingConditionOnProjectReference()
         {
             var projectReference = "some/to.csproj";
             var xproj = ProjectRootElement.Create();
@@ -171,7 +171,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_migrates_csproj_ProjectReference_in_xproj_including_condition_on_ProjectReference_parent()
+        public void ItMigratesCsprojProjectReferenceInXprojIncludingConditionOnProjectReferenceParent()
         {
             var projectReference = "some/to.csproj";
             var xproj = ProjectRootElement.Create();
@@ -205,7 +205,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_migrates_csproj_ProjectReference_in_xproj_including_condition_on_ProjectReference_parent_and_item()
+        public void ItMigratesCsprojProjectReferenceInXprojIncludingConditionOnProjectReferenceParentAndItem()
         {
             var projectReference = "some/to.csproj";
             var xproj = ProjectRootElement.Create();
@@ -240,17 +240,17 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
         }
 
         [Fact]
-        public void It_promotes_P2P_references_up_in_the_dependency_chain()
+        public void ItDoesNotPromoteP2PReferencesUpInTheDependencyChain()
         {
             var mockProj = MigrateProject("TestAppDependencyGraph", "ProjectA");
 
             var projectReferences = mockProj.Items.Where(
                 item => item.ItemType.Equals("ProjectReference", StringComparison.Ordinal));
-            projectReferences.Count().Should().Be(7);
+            projectReferences.Count().Should().Be(2);
         }
 
         [Fact]
-        public void It_promotes_FrameworkAssemblies_from_P2P_references_up_in_the_dependency_chain()
+        public void ItDoesNotPromoteFrameworkAssembliesFromP2PReferencesUpInTheDependencyChain()
         {
             var solutionDirectory = TestAssets.Get(TestAssetKinds.DesktopTestProjects, "TestAppWithFrameworkAssemblies")
                 .CreateInstance()
@@ -269,32 +269,11 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
                 item => item.ItemType == "Reference" &&
                 item.Include == "System.ComponentModel.DataAnnotations" &&
                 item.Parent.Condition == " '$(TargetFramework)' == 'net451' ");
-            frameworkAssemblyReferences.Count().Should().Be(1);
+            frameworkAssemblyReferences.Count().Should().Be(0);
         }
 
         [Fact]
-        public void All_promoted_P2P_references_are_marked_with_a_FromP2P_attribute()
+        public void NoP2PReferenceIsMarkedWithAFromP2PAttribute()
-        {
-            var expectedHoistedProjectReferences = new [] {
-                Path.Combine("..", "ProjectD", "ProjectD.csproj"),
-                Path.Combine("..", "ProjectE", "ProjectE.csproj"),
-                Path.Combine("..", "CsprojLibrary1", "CsprojLibrary1.csproj"),
-                Path.Combine("..", "CsprojLibrary2", "CsprojLibrary2.csproj"),
-                Path.Combine("..", "CsprojLibrary3", "CsprojLibrary3.csproj")
-            };
-
-            var mockProj = MigrateProject("TestAppDependencyGraph", "ProjectA");
-
-            var projectReferences = mockProj.Items
-                .Where(item =>
-                    item.ItemType == "ProjectReference" && item.GetMetadataWithName("FromP2P") != null)
-                .Select(i => i.Include);
-
-            projectReferences.Should().BeEquivalentTo(expectedHoistedProjectReferences);
-        }
-
-        [Fact]
-        public void All_non_promoted_P2P_references_are_not_marked_with_a_FromP2P_attribute()
         {
             var expectedNonHoistedProjectReferences = new [] {
                 Path.Combine("..", "ProjectB", "ProjectB.csproj"),
@@ -305,14 +284,16 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Tests
 
             var projectReferences = mockProj.Items
                 .Where(item =>
-                    item.ItemType == "ProjectReference" && item.GetMetadataWithName("FromP2P") == null)
+                    item.ItemType == "ProjectReference");
-                .Select(i => i.Include);
 
-            projectReferences.Should().BeEquivalentTo(expectedNonHoistedProjectReferences);
+            projectReferences.Should().HaveCount(c => c ==  2)
+                .And.OnlyContain(item => item.GetMetadataWithName("FromP2P") == null);
+
+            projectReferences.Select(i => i.Include).Should().BeEquivalentTo(expectedNonHoistedProjectReferences);
         }
 
         [Fact]
-        public void It_migrates_unqualified_dependencies_as_ProjectReference_when_a_matching_project_is_found()
+        public void ItMigratesUnqualifiedDependenciesAsProjectReferenceWhenAMatchingProjectIsFound()
         {
             var mockProj = MigrateProject("TestAppWithUnqualifiedDependencies", "ProjectA");
             var projectReferenceInclude = Path.Combine("..", "ProjectB", "ProjectB.csproj");            

test/dotnet-migrate.Tests/GivenThatIWantToMigrateTestApps.cs

@@ -484,20 +484,7 @@ namespace Microsoft.DotNet.Migration.Tests
             Restore(projectDirectory, projectName);
             BuildMSBuild(projectDirectory, projectName);
         }
-
+        
-        [Fact]
-        public void ItFailsGracefullyWhenMigratingAppWithMissingDependency()
-        {
-            string projectName = "MigrateAppWithMissingDep";
-            var projectDirectory = Path.Combine(GetTestGroupTestAssetsManager("NonRestoredTestProjects").CreateTestInstance(projectName).Path, "MyApp");
-
-            string migrationOutputFile = Path.Combine(projectDirectory, "migration-output.json");
-            File.Exists(migrationOutputFile).Should().BeFalse();
-            MigrateCommand.Run(new string[] { projectDirectory, "-r", migrationOutputFile, "--format-report-file-json" }).Should().NotBe(0);
-            File.Exists(migrationOutputFile).Should().BeTrue();
-            File.ReadAllText(migrationOutputFile).Should().Contain("MIGRATE1018");
-        }
-
         private void VerifyAutoInjectedDesktopReferences(string projectDirectory, string projectName, bool shouldBePresent)
         {
             if (projectName != null)