Invoke `dotnet run-api` to obtain virtual project #78648

RikkiGibson · 2025-05-19T22:01:26Z

Turn VirtualCSharpFileBasedProgramProject into a MEF component, which takes in a DotnetCliHelper.
Add a method MakeVirtualProjectContentAsync to shell out to dotnet run-api, do JSON [de]serialization, etc.
Run-api RPC testing is done by setting up a temp folder with a global.json demanding a quite recent nightly SDK. For now, these tests are skipped, except when the compilation symbol 'RoslynTestRunApi' is defined. This allows for testing in the local environment when the right SDK is available, while not breaking CI.

Out of date discussion

Question: how to do testing?

dotnet run-api is only available in nightly .NET 10 SDKs at the moment. So if we just shelled out to the SDK in the repo-level global.json, we wouldn't be able to use the API.

Is it possible for the LanguageServer test project to take a package reference to a nightly version of the dotnet CLI or some such? Then we could just introduce a test suite which sets up the proper DotnetCliHelper and proceeds with testing against the SDK which will have dotnet run-api available. Is there any such package that we could use @MiYanni?

If no "package based" solution exists, maybe we could add some kind of test fixture, build task, etc. which downloads the SDKs that we need for such tests and stashes them in some appropriate location?

Ideally, we could even allow testing against an old SDK version in different tests in the same project, so that we can verify what happens when dotnet run-api is not available in the user's SDK.

There might be analogous problems for MSBuildHost (testing how we handle behaviors of one or another specific MSBuild version). Tagging @jasonmalinowski in case you have any thoughts/suggestions.

jasonmalinowski · 2025-05-19T22:20:48Z

Ideally, we could even allow testing against an old SDK version in different tests in the same project, so that we can verify what happens when dotnet run-api is not available in the user's SDK.

