Make ISemanticSearchCopilotUIProvider import lazy to avoid loading VS.EA.Copilot #77516
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
3 tasks
|
@dotnet/roslyn-ide ptal |
jasonmalinowski
approved these changes
Mar 18, 2025
| @@ -64,7 +64,7 @@ internal sealed class SemanticSearchToolWindowImpl( | |||
| IStreamingFindUsagesPresenter resultsPresenter, | |||
| ITextUndoHistoryRegistry undoHistoryRegistry, | |||
| ISemanticSearchCopilotService copilotService, | |||
| ISemanticSearchCopilotUIProvider copilotUIProvider, | |||
| Lazy<ISemanticSearchCopilotUIProvider> copilotUIProvider, // lazy to avoid loading Microsoft.VisualStudio.LanguageServices.ExternalAccess.Copilot | |||
There was a problem hiding this comment.
It feels a bit weird to me that we have to add a Lazy to avoid an ExternalAccess load of our own assembly. Looking at the interfaces in question rather than having the ExternalAccess wrap our types we're using the ExternalAccess implementation which still eventually wraps our types?
This comment might be worth expanding on a bit on what's going on here....
There was a problem hiding this comment.
I'm not sure I understand.
We have a couple of interfaces ISemanticSearchCopilotUIProviderImpl and ITextBoxControlImpl defined in EA because Copilot needs to export their implementation.
We can't use these interfaces in VS layer since that would add reverse EA dependency. We need to define another interface ISemanticSearchCopilotUIProvider that is exported in EA and imported in VS layer.
333fred
added a commit
to 333fred/roslyn
that referenced
this pull request
Mar 20, 2025
* upstream/main: (169 commits) Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250319.2 (dotnet#77694) Update CSharpCopilotCodeAnalysisService.cs (dotnet#77691) Fix bug where exact path match would throw for additional files (dotnet#77583) raw strings Add CI validation of Semantic Search API lists (dotnet#77535) Fix test Cleanup and make semantic token processing and testing code more consistent Support textDocument/semanticTokens/full Fix SkipApplyOptimizationData parameter (dotnet#77677) Fix typo (dotnet#77678) Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250318.1 (dotnet#77672) Fix reflection Generate Documentation - Bug Fixes (dotnet#77641) Fix watch window completion window upon manual completion invocation (dotnet#77656) Make ISemanticSearchCopilotUIProvider import lazy to avoid loading VS.EA.Copilot (dotnet#77516) Fix generation of attribute with array constant Change vs deps flow (dotnet#77651) On-the-fly-docs Pass along additional context (dotnet#77510) Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250317.1 (dotnet#77649) Remove dead code, use generated regex, use filescoped namespace. ...
333fred
added a commit
that referenced
this pull request
Mar 20, 2025
* wip
* wip
* wip
* wip
* wip
* wip
* wip
* remove some commented out code
* still cleaning up
* revert file
* clean up + option
* wip
* dispose of disable of intellicode line completions
* comments
* comments
* add quota exceeded checks
* fix tests
* revert file
* remove unused optional params
* last few fixes
* feedback
* fix tests
* small fix
* fix
* feedback
* feedback
* removed per comments
* remove unused usings
* Fixer/Analyzer for implementing method
TODO
- Try invoking copilot service from Features layer (needs GenerateMethod APIs added to EA)
- Try invoking semantic search from Features layer
Reviews done:
- Skimmed through the proposal edit approach used for ///
- Excluded diff for ///, does not exist in copilot service
* feedback
* feedback
* move stuff around with new EA structure in roslyn
* wip
* feedback
* feedbacl
* fix
* feedback
* feedback
* feedback
* revert
* remove null check
* clean up
* fix
* fix tests
* local ad ons
* local ad ons
* Add main bits for generating method implementation
* todo next
* update APIs
* trial 1
* Fix compile after merge
* temp1
* temp2
* Implement method, end-to-end for one-off code action
* corrections
* cleanup
* cleanup preliminary check
* Fix bug
* cleanup and fixes
* remove local changes for pr
* correct spacing
* cleanup
* Add unit test - takes code block suggestion to replace with method body
* Add test, support expression body clause later
* Change the FAR API call
* using `.Parent.FirstAncestorOrSelf`
* Improve wrapper implementation
* Apply first round code review
* Merge DualChangeAction into DocumentChangeAction
* Make IDE3000 not configurable
This diagnostic is always hidden (no UI) for the sole support of the
associated code fix provider.
* Simplify analyzer code and improve robustness
* Simplifies fixer and applies most remaining feedback
* Fix compile related to DocumentChangeAction
* Cleanup messaging
* Add resources for messaging
* Adds a few basic tests
* Undo most recent analyzer changes,
to keep the throw statement/expression check
- Adds more test
- Removes dead code
- [ ] Cleanup more code
- [ ] Fix failing tests
- [ ] Rename API make consistent
- [ ] Revisist triple slash docs
- [ ] Change SyntaxNode to MemberDeclaration
- [ ] Remove unexpected null checks
* Apply PR feedback
- [x] Adds more test
- [x] Removes dead code
- [x] Cleans up more code
- [ ] Fix failing tests
- [x] Renames API to make consistent
- [x] Revisits triple slash docs
- [x] API Accepts SyntaxNode
- [x] Removes unreachable code
* Tests renamed
* Correct test setup
- and do minor Cleanup
* Improves readability and removes unreachable code
* Correct mistakes in tests
* Adds more unit tests
* Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055)
* Fixes Analyzer tests
- [ ] Fixer seems not activates in tests
* Makes sure original and replacement text are properly aligned
* Apply partial PR feedback
- Test correction on placement of diagnostic span
* cleanup
* Applies feedback to change API surface
* All tests pass
* tests are actually failing
* Fix Quota bug and show failing tests
* Update tests for error conditions where diagnostics are not corrected
* undo IsImplementNotImplementedExceptionEnabledAsync change
* Fixes remaining tests
* cleanup
* Add back referencedSymbols to API call to help with perf
* Gracefully comment when result is unexpected
* Remove unused resource
* Simplify error messages
* Cleanup tests
* Add missing entry for IDE3000
* Add Missing Help Link
* mid stage
* Removes QuotaExceeds property, received as message
* Update signature
* Fix missed out warning
* Fix test correctness issue picked up by CI
* Add logging
* Apply PR feedback
* Fix run code analysis on solution not reporting results
Fixes #77495
* Use HangMitigatingTimeout for cases where operations do not complete synchronously
* some PR feedback
* spacing
* Move test
* Apply most remaining feedback
* Applies feedback
* Add runtime async to official build (#77537)
* Signature now strictly uses MemberDeclarationSyntax
* Update message for analyzer referencing newer compiler than host (#77541)
* Updates tests
* field implementation is not supported as method or property
see test below
```
[InlineData("int myField;", typeof(FieldDeclarationSyntax))]
public async Task TestInvalidNodeReplacement(string syntax, Type type)
```
* Fix NFW due to invoking Workspace.RaiseEventForHandlers in the CA process (#77546)
NFW telemetry indicates Solution.Workspace is getting called in our server process. This is due to a recent change I made around immediate eventing. In the server process, there are no immediate (or standard) workspace event handlers, but due to this recent changewe were always calling RaiseEventForHandlers for the immediate handlers without checking for their presence.
* Implement field null resilience analysis (#77127)
Co-authored-by: Fred Silberberg <fred@silberberg.xyz>
* Fix incremental generator in deterministic key file (#77553)
* Apply PR feedback
* Disable downloading the runtime packages during restore and instead ensure that it happens when needed (pack) (#77515)
During the most recent SDK insertion, allocation regressions were reported in the C# editing speedometer test. Investigation yielded that these were because of restore failures in the test project as the runtime packages are not published at the point of the SDK insertion.
Looking at the MS.CodeAnalysis.LanguageServer project closer revealed that these runtime identifiers needed to only be set during the pack task. This PR moves setting the RuntimeIdentifiers property to only be done during the pack task, and thus the restore task won't attempt to download these packages. As the test doesn't perform a pack on the roslyn solution, this should prevent future SDK insertions from hitting this issue again.
* Simplifies assumptions - Only trigger fixer when service available
* nit spacing
* Fix preview window for 'implement NotImplementedException'
* Simmplify
* Remove uneeded helper
* Fix Stack Trace Explorer for additional documents (#77517)
Update IDocumentNavigationService implementation in VS to work on any TextDocument
Fix IDocumentNavigationService extension to use TextDocument
Fix Stack Trace Explorer to search for AdditionalDocuments when trying to find a match in file path or name
Partial fix for #77499
There might be more cases where finding symbols doesn't work but filepath+line navigation should now work
* Update dependencies from https://github.com/dotnet/arcade build 20250311.4 (#77569)
Microsoft.SourceBuild.Intermediate.arcade , Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.XliffTasks
From Version 9.0.0-beta.25111.5 -> To Version 9.0.0-beta.25161.4
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* Update PublishData.json (#77589)
* Update main-merge.yml to not run on forks
* Fix typo (#77595)
* Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250313.4 (#77604)
Microsoft.SourceBuild.Intermediate.source-build-reference-packages
From Version 10.0.616001 -> To Version 10.0.616304
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* Filter down the list of files we need to examing when looking for a :base(...) call in find refs
* Move feature-oriented helpers to appropriate location
* Ensure we don't touch Solution.Workspace if we don't have to
This is a bit more of a defense-in-depth fix to fix a potential NFW.
* Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250314.1 (#77623)
Microsoft.SourceBuild.Intermediate.source-build-reference-packages
From Version 10.0.616304 -> To Version 10.0.616401
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* Update dependencies from https://github.com/dotnet/arcade build 20250314.2 (#77624)
Microsoft.SourceBuild.Intermediate.arcade , Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.XliffTasks
From Version 9.0.0-beta.25161.4 -> To Version 9.0.0-beta.25164.2
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* Correct how we pick up options for `ICopilotOptionsService` (#77620)
* Corrections to how we pick up roslyn options settings
* Cleanup
* Set default for Implement with Copilot to be false
* update for copilot.general.editor.enableGenerateDocumentationComment
* Update BuildActionTelemetryTable tool
* Add feature flag controlling Copilot prompt in Semantic Search (#77562)
* Fix BuildActionTelemetryTable project
* Update CodeActionDescriptionsMap.
Updates program to generate new description map when analyzers are added.
* Fix typo
* Remove dead code, use generated regex, use filescoped namespace.
* Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250317.1 (#77649)
Microsoft.SourceBuild.Intermediate.source-build-reference-packages
From Version 10.0.616401 -> To Version 10.0.616701
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* On-the-fly-docs Pass along additional context (#77510)
* wip
* works end to end
* some feedback
* feedback
* fix bugs
* tests
* revert settings
* fix code analysis service
* fix formatting
* Feedback
* Change vs deps flow (#77651)
* Fix generation of attribute with array constant
* Make ISemanticSearchCopilotUIProvider import lazy to avoid loading VS.EA.Copilot (#77516)
* Fix watch window completion window upon manual completion invocation (#77656)
* Fix watch window completion window upon manual completion invocation
Addresses https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2287966
This recently regressed due to this recent PR: #77204
The issue here is that I changed the csharp debugger context's projection buffer creation code from always having a one char string as the second sourceSpan to sometimes having that string be empty. This allowed the completion context to include items for which a semicolon would close that scope. However, by having a zero length span, a seam was created for a tracking span at priorTrackingSpan to be introduced into the set of tracking positions that can contribute to completion. Editor completion doesn't handle this well, so we're best off always sending a non-zero length source Span, so we do so and just use a space as the value.
* Generate Documentation - Bug Fixes (#77641)
* fix some bugs
* Feedback
* Fix reflection
* Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250318.1 (#77672)
Microsoft.SourceBuild.Intermediate.source-build-reference-packages
From Version 10.0.616701 -> To Version 10.0.616801
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* Fix typo (#77678)
disalloed => disallowed.
* Fix SkipApplyOptimizationData parameter (#77677)
* Support textDocument/semanticTokens/full
* Cleanup and make semantic token processing and testing code more consistent
* Fix test
* Add CI validation of Semantic Search API lists (#77535)
* raw strings
* Fix bug where exact path match would throw for additional files (#77583)
From /pull/77517/files/45c0e103f76f36bed6004f836d3dcfeae4bfae0d#r1992506030
* Update CSharpCopilotCodeAnalysisService.cs (#77691)
* Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250319.2 (#77694)
Microsoft.SourceBuild.Intermediate.source-build-reference-packages
From Version 10.0.616801 -> To Version 10.0.616902
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
---------
Co-authored-by: Ankita Khera <ankitakhera@microsoft.com>
Co-authored-by: Maryam Ariyan <maariyan@microsoft.com>
Co-authored-by: Sam Harwell <Sam.Harwell@microsoft.com>
Co-authored-by: Rikki Gibson <rigibson@microsoft.com>
Co-authored-by: Todd Grunke <toddgrun@microsoft.com>
Co-authored-by: Jared Parsons <jared@paranoidcoding.org>
Co-authored-by: Cyrus Najmabadi <cyrus.najmabadi@gmail.com>
Co-authored-by: Andrew Hall <andrha@microsoft.com>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Jan Jones <janjones@microsoft.com>
Co-authored-by: Arun Chander <arkalyan@microsoft.com>
Co-authored-by: Kauwai Lucchesi <53876126+kauwai@users.noreply.github.com>
Co-authored-by: Jason Malinowski <jason.malinowski@microsoft.com>
Co-authored-by: Tomáš Matoušek <tmat@users.noreply.github.com>
Co-authored-by: Joey Robichaud <joseph.robichaud@microsoft.com>
Co-authored-by: Ankita Khera <40616383+akhera99@users.noreply.github.com>
Co-authored-by: David Barbet <dabarbet@microsoft.com>
Co-authored-by: Joey Robichaud <jorobich@microsoft.com>
Co-authored-by: David Wengier <david.wengier@microsoft.com>
Co-authored-by: Bill Wagner <wiwagn@microsoft.com>
Co-authored-by: PaddiM8 <hi@bakk.dev>
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.
Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2398928