-
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
Adds 💡 Implement Using Copilot
for NotImplementedException
#77299
Conversation
see test below ``` [InlineData("int myField;", typeof(FieldDeclarationSyntax))] public async Task TestInvalidNodeReplacement(string syntax, Type type) ```
src/Features/CSharp/Portable/Copilot/CSharpImplementNotImplementedExceptionFixProvider.cs
Show resolved
Hide resolved
src/Features/CSharp/Portable/Copilot/CSharpImplementNotImplementedExceptionFixProvider.cs
Outdated
Show resolved
Hide resolved
...Features/CSharpTest/Copilot/CSharpImplementNotImplementedExceptionDiagnosticAnalyzerTests.cs
Outdated
Show resolved
Hide resolved
@@ -70,6 +70,9 @@ public Test() | |||
[StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] | |||
public new string FixedCode { set => base.FixedCode = value; } | |||
|
|||
[StringSyntax(PredefinedEmbeddedLanguageNames.CSharpTest)] | |||
public new string BatchFixedCode { set => base.BatchFixedCode = value; } |
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.
@sharwell to check on this. curious why we haven't needed this yet.
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.
It's been around for a long time. It's needed for cases where Fix All in Document does not produce the same result as running the individual fixes in sequence.
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.
tentative signoff with Sam's signoff on the testing change, and the final changes requested done. Thanks. Looks awesome! Can't wait to try this out.
{ | ||
Contract.ThrowIfTrue(string.IsNullOrWhiteSpace(implementationDetails.Message)); | ||
replacement = AddErrorComment(methodOrProperty, implementationDetails.Message); | ||
} |
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.
much nicer.
I don't seem to have merge privileges on roslyn repo. I'll need help with merging PR once CI is complete |
@@ -16,6 +16,7 @@ | |||
</PropertyGroup> | |||
<ItemGroup Label="Project References"> | |||
<ProjectReference Include="..\..\..\Compilers\Core\Portable\Microsoft.CodeAnalysis.csproj" /> | |||
<ProjectReference Include="..\..\..\Compilers\CSharp\Portable\Microsoft.CodeAnalysis.CSharp.csproj" /> |
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 was not ok. @maryamariyan this has to be removed as it is a layering violation.
Offers 💡
Implement Using Copilot
whenNotImplementedException
is present in code.Implement_With_Copilot_Examples.mp4