Once we have a usable run-api in a shipped SDK (even if it's just preview), I'm OK telling users they have to move their SDK ahead to that version. Once things stabilize a bit more I'd say we have a longer support guarantee, but if users are trying to adopt this when it's really, really new, they can be on newer bits. That's fine.

jasonmalinowski · 2025-05-19T22:21:33Z

Paging @dibarbet for the testing question. I think we already have tests that download different SDK versions for testing; this would seem to be a completely reasonable case to include.

dibarbet · 2025-05-19T22:46:18Z

Paging @dibarbet for the testing question. I think we already have tests that download different SDK versions for testing; this would seem to be a completely reasonable case to include.

only on the vscode-csharp side, nothing in roslyn (afaik). there we use docker images to ensure we get an image with only the desired SDK on it. If there is one for .net 10, we can use that. See https://github.com/dotnet/vscode-csharp/blob/main/azure-pipelines.yml#L76

RikkiGibson · 2025-05-19T22:58:14Z

It looks like the docker solution probably only works for CI?

I am going to try and make progress by writing some functional tests which basically just dig into the local machine to find a particular new enough SDK. Before the PR is merged we will want to figure out a longer term solution.

I def think we don't need exhaustive testing for various versions of the SDK and various levels of support, but, I would like to verify that there is some kind of graceful fallback when too old of an SDK is used.

...uageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/VirtualProjectXmlProviderTests.cs

...eServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlProvider.cs

RikkiGibson · 2025-05-21T01:01:32Z

...Server/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/FileBasedProgramsProjectSystem.cs

@@ -30,10 +31,12 @@ internal sealed class FileBasedProgramsProjectSystem : LanguageServerProjectLoad
    private readonly ILspServices _lspServices;
    private readonly ILogger<FileBasedProgramsProjectSystem> _logger;
    private readonly IMetadataAsSourceFileService _metadataAsSourceFileService;
+    private readonly VirtualProjectXmlProvider _projectXmlProvider;


At this point I am thinking just placing this stuff in a FileBasedPrograms namespace and thinking of some distinctive names will be better. Naming this VirtualFileBasedProgramCSharpProjectProvider or some such.. It is so much to scan past, and it makes keeping track of the code elements harder, in my opinion. It feels important to be able to distinguish this easily from a "project system" and "project factory" and so on. There are many code elements in these layers which are all called "project" yet which are very different things. So I like keeping the word "virtual", it's used in reference to projects in this one place.

...Server/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/FileBasedProgramsProjectSystem.cs

...osoft.CodeAnalysis.LanguageServer/HostWorkspace/FileBasedProgramsWorkspaceProviderFactory.cs

MiYanni · 2025-05-21T18:50:46Z

@RikkiGibson

Is it possible for the LanguageServer test project to take a package reference to a nightly version of the dotnet CLI or some such? Then we could just introduce a test suite which sets up the proper DotnetCliHelper and proceeds with testing against the SDK which will have dotnet run-api available. Is there any such package that we could use @MiYanni?

There's not a mechanism from the SDK repo to take a dependency on a nightly build. Maybe the VMR has a way to do that?

RikkiGibson · 2025-05-21T18:55:23Z

Maybe the tests which exercise usage of run-api need to be "local run only" for the moment. Eventually we will get a new enough SDK at the repo level which can be used to run the tests in CI.

dibarbet · 2025-05-23T21:40:58Z

...uageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/VirtualProjectXmlProviderTests.cs

+        await globalJsonFile.WriteAllTextAsync("""
+            {
+              "sdk": {
+                "version": "10.0.100-preview.5.25265.12"


these tests should fail in CI (and look like they are). We would need to figure out some way of ensuring the preview SDK is available in helix machines.

I opted to keep things moving forward by just skipping the tests requiring preview SDK in CI.

dibarbet · 2025-05-23T21:47:17Z

...eServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlProvider.cs

+    internal async Task<(string VirtualProjectXml, ImmutableArray<SimpleDiagnostic> Diagnostics)?> GetVirtualProjectContentAsync(string documentFilePath, CancellationToken cancellationToken)
+    {
+        var workingDirectory = Path.GetDirectoryName(documentFilePath);
+        var process = dotnetCliHelper.Run(["run-api"], workingDirectory, shouldLocalizeOutput: true, redirectStandardInput: true);


we talked about this offline, but re-iterating here. It isn't ideal that we have to shell out to the CLI to retrieve this. Some problems I can think of that we may encounter

Regress startup perf (files are in misc to begin with, so we're going to incur an additional process startup cost every launch). We are currently trying to eliminate as many as possible (e.g. from acquiring runtime).

PATH / SDK lookup issues. VSCode doesn't always inherit the right path, depending on exactly how it is launched. MSBuildLocator already has fairly complicated resolution logic which we do not replicate here. This will likely fail in cases where msbuildlocator succeeds.

A better solution may be to have a static project that uses msbuild variables for TFM/project properties which is imported from some known file in the SDK. Then it just uses all the existing code (/msbuild locator) to load the project.

Not blocking this PR on this though.

I def understand the concerns about having to start up a new subprocess.

I wasn't sure what you meant by static project. Does that essentially mean have IDE pass in a project like the following:

<Project> <Import Project="FileBasedPrograms.props" />  <PropertyGroup> <TargetFramework>$(FileBasedProgramsTargetFramework)</TargetFramework> </PropertyGroup> <ItemGroup>  <PackageReference Include="@(FileBasedProgramsPackageReferences)" /> </ItemGroup> </Project>

...etc? So that when the design-time build runs, some task simply runs which sets up all the props and items it needs to use, reducing the number of process spawns, JIT warmups etc.

yup - for the references, maybe we could even pass them in as a property to the design time build if you didnt want a an msbuild task?

I chatted with @dibarbet on this a bit yesterday. We don't want the IDE being in the situation of having to figure out all the properties directly, especially as the directives we support change/expand over time. That said, if we can get away with having a build task figure out the majority of it, then maybe we could pass just a entrypoint file as a property and a majority of stuff could be figured out?

The other idea was we do go back to the idea of there being an API we call, but we'd load that API from the SDK. But that creates some risk since we have to deal with supporting many old versions, and how we isolate dependencies, etc.

dibarbet · 2025-05-23T21:49:09Z

...eServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlProvider.cs

+        await process.StandardInput.WriteAsync(inputJson);
+        process.StandardInput.Close();
+
+        process.ErrorDataReceived += (sender, args) => _logger.LogDebug($"dotnet run-api: {args.Data}");


should this be debug logging? If we think it will be common to have stderr output then fine with debug logging (as long as we indicate a failure elsewhere at a info/warn/error). Otherwise if this isn't expected, we definitely want warn/error logs.

Actually, it probably will be common, simply when user uses too old of an SDK. I will do some manual testing and adjust accordingly.

I played with using this with an old SDK, and, I think I feel good about using debug severity for this. Essentially user's SDK being too old doesn't mean that anything is wrong, it just means we won't light up this new experience. I adjusted things to try to handle that scenario gracefully.

...eServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlProvider.cs

dibarbet · 2025-05-23T21:51:53Z

...eServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlProvider.cs

+            return null;
+        }
+
+        var response = JsonSerializer.Deserialize(responseJson, RunFileApiJsonSerializerContext.Default.RunApiOutput);


do we want to catch errors and log here? Looks like it is currently throwing in the tests because it has some other error output.

Perhaps if the process returns a non-zero exit code we shouldn't even try to deserialize?

...eServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlProvider.cs

…asedPrograms/VirtualProjectXmlProvider.cs Co-authored-by: David Barbet <[email protected]>

* upstream/main: (162 commits) Invoke `dotnet run-api` to obtain virtual project (dotnet#78648) [main] Update dependencies from dotnet/arcade (dotnet#78578) [main] Source code updates from dotnet/dotnet (dotnet#78724) Do not convert tuple expression with error to a bad expression too early (dotnet#78645) Updat epersistence version Add helper for mapping between documents and hierarchy+itemids Extensions: nullability analysis for property access (dotnet#78646) Add IsNull property to IAsyncToken (dotnet#78718) Update MicrosoftVisualStudioExtensibilityTestingVersion (dotnet#78661) Always log PID avoid not needed compilationUnit clone (dotnet#78676) Fix Make operator glyph consistent witht eh rest of the members [main] Source code updates from dotnet/dotnet (dotnet#78710) fix: RS2007 table borders Run on UI thread Organize Handle error case: static ref field (No NullReferenceException) (dotnet#78707) Sort Fix crash with introduce local within a compilation unit ...

* Add XML Solution (SLNX) support to MSBuildWorkspace * Update `AbstractImplementAbstractClassCodeFixProvider.cs` source Signed-off-by: Emmanuel Ferdman <[email protected]> * IN progress * IN progress * in progress * in progress * Simplify * Simplify * Cleanup * Rename * Push through initial description * Rename * Removed * Add back disposal * Linked source * Rename * Update docs * Remove * Add comment * Add comment * Fix * Delay loading of CodeRefactoringProvider's till absolutely needed * CR feedback and tests. * Fix comment * CR feedback * Fix crash in 'introduce variable' on top-level statements * CR feedback * Seal certain types in LSP layer * Seal certain types in LSP layer * Publish PR validation to internal feeds * PooledObjects cleanup (#78382) * moved condition above top-level statements * REvert * Seal * Gracefully handle span mapping failing * Fix information logs getting logged as debug in VSCode * Don't unnecessarily create a document when only the document state is needed. (#78463) Cyrus noticed this during a chat where I was considering a larger change to this code. Might as well make the simple/safe change while I consider the larger change too. Other small changes: 1) Early exit if no changed documents 2) Unrelated change adding Checksum overload as I noticed a high frequency caller during solution load (ProjectSystemProjectOptionsProcessor.ReparseCommandLineIfChanged_NoLock) had an ImmutableArray and it was getting boxed and enumerator alloced. * Ensure loghub collects the now verbose level logs * Adjust some more logging messages * Cancel running requests when the connection terminates * Shorten log category name * Extensions: handle extensions in VB SymbolDisplay (#78512) * Switch to non-scouting queue to unblock CI (#78521) * Fix test helper message to avoid NRE (#78532) * Collect data about which code fixes end up making changes that conflict with other code fixes * docs * Revert * Source package fixes (#78534) * Extract helper to make it easier for other features to request copilot analysis * lint * lint * Extensions: only count extensions for determining identifier (#78523) * Cache extension method import info per project ID * Remove telemetry * Use pattern matching * Lint * Add VB ref assembly to semantic search (#78537) * Add support for FieldRva to EnC delta (#78033) * [main] Source code updates from dotnet/dotnet (#78527) * [VMR] Codeflow c53bdd8-c53bdd8 [[ commit created by automation ]] * Update dependencies from https://github.com/dotnet/dotnet build 267468 * [VMR] Codeflow b422f78-b422f78 [[ commit created by automation ]] * Update dependencies from https://github.com/dotnet/dotnet build 267725 * Update dependencies from https://github.com/dotnet/dotnet build 267776 --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Introduce -productBuild and -sourceBuild switches (#78519) * Introduce -productBuild and -sourceBuild switches * Update eng/build.sh Co-authored-by: Viktor Hofer <[email protected]> * Update build.sh * Add new options for SB and PB switches * Remove leftover SB arg --------- Co-authored-by: Viktor Hofer <[email protected]> * Document more steps in our C# release process (#78539) * Extensions: analyzer actions (#78319) * Update XAML EA to use DocumentUri instead of System.Uri * Update field value for NavBar Integration Tests (#78553) * fix * fix * Update GetFirstRelatedDocumentId to not return documents with the same path within the same project (#78475) GetFirstRelatedDocumentId had a mismatch in behavior for files within the same project that have the same path (shouldn't really happen, would cause compile errors). Previously, the code would return files from the same proejct when searching the cache, but not when doing the linear walk. Talked with Jason, and he indicated that it was better to have the code not return files from the same project, as callers wouldn't expect that. * Implement PDG.WithProjectsRemoved (#78428) This allows batch removal of projects from the ProjectDependencyGraph. This should eventually give us a small perf benefit, but only at the point where our projects can be disposed (or processed) in batches, which is not something we currently do. * Selectively persist the commandline to temporary storage (#78441) Only write command lines to temporary storage when they may later be needed. Persisting project command line arguments is surprisingly costly, as evidenced by the profile image below. As this storage is only needed when there is an effective ruleset path, we can limit when we persist to storage to be limited to that scenario. See PR for detailed performance information. * File based programs IDE support (#78488) Co-authored-by: Cyrus Najmabadi <[email protected]> Co-authored-by: Jason Malinowski <[email protected]> * Update dependencies from https://github.com/dotnet/arcade build 20250513.2 Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.XliffTasks From Version 9.0.0-beta.25255.5 -> To Version 9.0.0-beta.25263.2 * Extensions: pattern-based constructs (#78480) * Unset CompilerType from Csc (#78483) * Unset CompilerType from Csc * Remove CompilerType logic * Add Microsoft.CodeAnalysis.Extensions package (#78388) * Fix MoveType to create debuggable documents (#78554) * Move SpecializedCollections to Microsoft.CodeAnalysis.Collections namespace (#78466) * Add a reference to the tracking bug for a workaround * Fix embedded language classification inside multi-line string * Remove 'Opt' suffixes * Unify resource strings for special case checks * NRT * mIn progress * Simplify * Simplify * Remove localized strings from Collections source package (#78576) * Revert "Update XAML EA to use DocumentUri instead of System.Uri (#78555)" This reverts commit 2611c9a, reversing changes made to f4e6964. * Implement `Enum.IsDefined` check refactoring * Rename LSP messages for VisualStudio.Extensibility integration (#78598) Co-authored-by: Matteo Prosperi <[email protected]> * More improvements to source packages (#78587) * Editor configs for source packages * Add nullable enable * Include linked files in source packages * Suppress RS1024 * Remove duplicate nullability directives * Remove unnecessary extensions * Move FatalError and FailFast to Contracts package * Add disclaimer * Ensure we pass unique binlog paths to each BuildHost Otherwise if we launch multiple processes, they might step atop each other and cause locking issues. * Clean up our SpellingExclusions Somehow almost every line started with a UTF-8 BOM. This fixes it. * Track used assemblies of data section string literals (#78552) * Track used assemblies of data section string literals * Move reporting to module * Fix a failing test * Improve test * Add a comment * Support local functions in breadcrumbs * hotfix to fix restore and stop including bin/obj artifacts in directory with loose files (#78615) * Fix angle brackets in generics in hover * more directly walk the tree for local functions and add tests * Give .NET Framework build task in sdk different identity (#78584) closes #78565 * [VMR] Codeflow 015a854-015a854 [[ commit created by automation ]] * Update dependencies from https://github.com/dotnet/dotnet build 268651 No dependency updates to commit * [VMR] Codeflow 604dfc7-170498a [[ commit created by automation ]] * Update dependencies from https://github.com/dotnet/dotnet build 268722 No dependency updates to commit * review feedback * Fix option reading * Simplify equivalence keys * Make refactoring resilient against absence of `InvalidEnumArgumentException` type * Fix formatting * Update Workspace.MSBuild to reference Microsoft.Build.Framework. * LSP: Fix batch builds for file-based programs and fix `"dotnet.projects.binaryLogPath"` throwing an exception (#78644) Co-authored-by: Joey Robichaud <[email protected]> * Convert operand of `true`/`false` operator invoked as part of `&&`/`||`operator evaluation (#78629) Fixes #78609. * Extensions: rename syntax node (#78625) * Simplify * Use SolutionFileReader instead of calling out to BuildHost Removes the GetProjectsInSolution BuildHost API. Instead we can use the SolutionPersister library in processes to parse the projects from a solution file. * StaticLogMessage isn't pooling correctly Noticed this when looking at a profile and was curious why there were 3 MB of allocations of the StaticLogMessage type. LogMessage.Free sets _message to null before calling FreeCore, thus StaticLogMessage.FreeCore was never adding items back to it's pool. The ordering of LogMessage setting _message to null and calling FreeCore is important, so that can't be changed. Instead, add a flag to StaticLogMessage to indicate whether it's in a constructed state or not. * Fixing symbol publishing (#78650) These need to be manually listed given that they aren't published directly as a NuPkg * Compare symbols by metadata name when searching for eqivalent symbols in FAR engine * Extensions: disallow indexers (#78626) * Add untriaged label to all new issues (#78658) * Update resourceManagement.yml * Update resourceManagement.yml * Fixed crash in CSharpConvertToRecordRefactoringProvider. * Return arity check for method symbols * Fix doc comment * Update src/Features/Core/Portable/SymbolSearch/SymbolSearchOptions.cs * Fix * Make solution file reading async * Fix storage * Hot Reload: Handle document deletes (#78516) * Use SBRP version of M.CA for analyzers in the context of source build (#78668) * fix symbol publishing? (#78671) * Cleanup * Use ThrowIfFalse when resolving paths * Add test assertions * Applied code review suggestions. * Update Microsoft.Build.Tasks.CodeAnalysis.Sdk.csproj (#78681) * Fix nullable crash for field keyword in partial property (#78672) * Use VerifyDiagnostics format to report CompileAndVerify diagnostics (#78666) * Revert "Update Microsoft.Build.Tasks.CodeAnalysis.Sdk.csproj (#78681)" (#78687) This reverts commit 6d490de. * Revert "fix symbol publishing? (#78671)" (#78686) This reverts commit 2b6024e. * Add comment * [main] Source code updates from dotnet/dotnet (#78663) * [VMR] Codeflow c3c7ad6-c3c7ad6 [[ commit created by automation ]] * Update dependencies from https://github.com/dotnet/dotnet build 268973 No dependency updates to commit * [VMR] Codeflow d219c4f-d219c4f [[ commit created by automation ]] * Update dependencies from https://github.com/dotnet/dotnet build 269082 No dependency updates to commit --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Add a test to verify that behavior is expected and bug no longer occurs * [main] Source code updates from dotnet/dotnet (#78692) * [VMR] Codeflow 2b6024e-2b6024e [[ commit created by automation ]] * Update dependencies from https://github.com/dotnet/dotnet build 269352 No dependency updates to commit --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Modify ABWQ.AddWork to take in a ROS instead of an IEnumerable (#78691) I'm seeing this enumerator allocation as account for 50 MB (0.9%) in a customer trace for feedback ticket https://developercommunity.visualstudio.com/t/Razor-Development:-30-Second-CPU-Churn-A/10904811 This shows up a bit smaller in the C# editing speedometer test, but does show as 2.3 MB. Test insertion showed allocations were removed. See PR for more numbers and speedometer test results. * Add assert * Add test * Add support for 'fixed' * Fix crash with introduce local within a compilation unit * Sort * Handle error case: static ref field (No NullReferenceException) (#78707) * Organize * Run on UI thread * fix: RS2007 table borders * [main] Source code updates from dotnet/dotnet (#78710) * [VMR] Codeflow 533fde8-533fde8 [[ commit created by automation ]] * Update dependencies from https://github.com/dotnet/dotnet build 269499 No dependency updates to commit * Update dependencies from https://github.com/dotnet/dotnet build 269610 No dependency updates to commit --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Make operator glyph consistent witht eh rest of the members * Fix * avoid not needed compilationUnit clone (#78676) * Always log PID * Update MicrosoftVisualStudioExtensibilityTestingVersion (#78661) * update extensibility version * remove added package? * update minversion * test * update vswhere version * update versions.props vswhere * update min version * update coreeditor version ranges * Add IsNull property to IAsyncToken (#78718) * Extensions: nullability analysis for property access (#78646) * Add helper for mapping between documents and hierarchy+itemids * Updat epersistence version * Do not convert tuple expression with error to a bad expression too early (#78645) * [main] Source code updates from dotnet/dotnet (#78724) * [VMR] Codeflow f5705c8-57b0396 [[ commit created by automation ]] * Update dependencies from https://github.com/dotnet/dotnet build 269724 No dependency updates to commit --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * [main] Update dependencies from dotnet/arcade (#78578) * Update dependencies from https://github.com/dotnet/arcade build 20250513.5 Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.XliffTasks From Version 9.0.0-beta.25263.2 -> To Version 9.0.0-beta.25263.5 * Update dependencies from https://github.com/dotnet/arcade build 20250516.2 Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.XliffTasks From Version 9.0.0-beta.25263.2 -> To Version 9.0.0-beta.25266.2 * Update dependencies from https://github.com/dotnet/arcade build 2025052.1 Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.XliffTasks From Version 9.0.0-beta.25263.2 -> To Version 9.0.0-beta.25271.1 --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Invoke `dotnet run-api` to obtain virtual project (#78648) Co-authored-by: David Barbet <[email protected]> --------- Signed-off-by: Emmanuel Ferdman <[email protected]> Co-authored-by: Deepak Rathore (ALLYIS INC) <[email protected]> Co-authored-by: Emmanuel Ferdman <[email protected]> Co-authored-by: Cyrus Najmabadi <[email protected]> Co-authored-by: Cyrus Najmabadi <[email protected]> Co-authored-by: Evgeny Tvorun <[email protected]> Co-authored-by: Victor Pogor <[email protected]> Co-authored-by: David Barbet <[email protected]> Co-authored-by: Tomáš Matoušek <[email protected]> Co-authored-by: Jason Malinowski <[email protected]> Co-authored-by: Andrew Hall <[email protected]> Co-authored-by: Todd Grunke <[email protected]> Co-authored-by: Julien Couvreur <[email protected]> Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Joey Robichaud <[email protected]> Co-authored-by: Ella Hathaway <[email protected]> Co-authored-by: Viktor Hofer <[email protected]> Co-authored-by: Ankita Khera <[email protected]> Co-authored-by: Rikki Gibson <[email protected]> Co-authored-by: Jason Malinowski <[email protected]> Co-authored-by: Jan Jones <[email protected]> Co-authored-by: DoctorKrolic <[email protected]> Co-authored-by: Matteo Prosperi <[email protected]> Co-authored-by: Matteo Prosperi <[email protected]> Co-authored-by: Jared Parsons <[email protected]> Co-authored-by: Joey Robichaud <[email protected]> Co-authored-by: AlekseyTs <[email protected]> Co-authored-by: John Douglas Leitch <[email protected]> Co-authored-by: Matt Thalman <[email protected]> Co-authored-by: Bernd Baumanns <[email protected]> Co-authored-by: Thomas Shephard <[email protected]> Co-authored-by: DoctorKrolic <[email protected]> Co-authored-by: David Barbet <[email protected]>

jasonmalinowski · 2025-05-30T19:18:46Z

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/DotnetCliHelper.cs

    {
        _logger.LogDebug($"Running dotnet CLI command at {_dotnetExecutablePath.Value} in directory {workingDirectory} with arguments {arguments}");

        var startInfo = new ProcessStartInfo(_dotnetExecutablePath.Value)
        {
            CreateNoWindow = true,
            UseShellExecute = false,
+            RedirectStandardInput = redirectStandardInput,


Should this frankly always be true as well? Because it seems either that:

The command we're running won't read stdin (so it won't really matter).

The command we might be running might incidentally read stdin (like restore triggering an interactive credential provider) and we probably don't want it using some random stdin from...somewhere?

In case (2), I think it would essentially read the stdin of the parent process, which it seems like we wouldn't want.

jasonmalinowski · 2025-05-30T19:25:52Z

...er/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/FileBasedProgramsProjectSystem.cs

+        var content = await _projectXmlProvider.GetVirtualProjectContentAsync(documentPath, cancellationToken);
+        if (content is not var (virtualProjectContent, diagnostics))
+        {
+            // 'GetVirtualProjectContentAsync' will log errors when it fails


It could be clearer then to pass in the ILogger so that way it's clear it's expected to log to this rather than having it's own logger.

jasonmalinowski · 2025-05-30T19:27:02Z

...er/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/FileBasedProgramsProjectSystem.cs


        // When loading a virtual project, the path to the on-disk source file is not used. Instead the path is adjusted to end with .csproj.
        // This is necessary in order to get msbuild to apply the standard c# props/targets to the project.
-        var virtualProjectPath = VirtualCSharpFileBasedProgramProject.GetVirtualProjectPath(documentPath);
+        var virtualProjectPath = VirtualProjectXmlProvider.GetVirtualProjectPath(documentPath);


Should the name of the file also come from the API too?

jasonmalinowski · 2025-05-30T19:35:43Z

...eServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlProvider.cs

+    internal async Task<(string VirtualProjectXml, ImmutableArray<SimpleDiagnostic> Diagnostics)?> GetVirtualProjectContentAsync(string documentFilePath, CancellationToken cancellationToken)
+    {
+        var workingDirectory = Path.GetDirectoryName(documentFilePath);
+        var process = dotnetCliHelper.Run(["run-api"], workingDirectory, shouldLocalizeOutput: true, redirectStandardInput: true);


I chatted with @dibarbet on this a bit yesterday. We don't want the IDE being in the situation of having to figure out all the properties directly, especially as the directives we support change/expand over time. That said, if we can get away with having a build task figure out the majority of it, then maybe we could pass just a entrypoint file as a property and a majority of stuff could be figured out?

The other idea was we do go back to the idea of there being an API we call, but we'd load that API from the SDK. But that creates some risk since we have to deal with supporting many old versions, and how we isolate dependencies, etc.

jasonmalinowski · 2025-05-30T19:36:16Z

...eServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlProvider.cs

+
+        // Debug severity is used for these because we think it will be common for the user environment to have too old of an SDK for the call to work.
+        // Rather than representing a hard error condition, it represents a condition where we need to gracefully downgrade the experience.
+        process.ErrorDataReceived += (sender, args) => _logger.LogDebug($"dotnet run-api: {args.Data}");


Consider putting an [stderr] in this so the logs are a bit clearer for what they might be.

jasonmalinowski · 2025-05-30T19:41:00Z

...eServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlProvider.cs

+                if (project.Version > RunApiOutput.LatestKnownVersion)
+                {
+                    _logger.LogWarning($"'dotnet run-api' version '{project.Version}' is newer than latest known version {RunApiOutput.LatestKnownVersion}");
+                }


I don't know why we'd do this, since it means any bump to the version means we're now broken. I commented on the run-api PR that we need to rethink the versioning generally, but minimally I'd delete this code.

I didn't follow why deleting this code would make the situation better.

Because right now if you were to bump the version, you'll just immediately give up and fail. Without this, it might still work.

I'd say the feedback here is similar to global.json: the number of times I've had to had to go fetch an SDK to build another repo is infinitely higher than the number of times global.json actually helped me.

jasonmalinowski · 2025-05-30T19:41:36Z

...eServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlProvider.cs

+    internal static bool IsFileBasedProgram(string documentFilePath, SourceText text)
+    {
+        // TODO: this needs to be adjusted to be more sustainable.
+        // When we adopt the dotnet run-api, we need to get rid of this or adjust it to be more sustainable (e.g. using the appropriate document to get a syntax tree)


So now that you have run-api being consumed, what's the plan for this?

jasonmalinowski · 2025-05-30T19:44:51Z

...uageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/VirtualProjectXmlProviderTests.cs

+        public override bool ShouldSkip =>
+#if RoslynTestRunApi
+            false;
+#else
+            true;
+#endif


Do we have a bug tracking enabling this?

jasonmalinowski · 2025-05-30T19:45:53Z

...uageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/VirtualProjectXmlProviderTests.cs

+        var contentNullable = await projectProvider.GetVirtualProjectContentAsync(appFile.Path, CancellationToken.None);
+        var content = contentNullable.Value;
+        var virtualProjectXml = content.VirtualProjectXml;
+        LoggerFactory.CreateLogger<VirtualProjectXmlProviderTests>().LogTrace(virtualProjectXml);


Why not use the xunit logging support?

The base type is adding a logger provider which wraps the xunit logger. It didn't feel necessary to directly use the xunit logger given that.

jasonmalinowski · 2025-05-30T19:46:51Z

...uageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/VirtualProjectXmlProviderTests.cs

+        Assert.Equal(appFile.Path, diagnostic.Location.Path);
+
+        // LinePositionSpan is not deserializing properly.
+        // Address when implementing editor squiggles. https://github.com/dotnet/roslyn/issues/78688


Seems like we should fix that now, since otherwise we're reporting bad locations to the error list?

Nothing is including the diagnostics from run-api in the error list currently.

@jasonmalinowski

#78788) From discussion with @jasonmalinowski, we think we should keep in the "directly creating" virtual project behavior that we had prior to #78648, at minimum until we can be pretty sure .NET 10 users will have an SDK with run-api available. Without this change, users running with .NET 10 preview 4 SDK will not get intellisense in VS Code. They would have to install a nightly .NET 10 or wait for preview 5 to come out. I did manually verify that this change works when SDK is pinned to preview 4. The log message about taking the fallback path gets written out and so on. Most of the change here is copy/pasted from deleted lines in the linked PR #78648. FYI @dibarbet we probably want to make sure this is in the next prerelease that goes out. Apologies for the inconvenience.

WIP

6e51543

dotnet-policy-service bot added the VSCode label May 19, 2025

github-actions bot added the Area-IDE label May 19, 2025

build-analysis bot mentioned this pull request May 20, 2025

The active test run was aborted. Reason: Test host process crashed dotnet/dnceng#451

Open

3 tasks

RikkiGibson added 2 commits May 19, 2025 17:03

scaffold out a test

a06c028

Add some basic run-api RPC tests

adebf2b

RikkiGibson commented May 21, 2025

View reviewed changes

...uageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/VirtualProjectXmlProviderTests.cs Outdated Show resolved Hide resolved

RikkiGibson commented May 21, 2025

View reviewed changes

...eServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlProvider.cs Outdated Show resolved Hide resolved

RikkiGibson commented May 21, 2025

View reviewed changes

...Server/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/FileBasedProgramsProjectSystem.cs Outdated Show resolved Hide resolved

RikkiGibson commented May 21, 2025

View reviewed changes

...osoft.CodeAnalysis.LanguageServer/HostWorkspace/FileBasedProgramsWorkspaceProviderFactory.cs Outdated Show resolved Hide resolved

RikkiGibson added 6 commits May 21, 2025 14:39

cleanup/simplify run-api test

f9c91c5

experiment with checksums to avoid unwanted restores on C# code edits

edaadf3

cleanup

c50833a

remove checksum stuff

6cbbdf0

revert a few things and simplify

1159e30

move a few files

1febb54

RikkiGibson requested a review from dibarbet May 22, 2025 23:01

RikkiGibson marked this pull request as ready for review May 22, 2025 23:01

RikkiGibson requested a review from a team as a code owner May 22, 2025 23:01

dibarbet reviewed May 23, 2025

View reviewed changes

RikkiGibson added 2 commits May 27, 2025 13:40

Address some feedback

0160546

Add conditional symbol to skip tests requiring preview SDK

648aa23

RikkiGibson requested a review from dibarbet May 27, 2025 22:58

RikkiGibson changed the title ~~(WIP) Invoke dotnet run-api to obtain virtual project~~ Invoke dotnet run-api to obtain virtual project May 27, 2025

dibarbet approved these changes May 28, 2025

View reviewed changes

...eServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlProvider.cs Outdated Show resolved Hide resolved

Update src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/FileB…

f455432

…asedPrograms/VirtualProjectXmlProvider.cs Co-authored-by: David Barbet <[email protected]>

RikkiGibson merged commit f750f70 into dotnet:main May 28, 2025
25 checks passed

RikkiGibson deleted the fbp-run-api branch May 28, 2025 20:45

dotnet-policy-service bot added this to the Next milestone May 28, 2025

jasonmalinowski reviewed May 30, 2025

View reviewed changes

dotnet-bot mentioned this pull request May 31, 2025

[Automated] PRs inserted in VS build main-10630.40 #78777

Closed

RikkiGibson mentioned this pull request Jun 2, 2025

Directly create virtual project when dotnet run-api is missing for now #78788

Merged

This was referenced Jun 3, 2025

[Automated] PRs inserted in VS build feature.debugger.main-10702.70 #78803

Closed

[Automated] PRs inserted in VS build feature.Wix5-10703.29 #78812

Closed

[Automated] PRs inserted in VS build feature.telemetry.d18OpenTelemetry-10703.197 #78814

Closed

RikkiGibson mentioned this pull request Jun 9, 2025

Enable VirtualProjectXmlProviderTests in CI #78879

Open

RikkiGibson added a commit to RikkiGibson/roslyn that referenced this pull request Jun 9, 2025

Address feedback from dotnet#78648

07fded2

RikkiGibson mentioned this pull request Jun 9, 2025

Address feedback from #78648 #78880

Merged

RikkiGibson added a commit that referenced this pull request Jun 20, 2025

Address feedback from #78648 (#78880)

90229b2

Invoke dotnet run-api to obtain virtual project #78648

Invoke dotnet run-api to obtain virtual project #78648

Uh oh!

Conversation

RikkiGibson commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Question: how to do testing?

Uh oh!

jasonmalinowski commented May 19, 2025

Uh oh!

jasonmalinowski commented May 19, 2025

Uh oh!

dibarbet commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RikkiGibson commented May 19, 2025

Uh oh!

Uh oh!

Uh oh!

RikkiGibson May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MiYanni commented May 21, 2025

Uh oh!

RikkiGibson commented May 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dibarbet May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dibarbet May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Invoke `dotnet run-api` to obtain virtual project #78648

Invoke `dotnet run-api` to obtain virtual project #78648

RikkiGibson commented May 19, 2025 •

edited

Loading

dibarbet commented May 19, 2025 •

edited

Loading

RikkiGibson May 21, 2025 •

edited

Loading

dibarbet May 23, 2025 •

edited

Loading

dibarbet May 23, 2025 •

edited

Loading

RikkiGibson May 30, 2025 •

edited

Loading