Correctly handle <ProjectReferences> that don't produce references

<ProjectReference> elements can have metadata that specify what type of item to create, or that they shouldn't create items at all. The existing code that was processing references wasn't checking for this metadata, so you could end up with project references between projects that shouldn't be there. The approach to take here is to never look at the <ProjectReference> items directly, rather only look at the ReferencePath items that are produced, and if we see metadata that indicates it came from a project, convert it to a project reference at that point.
OmniSharp · Sep 25, 2020 · 198ace8 · 198ace8
1 parent ed3d417
commit 198ace8
Show file tree

Hide file tree

Showing 8 changed files with 118 additions and 27 deletions.
diff --git a/src/OmniSharp.MSBuild/ProjectFile/MetadataNames.cs b/src/OmniSharp.MSBuild/ProjectFile/MetadataNames.cs
@@ -5,7 +5,7 @@ internal static class MetadataNames
         public const string FullPath = nameof(FullPath);
         public const string IsImplicitlyDefined = nameof(IsImplicitlyDefined);
         public const string Project = nameof(Project);
-        public const string OriginalItemSpec = nameof(OriginalItemSpec);
+        public const string MSBuildSourceProjectFile = nameof(MSBuildSourceProjectFile);
         public const string ReferenceSourceTarget = nameof(ReferenceSourceTarget);
         public const string Version = nameof(Version);
         public const string Aliases = nameof(Aliases);

