From c2da70533bb534b8aae979ea5ddb12f8cd994f1e Mon Sep 17 00:00:00 2001 From: Jason Malinowski Date: Fri, 25 Sep 2020 14:43:25 -0700 Subject: [PATCH] Correctly handle that don't produce references 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 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. --- .../ProjectFile/MetadataNames.cs | 2 +- .../ProjectFileInfo.ProjectData.cs | 40 ++++++---------- .../Analyzer/Analyzer.csproj | 7 +++ .../Analyzer/Class1.cs | 8 ++++ .../ConsumingProject/ConsumingProject.csproj | 14 ++++++ .../ConsumingProject/Program.cs | 16 +++++++ .../ProjectWithAnalyzersFromReference.sln | 48 +++++++++++++++++++ .../ProjectFileInfoTests.cs | 14 ++++++ 8 files changed, 122 insertions(+), 27 deletions(-) create mode 100644 test-assets/test-projects/ProjectWithAnalyzersFromReference/Analyzer/Analyzer.csproj create mode 100644 test-assets/test-projects/ProjectWithAnalyzersFromReference/Analyzer/Class1.cs create mode 100644 test-assets/test-projects/ProjectWithAnalyzersFromReference/ConsumingProject/ConsumingProject.csproj create mode 100644 test-assets/test-projects/ProjectWithAnalyzersFromReference/ConsumingProject/Program.cs create mode 100644 test-assets/test-projects/ProjectWithAnalyzersFromReference/ProjectWithAnalyzersFromReference.sln diff --git a/src/OmniSharp.MSBuild/ProjectFile/MetadataNames.cs b/src/OmniSharp.MSBuild/ProjectFile/MetadataNames.cs index 56e54025e4..3f50d0dedb 100644 --- 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 index 92f4b7b19a..9fa7906ea1 100644 --- 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(); var projectReferenceAliases = ImmutableDictionary.CreateBuilder(); - var projectReferencesAdded = new HashSet(); - 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(); var referenceAliases = ImmutableDictionary.CreateBuilder(); 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 new file mode 100644 index 0000000000..9f5c4f4abb --- /dev/null +++ b/test-assets/test-projects/ProjectWithAnalyzersFromReference/Analyzer/Analyzer.csproj @@ -0,0 +1,7 @@ + + + + netstandard2.0 + + + diff --git a/test-assets/test-projects/ProjectWithAnalyzersFromReference/Analyzer/Class1.cs b/test-assets/test-projects/ProjectWithAnalyzersFromReference/Analyzer/Class1.cs new file mode 100644 index 0000000000..7b90a3ec53 --- /dev/null +++ b/test-assets/test-projects/ProjectWithAnalyzersFromReference/Analyzer/Class1.cs @@ -0,0 +1,8 @@ +using System; + +namespace ExternAlias.Lib +{ + public class Class1 + { + } +} diff --git a/test-assets/test-projects/ProjectWithAnalyzersFromReference/ConsumingProject/ConsumingProject.csproj b/test-assets/test-projects/ProjectWithAnalyzersFromReference/ConsumingProject/ConsumingProject.csproj new file mode 100644 index 0000000000..a1d4ab93db --- /dev/null +++ b/test-assets/test-projects/ProjectWithAnalyzersFromReference/ConsumingProject/ConsumingProject.csproj @@ -0,0 +1,14 @@ + + + + Exe + netcoreapp2.1 + + + + Analyzer + false + + + + diff --git a/test-assets/test-projects/ProjectWithAnalyzersFromReference/ConsumingProject/Program.cs b/test-assets/test-projects/ProjectWithAnalyzersFromReference/ConsumingProject/Program.cs new file mode 100644 index 0000000000..c75f57a9c5 --- /dev/null +++ b/test-assets/test-projects/ProjectWithAnalyzersFromReference/ConsumingProject/Program.cs @@ -0,0 +1,16 @@ +extern alias abc; +extern alias def; +using System; + +namespace ExternAlias.App +{ + class Program + { + static void Main(string[] args) + { + new abc::ExternAlias.Lib.Class1(); + new def::ExternAlias.Lib.Class1(); + Console.WriteLine("Hello World!"); + } + } +} diff --git a/test-assets/test-projects/ProjectWithAnalyzersFromReference/ProjectWithAnalyzersFromReference.sln b/test-assets/test-projects/ProjectWithAnalyzersFromReference/ProjectWithAnalyzersFromReference.sln new file mode 100644 index 0000000000..016d1c4a7e --- /dev/null +++ b/test-assets/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 index d9cd52f13e..7c3aa13d12 100644 --- 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); + } + } } }