Add public IMethodSymbol.IsPartialDefinition

[tmat]

Using only symbol APIs (i.e. not syntax) it is currently impossible to distinguish between regular method and partial definition that does not have an implementation part.

In both cases IMethodSymbol.PartialDefinitionPart and IMethodSymbol.PartialImplementationPart are null.

Proposal

Add IMethodSymbol.IsPartialDefinition that returns true for partial definition symbols. These are source-only artifacts and thus PE symbols would always return false.

[tmat]

@jcouv @jaredpar Need this API for EnC implementation. Can work around by inspecting syntax for now.

[jaredpar]

@333fred to prioritize the API request

[svick]

Isn't this pretty much a duplicate of the recently rejected #50736?

[tmat]

Yes, it's similar. Unlike IsPartial, IsPartialDefinition can be implemented for all method symbols consistently (returns false for metadata symbols since definitions are not emitted).

[333fred]

@tmat I do have to wonder, though, whether the workaround @CyrusNajmabadi suggested in checking syntax is good enough. I personally don't find the reasons about not having the API compelling (we could have had just a single symbol for partials with info merged in the first place, having the public link between definition and implementation feels like a more syntactic thing), but if we want to hold to that reasoning it seems like it would still apply here.

[tmat]

The symbol API is incomplete. It doesn't allow one to reason about symbols that we expose. Sure, I can use syntax but then why do we have symbol APIs in the first place?

[tmat]

Falling back to syntax requires me to special case C# and VB making things more complicated than a simple property.

[333fred]

@CyrusNajmabadi, do you have any comments?

[CyrusNajmabadi]

I always found the API here a bit weird. There's a link between the partials if both partials exist. But nothing there if the other doesn't exist. It's a bit wonky. I would be totally fine with a IsPartialDefinition here.

[aelij]

Let me just say this - why is an API that's obviously useful and extremely unlikely to ever change internal? If it's useful within Roslyn, it can definitely be useful for external devs. In other words - what's the downside?

[CyrusNajmabadi]

and extremely unlikely to ever change internal

I'm not convinced of that. I actually think the partial space is an area we're going to be changing a lot more around. We already revised partial methods recently to help out source generators, and i doubt it's the last thing we'll do in that space.

[aelij]

@CyrusNajmabadi I'm not convinced either :) You can say that about everything in the symbol API. Unless you're going to introduce a serious breaking change into the language, an indicator whether a method is partial or not will be stable.

[333fred]

And if we decided to allow partial methods to be defined multiple times? We might want to have an entirely different API shape at that point. There is pretty much no API decision that we can make that is cut and dry. Every public API needs direct testing, incurs maintenance burden, and increases the concept count for people working with our APIs. The underlying implementation of partial methods could change completely, in which case we'd have to either deprecate this API or go through contortions to support the existing shape.

None of this is to say we shouldn't do this particular API. However, no API decision is as simple as YOLO.

[CyrusNajmabadi]

You can say that about everything in the symbol API

Yes. Which is why we're very hesitant and very rarely change things.

an indicator whether a method is partial or not will be stable.

I don't know if that's true. I coudl easily imagine, for example, new types of 'partial' (like the replace/original idea we had a while back). I could see this being an enum or a much more complex representation in the future if necessary.

[alrz]

Does it make sense to add the API as an extension? Is it possible that it could be implemented more efficient internally?

[aelij]

I coudl easily imagine, for example, new types of 'partial' (like the replace/original idea we had a while back). I could see this being an enum or a much more complex representation in the future if necessary.

Yeah, I thought about that too, but even then, you could have an IsPartial and PartialKind properties. The price of not adding it now is API stagnation. Partial has been there for a long time. Will we forever be fearful of language evolution?

However, no API decision is as simple as YOLO.

Haha, I agree for sure. Having written analyzers for the other .NET code analysis platform, I truly appreciate how stable the Roslyn API has been. But is it possible you're being slightly overly cautious? From my experience, Roslyn has been a lot less useful without consuming internals (could you imagine writing VS on top of the public API?). I was willing to pay the price of the odd compilation error during upgrades. A deprecated property seems like an even smaller price to pay.

Just my 2¢.

[CyrusNajmabadi]

Will we forever be fearful of language evolution?

Yes :) I am constantly fearful fo this. Because i routinely run into being non-fearful and then regretting it around 3 month later. :)

[aelij]

@alrz That's what IgnoresAccessChecksToGenerator was made for ;)