Merge pull request #2613 from mattwar/Bug1167536

Handle recursive project references in vs shim

src/VisualStudio/CSharp/Impl/ProjectSystemShim/CSharpProject.cs

@@ -69,5 +69,10 @@ protected override void UpdateAnalyzerRules()
 
             this.SetOptions(this.CreateCompilationOptions(), this.CreateParseOptions());
         }
+
+        internal void AddProjectReference(CSharpProject project)
+        {
+            base.AddProjectReference(new ProjectReference(project.Id));
+        }
     }
 }

src/VisualStudio/CSharp/Test/CSharpVisualStudioTest.csproj

@@ -164,6 +164,7 @@
     <Compile Include="Debugging\DataTipInfoGetterTests.cs" />
     <Compile Include="Debugging\LocationInfoGetterTests.cs" />
     <Compile Include="Debugging\NameResolverTests.cs" />
+    <Compile Include="ProjectSystemShim\CSharpReferencesTests.cs" />
     <Compile Include="ProjectSystemShim\SourceFileHandlingTests.cs" />
     <Content Include="Debugging\ProximityExpressionsGetterTestFile.cs" />
     <Compile Include="Debugging\ProximityExpressionsGetterTests.cs" />

src/VisualStudio/CSharp/Test/ProjectSystemShim/CSharpReferencesTests.cs

+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Runtime.InteropServices;
+using System.Text;
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.VisualStudio.LanguageServices.CSharp.ProjectSystemShim;
+using Microsoft.VisualStudio.LanguageServices.CSharp.ProjectSystemShim.Interop;
+using Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim.Framework;
+using Roslyn.Test.Utilities;
+using Xunit;
+
+namespace Roslyn.VisualStudio.CSharp.UnitTests.ProjectSystemShim
+{
+    using static CSharpHelpers;
+
+    public class CSharpReferenceTests
+    {
+        [Fact]
+        [Trait(Traits.Feature, Traits.Features.ProjectSystemShims)]
+        public void AddingReferenceToProjectMetadataPromotesToProjectReference()
+        {
+            using (var environment = new TestEnvironment())
+            {
+                var project1 = CreateCSharpProject(environment, "project1");
+                environment.ProjectTracker.UpdateProjectBinPath(project1, null, @"c:\project1.dll");
+
+                var project2 = CreateCSharpProject(environment, "project2");
+                environment.ProjectTracker.UpdateProjectBinPath(project2, null, @"c:\project2.dll");
+
+                // since this is known to be the output path of project1, the metadata reference is converted to a project reference
+                project2.OnImportAdded(@"c:\project1.dll", "project1");
+
+                Assert.Equal(true, project2.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project1.Id));
+
+                project2.Disconnect();
+                project1.Disconnect();
+            }
+        }
+
+        [Fact]
+        [Trait(Traits.Feature, Traits.Features.ProjectSystemShims)]
+        public void AddCylicProjectMetadataReferences()
+        {
+            using (var environment = new TestEnvironment())
+            {
+                var project1 = CreateCSharpProject(environment, "project1");
+                environment.ProjectTracker.UpdateProjectBinPath(project1, null, @"c:\project1.dll");
+
+                var project2 = CreateCSharpProject(environment, "project2");
+                environment.ProjectTracker.UpdateProjectBinPath(project2, null, @"c:\project2.dll");
+
+                project1.AddProjectReference(project2);
+
+                // normally this metadata reference would be elevated to a project reference, but fails because of cyclicness
+                project2.OnImportAdded(@"c:\project1.dll", "project1");
+
+                Assert.Equal(true, project1.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project2.Id));
+                Assert.Equal(false, project2.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project1.Id));
+
+                project2.Disconnect();
+                project1.Disconnect();
+            }
+        }
+
+        [Fact]
+        [Trait(Traits.Feature, Traits.Features.ProjectSystemShims)]
+        public void AddCylicProjectReferences()
+        {
+            using (var environment = new TestEnvironment())
+            {
+                var project1 = CreateCSharpProject(environment, "project1");
+                var project2 = CreateCSharpProject(environment, "project2");
+
+                project1.AddProjectReference(project2);
+                project2.AddProjectReference(project1);
+
+                Assert.Equal(true, project1.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project2.Id));
+                Assert.Equal(false, project2.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project1.Id));
+
+                project2.Disconnect();
+                project1.Disconnect();
+            }
+        }
+
+        [Fact]
+        [Trait(Traits.Feature, Traits.Features.ProjectSystemShims)]
+        public void AddCylicProjectReferencesDeep()
+        {
+            using (var environment = new TestEnvironment())
+            {
+                var project1 = CreateCSharpProject(environment, "project1");
+                var project2 = CreateCSharpProject(environment, "project2");
+                var project3 = CreateCSharpProject(environment, "project3");
+                var project4 = CreateCSharpProject(environment, "project4");
+
+                project1.AddProjectReference(project2);
+                project2.AddProjectReference(project3);
+                project3.AddProjectReference(project4);
+                project4.AddProjectReference(project1);
+
+                Assert.Equal(true, project1.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project2.Id));
+                Assert.Equal(true, project2.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project3.Id));
+                Assert.Equal(true, project3.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project4.Id));
+                Assert.Equal(false, project4.GetCurrentProjectReferences().Any(pr => pr.ProjectId == project1.Id));
+
+                project4.Disconnect();
+                project3.Disconnect();
+                project2.Disconnect();
+                project1.Disconnect();
+            }
+        }
+
+    }
+}
\ No newline at end of file

