-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Adds 💡 Implement Using Copilot
for NotImplementedException
#77299
base: main
Are you sure you want to change the base?
Adds 💡 Implement Using Copilot
for NotImplementedException
#77299
Conversation
…icate-for-features
src/Features/CSharp/Portable/Copilot/CSharpImplementNotImplementedExceptionFixProvider.cs
Outdated
Show resolved
Hide resolved
src/Features/CSharp/Portable/Copilot/CSharpImplementNotImplementedExceptionFixProvider.cs
Show resolved
Hide resolved
src/Features/CSharp/Portable/Copilot/CSharpImplementNotImplementedExceptionFixProvider.cs
Outdated
Show resolved
Hide resolved
...pTest/Copilot/CSharpImplementNotImplementedExceptionFixProviderTests.CodeBlockSuggestions.cs
Show resolved
Hide resolved
src/Features/ExternalAccess/Copilot/GenerateImplementation/ImplementationDetailsWrapper.cs
Outdated
Show resolved
Hide resolved
ImmutableDictionary<SyntaxNode, ImmutableArray<ReferencedSymbol>> methodOrProperties, | ||
CancellationToken cancellationToken) | ||
{ | ||
if (GenerateImplementationService is null) |
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.
This bit makes it clear that GenerateImplemenationService
shouldn't be declared as nullable. Why are null checks needed for fields that have been proven non-null in the constructor?
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.
So, it is totally possible for the service to be null, and that is when the the EA layer API that I am adding is not yet available to the copilot service. in that case, this will be null. The reason for null checks in ctor I believe is to report when this unwanted state is present. that's why I mentioned the dual insertion, when the newer roslyn package gets ingested by copilot service then this service will no longer be null
Offers 💡
Implement Using Copilot
whenNotImplementedException
is present in code.FixAll_Faster.mp4