File-based program directive diagnostics in editor #79421

RikkiGibson · 2025-07-16T18:31:14Z

#78688

Creates a new IDiagnosticSourceProvider, which produces IDiagnosticSource with IsLiveSource() => false.
When VirtualProjectXmlProvider gets a virtual project, it caches the diagnostics from the run-api result in a dictionary
The IDiagnosticSource has a reference to the VirtualProjectXmlProvider and accesses the latest diagnostics for the project from it
Right now this works well enough to display directive diagnostics when a file is first opened.
~~TODO:~~ When GetVirtualProjectContentAsync gets a new project back from run-api, it needs to signal that the new diagnostics are available, and the associated IDiagnosticSource should be queried again. I'm not sure how to do this. Edit: resolved by using IDiagnosticRefresher.
Looking for feedback on the location to keep the cached diagnostics, etc. Possibly the diagnostics should be "unloaded" if the associated project is unloaded. Possibly even these should be cached in the FileBasedProgramsProjectSystem and looked up somehow.
- Since IDiagnosticSourceProvider needs to be registered through MEF the current factoring felt fairly straightforward. However we could imagine wanting to also surface the design time build diagnostics through this provider somehow. In which case we would need the project system to cache diagnostics instead, in some location where the IDiagnosticSource could access them.
- Maybe the IDiagnosticSourceProvider could be a parameter to the FileBasedProgramsProjectSystem and when the project system has diagnostics it wants cached/potentially surfaced it could just call into the diagnostic source provider.

src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/FileBasedPrograms/RunApiModels.cs

...t.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlDiagnosticSourceProvider.cs

CyrusNajmabadi · 2025-07-16T19:31:19Z

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

+                using (await _gate.DisposableWaitAsync(cancellationToken))
+                {
+                    _diagnosticsByFilePath[documentFilePath] = project.Diagnostics;
+                }
                return (project.Content, project.Diagnostics);
            }


the staleness issue semes real here. it looks like we could return incorrect diagnostics for a solution snapshot. and it's unclear what would snap the ide out of that state.

This is updated each time a design time build occurs, which for the moment occurs whenever the file-based program is saved to disk.

The cached diagnostics would probably need to be updated in the other code paths also. e.g. if run-api completely fails, the stale diagnostics we already had in the cache are likely not useful and should be cleared out.

it looks like we could return incorrect diagnostics for a solution snapshot. and it's unclear what would snap the ide out of that state.

I don't think I fully grasp this concern or what should be done to address it.

CyrusNajmabadi · 2025-07-16T21:38:06Z

So, when the "Save" happens, what ensures that the entire pipeline even knows to run again to then get these cached diagnostics?

For context, saving a file doesn't not change our content checksums, and we often (but not always) drive these experiences off of content checksums. So we'd need something else to mix into the system to even know to get diagnostics agian. And importantly, that would either have to run after the diagnostics were definitely computed and cached, or it would have to wait on those being computed when it did its work in order for the user to get the correct diagnostics results.

All this is possible. We just have to be careful and clear (and the code should be evident as to how it is ccomplishing this). Thanks! :)

RikkiGibson · 2025-07-16T21:44:15Z

So, when the "Save" happens, what ensures that the entire pipeline even knows to run again to then get these cached diagnostics?

A file watcher will kick off a design time build when the file-based program is saved.

For the pipeline to run again, the TODO from PR description needs to be addressed.

TODO: When GetVirtualProjectContentAsync gets a new project back from run-api, it needs to signal that the new diagnostics are available, and the associated IDiagnosticSource should be queried again. I'm not sure how to do this.

CyrusNajmabadi · 2025-07-16T21:55:05Z

TODO: When GetVirtualProjectContentAsync gets a new project back from run-api, it needs to signal that the new diagnostics are available, and the associated IDiagnosticSource should be queried again. I'm not sure how to do this.

Answered offline.

RikkiGibson · 2025-07-16T22:40:08Z

Added use of IDiagnosticRefresher. Seems decently responsive for the moment. We should still track down making this work with in-memory-only changes in the near-to-medium term so that long design time builds, autosave being turned off, etc. will not mess up the user experience.

fbp-directive-diagnostics.mp4

Also, if we break things so badly that run-api doesn't even give back a virtual project, we should probably clear out any stale diagnostics from the last run.

...t.CodeAnalysis.LanguageServer/FileBasedPrograms/VirtualProjectXmlDiagnosticSourceProvider.cs

dibarbet · 2025-07-16T23:32:34Z

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

