Address PR Feedback

src/Microsoft.DotNet.ProjectJsonMigration/ProjectDependencyFinder.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using Microsoft.Build.Construction;
 using Microsoft.DotNet.ProjectModel;
 using System.Linq;
 using System.IO;
@@ -14,18 +15,32 @@ namespace Microsoft.DotNet.ProjectJsonMigration
 {
     public class ProjectDependencyFinder 
     {
-        public IEnumerable<ProjectDependency> ResolveProjectDependencies(IEnumerable<ProjectContext> projectContexts)
+        public IEnumerable<ProjectDependency> ResolveProjectDependencies(string projectDir, string xprojFile = null)
+        {
+            var projectContexts = ProjectContext.CreateContextForEachFramework(projectDir);
+            xprojFile = xprojFile ?? FindXprojFile(projectDir);
+
+            ProjectRootElement xproj = null;
+            if (xprojFile != null)
+            {
+                xproj = ProjectRootElement.Open(xprojFile);
+            }
+
+            return ResolveProjectDependencies(projectContexts, ResolveXProjProjectDependencyNames(xproj));
+        }
+
+        public IEnumerable<ProjectDependency> ResolveProjectDependencies(IEnumerable<ProjectContext> projectContexts, IEnumerable<string> preResolvedProjects=null)
         {
             foreach(var projectContext in projectContexts)
             {
-                foreach(var projectDependency in ResolveProjectDependencies(projectContext))
+                foreach(var projectDependency in ResolveProjectDependencies(projectContext, preResolvedProjects))
                 {
                     yield return projectDependency;
                 }
             }
         }
 
-        public IEnumerable<ProjectDependency> ResolveProjectDependencies(ProjectContext projectContext, HashSet<string> preResolvedProjects=null)
+        public IEnumerable<ProjectDependency> ResolveProjectDependencies(ProjectContext projectContext, IEnumerable<string> preResolvedProjects=null)
         {
             preResolvedProjects = preResolvedProjects ?? new HashSet<string>();
 
@@ -59,6 +74,40 @@ namespace Microsoft.DotNet.ProjectJsonMigration
             return projectDependencies;
         }
 
+        private IEnumerable<string> ResolveXProjProjectDependencyNames(ProjectRootElement xproj)
+        {
+            var xprojDependencies = ResolveXProjProjectDependencies(xproj).SelectMany(r => r.Includes());
+            return new HashSet<string>(xprojDependencies.Select(p => Path.GetFileNameWithoutExtension(p)));
+        }
+
+        internal IEnumerable<ProjectItemElement> ResolveXProjProjectDependencies(ProjectRootElement xproj)
+        {
+            if (xproj == null)
+            {
+                MigrationTrace.Instance.WriteLine($"{nameof(ProjectDependencyFinder)}: No xproj file given.");
+                return Enumerable.Empty<ProjectItemElement>();
+            }
+
+            return xproj.Items
+                        .Where(i => i.ItemType == "ProjectReference")
+                        .Where(p => p.Includes().Any(
+                                    include => string.Equals(Path.GetExtension(include), ".csproj", StringComparison.OrdinalIgnoreCase)));
+        }
+
+        internal string FindXprojFile(string projectDirectory)
+        {
+            var allXprojFiles = Directory.EnumerateFiles(projectDirectory, "*.xproj", SearchOption.TopDirectoryOnly);
+
+            if (allXprojFiles.Count() > 1)
+            {
+                MigrationErrorCodes
+                    .MIGRATE1017($"Multiple xproj files found in {projectDirectory}, please specify which to use")
+                    .Throw();
+            }
+
+            return allXprojFiles.FirstOrDefault();
+        }
+
         private Dictionary<string, ProjectDependency> FindPossibleProjectDependencies(string projectJsonFilePath)
         {
             var projectRootDirectory = GetRootFromProjectJson(projectJsonFilePath);

src/Microsoft.DotNet.ProjectJsonMigration/ProjectMigrator.cs

@@ -40,14 +40,14 @@ namespace Microsoft.DotNet.ProjectJsonMigration
                 throw new ArgumentNullException();
             }
 
-            MigrateHelper(rootSettings);
+            MigrateProject(rootSettings);
 
             if (skipProjectReferences)
             {
                 return;
             }
 
-            var projectDependencies = ResolveTransitiveClosureProjectDependencies(rootSettings.ProjectDirectory);
+            var projectDependencies = ResolveTransitiveClosureProjectDependencies(rootSettings.ProjectDirectory, rootSettings.ProjectXProjFilePath);
 
             foreach(var project in projectDependencies)
             {
@@ -56,15 +56,14 @@ namespace Microsoft.DotNet.ProjectJsonMigration
                                                      projectDir,
                                                      rootSettings.SdkPackageVersion,
                                                      rootSettings.MSBuildProjectTemplate);
-                MigrateHelper(settings);
+                MigrateProject(settings);
             }
         }
 
