-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Edit and continue diagnostic source #72787
Conversation
1492505
to
e828e3a
Compare
src/EditorFeatures/Core/EditAndContinue/EditAndContinueLanguageService.cs
Show resolved
Hide resolved
src/EditorFeatures/Core/EditAndContinue/EditAndContinueLanguageService.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/Core/EditAndContinue/EditAndContinueLanguageService.cs
Show resolved
Hide resolved
...Features/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueDiagnosticSource.cs
Outdated
Show resolved
Hide resolved
...tures/LanguageServer/Protocol/Handler/Diagnostics/AbstractWorkspacePullDiagnosticsHandler.cs
Show resolved
Hide resolved
...tures/LanguageServer/Protocol/Handler/Diagnostics/AbstractWorkspacePullDiagnosticsHandler.cs
Show resolved
Hide resolved
...Features/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueDiagnosticSource.cs
Outdated
Show resolved
Hide resolved
...Features/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueDiagnosticSource.cs
Outdated
Show resolved
Hide resolved
...Features/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueDiagnosticSource.cs
Outdated
Show resolved
Hide resolved
@dibarbet @CyrusNajmabadi Updated per our discussion. |
I did not need to add any special handling to clear diagnostics. Added a test to cover it. Seems to work. |
...Features/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueDiagnosticSource.cs
Outdated
Show resolved
Hide resolved
...Features/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueDiagnosticSource.cs
Outdated
Show resolved
Hide resolved
...Features/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueDiagnosticSource.cs
Outdated
Show resolved
Hide resolved
src/Features/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueSessionState.cs
Show resolved
Hide resolved
src/Features/LanguageServer/Protocol/Features/EditAndContinue/EditAndContinueSessionState.cs
Show resolved
Hide resolved
...tures/LanguageServer/Protocol/Handler/Diagnostics/AbstractWorkspacePullDiagnosticsHandler.cs
Show resolved
Hide resolved
...tures/LanguageServer/Protocol/Handler/Diagnostics/AbstractWorkspacePullDiagnosticsHandler.cs
Show resolved
Hide resolved
...es/LanguageServer/Protocol/Handler/Diagnostics/DiagnosticSources/DocumentDiagnosticSource.cs
Show resolved
Hide resolved
return ValueTaskFactory.FromResult(DocumentPullDiagnosticHandler.GetDiagnosticSources(DiagnosticKind.All, nonLocalDocumentDiagnostics, taskList: false, context, GlobalOptions)); | ||
var source = nonLocalDocumentDiagnostics | ||
? DocumentPullDiagnosticHandler.GetNonLocalDiagnosticSource(context, GlobalOptions) | ||
: DocumentPullDiagnosticHandler.GetDiagnosticSource(DiagnosticKind.All, context); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh - we already create buckets in VSCode - I completely forgot about this. Its for diagnostics reported on the document during compilation end
Line 43 in b67df4c
Identifier = DocumentNonLocalDiagnosticIdentifier.ToString() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signing off on teh pull-diags changes. Just nits and small requests for explanations or docs. Glad it was so simple :)
return ValueTaskFactory.FromResult(DocumentPullDiagnosticHandler.GetDiagnosticSources(DiagnosticKind.All, nonLocalDocumentDiagnostics, taskList: false, context, GlobalOptions)); | ||
var source = nonLocalDocumentDiagnostics | ||
? DocumentPullDiagnosticHandler.GetNonLocalDiagnosticSource(context, GlobalOptions) | ||
: DocumentPullDiagnosticHandler.GetDiagnosticSource(DiagnosticKind.All, context); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh - we already create buckets in VSCode - I completely forgot about this. Its for diagnostics reported on the document during compilation end
Line 43 in b67df4c
Identifier = DocumentNonLocalDiagnosticIdentifier.ToString() |
return new(GetDiagnosticSources(diagnosticKind: default, nonLocalDocumentDiagnostics: false, taskList: true, context, GlobalOptions)); | ||
return context.GetTrackedDocument<Document>() is { } document ? new TaskListDiagnosticSource(document, GlobalOptions) : null; | ||
|
||
if (category == PullDiagnosticCategories.EditAndContinue) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently these won't ever show in VSCode (because we only register the kind for VS diagnostics).
For VSCode, buckets are named 'identifiers', and they can only be registered dynamically like we do here -
Line 43 in b67df4c
Identifier = DocumentNonLocalDiagnosticIdentifier.ToString() |
And unfortunately it has a different field name. If you add add the identifier, in the Public doc pull handler, you'll also need to add the code to check for the identifier and create a source for it.
3a68276
to
5937238
Compare
5983785
to
3e2e3ce
Compare
…tinueDiagnosticAnalyzer
…pace pull handler
3e2e3ce
to
6a03c2a
Compare
Directly hook EnC document analysis into pull diagnostic handler instead of going through legacy incremental analyzer system.
Since pull diagnostics ae implemented in the LSP layer we need to expose the EnC session state to this layer via a new MEF component (
IEditAndContinueSessionTracker
). Previously the state was directly onEditAndContinueLanguageService
, which is in Editor Features layer. We also need theIActiveStatementSpanLocator
, which reports the current locations of active statements to the EnC analysis, to be available in LSP layer.There are three kinds of EnC diagnostics: 1) rude edits that are determined from document semantic analysis and 2) emit errors reported by the compiler when we try to emit deltas and 3) issues related to the current state of the debuggee.
When pull request for semantic diagnostics is processed and the EnC session is active we analyze the document for rude edits and report them. We don't attempt to emit the deltas at that point or check the debuggee status. Emitting deltas is expensive and the diagnostics that are only reported in emit phase are rare. The debuggee status may change at any point during the edit session and it might be confusing to report these diagnostics until the user is actually applying the change. Instead, we store emit and debuggee state relate diagnostics to the EnC session state and when we pull we simply include the stored diagnostics. We invalidate and refresh the pull diagnostics whenever the EnC session state changes (start session, enter/exit break state, end session).