MSBuildWorkspace does not cleanly load VB project on Linux or MacOS

[JoeRobich]

Version Used: 4.10 Preview 1

Reported in #69225

Steps to Reproduce:

1. dotnet new console --language VB
2. use MSBuildWorkspace.OpenProjectAsync to load the newly created VB project

Expected Behavior:
Project loads without any workspace diagnostics and compilation reports no compiler diagnostics.

Actual Behavior:
Compilation reports "BC30002": Type 'Global.Microsoft.VisualBasic.ApplicationServices.ApplicationBase' is not defined.

[paul1956]

@JoeRobich I believe that is part of the WinForms VB Runtime not .Net Core, there have been some requests to move it and other parts of the VB Runtime down to Core since it would enable easier access from C# and allow VB code on Linux and Mac, but I have not seen any movement. There is very little in ApplicationBase (120 lines) but ApplicationBase.Info uses reflection on Windows to access Assembly and if you need that more work would be required.

[arunchndr]

Joey - please move this to the runtime repo when ready.

[JoeRobich]

After doing a little more investigation, this isn't an issue for the Runtime or WinForms teams.

1. The reported diagnostics are located in the "MyTemplateLocation", meaning the compiler provided template.
2. The compiler doesn't report any diagnostics when performing a command line build.
3. The binarylog of the command line build shows the _MyType should be defined as "Empty"
   
4. The MyTemplate should not reference 'Global.Microsoft.VisualBasic.ApplicationServices.ApplicationBase' or any other type when _MyType is "Empty"
5. The MSBuildWorkspace loaded project's ParseOptions show only 2 PreprocessorSymbol and neither are _MyType
6. When _MyType is "" the template treats it as a Windows type which explains why it is referencing these classes that aren't available.

So why are we not getting the proper _MyType?

1. The projectFileInfo.CommandLineArgs loaded from the ProjectFile in MSBuildProjectLoader.Worker shows many defined symbols but some odd character escaping.
2. Checking the json read in RpcClient, the define looks like "/define:\"CONFIG=/\"Debug/\",DEBUG=-1,TRACE=-1,PLATFORM=/\"AnyCPU/\",NET=-1,NET9_0=-1,NETCOREAPP=-1,_MyType=/\"Empty/\",NET5_0_
3. My type is defined but oddly escaped _MyType=/\"Empty/\"
4. The CommandLineArgs come from reading the VbcCommandLineArgs Item from the design time build.
5. Checking the VbcCommandLineArgs Item from a design-time build binlog, _MyList is report /"Empty/"
   
6. Comparing this to the Vbc task's CommandLineArguments and I see the quotes properly escaped.
   
7. This is happening on non-Windows platforms because MSBuild is "fixing up" the ItemSpec (replacing the Windows default path separator '\' with '/') when creating the VbcCommandLineArgs TaskItem.
8. The JsonSerializer is then turns /"Empty/" into /\"Empty/\" when escaping the double quotes.
9. Going back to the MSBuildProjectLoader.Worker and inspecting the parsed commandLineArgs.Errors there is indeed an error that is not surfaced.

error BC31030: Conditional compilation constant 'CONFIG= ^^ ^^ /"Debug/"' is not valid: Syntax error in conditional compilation expression.

Options:

1. We could stop using the *CommandLineArgs Items as we already maintain *CommandLineArgumentReader classes which read build Properties (importantly not Items) to gather the same information.
2. We could treat /" in the CommandLineArgs ItemSpec as an escaped double quote. However, there might be false positives.
3. MSBuild could change the ItemSpec behavior. Likely the riskiest option at this point.

cc: @jasonmalinowski & @rainersigwald for thoughts

[jasonmalinowski]

Is that TaskItem constructor being called here?

roslyn/src/Compilers/Core/MSBuildTask/ManagedToolTask.cs

Line 162 in fac15a8

items[i] = new TaskItem(commandLineArgs[i]);

Because if so, it looks like there's another constructor that doesn't do escaping:

https://github.com/dotnet/msbuild/blob/4c6a5a963ceb38f77af4e57d28669872b616a8dc/src/Utilities/TaskItem.cs#L88-L96

Can we call that one instead?

[JoeRobich]

Can we call that one instead?

Unfortunately it does call the other constructor which does the escaping. https://github.com/dotnet/msbuild/blob/4c6a5a963ceb38f77af4e57d28669872b616a8dc/src/Utilities/TaskItem.cs#L100

We could create out own ITaskItem implementation to wrap the args and pass it to this constructor. https://github.com/dotnet/msbuild/blob/4c6a5a963ceb38f77af4e57d28669872b616a8dc/src/Utilities/TaskItem.cs#L120-L123

Also, thank you for linking me to where this TaskItem was being created on our end.

[jasonmalinowski]

@rainersigwald Any thoughts here? Invoking that copy constructor seems like a possible workaround.

[rainersigwald]

. . . how on Earth is this the first time that unconditional correction has come up? We should definitely have an API to avoid it. (edit: dotnet/msbuild#11083)

I think you're right that the best workaround is a custom ITaskItem--I don't think you'll need to create a TaskItem from it though, you should be able to return your type IIRC.