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
IInvocationOperation and IObjectCreationOperation should expose the underlying IMethodReferenceOperation #26206
Comments
Thanks! Can we rope in the compatability council? My recollection is that we have fixed semantic model calls in the past when they returned incorrect results. this could certainly break anyone who took a dependency on the old values. But, in practice, just having good behavior rather than preserving bugs won out and was healthier for the ecosystem as a whole. Considering how new IOperation is, i'm hoping we can do the same for it here. |
I think IInvocationOperation should implement IMemberReferenceOperation instead, see #23084 |
That's def an option! Though i think it will lead to a lot of surprise from users. Namely that they can register for method references, and then not actually hear about method references (i.e. exactly the issue that bit this feature :D)**. That said, it would be a viable option that avoids the compat issue. So that's a huge pro going for it. ** However, anyone who cares about methods for a feature will almost certainly try things out with an actual invocation. So it's seems nearly impossible that they would miss this. if we go this route we should probably document IMethodReference and IPropertyReference and say that you may have to also register for IInvocationExpression to hear about Methods/properties if they're used in invocation scenarios. |
Documentation clearly states that IMethodReferenceOperation "Represents a reference to a method other than as the target of an invocation." So, I don't see why one would be surprised. |
Because humans are humans :) Note that IPropertyRef does not say the same. Should i assume that me.Prop(1, 2, 3) in VB has an IInvocationOp and an IPropertyReferenceOp? :) (i honestly don't know. either IOp tree wouldn't surprise me). |
Well, if that is the reason, then no matter what we do some humans will be surprised.
You shouldn't assume anything, you should find an answer to the question. :-) |
@mavasani Am I correct in my interpretation that the current proposal would not be a breaking change for analyzers based on the 2.6 API? |
I think we have few proposals here:
I think our general view is 3. is probably our best option, especially given analyzer authors will quickly figure out that registering just for |
👍 on option '3' for me as well. |
Just a note: I think even IObjectCreationOperation should implement IMethodReferenceOperation. I will update the title accordingly. |
See discussion here: #26188 (comment)
The text was updated successfully, but these errors were encountered: