search

phmonte / Buildalyzer

A utility to perform design-time builds of .NET projects without having to think too hard about it.
MIT License
611 stars 94 forks source link

Support not referencing VB #168

Closed svick closed 3 years ago

svick commented 3 years ago

I want to use Buildalyzer.Workspaces, but without referencing Microsoft.CodeAnalysis.VisualBasic.Workspaces (because I have a custom build of Microsoft.CodeAnalysis.CSharp.Worspaces). When I try to do that, I get the following exception:

System.IO.FileNotFoundException: Could not load file or assembly 'Microsoft.CodeAnalysis.VisualBasic, Version=3.8.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35'. The system cannot find the file specified. File name: 'Microsoft.CodeAnalysis.VisualBasic, Version=3.8.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' at Buildalyzer.Workspaces.AnalyzerResultExtensions.CreateParseOptions(IAnalyzerResult analyzerResult, String languageName) at Buildalyzer.Workspaces.AnalyzerResultExtensions.GetProjectInfo(IAnalyzerResult analyzerResult, Workspace workspace, ProjectId projectId) at Buildalyzer.Workspaces.AnalyzerResultExtensions.AddToWorkspace(IAnalyzerResult analyzerResult, Workspace workspace, Boolean addProjectReferences) at Buildalyzer.Workspaces.AnalyzerResultExtensions.GetWorkspace(IAnalyzerResult analyzerResult, Boolean addProjectReferences)

This happens because of the following code in CreateCompilationOptions (and similar, but longer code in CreateParseOptions):

https://github.com/daveaglick/Buildalyzer/blob/da88770a2952f7894c05cd0866ef9d766f78aaea/src/Buildalyzer.Workspaces/AnalyzerResultExtensions.cs#L192-L200

This means that JIT compiling the method requires a reference to Microsoft.CodeAnalysis.VisualBasic, even if I only ever want to create C# options.

Extracting the language-specific code into a local function seems to fix this issue.

daveaglick commented 3 years ago

What an interesting side effect of local functions! I guess because they're "lifted" into separate methods? In any case, I've hit the merge button and am planning a release within the next day or two once I clear the remaining PR backlog. Thanks for the contribution, and sorry for the delay!

svick commented 3 years ago

I guess because they're "lifted" into separate methods?

Yes, exactly. Using regular methods would achieve the same effect, but I thought local functions were more appropriate here.