From 7f45a59ea4bab5ba9608fea22ac9aeb22dd851e5 Mon Sep 17 00:00:00 2001 From: Jason Malinowski Date: Fri, 25 Sep 2020 14:43:25 -0700 Subject: [PATCH 1/2] 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 | 43 ++++++----------- .../Analyzer/Analyzer.csproj | 7 +++ .../Analyzer/Class1.cs | 8 ++++ .../ConsumingProject/ConsumingProject.csproj | 14 ++++++ .../ConsumingProject/Program.cs | 12 +++++ .../ProjectWithAnalyzersFromReference.sln | 48 +++++++++++++++++++ .../ProjectFileInfoTests.cs | 14 ++++++ 8 files changed, 118 insertions(+), 30 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..721e569339 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; @@ -395,9 +383,6 @@ private static RuleSet ResolveRulesetIfAny(MSB.Execution.ProjectInstance project return null; } - private static bool IsCSharpProject(string filePath) - => filePath.EndsWith(".csproj", StringComparison.OrdinalIgnoreCase); - private static bool FileNameIsNotGenerated(string filePath) => !Path.GetFileName(filePath).StartsWith("TemporaryGeneratedFile_", StringComparison.OrdinalIgnoreCase); 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..074d17f5d9 --- /dev/null +++ b/test-assets/test-projects/ProjectWithAnalyzersFromReference/Analyzer/Class1.cs @@ -0,0 +1,8 @@ +using System; + +namespace Analyzer +{ + 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..829e243c17 --- /dev/null +++ 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/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); + } + } } } From 1c487eacf7a4e0cd7db3832e2e4961bed5f692a1 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Sat, 26 Sep 2020 17:54:35 -0700 Subject: [PATCH 2/2] Use ProjectReferenceOriginalItemSpec for getting package reference path --- src/OmniSharp.MSBuild/ProjectFile/MetadataNames.cs | 2 +- .../ProjectFile/ProjectFileInfo.ProjectData.cs | 14 +++++++++----- .../ProjectFile/ProjectFileInfo.cs | 4 ++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/OmniSharp.MSBuild/ProjectFile/MetadataNames.cs b/src/OmniSharp.MSBuild/ProjectFile/MetadataNames.cs index 3f50d0dedb..70ce5eeebf 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 MSBuildSourceProjectFile = nameof(MSBuildSourceProjectFile); + public const string ProjectReferenceOriginalItemSpec = nameof(ProjectReferenceOriginalItemSpec); 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 721e569339..bfa3d0031d 100644 --- a/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.ProjectData.cs +++ b/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.ProjectData.cs @@ -240,8 +240,10 @@ public static ProjectData Create(MSB.Evaluation.Project project) documentationFile, preprocessorSymbolNames, suppressedDiagnosticIds, warningsAsErrors, warningsNotAsErrors, signAssembly, assemblyOriginatorKeyFile, treatWarningsAsErrors, defaultNamespace, runAnalyzers, runAnalyzersDuringLiveAnalysis, ruleset: null); } - public static ProjectData Create(MSB.Execution.ProjectInstance projectInstance, MSB.Evaluation.Project project) + public static ProjectData Create(string projectFilePath, MSB.Execution.ProjectInstance projectInstance, MSB.Evaluation.Project project) { + var projectFolderPath = Path.GetDirectoryName(projectFilePath); + var guid = PropertyConverter.ToGuid(projectInstance.GetPropertyValue(PropertyNames.ProjectGuid)); var name = projectInstance.GetPropertyValue(PropertyNames.ProjectName); var assemblyName = projectInstance.GetPropertyValue(PropertyNames.AssemblyName); @@ -300,14 +302,16 @@ public static ProjectData Create(MSB.Execution.ProjectInstance projectInstance, // MSBuild logic is adding or removing properties too. if (StringComparer.OrdinalIgnoreCase.Equals(referenceSourceTarget, ItemNames.ProjectReference)) { - var sourceProjectFile = referencePathItem.GetMetadataValue(MetadataNames.MSBuildSourceProjectFile); - if (sourceProjectFile.EndsWith(".csproj", StringComparison.OrdinalIgnoreCase)) + var projectReferenceOriginalItemSpec = referencePathItem.GetMetadataValue(MetadataNames.ProjectReferenceOriginalItemSpec); + if (projectReferenceOriginalItemSpec.EndsWith(".csproj", StringComparison.OrdinalIgnoreCase)) { - projectReferences.Add(sourceProjectFile); + var projectReferenceFilePath = Path.GetFullPath(Path.Combine(projectFolderPath, projectReferenceOriginalItemSpec)); + + projectReferences.Add(projectReferenceFilePath); if (!string.IsNullOrEmpty(aliases)) { - projectReferenceAliases[sourceProjectFile] = aliases; + projectReferenceAliases[projectReferenceFilePath] = aliases; } continue; diff --git a/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.cs b/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.cs index 6c52b5a9da..3131bf161a 100644 --- a/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.cs +++ b/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.cs @@ -119,7 +119,7 @@ public static (ProjectFileInfo, ImmutableArray, ProjectLoaded return (null, diagnostics, null); } - var data = ProjectData.Create(projectInstance, project); + var data = ProjectData.Create(filePath, projectInstance, project); var projectFileInfo = new ProjectFileInfo(projectIdInfo, filePath, data, sessionId, dotNetInfo); var eventArgs = new ProjectLoadedEventArgs(projectIdInfo.Id, project, @@ -143,7 +143,7 @@ public static (ProjectFileInfo, ImmutableArray, ProjectLoaded return (null, diagnostics, null); } - var data = ProjectData.Create(projectInstance, project); + var data = ProjectData.Create(FilePath, projectInstance, project); var projectFileInfo = new ProjectFileInfo(ProjectIdInfo, FilePath, data, SessionId, DotNetInfo); var eventArgs = new ProjectLoadedEventArgs(Id, project,