Merge pull request #3243 from jasonmalinowski/deal-with-projectreference-circularities

Deal with solutions with circular project references in MSBuildWorkspace

src/Workspaces/Core/Desktop/Workspace/MSBuild/MSBuildWorkspace.cs

@@ -411,7 +411,7 @@ public async Task<Solution> OpenSolutionAsync(string solutionFilePath, Cancellat
             }
 
             // a list to accumulate all the loaded projects
-            var loadedProjects = new List<ProjectInfo>();
+            var loadedProjects = new Dictionary<ProjectId, ProjectInfo>();
 
             // load all the projects
             foreach (var project in solutionFile.ProjectsInOrder)
@@ -463,7 +463,7 @@ public async Task<Solution> OpenSolutionAsync(string solutionFilePath, Cancellat
             }
 
             // a list to accumulate all the loaded projects
-            var loadedProjects = new List<ProjectInfo>();
+            var loadedProjects = new Dictionary<ProjectId, ProjectInfo>();
 
             var reportMode = this.SkipUnrecognizedProjects ? ReportMode.Log : ReportMode.Throw;
 
@@ -487,7 +487,7 @@ public async Task<Solution> OpenSolutionAsync(string solutionFilePath, Cancellat
 #endif
 
             // construct workspace from loaded project infos
-            this.OnSolutionAdded(SolutionInfo.Create(SolutionId.CreateNewId(debugName: absoluteSolutionPath), version, absoluteSolutionPath, loadedProjects));
+            this.OnSolutionAdded(SolutionInfo.Create(SolutionId.CreateNewId(debugName: absoluteSolutionPath), version, absoluteSolutionPath, loadedProjects.Values));
 
             this.UpdateReferencesAfterAdd();
 
@@ -510,11 +510,11 @@ public async Task<Project> OpenProjectAsync(string projectFilePath, Cancellation
                 IProjectFileLoader loader;
                 if (this.TryGetLoaderFromProjectPath(projectFilePath, ReportMode.Throw, out loader))
                 {
-                    var loadedProjects = new List<ProjectInfo>();
+                    var loadedProjects = new Dictionary<ProjectId, ProjectInfo>();
                     var projectId = await GetOrLoadProjectAsync(fullPath, loader, this.LoadMetadataForReferencedProjects, loadedProjects, cancellationToken).ConfigureAwait(false);
 
                     // add projects to solution
-                    foreach (var project in loadedProjects)
+                    foreach (var project in loadedProjects.Values)
                     {
                         this.OnProjectAdded(project);
                     }
@@ -618,7 +618,7 @@ private Solution UpdateReferencesAfterAdd(Solution solution)
             return solution;
         }
 
-        private async Task<ProjectId> GetOrLoadProjectAsync(string projectFilePath, IProjectFileLoader loader, bool preferMetadata, List<ProjectInfo> loadedProjects, CancellationToken cancellationToken)
+        private async Task<ProjectId> GetOrLoadProjectAsync(string projectFilePath, IProjectFileLoader loader, bool preferMetadata, Dictionary<ProjectId, ProjectInfo> loadedProjects, CancellationToken cancellationToken)
         {
             var projectId = GetProjectId(projectFilePath);
             if (projectId == null)
@@ -629,7 +629,7 @@ private async Task<ProjectId> GetOrLoadProjectAsync(string projectFilePath, IPro
             return projectId;
         }
 
-        private async Task<ProjectId> LoadProjectAsync(string projectFilePath, IProjectFileLoader loader, bool preferMetadata, List<ProjectInfo> loadedProjects, CancellationToken cancellationToken)
+        private async Task<ProjectId> LoadProjectAsync(string projectFilePath, IProjectFileLoader loader, bool preferMetadata, Dictionary<ProjectId, ProjectInfo> loadedProjects, CancellationToken cancellationToken)
         {
             System.Diagnostics.Debug.Assert(projectFilePath != null);
             System.Diagnostics.Debug.Assert(loader != null);
@@ -693,7 +693,7 @@ private async Task<ProjectId> LoadProjectAsync(string projectFilePath, IProjectF
 
             // project references
             var resolvedReferences = await this.ResolveProjectReferencesAsync(
-                projectFilePath, projectFileInfo.ProjectReferences, preferMetadata, loadedProjects, cancellationToken).ConfigureAwait(false);
+                projectId, projectFilePath, projectFileInfo.ProjectReferences, preferMetadata, loadedProjects, cancellationToken).ConfigureAwait(false);
 
             var metadataReferences = projectFileInfo.MetadataReferences
                 .Concat(resolvedReferences.MetadataReferences);
@@ -714,6 +714,7 @@ private async Task<ProjectId> LoadProjectAsync(string projectFilePath, IProjectF
             }
 
             loadedProjects.Add(
+                projectId,
                 ProjectInfo.Create(
                     projectId,
                     version,
@@ -800,10 +801,11 @@ private class ResolvedReferences
         }
 
         private async Task<ResolvedReferences> ResolveProjectReferencesAsync(
+            ProjectId thisProjectId,
             string thisProjectPath,
             IReadOnlyList<ProjectFileReference> projectFileReferences,
             bool preferMetadata,
-            List<ProjectInfo> loadedProjects,
+            Dictionary<ProjectId, ProjectInfo> loadedProjects,
             CancellationToken cancellationToken)
         {
             var resolvedReferences = new ResolvedReferences();
@@ -842,8 +844,25 @@ private class ResolvedReferences
                     {
                         // load the project
                         var projectId = await this.GetOrLoadProjectAsync(fullPath, loader, preferMetadata, loadedProjects, cancellationToken).ConfigureAwait(false);
-                        resolvedReferences.ProjectReferences.Add(new ProjectReference(projectId, projectFileReference.Aliases));
-                        continue;
+
+                        // If that other project already has a reference on us, this will cause a circularity.
+                        // This check doesn't need to be in the "already loaded" path above, since in any circularity this path
+                        // must be taken at least once.
+                        if (ProjectAlreadyReferencesProject(loadedProjects, projectId, targetProject: thisProjectId))
+                        {
+                            // We'll try to make this metadata if we can
+                            var projectMetadata = await this.GetProjectMetadata(fullPath, projectFileReference.Aliases, _properties, cancellationToken).ConfigureAwait(false);
+                            if (projectMetadata != null)
+                            {
+                                resolvedReferences.MetadataReferences.Add(projectMetadata);
+                            }
+                            continue;
+                        }
+                        else
+                        {
+                            resolvedReferences.ProjectReferences.Add(new ProjectReference(projectId, projectFileReference.Aliases));
+                            continue;
+                        }
                     }
                 }
                 else
@@ -859,6 +878,18 @@ private class ResolvedReferences
             return resolvedReferences;
         }
 
+        /// <summary>
+        /// Returns true if the project identified by <paramref name="fromProject"/> has a reference (even indirectly)
+        /// on the project identified by <paramref name="targetProject"/>.
+        /// </summary>
+        private bool ProjectAlreadyReferencesProject(Dictionary<ProjectId, ProjectInfo> loadedProjects, ProjectId fromProject, ProjectId targetProject)
+        {
+            ProjectInfo info;
+
+            return loadedProjects.TryGetValue(fromProject, out info) && info.ProjectReferences.Any(pr => pr.ProjectId == targetProject ||
+                ProjectAlreadyReferencesProject(loadedProjects, pr.ProjectId, targetProject));
+        }
+
         /// <summary>
         /// Gets a MetadataReference to a project's output assembly.
         /// </summary>

src/Workspaces/CoreTest/ServicesTest.csproj

@@ -301,6 +301,11 @@
   <ItemGroup>
     <EmbeddedResource Include="TestFiles\CSharpProject_CSharpProject_ExternAlias2.csproj" />
   </ItemGroup>
+  <ItemGroup>
+    <EmbeddedResource Include="TestFiles\CircularProjectReferences\CircularCSharpProject1.csproj" />
+    <EmbeddedResource Include="TestFiles\CircularProjectReferences\CircularCSharpProject2.csproj" />
+    <EmbeddedResource Include="TestFiles\CircularProjectReferences\CircularSolution.sln" />
+  </ItemGroup>
   <ImportGroup Label="Targets">
     <Import Project="..\..\Tools\Microsoft.CodeAnalysis.Toolset.Open\Targets\VSL.Imports.targets" />
     <Import Project="..\..\..\build\VSL.Imports.Closed.targets" />

src/Workspaces/CoreTest/TestFiles/CircularProjectReferences/CircularCSharpProject1.csproj

+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright (c)  Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information. -->
+<Project ToolsVersion="4.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <ProjectGuid>{686DD036-86AA-443E-8A10-DDB43266A8C4}</ProjectGuid>
+    <OutputType>Library</OutputType>
+    <AppDesignerFolder>Properties</AppDesignerFolder>
+    <RootNamespace>Library1</RootNamespace>
+    <AssemblyName>Library1</AssemblyName>
+    <TargetFrameworkVersion>v4.5</TargetFrameworkVersion>
+    <FileAlignment>512</FileAlignment>
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
+    <DebugSymbols>true</DebugSymbols>
+    <DebugType>full</DebugType>
+    <Optimize>false</Optimize>
+    <OutputPath>bin\Debug\</OutputPath>
+    <DefineConstants>DEBUG;TRACE</DefineConstants>
+    <ErrorReport>prompt</ErrorReport>
+    <WarningLevel>4</WarningLevel>
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
+    <DebugType>pdbonly</DebugType>
+    <Optimize>true</Optimize>
+    <OutputPath>bin\Release\</OutputPath>
+    <DefineConstants>TRACE</DefineConstants>
+    <ErrorReport>prompt</ErrorReport>
+    <WarningLevel>4</WarningLevel>
+  </PropertyGroup>
+  <ItemGroup>
+    <Reference Include="Microsoft.CSharp" />
+    <Reference Include="System" />
+    <Reference Include="System.Core" />
+    <Reference Include="System.Data" />
+    <Reference Include="System.Data.DataSetExtensions" />
+    <Reference Include="System.Xml" />
+    <Reference Include="System.Xml.Linq" />
+  </ItemGroup>
+  <ItemGroup>
+    <Compile Include="Properties\AssemblyInfo.cs" />
+    <ProjectReference Include="CircularCSharpProject2.csproj">
+      <Project>{C71872E2-0D54-4C4E-B6A9-8C1726B7B78C}</Project>
+      <Name>CSharpProject2</Name>
+    </ProjectReference>
+  </ItemGroup>
+  <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
+  <!-- To modify your build process, add your task inside one of the targets below and uncomment it. 
+       Other similar extension points exist, see Microsoft.Common.targets.
+  <Target Name="BeforeBuild">
+  </Target>
+  <Target Name="AfterBuild">
+  </Target>
+  -->
+</Project>

src/Workspaces/CoreTest/TestFiles/CircularProjectReferences/CircularCSharpProject2.csproj

+<?xml version="1.0" encoding="utf-8"?>
+<!-- Copyright (c)  Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information. -->
+<Project ToolsVersion="4.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <ProjectGuid>{C71872E2-0D54-4C4E-B6A9-8C1726B7B78C}</ProjectGuid>
+    <OutputType>Library</OutputType>
+    <AppDesignerFolder>Properties</AppDesignerFolder>
+    <RootNamespace>Library2</RootNamespace>
+    <AssemblyName>Library2</AssemblyName>
+    <TargetFrameworkVersion>v4.5</TargetFrameworkVersion>
+    <FileAlignment>512</FileAlignment>
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
+    <DebugSymbols>true</DebugSymbols>
+    <DebugType>full</DebugType>
+    <Optimize>false</Optimize>
+    <OutputPath>bin\Debug\</OutputPath>
+    <DefineConstants>DEBUG;TRACE</DefineConstants>
+    <ErrorReport>prompt</ErrorReport>
+    <WarningLevel>4</WarningLevel>
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
+    <DebugType>pdbonly</DebugType>
+    <Optimize>true</Optimize>
+    <OutputPath>bin\Release\</OutputPath>
+    <DefineConstants>TRACE</DefineConstants>
+    <ErrorReport>prompt</ErrorReport>
+    <WarningLevel>4</WarningLevel>
+  </PropertyGroup>
+  <ItemGroup>
+    <Reference Include="Microsoft.CSharp" />
+    <Reference Include="System" />
+    <Reference Include="System.Core" />
+    <Reference Include="System.Data" />
+    <Reference Include="System.Data.DataSetExtensions" />
+    <Reference Include="System.Xml" />
+    <Reference Include="System.Xml.Linq" />
+  </ItemGroup>
+  <ItemGroup>
+    <Compile Include="Properties\AssemblyInfo.cs" />
+    <ProjectReference Include="CircularCSharpProject1.csproj">
+      <Project>{686DD036-86AA-443E-8A10-DDB43266A8C4}</Project>
+      <Name>CSharpProject2</Name>
+    </ProjectReference>
+  </ItemGroup>
+  <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
+  <!-- To modify your build process, add your task inside one of the targets below and uncomment it. 
+       Other similar extension points exist, see Microsoft.Common.targets.
+  <Target Name="BeforeBuild">
+  </Target>
+  <Target Name="AfterBuild">
+  </Target>
+  -->
+</Project>

src/Workspaces/CoreTest/TestFiles/CircularProjectReferences/CircularSolution.sln

+
+Microsoft Visual Studio Solution File, Format Version 11.00
+# Visual Studio 2010
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "CircularCSharpProject1", "CircularCSharpProject1.csproj", "{686DD036-86AA-443E-8A10-DDB43266A8C4}"
+EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "CircularCSharpProject2", "CircularCSharpProject2.csproj", "{C71872E2-0D54-4C4E-B6A9-8C1726B7B78C}"
+EndProject
+Global
+	GlobalSection(SolutionConfigurationPlatforms) = preSolution
+		Debug|Any CPU = Debug|Any CPU
+		Release|Any CPU = Release|Any CPU
+	EndGlobalSection
+	GlobalSection(ProjectConfigurationPlatforms) = postSolution
+	EndGlobalSection
+	GlobalSection(SolutionProperties) = preSolution
+		HideSolutionNode = FALSE
+	EndGlobalSection
+EndGlobal

src/Workspaces/CoreTest/WorkspaceTests/MSBuildWorkspaceTests.cs

@@ -179,6 +179,29 @@ public void TestOpenSolution_DuplicateProjectGuids()
             var solution = MSBuildWorkspace.Create().OpenSolutionAsync(GetSolutionFileName("DuplicatedGuids.sln")).Result;
         }
 
+        [Fact, Trait(Traits.Feature, Traits.Features.Workspace)]
+        [WorkItem(831379, "DevDiv")]
+        public void GetCompilationWithCircularProjectReferences()
+        {
+            CreateFiles(GetSolutionWithCircularProjectReferences());
+
+            var solution = MSBuildWorkspace.Create().OpenSolutionAsync(GetSolutionFileName("CircularSolution.sln")).Result;
+
+            // Verify we can get compilations for both projects
+            var projects = solution.Projects.ToArray();
+
+            // Exactly one of them should have a reference to the other. Which one it is is unspecced
+            Assert.True(projects[0].ProjectReferences.Any(r => r.ProjectId == projects[1].Id) ||
+                        projects[1].ProjectReferences.Any(r => r.ProjectId == projects[0].Id));
+
+            var compilation1 = projects[0].GetCompilationAsync().Result;
+            var compilation2 = projects[1].GetCompilationAsync().Result;
+
+            // Exactly one of them should have a compilation to the other. Which one it is is unspecced
+            Assert.True(compilation1.References.OfType<CompilationReference>().Any(c => c.Compilation == compilation2) ||
+                        compilation2.References.OfType<CompilationReference>().Any(c => c.Compilation == compilation1));
+        }
+
         [Fact, Trait(Traits.Feature, Traits.Features.Workspace)]
         public void TestOutputFilePaths()
         {

src/Workspaces/CoreTest/WorkspaceTests/WorkspaceTestBase.cs

@@ -162,6 +162,16 @@ protected FileSet GetSolutionWithDuplicatedGuidFiles()
             });
         }
 
+        protected FileSet GetSolutionWithCircularProjectReferences()
+        {
+            return new FileSet(new Dictionary<string, object>
+            {
+                { @"CircularSolution.sln", GetResourceText("CircularProjectReferences.CircularSolution.sln") },
+                { @"CircularCSharpProject1.csproj", GetResourceText("CircularProjectReferences.CircularCSharpProject1.csproj") },
+                { @"CircularCSharpProject2.csproj", GetResourceText("CircularProjectReferences.CircularCSharpProject2.csproj") },
+            });
+        }
+
         public static byte[] GetResourceBytes(string fileName)
         {
             var fullName = @"Microsoft.CodeAnalysis.UnitTests.TestFiles." + fileName;