src/VisualStudio/Core/Def/Implementation/ProjectSystem/AbstractProject.cs

@@ -414,9 +414,12 @@ protected int AddMetadataReferenceAndTryConvertingToProjectReferenceIfPossible(s
             if (ProjectTracker.TryGetProjectByBinPath(filePath, out project))
             {
                 var projectReference = new ProjectReference(project.Id, properties.Aliases, properties.EmbedInteropTypes);
-                AddProjectReference(projectReference);
-                _metadataFileNameToConvertedProjectReference.Add(filePath, projectReference);
-                return VSConstants.S_OK;
+                if (CanAddProjectReference(projectReference))
+                {
+                    AddProjectReference(projectReference);
+                    _metadataFileNameToConvertedProjectReference.Add(filePath, projectReference);
+                    return VSConstants.S_OK;
+                }
             }
 
             if (!File.Exists(filePath))
@@ -512,7 +515,7 @@ private void OnAnalyzerChanged(object sender, EventArgs e)
         protected void AddProjectReference(ProjectReference projectReference)
         {
             // dev11 is sometimes calling us multiple times for the same data
-            if (_projectReferences.Contains(projectReference))
+            if (!CanAddProjectReference(projectReference))
             {
                 return;
             }
@@ -531,6 +534,62 @@ protected void AddProjectReference(ProjectReference projectReference)
             }
         }
 
+        protected bool CanAddProjectReference(ProjectReference projectReference)
+        {
+            if (projectReference.ProjectId == this.Id)
+            {
+                // cannot self reference
+                return false;
+            }
+
+            if (_projectReferences.Contains(projectReference))
+            {
+                // already have this reference
+                return false;
+            }
+
+            var project = this.ProjectTracker.GetProject(projectReference.ProjectId);
+            if (project != null)
+            {
+                // cannot add a reference to a project that references us (it would make a cycle)
+                return !project.TransitivelyReferences(this.Id);
+            }
+
+            return true;
+        }
+
+        private bool TransitivelyReferences(ProjectId projectId)
+        {
+            return TransitivelyReferencesWorker(projectId, new HashSet<ProjectId>());
+        }
+
+        private bool TransitivelyReferencesWorker(ProjectId projectId, HashSet<ProjectId> visited)
+        {
+            visited.Add(this.Id);
+
+            foreach (var pr in this._projectReferences)
+            {
+                if (projectId == pr.ProjectId)
+                {
+                    return true;
+                }
+
+                if (!visited.Contains(pr.ProjectId))
+                {
+                    var project = this.ProjectTracker.GetProject(pr.ProjectId);
+                    if (project != null)
+                    {
+                        if (project.TransitivelyReferencesWorker(projectId, visited))
+                        {
+                            return true;
+                        }
+                    }
+                }
+            }
+
+            return false;
+        }
+
         protected void RemoveProjectReference(ProjectReference projectReference)
         {
             Contract.ThrowIfFalse(_projectReferences.Remove(projectReference));
@@ -803,10 +862,13 @@ internal void TryProjectConversionForIntroducedOutputPath(string binPath, Abstra
                     metadataReference.Properties.Aliases,
                     metadataReference.Properties.EmbedInteropTypes);
 
-                RemoveMetadataReferenceCore(metadataReference, disposeReference: true);
-                AddProjectReference(projectReference);
+                if (CanAddProjectReference(projectReference))
+                {
+                    RemoveMetadataReferenceCore(metadataReference, disposeReference: true);
+                    AddProjectReference(projectReference);
 
-                _metadataFileNameToConvertedProjectReference.Add(binPath, projectReference);
+                    _metadataFileNameToConvertedProjectReference.Add(binPath, projectReference);
+                }
             }
         }
 

src/VisualStudio/Core/Test/ProjectSystemShim/VisualBasicSpecialReferencesTests.vb

