fix languages.getDiagnostics() problem-matchers diagnostics duplication #290278
+7
−9
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #256118
ISSUE
When opening files after starting a task which outputs diagnostic entries for that file,
languages.getDiagnostics()returns duplicated (combined) diagnostic entries coming from extension and from task's problem matcher for that file. If a file is opened before the task startup, there is no duplicates, which is correct.Expected: when a file is open and a task is running,
getDiagnostics()should output only the list which comes from extension, regardless of the opening a file and task starting actions order, and return the list from task's problem matcher for closed files.the path taken to the fix
DiagnosticCollectionssimultaneously, thetypescriptand the_generated_mirrornamed collections._generated_mirror(mirrorCollection) is managed through ExtHostDiagnostics.$acceptMarkersChange and is a storage of diagnostics coming from tasks.$acceptMarkersChangegets a 'clearing' call (empty list of diagnostics for closed resource uri). Shortly after,$acceptMarkersChangegets repopulated by task's outputs, for each resource uri, again, excluding the open ones. Seems it is the same action as when the task is started. Though this may be counterperformant, since it could be called as update for the closed file only, instead of passing the entire list, the result is correct.$acceptMarkersChangedoesn't gets a clearing call for opened uri, which results in diagnostics lists to exists in two collections, which leads to duplicated entries ofgetDiagnostics()output.$acceptMarkersChange, which is MainThreadDiagnostics._forwardMarkers and found that, when a file is opened, the clearing call to$acceptMarkersChangefor opening uri is filtered byforeignMarkerData.length > 0check. It seems that$acceptMarkersChangeis designed to expect a clearing call on file open, so, this check probably should be removed. Which, in turn, makes theallMarkerData.length === 0check excessive, so I have removed it too, for the sake of readability.mirrorCollectioninside$acceptMarkersChange. Otherwise it keeps storing empty lists even when problem matcher task is terminated, which looks like a kind of leak to me.This doesn't solves the issue I have described in #256118 (comment), however. But now it doesn't looks like it is related to getDiagnostics, since I didn't found any side effects of calling it. It needs more testing, but it is probably related to the use of
vscode.window.showTextDocumentfrom extension host. When closing a file, which was previously opened by that, markerService doesn't get updated by diagnostics info for that file. Problems View list, same as getDiagnostics output, keep outputting entries, which was from typescript extension, by the moment of file closing. And after some time of VS Code idling, some sort of resources cleanup happens, which removes entries from the markers service and leads to losing the tracking of problems for that file. This deserves a separate issue, if the one from team, reading this, doesn't knows how to fix it with a swipe ;)