Make Implement with Copilot light up on member name (#77769)

Every time a developer uses the 💡 `Generate Method` feature in VS,
then with a ctrl+dot the cursor gets placed on the name of the newly
generated method which by default contains `throw new
NotImplementedException()`:


https://github.com/user-attachments/assets/5aa69115-6660-43c4-b711-eaa0f4772d8d

To help the discoverability and usability of `Implement with Copilot`,
especially for the developers frequently using the `Generate Method`
feature, I am making the analyzer diagnostic for `Implement with
Copilot` also light up on the method or property name containing the
`NotImplementedException` throw nodes.

Author: CyrusNajmabadi

src/Features/CSharp/Portable/Copilot/CSharpImplementNotImplementedExceptionDiagnosticAnalyzer.cs

@@ -28,30 +28,92 @@ protected override void InitializeWorker(AnalysisContext context)
         context.RegisterCompilationStartAction(context =>
         {
             var notImplementedExceptionType = context.Compilation.GetTypeByMetadataName(typeof(NotImplementedException).FullName!);
-            if (notImplementedExceptionType != null)
-                context.RegisterOperationAction(context => AnalyzeThrow(context, notImplementedExceptionType), OperationKind.Throw);
+            if (notImplementedExceptionType is null)
+                return;
+
+            context.RegisterOperationBlockAction(context => AnalyzeOperationBlock(context, notImplementedExceptionType));
         });
     }
 
-    private void AnalyzeThrow(OperationAnalysisContext context, INamedTypeSymbol notImplementedExceptionType)
+    private void AnalyzeOperationBlock(
+        OperationBlockAnalysisContext context,
+        INamedTypeSymbol notImplementedExceptionType)
     {
-        var throwOperation = (IThrowOperation)context.Operation;
-        if (throwOperation is
+        foreach (var block in context.OperationBlocks)
+            AnalyzeBlock(block);
+
+        void AnalyzeBlock(IOperation block)
+        {
+            foreach (var operation in block.DescendantsAndSelf())
             {
-                Exception: IConversionOperation
+                if (operation is IThrowOperation
+                    {
+                        Exception: IConversionOperation
+                        {
+                            Operand: IObjectCreationOperation
+                            {
+                                Constructor.ContainingType: INamedTypeSymbol constructedType,
+                            },
+                        },
+                        Syntax: var throwSyntax
+                    } throwOperation &&
+                    notImplementedExceptionType.Equals(constructedType))
                 {
-                    Operand: IObjectCreationOperation
+                    // Report diagnostic for each throw operation
+                    context.ReportDiagnostic(Diagnostic.Create(
+                        Descriptor,
+                        throwOperation.Syntax.GetLocation()));
+
+                    // If the throw is the top-level operation in the containing symbol, report a diagnostic on the
+                    // symbol as well. Note: consider reporting on the entire symbol, instead of just the name.  And in
+                    // this case, do not report directly on the throw as well.
+                    if (ShouldReportAsTopLevel(block, operation))
                     {
-                        Constructor.ContainingType: INamedTypeSymbol constructedType,
-                    },
-                },
-                Syntax: ThrowExpressionSyntax or ThrowStatementSyntax,
-            } &&
-            notImplementedExceptionType.Equals(constructedType))
+                        foreach (var location in context.OwningSymbol.Locations)
+                        {
+                            if (location.SourceTree == context.FilterTree)
+                            {
+                                context.ReportDiagnostic(Diagnostic.Create(
+                                    Descriptor,
+                                    location));
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    private static bool ShouldReportAsTopLevel(IOperation block, IOperation operation)
+    {
+        if (block is IBlockOperation { Operations: [var child] })
         {
-            context.ReportDiagnostic(Diagnostic.Create(
-                Descriptor,
-                throwOperation.Syntax.GetLocation()));
+            // Handle: { throw new NotImplementedException(); }
+            if (child == operation)
+                return true;
+
+            // Handle: => throw new NotImplementedException();
+            if (child is IReturnOperation
+                {
+                    ReturnedValue: IConversionOperation
+                    {
+                        Operand: var operand
+                    }
+                } && operand == operation &&
+                // Excluding property declarations with expression bodies
+                // Because their location is an exact match with the throw operation
+                // and we don't want to report the same location twice
+                operation.Syntax.Parent is not ArrowExpressionClauseSyntax { Parent: PropertyDeclarationSyntax })
+            {
+                // Include expression-bodied methods and get accessors
+                return true;
+            }
+
+            // Handle expression-bodied set accessors
+            if (child is IExpressionStatementOperation { Operation: var throwOperation } && throwOperation == operation)
+                return true;
         }
+
+        return false;
     }
 }

src/Features/CSharp/Portable/Copilot/CSharpImplementNotImplementedExceptionFixProvider.cs

@@ -55,10 +55,11 @@ await copilotService.IsImplementNotImplementedExceptionsAvailableAsync(cancellat
             return;
         }
 
-        var throwNode = context.Diagnostics[0].Location.FindNode(getInnermostNodeForTie: true, cancellationToken);
+        var diagnosticNode = context.Diagnostics[0].Location.FindNode(getInnermostNodeForTie: true, cancellationToken);
 
         // Preliminary analysis before registering fix
-        var methodOrProperty = throwNode.FirstAncestorOrSelf<MemberDeclarationSyntax>();
+        var methodOrProperty = diagnosticNode.FirstAncestorOrSelf<MemberDeclarationSyntax>();
+
         if (methodOrProperty is BasePropertyDeclarationSyntax or BaseMethodDeclarationSyntax)
         {
             // Pull out the computation into a lazy computation here.  That way if we compute (and thus cache) the
@@ -84,8 +85,9 @@ protected override async Task FixAllAsync(
 
         foreach (var diagnostic in diagnostics)
         {
-            var throwNode = diagnostic.Location.FindNode(getInnermostNodeForTie: true, cancellationToken);
-            var methodOrProperty = throwNode.FirstAncestorOrSelf<MemberDeclarationSyntax>();
+            var diagnosticNode = diagnostic.Location.FindNode(getInnermostNodeForTie: true, cancellationToken);
+            var methodOrProperty = diagnosticNode.FirstAncestorOrSelf<MemberDeclarationSyntax>();
+
             Contract.ThrowIfFalse(methodOrProperty is BasePropertyDeclarationSyntax or BaseMethodDeclarationSyntax);
 
             if (!memberReferencesBuilder.ContainsKey(methodOrProperty))