search

dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
19.11k stars 4.04k forks source link

Should Microsoft.CodeAnalysis.Workspaces.MSBuild be exposing MSBuild references? #75058

Open ericstj opened 2 months ago

ericstj commented 2 months ago

Version Used: 4.11.0

Steps to Reproduce:

  1. Reference latest 4.11.0
  2. Build
  3. Observe warnings and over 33 assemblies in the output including private copies of most of MSBuild assemblies.

Diagnostic Id:

 warning NU1903: Package 'System.Formats.Asn1' 7.0.0 has a known high severity vulnerability, https://github.com/advisories/GHSA-447r-wph3-92pm
 warning NU1903: Package 'System.Text.Json' 8.0.0 has a known high severity vulnerability, https://github.com/advisories/GHSA-hh2w-p6rv-4g7w

Expected Behavior: Do we really need all these assemblies exposed to the consuming project? I thought the purpose of this component was to avoid exposing MSBuild details to the caller and instead load from out-of-proc using a host with MSBuildLocator.

I think we should have minimal depdencies here. Seems all but Microsoft.Build.Framework could be made PrivateAssets

Actual Behavior: All of MSBuild including it's runtime dependencies are exposed to consumers.

I did a quick and dirty experiment where I ran GenAPI on Microsoft.CodeAnalysis.Workspaces.MSBuild to see what of MSBuild it actually exposes to users. It seems the only type is Microsoft.Build.Framework.ILogger. Here's that prototype: wsRefProto.zip

I also disassembled Microsoft.CodeAnalysis.Workspaces.MSBuild to see what types it was actually using in the runtime implementation. Seems it's only using:

So you can at the very least remove your references to https://github.com/dotnet/roslyn/blob/7b75058bf667721fb985519fe800ffe44580457a/src/Workspaces/Core/MSBuild/Microsoft.CodeAnalysis.Workspaces.MSBuild.csproj#L26

If you made proxy types for those and loaded through reflection (from wherever the locator found them) you could also remove the Microsoft.Build dependency.

If you made an API breaking change, you could replace the ILogger dependency with a wrapper and remove the Microsoft.Build.Framework dependency.

dotnet-issue-labeler[bot] commented 2 months ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

dotnet-issue-labeler[bot] commented 2 months ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.