@@ -125,5 +125,97 @@ Namespace Microsoft.VisualStudio.LanguageServices.UnitTests.ProjectSystemShim
                 project.Disconnect()
             End Using
         End Sub
+
+        <Fact()>
+        <Trait(Traits.Feature, Traits.Features.ProjectSystemShims)>
+        Public Sub AddingReferenceToProjectMetataPromotesToProjectReference()
+            Using environment = New TestEnvironment()
+
+                Dim project1 = CreateVisualBasicProject(environment, "project1")
+                environment.ProjectTracker.UpdateProjectBinPath(project1, Nothing, "C:\project1.dll")
+
+                Dim project2 = CreateVisualBasicProject(environment, "project2")
+                environment.ProjectTracker.UpdateProjectBinPath(project2, Nothing, "C:\project2.dll")
+
+                ' since this is known to be the output path of project1, the metadata reference is converted to a project reference
+                project2.AddMetaDataReference("c:\project1.dll", True)
+
+                Assert.Equal(True, project2.GetCurrentProjectReferences().Any(Function(pr) pr.ProjectId = project1.Id))
+
+                project2.Disconnect()
+                project1.Disconnect()
+            End Using
+        End Sub
+
+        <Fact()>
+        <Trait(Traits.Feature, Traits.Features.ProjectSystemShims)>
+        Public Sub AddCyclicProjectMetadataReferences()
+            Using environment = New TestEnvironment()
+
+                Dim project1 = CreateVisualBasicProject(environment, "project1")
+                environment.ProjectTracker.UpdateProjectBinPath(project1, Nothing, "C:\project1.dll")
+
+                Dim project2 = CreateVisualBasicProject(environment, "project2")
+                environment.ProjectTracker.UpdateProjectBinPath(project2, Nothing, "C:\project2.dll")
+
+                project1.AddProjectReference(project2)
+
+                ' normally this metadata reference would be elevated to a project reference, but fails because of cyclicness
+                project2.AddMetaDataReference("c:\project1.dll", True)
+
+                Assert.Equal(True, project1.GetCurrentProjectReferences().Any(Function(pr) pr.ProjectId = project2.Id))
+                Assert.Equal(False, project2.GetCurrentProjectReferences().Any(Function(pr) pr.ProjectId = project1.Id))
+
+                project2.Disconnect()
+                project1.Disconnect()
+            End Using
+        End Sub
+
+        <Fact()>
+        <Trait(Traits.Feature, Traits.Features.ProjectSystemShims)>
+        Public Sub AddCyclicProjectReferences()
+            Using environment = New TestEnvironment()
+
+                Dim project1 = CreateVisualBasicProject(environment, "project1")
+                Dim project2 = CreateVisualBasicProject(environment, "project2")
+
+                project1.AddProjectReference(project2)
+                project2.AddProjectReference(project1)
+
+                Assert.Equal(True, project1.GetCurrentProjectReferences().Any(Function(pr) pr.ProjectId = project2.Id))
+                Assert.Equal(False, project2.GetCurrentProjectReferences().Any(Function(pr) pr.ProjectId = project1.Id))
+
+                project2.Disconnect()
+                project1.Disconnect()
+            End Using
+        End Sub
+
+        <Fact()>
+        <Trait(Traits.Feature, Traits.Features.ProjectSystemShims)>
+        Public Sub AddCyclicProjectReferencesDeep()
+            Using environment = New TestEnvironment()
+
+                Dim project1 = CreateVisualBasicProject(environment, "project1")
+                Dim project2 = CreateVisualBasicProject(environment, "project2")
+                Dim project3 = CreateVisualBasicProject(environment, "project3")
+                Dim project4 = CreateVisualBasicProject(environment, "project4")
+
+                project1.AddProjectReference(project2)
+                project2.AddProjectReference(project3)
+                project3.AddProjectReference(project4)
+                project4.AddProjectReference(project1)
+
+                Assert.Equal(True, project1.GetCurrentProjectReferences().Any(Function(pr) pr.ProjectId = project2.Id))
+                Assert.Equal(True, project2.GetCurrentProjectReferences().Any(Function(pr) pr.ProjectId = project3.Id))
+                Assert.Equal(True, project3.GetCurrentProjectReferences().Any(Function(pr) pr.ProjectId = project4.Id))
+                Assert.Equal(False, project4.GetCurrentProjectReferences().Any(Function(pr) pr.ProjectId = project1.Id))
+
+                project4.Disconnect()
+                project3.Disconnect()
+                project2.Disconnect()
+                project1.Disconnect()
+            End Using
+        End Sub
+
     End Class
 End Namespace