Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly handle <ProjectReferences> that don't produce references #1956

Merged
merged 2 commits into from
Sep 28, 2020
Merged

Correctly handle <ProjectReferences> that don't produce references #1956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7f45a59
Correctly handle <ProjectReferences> that don't produce references
jasonmalinowski Sep 25, 2020
1c487ea
Use ProjectReferenceOriginalItemSpec for getting package reference path
JoeRobich Sep 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/OmniSharp.MSBuild/ProjectFile/MetadataNames.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ProjectReferenceOriginalItemSpec = nameof(ProjectReferenceOriginalItemSpec);
public const string ReferenceSourceTarget = nameof(ReferenceSourceTarget);
public const string Version = nameof(Version);
public const string Aliases = nameof(Aliases);
Expand Down
49 changes: 19 additions & 30 deletions src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.ProjectData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -286,41 +288,32 @@ 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 projectReferenceOriginalItemSpec = referencePathItem.GetMetadataValue(MetadataNames.ProjectReferenceOriginalItemSpec);
if (projectReferenceOriginalItemSpec.EndsWith(".csproj", StringComparison.OrdinalIgnoreCase))
{
var projectReferenceFilePath = Path.GetFullPath(Path.Combine(projectFolderPath, projectReferenceOriginalItemSpec));

projectReferences.Add(projectReferenceFilePath);

if (!string.IsNullOrEmpty(aliases))
{
projectReferenceAliases[projectReferenceFilePath] = aliases;
}

continue;
}
}
Expand All @@ -330,7 +323,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;
Expand Down Expand Up @@ -395,9 +387,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);

Expand Down
4 changes: 2 additions & 2 deletions src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public static (ProjectFileInfo, ImmutableArray<MSBuildDiagnostic>, 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,
Expand All @@ -143,7 +143,7 @@ public static (ProjectFileInfo, ImmutableArray<MSBuildDiagnostic>, 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,
Expand Down
7 changes: 7 additions & 0 deletions test-assets/test-projects/ProjectWithAnalyzersFromReference/Analyzer/Analyzer.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
</PropertyGroup>

</Project>
8 changes: 8 additions & 0 deletions test-assets/test-projects/ProjectWithAnalyzersFromReference/Analyzer/Class1.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
using System;

namespace Analyzer
{
public class Class1
{
}
}
14 changes: 14 additions & 0 deletions .../test-projects/ProjectWithAnalyzersFromReference/ConsumingProject/ConsumingProject.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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>
Comment on lines +9 to +10
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a pattern we're seeing is more common with people writing source generators, since you often want to have the generator in your same solution as the project itself. It's not the best supported because reloading the analyzer is often difficult if it changes on disk.

</ProjectReference>
</ItemGroup>

</Project>
12 changes: 12 additions & 0 deletions test-assets/test-projects/ProjectWithAnalyzersFromReference/ConsumingProject/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
using System;

namespace ConsumingProject.App
{
class Program
{
static void Main(string[] args)
{
Console.WriteLine("Hello World!");
}
}
}
48 changes: 48 additions & 0 deletions ...ets/test-projects/ProjectWithAnalyzersFromReference/ProjectWithAnalyzersFromReference.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
14 changes: 14 additions & 0 deletions tests/OmniSharp.MSBuild.Tests/ProjectFileInfoTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
}