+
+                    // check for difference, and signal to host to update if so.
+                    if (previousCachedDiagnostics.IsDefault || !project.Diagnostics.SequenceEqual(previousCachedDiagnostics))
+                        diagnosticRefresher.RequestWorkspaceRefresh();


this looks correct. generally the guidance I have on using the refresher is to use it as sparingly as possible because it refreshes all diagnostics. It looks like you're already checking to make sure there are changes which is good.

But the less that goes through this the better. Ideally we can get most diagnostics from the actual Roslyn snapshots, and then the normal on-type updates handle most changes.

Ideally we can get most diagnostics from the actual Roslyn snapshots, and then the normal on-type updates handle most changes.

What would it mean to do this? Does that require having a "live source" for diagnostics? e.g. getting the diagnostics through analyzer, or running run-api on the "snapshot" of the file-based program?

dibarbet · 2025-07-16T23:34:48Z

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

+                    _diagnosticsByFilePath[documentFilePath] = project.Diagnostics;
+
+                    // check for difference, and signal to host to update if so.
+                    if (previousCachedDiagnostics.IsDefault || !project.Diagnostics.SequenceEqual(previousCachedDiagnostics))


How do we remove diagnostics? It looks really bad when we have stale diagnostics, so we definitely need to ensure we're clearing them when the file based project is deleted / renamed / otherwise changes contexts?

We also probably need to remove them from the _diagnosticsByFilePath map if the file/project is deleted, otherwise we have a small leak

I'm wrapping this method to try and ensure that the cache is always dealt with appropriately, whatever the response from run-api.

…ostics

RikkiGibson · 2025-07-25T01:52:30Z

Handled unloading. Quite possibly the caching should be in FileBasedProgramsProjectSystem rather than VirtualProjectXmlProvider. I don't have a strong opinion on it.

…ostics

RikkiGibson · 2025-07-28T17:10:36Z

@dibarbet @jasonmalinowski for review

CyrusNajmabadi · 2025-07-28T17:15:52Z

@dibarbet @jasonmalinowski for review

Can you do a final pass through feedback plz :)

…ostics

dibarbet · 2025-07-28T20:24:25Z

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

@@ -153,4 +153,9 @@ public async ValueTask TryRemoveMiscellaneousDocumentAsync(DocumentUri uri)
            Preferred: buildHostKind,
            Actual: buildHostKind);
    }
+
+    protected override async ValueTask OnProjectUnloadedAsync(string projectFilePath)


Trying to figure out the semantics of OnProjectUnloadedAsync

Does this get called when the file is closed in the editor

Does this get called when the file moves from a loose file, into an actual project?

Yes.

No. I'm adding a comment to explain in more details.

Is there any way we can tell when that transition happens? So that we can clear out diagnostics if its not a file based program any longer?

Maybe it would be possible to verify that the workspace kind is still the language server workspace? IIRC there would be a workspace changed event triggering a diagnostics refresh, so there should be a diagnostics requests to the server in that scenario already.

Is there any way we can tell when that transition happens? So that we can clear out diagnostics if its not a file based program any longer?

Hypothetically, upon reload, we could see if some of the targets we unloaded were associated with a different ProjectFactory than before, and react to that somehow.

roslyn/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LanguageServerProjectLoader.cs

Line 266 in 8fefbab

target.Dispose();

Note also the assert that tracked targets for a given project, are all associated with only one projectFactory at a given time:

roslyn/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/LanguageServerProjectLoader.cs

Line 287 in 8fefbab

Debug.Assert(newProjectTargets.All(target => target.ProjectFactory == projectFactory));

In practice I do not think we are going to need a special behavior here. Each design time build, either of FBP or "rich misc file", is going to result in run-api reporting some set of diagnostics, and we will always surface the latest set for the project. And, we expect run-api is not going to report anything for the "non-FBP" case. It would be hard to observe it reporting in the non-FBP case, since it only reports on bad "app directives" currently, and presence of those will mark the file as an FBP.

Long and short is I do not expect an unwanted/buggy behavior here from failing to take an explicit step to clear the diagnostics out when transitioning between FBP and non-FBP. (I'll verify this manually but I keep having to change between different branches and would like to fix in a follow up in the case we really do have a problem here.)

RikkiGibson added 2 commits July 15, 2025 20:43

WIP

dd2cf21

more

b7bd8bc

github-actions bot added the Area-IDE label Jul 16, 2025

dotnet-policy-service bot added the VSCode label Jul 16, 2025