diff --git a/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.ProjectData.cs b/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.ProjectData.cs
@@ -286,41 +286,30 @@ public static ProjectData Create(MSB.Execution.ProjectInstance projectInstance,
 
                 var projectReferences = ImmutableArray.CreateBuilder<string>();
                 var projectReferenceAliases = ImmutableDictionary.CreateBuilder<string, string>();
-                var projectReferencesAdded = new HashSet<string>();
-                foreach (var projectReferenceItem in projectInstance.GetItems(ItemNames.ProjectReference))
-                {
-                    var fullPath = projectReferenceItem.GetMetadataValue(MetadataNames.FullPath);
-
-                    if (IsCSharpProject(fullPath) && projectReferencesAdded.Add(fullPath))
-                    {
-                        projectReferences.Add(fullPath);
-
-                        var aliases = projectReferenceItem.GetMetadataValue(MetadataNames.Aliases);
-                        if (!string.IsNullOrEmpty(aliases))
-                        {
-                            projectReferenceAliases[fullPath] = aliases;
-                        }
-                    }
-                }
 
                 var references = ImmutableArray.CreateBuilder<string>();
                 var referenceAliases = ImmutableDictionary.CreateBuilder<string, string>();
                 foreach (var referencePathItem in projectInstance.GetItems(ItemNames.ReferencePath))
                 {
                     var referenceSourceTarget = referencePathItem.GetMetadataValue(MetadataNames.ReferenceSourceTarget);
+                    var aliases = referencePathItem.GetMetadataValue(MetadataNames.Aliases);
 
+                    // If this reference came from a project reference, count it as such. We never want to directly look
+                    // at the ProjectReference items in the project, as those don't always create project references
+                    // if things like OutputItemType or ReferenceOutputAssembly are set. It's also possible that other
+                    // MSBuild logic is adding or removing properties too.
                     if (StringComparer.OrdinalIgnoreCase.Equals(referenceSourceTarget, ItemNames.ProjectReference))
                     {
-                        // If the reference was sourced from a project reference, we have two choices:
-                        //
-                        //   1. If the reference is a C# project reference, we shouldn't add it because it'll just duplicate
-                        //      the project reference.
-                        //   2. If the reference is *not* a C# project reference, we should keep this reference because the
-                        //      project reference was already removed.
-
-                        var originalItemSpec = referencePathItem.GetMetadataValue(MetadataNames.OriginalItemSpec);
-                        if (originalItemSpec.EndsWith(".csproj", StringComparison.OrdinalIgnoreCase))
+                        var sourceProjectFile = referencePathItem.GetMetadataValue(MetadataNames.MSBuildSourceProjectFile);
+                        if (sourceProjectFile.EndsWith(".csproj", StringComparison.OrdinalIgnoreCase))
                         {
+                            projectReferences.Add(sourceProjectFile);
+
+                            if (!string.IsNullOrEmpty(aliases))
+                            {
+                                projectReferenceAliases[sourceProjectFile] = aliases;
+                            }
+
                             continue;
                         }
                     }
@@ -330,7 +319,6 @@ public static ProjectData Create(MSB.Execution.ProjectInstance projectInstance,
                     {
                         references.Add(fullPath);
 
-                        var aliases = referencePathItem.GetMetadataValue(MetadataNames.Aliases);
                         if (!string.IsNullOrEmpty(aliases))
                         {
                             referenceAliases[fullPath] = aliases;

diff --git a/test-assets/test-projects/ProjectWithAnalyzersFromReference/Analyzer/Analyzer.csproj b/test-assets/test-projects/ProjectWithAnalyzersFromReference/Analyzer/Analyzer.csproj
@@ -0,0 +1,7 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFramework>netstandard2.0</TargetFramework>
+  </PropertyGroup>
+
+</Project>
diff --git a/test-assets/test-projects/ProjectWithAnalyzersFromReference/Analyzer/Class1.cs b/test-assets/test-projects/ProjectWithAnalyzersFromReference/Analyzer/Class1.cs
@@ -0,0 +1,8 @@
+using System;
+
+namespace Analyzer
+{
+    public class Class1
+    {
+    }
+}
diff --git a/.../test-projects/ProjectWithAnalyzersFromReference/ConsumingProject/ConsumingProject.csproj b/.../test-projects/ProjectWithAnalyzersFromReference/ConsumingProject/ConsumingProject.csproj
@@ -0,0 +1,14 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <TargetFramework>netcoreapp2.1</TargetFramework>
+  </PropertyGroup>
+  <ItemGroup>
+    <ProjectReference Include="..\Analyzer\Analyzer.csproj">
+      <OutputItemType>Analyzer</OutputItemType>
+      <ReferenceOutputAssembly>false</ReferenceOutputAssembly>
+    </ProjectReference>
+  </ItemGroup>
+
+</Project>
diff --git a/test-assets/test-projects/ProjectWithAnalyzersFromReference/ConsumingProject/Program.cs b/test-assets/test-projects/ProjectWithAnalyzersFromReference/ConsumingProject/Program.cs
@@ -0,0 +1,12 @@
+using System;
+
+namespace ConsumingProject.App
+{
+    class Program
+    {
+        static void Main(string[] args)
+        {
+            Console.WriteLine("Hello World!");
+        }
+    }
+}
diff --git a/...ets/test-projects/ProjectWithAnalyzersFromReference/ProjectWithAnalyzersFromReference.sln b/...ets/test-projects/ProjectWithAnalyzersFromReference/ProjectWithAnalyzersFromReference.sln
@@ -0,0 +1,48 @@
+
+Microsoft Visual Studio Solution File, Format Version 12.00
+# Visual Studio 15
+VisualStudioVersion = 15.0.26124.0
+MinimumVisualStudioVersion = 15.0.26124.0
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Analyzer", "Analyzer\Analyzer.csproj", "{447E6D15-63B0-47F3-9E44-D6F0D0087C46}"
+EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ConsumingProject", "ConsumingProject\ConsumingProject.csproj", "{ECC29DFA-3AC0-474E-9C5C-C8AD4D3DD17D}"
+EndProject
+Global
+	GlobalSection(SolutionConfigurationPlatforms) = preSolution
+		Debug|Any CPU = Debug|Any CPU
+		Debug|x64 = Debug|x64
+		Debug|x86 = Debug|x86
+		Release|Any CPU = Release|Any CPU
+		Release|x64 = Release|x64
+		Release|x86 = Release|x86
+	EndGlobalSection
+	GlobalSection(SolutionProperties) = preSolution
+		HideSolutionNode = FALSE
+	EndGlobalSection
+	GlobalSection(ProjectConfigurationPlatforms) = postSolution
+		{447E6D15-63B0-47F3-9E44-D6F0D0087C46}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{447E6D15-63B0-47F3-9E44-D6F0D0087C46}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{447E6D15-63B0-47F3-9E44-D6F0D0087C46}.Debug|x64.ActiveCfg = Debug|Any CPU
+		{447E6D15-63B0-47F3-9E44-D6F0D0087C46}.Debug|x64.Build.0 = Debug|Any CPU
+		{447E6D15-63B0-47F3-9E44-D6F0D0087C46}.Debug|x86.ActiveCfg = Debug|Any CPU
+		{447E6D15-63B0-47F3-9E44-D6F0D0087C46}.Debug|x86.Build.0 = Debug|Any CPU
+		{447E6D15-63B0-47F3-9E44-D6F0D0087C46}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{447E6D15-63B0-47F3-9E44-D6F0D0087C46}.Release|Any CPU.Build.0 = Release|Any CPU
+		{447E6D15-63B0-47F3-9E44-D6F0D0087C46}.Release|x64.ActiveCfg = Release|Any CPU
+		{447E6D15-63B0-47F3-9E44-D6F0D0087C46}.Release|x64.Build.0 = Release|Any CPU
+		{447E6D15-63B0-47F3-9E44-D6F0D0087C46}.Release|x86.ActiveCfg = Release|Any CPU
+		{447E6D15-63B0-47F3-9E44-D6F0D0087C46}.Release|x86.Build.0 = Release|Any CPU
+		{ECC29DFA-3AC0-474E-9C5C-C8AD4D3DD17D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{ECC29DFA-3AC0-474E-9C5C-C8AD4D3DD17D}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{ECC29DFA-3AC0-474E-9C5C-C8AD4D3DD17D}.Debug|x64.ActiveCfg = Debug|Any CPU
+		{ECC29DFA-3AC0-474E-9C5C-C8AD4D3DD17D}.Debug|x64.Build.0 = Debug|Any CPU
+		{ECC29DFA-3AC0-474E-9C5C-C8AD4D3DD17D}.Debug|x86.ActiveCfg = Debug|Any CPU
+		{ECC29DFA-3AC0-474E-9C5C-C8AD4D3DD17D}.Debug|x86.Build.0 = Debug|Any CPU
+		{ECC29DFA-3AC0-474E-9C5C-C8AD4D3DD17D}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{ECC29DFA-3AC0-474E-9C5C-C8AD4D3DD17D}.Release|Any CPU.Build.0 = Release|Any CPU
+		{ECC29DFA-3AC0-474E-9C5C-C8AD4D3DD17D}.Release|x64.ActiveCfg = Release|Any CPU
+		{ECC29DFA-3AC0-474E-9C5C-C8AD4D3DD17D}.Release|x64.Build.0 = Release|Any CPU
+		{ECC29DFA-3AC0-474E-9C5C-C8AD4D3DD17D}.Release|x86.ActiveCfg = Release|Any CPU
+		{ECC29DFA-3AC0-474E-9C5C-C8AD4D3DD17D}.Release|x86.Build.0 = Release|Any CPU
+	EndGlobalSection
+EndGlobal
diff --git a/tests/OmniSharp.MSBuild.Tests/ProjectFileInfoTests.cs b/tests/OmniSharp.MSBuild.Tests/ProjectFileInfoTests.cs
@@ -229,5 +229,19 @@ public async Task WarningsAsErrors()
                 Assert.Equal(ReportDiagnostic.Suppress, compilationOptions.SpecificDiagnosticOptions["CS7081"]);
             }
         }
+
+        [Fact]
+        public async Task ProjectReferenceProducingAnalyzerItems()
+        {
+            using (var host = CreateOmniSharpHost())
+            using (var testProject = await _testAssets.GetTestProjectAsync("ProjectWithAnalyzersFromReference"))
+            {
+                var projectFilePath = Path.Combine(testProject.Directory, "ConsumingProject", "ConsumingProject.csproj");
+                var projectFileInfo = CreateProjectFileInfo(host, testProject, projectFilePath);
+                Assert.Empty(projectFileInfo.ProjectReferences);
+                var analyzerFileReference = Assert.Single(projectFileInfo.Analyzers);
+                Assert.EndsWith("Analyzer.dll", analyzerFileReference);
+            }
+        }
     }
 }