-        private IEnumerable<ProjectDependency> ResolveTransitiveClosureProjectDependencies(string rootProject)
+        private IEnumerable<ProjectDependency> ResolveTransitiveClosureProjectDependencies(string rootProject, string xprojFile)
         {
             HashSet<ProjectDependency> projectsMap = new HashSet<ProjectDependency>(new ProjectDependencyComparer());
-            var projectContexts = ProjectContext.CreateContextForEachFramework(rootProject);
+            var projectDependencies = _projectDependencyFinder.ResolveProjectDependencies(rootProject, xprojFile);
-            var projectDependencies = _projectDependencyFinder.ResolveProjectDependencies(projectContexts);
             Queue<ProjectDependency> projectsQueue = new Queue<ProjectDependency>(projectDependencies);
 
             while(projectsQueue.Count() != 0)
@@ -78,8 +77,8 @@ namespace Microsoft.DotNet.ProjectJsonMigration
 
                 projectsMap.Add(projectDependency);
 
-                projectContexts = ProjectContext.CreateContextForEachFramework(projectDependency.ProjectFilePath);
+                var projectDir = Path.GetDirectoryName(projectDependency.ProjectFilePath);
-                projectDependencies = _projectDependencyFinder.ResolveProjectDependencies(projectContexts);
+                projectDependencies = _projectDependencyFinder.ResolveProjectDependencies(projectDir);
 
                 foreach(var project in projectDependencies)
                 {
@@ -90,13 +89,14 @@ namespace Microsoft.DotNet.ProjectJsonMigration
             return projectsMap;
         }
 
-        private void MigrateHelper(MigrationSettings migrationSettings)
+        private void MigrateProject(MigrationSettings migrationSettings)
         {
             var migrationRuleInputs = ComputeMigrationRuleInputs(migrationSettings);
 
             if (IsMigrated(migrationSettings, migrationRuleInputs))
             {
                 // TODO : Adding user-visible logging
+                MigrationTrace.Instance.WriteLine($"{nameof(ProjectMigrator)}: Skip migrating {migrationSettings.ProjectDirectory}, it is already migrated.");
                 return;
             }
 
@@ -110,7 +110,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration
         private MigrationRuleInputs ComputeMigrationRuleInputs(MigrationSettings migrationSettings)
         {
             var projectContexts = ProjectContext.CreateContextForEachFramework(migrationSettings.ProjectDirectory);
-            var xprojFile = migrationSettings.ProjectXProjFilePath ?? FindXprojFile(migrationSettings.ProjectDirectory);
+            var xprojFile = migrationSettings.ProjectXProjFilePath ?? _projectDependencyFinder.FindXprojFile(migrationSettings.ProjectDirectory);
             
             ProjectRootElement xproj = null;
             if (xprojFile != null)
@@ -130,20 +130,6 @@ namespace Microsoft.DotNet.ProjectJsonMigration
             return new MigrationRuleInputs(projectContexts, templateMSBuildProject, itemGroup, propertyGroup, xproj);
         }
 
-        private string FindXprojFile(string projectDirectory)
-        {
-            var allXprojFiles = Directory.EnumerateFiles(projectDirectory, "*.xproj", SearchOption.TopDirectoryOnly);
-
-            if (allXprojFiles.Count() > 1)
-            {
-                MigrationErrorCodes
-                    .MIGRATE1017($"Multiple xproj files found in {projectDirectory}, please specify which to use")
-                    .Throw();
-            }
-
-            return allXprojFiles.FirstOrDefault();
-        }
-
         private void VerifyInputs(MigrationRuleInputs migrationRuleInputs, MigrationSettings migrationSettings)
         {
             VerifyProject(migrationRuleInputs.ProjectContexts, migrationSettings.ProjectDirectory);

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

@@ -30,7 +30,7 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
         {
             _projectDirectory = migrationSettings.ProjectDirectory;
 
-            var migratedXProjDependencyPaths = MigrateXProjProjectDependencies(migrationSettings, migrationRuleInputs);
+            var migratedXProjDependencyPaths = MigrateXProjProjectDependencies(migrationRuleInputs);
             var migratedXProjDependencyNames = new HashSet<string>(migratedXProjDependencyPaths.Select(p => Path.GetFileNameWithoutExtension(p)));
 
             AddPropertyTransformsToCommonPropertyGroup(migrationRuleInputs.CommonPropertyGroup);
@@ -40,23 +40,17 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
                 migrationRuleInputs.OutputMSBuildProject);
         }
 
-        private IEnumerable<string> MigrateXProjProjectDependencies(MigrationSettings migrationSettings, MigrationRuleInputs migrationRuleInputs)
+        private IEnumerable<string> MigrateXProjProjectDependencies(MigrationRuleInputs migrationRuleInputs)
         {
-            var xproj = migrationRuleInputs.ProjectXproj;
+            var csprojReferenceItems = _projectDependencyFinder.ResolveXProjProjectDependencies(migrationRuleInputs.ProjectXproj);
-            if (xproj == null)
+
+            if (!csprojReferenceItems.Any())
             {
-                MigrationTrace.Instance.WriteLine($"{nameof(MigrateProjectDependenciesRule)}: No xproj file given.");
                 return Enumerable.Empty<string>();
             }
 
             var csprojTransformedReferences = new List<ProjectItemElement>();
 
-            var csprojReferenceItems = xproj.Items
-                .Where(i => i.ItemType == "ProjectReference")
-                .Where(p => 
-                    p.Includes().Any(
-                        include => string.Equals(Path.GetExtension(include), ".csproj", StringComparison.OrdinalIgnoreCase)));
-
             foreach (var csprojReferenceItem in csprojReferenceItems)
             {
                 var conditionChain = csprojReferenceItem.ConditionChain();
@@ -72,7 +66,6 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
                 csprojTransformedReferences.Add(transformItem);
             }
 
-            
             MigrationTrace.Instance.WriteLine($"{nameof(MigrateProjectDependenciesRule)}: Migrating {csprojTransformedReferences.Count()} xproj to csproj references");
 
             foreach (var csprojTransformedReference in csprojTransformedReferences)
@@ -91,7 +84,6 @@ namespace Microsoft.DotNet.ProjectJsonMigration.Rules
             foreach (var projectContext in projectContexts)
             {
                 var projectDependencies = _projectDependencyFinder.ResolveProjectDependencies(projectContext, migratedXProjDependencyNames);
-                var projectExports = projectContext.CreateExporter("_").GetDependencies();
 
                 var projectDependencyTransformResults = projectDependencies.Select(p => ProjectDependencyTransform.Transform(p));