Make Implement with Copilot light up on member name

[maryamariyan]

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():

Implement_with_Copilot_GH_Clip.mp4

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.

[JoeRobich]

What are the downsides of reporting one diagnostic with the span of the entire method?

[maryamariyan]

What are the downsides of reporting one diagnostic with the span of the entire method?

I considered that too. just went with this flow instead. can't think of downsides, just that I'd have to make new changes :) I like that the code action is specifically on the throw.

I just know that I got specific feedback that having it light up on method name makes sense.
@phil-allen-msft do you have opinions on this?

[CyrusNajmabadi]

can't think of downsides,

The downside (including being on the method signature) is that this now shows up for anywhere in teh method no matter what you are on (and even if you don't see the throw-new-... part.

My recommendation woudl be that we only show this on the method itself if hte method is trivial. So it's a method that only contains the throw. Otherwise, we're making a strange connection here between a particular single expression in a method, with its header, which is not something we do elsewhere.

[CyrusNajmabadi]

why is the impl map keyed on string?

also, how would the impl map work if you have multiple methods with the same name. something is weird here.

[maryamariyan]

it's just to mock copilot behavior for testing

[maryamariyan]

how would the impl map work if you have multiple methods with the same name

The method BuildResult here is mocking copilot behavior. In reality we match on other parts of the member such as params, return, etc. so that means we properly support overload use cases. The logic you see here is an oversimplification of what copilot service does and I am just mocking it here.

as far as roslyn is concerned, it gives and receives member nodes and therefore we are properly keying off of member, not any string whatsoever