Handle C# ref return when converting VB->C#

[TymurGubayev]

Problem

• VB -> C#: incorrect output code if ref return from a C# lib is involved #1067: VB -> C#: incorrect output code if ref return from a C# lib is involved
• Related problem described in this comment: var withBlock = lst[i]; should be ref var withBlock = ref lst[i]; if ref return from a C# lib is involved

Solution

• The changes in code are actually minor: I added checks for prop.ReturnsByRef || prop.ReturnsByRefReadonly in a few places.
  I had to ignore a project in ConvertWholeSolutionAsync (C#->VB) because of the ref return.
• Currently, the MultiFileSolutionAndProjectTests tests will fail because of System.InvalidOperationException: Unexpected value 'PropertyAccess' of type 'Microsoft.CodeAnalysis.VisualBasic.BoundKind' dotnet/roslyn#68194
  This is fixed in Roslyn 4.8, but I haven't touched the references.
• At least one test covering the code changed

[TymurGubayev]

@GrahamTheCoder to make the tests work, we'd need to either upgrade Roslyn to at least 4.8, or comment the with-block tests out.

[GrahamTheCoder]

There's now a prerelease that seems to generally work, so I've pushed that.
The test "OutOptionalArgumentAsync" no longer detects the out parameters correctly in the new version (unrelated to your changes). This means people using that version of VS will see this regression on the current code in master anyway, so I'll skip the test and merge this, and open an issue for that skipped test

[GrahamTheCoder]

Just spotted the project level tests are having an issue too with the new version of roslyn. I'll have to come back to this and the other issue another day now I'm afraid. Ideas/fixes welcome. Would be good to upgrade to latest if possible.

[TymurGubayev]

My findings so far:

• upgrading Microsoft.Build[.Tasks.Core] 17.4.0 -> 17.10.4 but leaving Roslyn at 4.1.0 leaves us with the System.InvalidOperationException : Unexpected value 'PropertyAccess' of type 'Microsoft.CodeAnalysis.VisualBasic.BoundKind'.

• upgrading Roslyn 4.1.0 -> 4.8.0 causes Cannot convert SimpleImportsClauseSyntax, System.InvalidOperationException: Service of type 'Microsoft.CodeAnalysis.Classification.IEmbeddedLanguageClassificationService' is required to accomplish the task but is not available from the workspace. (i.e. the After upgrading Microsoft.CodeAnalysis.CSharp.Workspace from 4.2.0 to 4.3.0 it started throwing at runtime dotnet/roslyn#63921)

• upgrading Roslyn 4.8.0 -> 4.9.2 (and further) causes System.InvalidOperationException : Unable to locate the .NET Framework build host at C:\Users\username\AppData\Local\Temp\d087edd0-b2a6-452c-9149-94c9f59c027b\d087edd0-b2a6-452c-9149-94c9f59c027b\assembly\dl3\9fbdf688\000bbd93_706bda01\BuildHost-net472\Microsoft.CodeAnalysis.Workspaces.MSBuild.BuildHost.exe - line 206

The path it's looking at is Path.Combine(Path.GetDirectoryName(typeof(BuildHostProcessManager).Assembly.Location), "BuildHost-net472", "Microsoft.CodeAnalysis.Workspaces.MSBuild.BuildHost.exe") (from Roslyn's BuildHostProcessManager.cs, GetDotNetFrameworkBuildHostPath())
Everything up to /BuildHost-net472/ exists.

To me this looks like a Roslyn issue, but I have no idea what's going on there.

[TymurGubayev]

I think the problem is in the combination of Xunit and some change in Roslyn.
With Roslyn 4.8.0, the Microsoft.CodeAnalysis.Workspaces.MSBuild.BuildHost.EXE is loaded from %temp%\...guids...\assembly\dl3\<id1>\<id2>\Microsoft.CodeAnalysis.Workspaces.MSBuild.BuildHost.exe, not from a BuildHost-net472/-subfolder.

Also, a similar setup using MSTest works fine.

[GrahamTheCoder]

OK yeah looks like we're stuck. Let's merge this fix with the tests marked as skipped and a comment linking to this issue or directly to the upstream bugs if/when logged upstream.

[TymurGubayev]

I'm not even sure whose issue this one is: Xunit or Roslyn :)

[TymurGubayev]

Btw, I tried following workaround:

private void CopyBuildHost()
{
    Assembly[] loadedAssemblies = AppDomain.CurrentDomain.GetAssemblies();
    Assembly msBuildAssembly = Array.Find(loadedAssemblies, assembly => assembly.FullName.Contains("Microsoft.CodeAnalysis.Workspaces.MSBuild"));

    var BuildHostDirName = "BuildHost-net472";
    if (!Directory.Exists($"./{BuildHostDirName}")) return;

    var targetFolder = Path.Combine(Path.GetDirectoryName(msBuildAssembly.Location), BuildHostDirName);
        
    Directory.CreateDirectory(targetFolder);
    foreach (string file in Directory.GetFiles(BuildHostDirName))
    {
        string fileName = Path.GetFileName(file);
        string destinationPath = Path.Combine(targetFolder, fileName);
        File.Copy(file, destinationPath, false);
    }
}

which got me System.IO.IOException : The pipe has been ended. instead.

[GrahamTheCoder]

I was getting System.IO.IOException : The pipe has been ended. on the latest version of vs dependencies + latest prerelease of codeanalysis the whole time, so maybe something is a bit different on your local setup. e.g. I run the tests through ReSharper's test runner which may have made a difference
I wondered if it just meant the two versions weren't compatible since the last numbers don't match as they usually do in: https://docs.microsoft.com/en-us/visualstudio/extensibility/roslyn-version-support?view=vs-2019
Or maybe I need to install a preview version of VS locally that is a high enough version - though I'm on a pretty recent one I think.