Whenever we get around to revamping how the checkers are allocated, I think the fix is going to be to (in the editor) collect global diags from diagnostic-producing checkers, and then re-collect them whenever we check a new file, then expose those diags there.

This used to be here because we could fail to get file parsing diags. But then we madeParsedCommandLine mandatory, so this call always succeeds, so there's no reason to special case anything.

Note above how we get config file diags, then get options diags, and then get global diags. This means we could have the same diags repeated twice!

(This func does not sort and dedupe, for some reason)

This list already containsGetConfigFileParsingDiagnostics andGetGlobalDiagnostics, so this is a noop.

Strada use to add them to semantic Diagnostics - this ensured that all callers of this API didnt have to worry about it?

We can do that, if and only if the caller passes in anil file, theoretically.

Pushed a change which I think works to deleteGetGlobalDiagnostics too.

well - strada did the exact opposite. It added to "per file semanticDiagnostics" because thats how editor gets diagnostics and it shouldnt miss them?

functiongetDiagnosticsWorker(sourceFile:SourceFile,nodesToCheck:Node[]|undefined):Diagnostic[]{if(sourceFile){ensurePendingDiagnosticWorkComplete();// Some global diagnostics are deferred until they are needed and// may not be reported in the first call to getGlobalDiagnostics.// We should catch these changes and report them.constpreviousGlobalDiagnostics=diagnostics.getGlobalDiagnostics();constpreviousGlobalDiagnosticsSize=previousGlobalDiagnostics.length;checkSourceFileWithEagerDiagnostics(sourceFile,nodesToCheck);constsemanticDiagnostics=diagnostics.getDiagnostics(sourceFile.fileName);if(nodesToCheck){// No need to get global diagnostics.returnsemanticDiagnostics;}constcurrentGlobalDiagnostics=diagnostics.getGlobalDiagnostics();if(currentGlobalDiagnostics!==previousGlobalDiagnostics){// If the arrays are not the same reference, new diagnostics were added.constdeferredGlobalDiagnostics=relativeComplement(previousGlobalDiagnostics,currentGlobalDiagnostics,compareDiagnostics);returnconcatenate(deferredGlobalDiagnostics,semanticDiagnostics);}elseif(previousGlobalDiagnosticsSize===0&&currentGlobalDiagnostics.length>0){// If the arrays are the same reference, but the length has changed, a single// new diagnostic was added as DiagnosticCollection attempts to reuse the// same array.returnconcatenate(currentGlobalDiagnostics,semanticDiagnostics);}returnsemanticDiagnostics;}// Global diagnostics are always added when a file is not provided to// getDiagnosticsforEach(host.getSourceFiles(),file=>checkSourceFileWithEagerDiagnostics(file));returndiagnostics.getDiagnostics();}

Not suire if we have enough coverage for diagnostics in fourslash but all this logic was for editor to not miss reporting errors as for command line you can always get all semantic diagnostics and then global diagnsotics?

Not suire if we have enough coverage for diagnostics in fourslash but all this logic was for editor to not miss reporting errors as for command line you can always get all semantic diagnostics and then global diagnsotics?

The current LS can miss these global diagnostics, yes. Though, that isn't new...

The last commit I did was bad, though, because it locks off ever asking for global diags in the LS where we never ask for all files, so I'll undo that

Not suire if we have enough coverage for diagnostics in fourslash but all this logic was for editor to not miss reporting errors as for command line you can always get all semantic diagnostics and then global diagnsotics?

The current LS can miss these global diagnostics, yes. Though, that isn't new...

It is new right, given we use to add the new diagnostics to semantic diagnostics and send it back to client ?

getBindAndCheckDiagnosticsForFile

It shouldnt be part of this as these are the ones that are cached in buildInfo and need to be consistent

I'm confused enough with how to make this work that I wish we just didn't have global diags at all, they just really do depend on whether or not a file was checked and in which order, which does not feel reasonable to manage?

Havent checked how diagnostics are handled in corsa but with push diagnostics we can control the order but not sure how to handle that as part of pull diagnostics esp since global diagnostics are checker diagnostics without any file info. (may be we just need to make sure that checker cannot generate global diagnostics) ?