Remove legacy IDiagnosticService #72660
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
CyrusNajmabadi
commented
Mar 22, 2024
| private static readonly TestComposition s_compositionWithMockDiagnosticUpdateSourceRegistrationService = EditorTestCompositions.EditorFeatures | ||
| .AddExcludedPartTypes(typeof(IDiagnosticUpdateSourceRegistrationService)) | ||
| .AddParts(typeof(MockDiagnosticUpdateSourceRegistrationService)); | ||
| private static readonly TestComposition s_compositionWithMockDiagnosticUpdateSourceRegistrationService = EditorTestCompositions.EditorFeatures; |
There was a problem hiding this comment.
note: the names of this compositions need to change.
CyrusNajmabadi
commented
Mar 22, 2024
...torFeatures/CSharpTest/Diagnostics/DiagnosticAnalyzerDriver/DiagnosticAnalyzerDriverTests.cs
Outdated
Show resolved
Hide resolved
…iver/DiagnosticAnalyzerDriverTests.cs
CyrusNajmabadi
commented
Mar 22, 2024
| using Roslyn.Utilities; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.ExternalAccess.VSTypeScript; | ||
|
|
||
| [Export(typeof(IVSTypeScriptDiagnosticService)), Shared] | ||
| [method: ImportingConstructor] | ||
| [method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] | ||
| internal sealed class VSTypeScriptDiagnosticService(IDiagnosticService service) : IVSTypeScriptDiagnosticService | ||
| internal sealed class VSTypeScriptDiagnosticService() : IVSTypeScriptDiagnosticService |
There was a problem hiding this comment.
this is the only place that actually takes the events from IDiagnosticService and forwards them along. But it's only for "push style" diagnostics, which we've moved off of, and which TS also definitely does not want. @MariaSolOs for awareness. this should go nicely with yoru move to LSP pull diags.
CyrusNajmabadi
commented
Mar 22, 2024
| @@ -49,7 +47,6 @@ public async Task TestGetFirstDiagnosticWithFixAsync() | |||
| "; | |||
| using var workspace = TestWorkspace.CreateCSharp(code, composition: s_compositionWithMockDiagnosticUpdateSourceRegistrationService, openDocuments: true); | |||
|
|
|||
| Assert.IsType<MockDiagnosticUpdateSourceRegistrationService>(workspace.GetService<IDiagnosticUpdateSourceRegistrationService>()); | |||
There was a problem hiding this comment.
tons of asserts taht tests were actually not even using actual product code...
CyrusNajmabadi
commented
Mar 22, 2024
| @@ -39,8 +39,7 @@ private static string Inspect(DiagnosticData d) | |||
| (!string.IsNullOrWhiteSpace(d.DataLocation.UnmappedFileSpan.Path) ? $" {d.DataLocation.UnmappedFileSpan.Path}({d.DataLocation.UnmappedFileSpan.StartLinePosition.Line}, {d.DataLocation.UnmappedFileSpan.StartLinePosition.Character}, {d.DataLocation.UnmappedFileSpan.EndLinePosition.Line}, {d.DataLocation.UnmappedFileSpan.EndLinePosition.Character}):" : "") + | |||
| $" {d.Message}"; | |||
|
|
|||
| [Theory] | |||
| [CombinatorialData] | |||
| [Theory, CombinatorialData, Obsolete] | |||
There was a problem hiding this comment.
Obsolete needed as this tests the obsolete constructor of EnCUpdateSource
|
@mavasani ptal. |
|
@mavasani ptal |
mavasani
approved these changes
Mar 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The only client even listening for the events from this is TS. Note: they are moving to LSP pull diags. So i'm goign to wait on them to get that in, then remove all of this. Tagging @MariaSolOs as well.